maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Aleksey,

On Apr 18, Aleksey Midenkov wrote:
> >
> > I have a second patch that renames flags to have a very well-defined
> > meaning and name.
> >
> >   OPTION_KEEP_LOG -> OPTION_BINLOG_THIS_TRX
> >   log_current_statement -> OPTION_BINLOG_THIS_STMT
> 
> Where is your patch?

It's really just renaming, almost nothing else.
But here it is, attached.

Regards,
Sergei
VP of MariaDB Server Engineering
and security@xxxxxxxxxxx

commit 0593fd1926f
Author: Sergei Golubchik <serg@xxxxxxxxxxx>
Date:   Mon Apr 11 22:06:21 2022 +0200

    cleanup: log_current_statement and OPTION_KEEP_LOG
    
    rename OPTION_KEEP_LOG -> OPTION_BINLOG_THIS_TRX.
    Meaning: transaction cache will be written to binlog even on rollback.
    
    convert log_current_statement to OPTION_BINLOG_THIS_STMT.
    Meaning: the statement will be written to binlog (or trx binlog cache)
    even if it normally wouldn't be.
    
    setting OPTION_BINLOG_THIS_STMT must always set OPTION_BINLOG_THIS_TRX,
    otherwise the statement won't be logged if the transaction is rolled back.
    Use OPTION_BINLOG_THIS to set both.

diff --git a/sql/log.cc b/sql/log.cc
index 70ceecc66f8..02776f3b43e 100644
--- a/sql/log.cc
+++ b/sql/log.cc
@@ -2036,7 +2036,7 @@ inline bool is_prepared_xa(THD *thd)
 /*
   We flush the cache wrapped in a beging/rollback if:
     . aborting a single or multi-statement transaction and;
-    . the OPTION_KEEP_LOG is active or;
+    . the OPTION_BINLOG_THIS_TRX is active or;
     . the format is STMT and a non-trans table was updated or;
     . the format is MIXED and a temporary non-trans table was
       updated or;
@@ -2047,7 +2047,7 @@ static bool trans_cannot_safely_rollback(THD *thd, bool all)
 {
   DBUG_ASSERT(ending_trans(thd, all));
 
-  return ((thd->variables.option_bits & OPTION_KEEP_LOG) ||
+  return ((thd->variables.option_bits & OPTION_BINLOG_THIS_TRX) ||
           (trans_has_updated_non_trans_table(thd) &&
            thd->wsrep_binlog_format() == BINLOG_FORMAT_STMT) ||
           (thd->transaction->all.has_modified_non_trans_temp_table() &&
@@ -2446,10 +2446,10 @@ static int binlog_savepoint_rollback(handlerton *hton, THD *thd, void *sv)
    */
   if (unlikely(thd->wsrep_trx().is_streaming() ||
                (trans_has_updated_non_trans_table(thd)) ||
-               (thd->variables.option_bits & OPTION_KEEP_LOG)))
+               (thd->variables.option_bits & OPTION_BINLOG_THIS_TRX)))
 #else
   if (unlikely(trans_has_updated_non_trans_table(thd) ||
-               (thd->variables.option_bits & OPTION_KEEP_LOG)))
+               (thd->variables.option_bits & OPTION_BINLOG_THIS_TRX)))
 #endif /* WITH_WSREP */
   {
     char buf[1024];
diff --git a/sql/slave.cc b/sql/slave.cc
index 3c5b830fbe2..c0080e4df74 100644
--- a/sql/slave.cc
+++ b/sql/slave.cc
@@ -1368,13 +1368,13 @@ static bool sql_slave_killed(rpl_group_info *rgi)
   if (rli->sql_driver_thd->killed || rli->abort_slave)
   {
     /*
-      The transaction should always be binlogged if OPTION_KEEP_LOG is
+      The transaction should always be binlogged if OPTION_BINLOG_THIS_TRX is
       set (it implies that something can not be rolled back). And such
       case should be regarded similarly as modifing a
       non-transactional table because retrying of the transaction will
       lead to an error or inconsistency as well.
 
-      Example: OPTION_KEEP_LOG is set if a temporary table is created
+      Example: OPTION_BINLOG_THIS_TRX is set if a temporary table is created
       or dropped.
 
       Note that transaction.all.modified_non_trans_table may be 1
@@ -1384,7 +1384,7 @@ static bool sql_slave_killed(rpl_group_info *rgi)
     */
 
     if ((thd->transaction->all.modified_non_trans_table ||
-         (thd->variables.option_bits & OPTION_KEEP_LOG)) &&
+         (thd->variables.option_bits & OPTION_BINLOG_THIS_TRX)) &&
         rli->is_in_group())
     {
       char msg_stopped[]=
@@ -1396,10 +1396,10 @@ static bool sql_slave_killed(rpl_group_info *rgi)
         "documentation for details).";
 
       DBUG_PRINT("info", ("modified_non_trans_table: %d  OPTION_BEGIN: %d  "
-                          "OPTION_KEEP_LOG: %d  is_in_group: %d",
+                          "OPTION_BINLOG_THIS_TRX: %d  is_in_group: %d",
                           thd->transaction->all.modified_non_trans_table,
                           MY_TEST(thd->variables.option_bits & OPTION_BEGIN),
-                          MY_TEST(thd->variables.option_bits & OPTION_KEEP_LOG),
+                          MY_TEST(thd->variables.option_bits & OPTION_BINLOG_THIS_TRX),
                           rli->is_in_group()));
 
       if (rli->abort_slave)
diff --git a/sql/sql_class.h b/sql/sql_class.h
index 686e6e70766..d68588b4969 100644
--- a/sql/sql_class.h
+++ b/sql/sql_class.h
@@ -3555,8 +3555,7 @@ class THD: public THD_count, /* this must be first */
   /* set during loop of derived table processing */
   bool       derived_tables_processing;
   bool       tablespace_op;	/* This is TRUE in DISCARD/IMPORT TABLESPACE */
-  /* True if we have to log the current statement */
-  bool	     log_current_statement;
+
   /**
     True if a slave error. Causes the slave to stop. Not the same
     as the statement execution error (is_error()), since
diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc
index 707b8a0d3bf..cfc20e3d408 100644
--- a/sql/sql_insert.cc
+++ b/sql/sql_insert.cc
@@ -4336,7 +4336,7 @@ void select_insert::abort_result_set()
     changed= (info.copied || info.deleted || info.updated);
     transactional_table= table->file->has_transactions_and_rollback();
     if (thd->transaction->stmt.modified_non_trans_table ||
-        thd->log_current_statement)
+        thd->variables.option_bits & OPTION_BINLOG_THIS_STMT)
     {
         if (!can_rollback_data())
           thd->transaction->all.modified_non_trans_table= TRUE;
@@ -5217,7 +5217,7 @@ void select_create::abort_result_set()
 
     drop_open_table(thd, table, &create_table->db, &create_table->table_name);
     table=0;                                    // Safety
-    if (thd->log_current_statement)
+    if (thd->variables.option_bits & OPTION_BINLOG_THIS_STMT)
     {
       if (mysql_bin_log.is_open())
       {
diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc
index 078ea0dae9b..1d240edb9cd 100644
--- a/sql/sql_parse.cc
+++ b/sql/sql_parse.cc
@@ -4915,7 +4915,7 @@ mysql_execute_command(THD *thd, bool is_called_from_prepared_stmt)
       status_var_increment(thd->status_var.com_drop_tmp_table);
 
       /* So that DROP TEMPORARY TABLE gets to binlog at commit/rollback */
-      thd->variables.option_bits|= OPTION_KEEP_LOG;
+      thd->variables.option_bits|= OPTION_BINLOG_THIS_TRX;
     }
     /*
       If we are a slave, we should add IF EXISTS if the query executed
@@ -7569,7 +7569,7 @@ void THD::reset_for_next_command(bool do_clear_error)
 
   query_start_sec_part_used= 0;
   is_fatal_error= time_zone_used= 0;
-  log_current_statement= 0;
+  variables.option_bits&= ~OPTION_BINLOG_THIS_STMT;
 
   /*
     Clear the status flag that are expected to be cleared at the
@@ -7578,12 +7578,12 @@ void THD::reset_for_next_command(bool do_clear_error)
   server_status&= ~SERVER_STATUS_CLEAR_SET;
   /*
     If in autocommit mode and not in a transaction, reset
-    OPTION_STATUS_NO_TRANS_UPDATE | OPTION_KEEP_LOG to not get warnings
+    OPTION_STATUS_NO_TRANS_UPDATE | OPTION_BINLOG_THIS_TRX to not get warnings
     in ha_rollback_trans() about some tables couldn't be rolled back.
   */
   if (!in_multi_stmt_transaction_mode())
   {
-    variables.option_bits&= ~OPTION_KEEP_LOG;
+    variables.option_bits&= ~OPTION_BINLOG_THIS_TRX;
     transaction->all.reset();
   }
   DBUG_ASSERT(security_ctx== &main_security_ctx);
diff --git a/sql/sql_priv.h b/sql/sql_priv.h
index f7d8ef0da67..0ce6d9dccef 100644
--- a/sql/sql_priv.h
+++ b/sql/sql_priv.h
@@ -134,7 +134,7 @@
 #define OPTION_BEGIN            (1ULL << 20)    // THD, intern
 #define OPTION_TABLE_LOCK       (1ULL << 21)    // THD, intern
 #define OPTION_QUICK            (1ULL << 22)    // SELECT (for DELETE)
-#define OPTION_KEEP_LOG         (1ULL << 23)    // THD, user
+#define OPTION_BINLOG_THIS_TRX  (1ULL << 23)    // THD
 
 /* The following is used to detect a conflict with DISTINCT */
 #define SELECT_ALL              (1ULL << 24)    // SELECT, user, parser
@@ -175,6 +175,9 @@
 */
 #define OPTION_MASTER_SQL_ERROR         (1ULL << 35)
 
+#define OPTION_BINLOG_THIS_STMT         (1ULL << 36) // THD
+#define OPTION_BINLOG_THIS (OPTION_BINLOG_THIS_STMT | OPTION_BINLOG_THIS_TRX)
+
 #define OPTION_SKIP_REPLICATION         (1ULL << 37) // THD, user
 #define OPTION_RPL_SKIP_PARALLEL        (1ULL << 38)
 #define OPTION_NO_QUERY_CACHE           (1ULL << 39) // SELECT, user
diff --git a/sql/sql_table.cc b/sql/sql_table.cc
index b07efb29bba..e41629bf85f 100644
--- a/sql/sql_table.cc
+++ b/sql/sql_table.cc
@@ -4377,8 +4377,7 @@ int create_table_impl(THD *thd,
       */
       if (table_creation_was_logged)
       {
-        thd->variables.option_bits|= OPTION_KEEP_LOG;
-        thd->log_current_statement= 1;
+        thd->variables.option_bits|= OPTION_BINLOG_THIS;
         create_info->table_was_deleted= 1;
       }
     }
@@ -4436,8 +4435,7 @@ int create_table_impl(THD *thd,
           We have to log this query, even if it failed later to ensure the
           drop is done.
         */
-        thd->variables.option_bits|= OPTION_KEEP_LOG;
-        thd->log_current_statement= 1;
+        thd->variables.option_bits|= OPTION_BINLOG_THIS;
         create_info->table_was_deleted= 1;
         lex_string_set(&create_info->org_storage_engine_name,
                        ha_resolve_storage_engine_name(db_type));
@@ -4460,9 +4458,9 @@ int create_table_impl(THD *thd,
         */
 
         /* Log CREATE IF NOT EXISTS on slave for distributed engines */
-        if (thd->slave_thread && (db_type && db_type->flags &
-                                  HTON_IGNORE_UPDATES))
-          thd->log_current_statement= 1;
+        if (thd->slave_thread && db_type &&
+            db_type->flags & HTON_IGNORE_UPDATES)
+          thd->variables.option_bits|= OPTION_BINLOG_THIS;
         goto warn;
       }
       else
@@ -4827,7 +4825,7 @@ bool mysql_create_table(THD *thd, TABLE_LIST *create_table,
     thd->transaction->stmt.mark_created_temp_table();
 
   /* Write log if no error or if we already deleted a table */
-  if (likely(!result) || thd->log_current_statement)
+  if (!result || thd->variables.option_bits & OPTION_BINLOG_THIS_STMT)
   {
     if (unlikely(result) && create_info->table_was_deleted &&
         pos_in_locked_tables)
@@ -5286,7 +5284,7 @@ bool mysql_create_like_table(THD* thd, TABLE_LIST* table,
                                     &is_trans, C_ORDINARY_CREATE,
                                     table)) > 0);
   /* Remember to log if we deleted something */
-  do_logging= thd->log_current_statement;
+  do_logging= thd->variables.option_bits & OPTION_BINLOG_THIS_STMT;
   if (res)
     goto err;
 
@@ -11869,7 +11867,7 @@ bool Sql_cmd_create_table_like::execute(THD *thd)
         if (!(res= handle_select(thd, lex, result, 0)))
         {
           if (create_info.tmp_table())
-            thd->variables.option_bits|= OPTION_KEEP_LOG;
+            thd->variables.option_bits|= OPTION_BINLOG_THIS_TRX;
         }
         delete result;
       }
@@ -11913,7 +11911,7 @@ bool Sql_cmd_create_table_like::execute(THD *thd)
     {
       /* So that CREATE TEMPORARY TABLE gets to binlog at commit/rollback */
       if (create_info.tmp_table())
-        thd->variables.option_bits|= OPTION_KEEP_LOG;
+        thd->variables.option_bits|= OPTION_BINLOG_THIS_TRX;
       /* in case of create temp tables if @@session_track_state_change is
          ON then send session state notification in OK packet */
       if (create_info.options & HA_LEX_CREATE_TMP_TABLE)
diff --git a/sql/sys_vars.cc b/sql/sys_vars.cc
index 7127cbc00f6..921f6a53e98 100644
--- a/sql/sys_vars.cc
+++ b/sql/sys_vars.cc
@@ -4399,7 +4399,7 @@ static bool fix_autocommit(sys_var *self, THD *thd, enum_var_type type)
       transaction implicitly at the end (@sa stmt_causes_implicitcommit()).
     */
     thd->variables.option_bits&=
-                 ~(OPTION_BEGIN | OPTION_KEEP_LOG | OPTION_NOT_AUTOCOMMIT |
+                 ~(OPTION_BEGIN | OPTION_BINLOG_THIS_TRX | OPTION_NOT_AUTOCOMMIT |
                    OPTION_GTID_BEGIN);
     thd->transaction->all.modified_non_trans_table= false;
     thd->transaction->all.m_unsafe_rollback_flags&= ~THD_TRANS::DID_WAIT;
diff --git a/sql/transaction.cc b/sql/transaction.cc
index b1e98be56bc..123f9283d23 100644
--- a/sql/transaction.cc
+++ b/sql/transaction.cc
@@ -133,7 +133,7 @@ bool trans_begin(THD *thd, uint flags)
 #endif /* WITH_WSREP */
   }
 
-  thd->variables.option_bits&= ~(OPTION_BEGIN | OPTION_KEEP_LOG);
+  thd->variables.option_bits&= ~(OPTION_BEGIN | OPTION_BINLOG_THIS_TRX);
 
   /*
     The following set should not be needed as transaction state should
@@ -280,7 +280,7 @@ bool trans_commit(THD *thd)
   else
     repl_semisync_master.wait_after_commit(thd, FALSE);
 #endif
-  thd->variables.option_bits&= ~(OPTION_BEGIN | OPTION_KEEP_LOG);
+  thd->variables.option_bits&= ~(OPTION_BEGIN | OPTION_BINLOG_THIS_TRX);
   thd->transaction->all.reset();
   thd->lex->start_transaction_opt= 0;
 
@@ -329,7 +329,7 @@ bool trans_commit_implicit(THD *thd)
     res= MY_TEST(ha_commit_trans(thd, TRUE));
   }
 
-  thd->variables.option_bits&= ~(OPTION_BEGIN | OPTION_KEEP_LOG);
+  thd->variables.option_bits&= ~(OPTION_BEGIN | OPTION_BINLOG_THIS_TRX);
   thd->transaction->all.reset();
 
   /* The transaction should be marked as complete in P_S. */
@@ -374,7 +374,7 @@ bool trans_rollback(THD *thd)
   repl_semisync_master.wait_after_rollback(thd, FALSE);
 #endif
   /* Reset the binlog transaction marker */
-  thd->variables.option_bits&= ~(OPTION_BEGIN | OPTION_KEEP_LOG |
+  thd->variables.option_bits&= ~(OPTION_BEGIN | OPTION_BINLOG_THIS_TRX |
                                  OPTION_GTID_BEGIN);
   thd->transaction->all.reset();
   thd->lex->start_transaction_opt= 0;
@@ -424,7 +424,7 @@ bool trans_rollback_implicit(THD *thd)
     of new transacton in @@autocommit=1 mode. This is necessary to
     preserve backward compatibility.
   */
-  thd->variables.option_bits&= ~(OPTION_KEEP_LOG);
+  thd->variables.option_bits&= ~(OPTION_BINLOG_THIS_TRX);
   thd->transaction->all.reset();
 
   /* Rollback should clear transaction_rollback_request flag. */
@@ -669,7 +669,7 @@ bool trans_rollback_to_savepoint(THD *thd, LEX_CSTRING name)
 
   if (ha_rollback_to_savepoint(thd, sv))
     res= TRUE;
-  else if (((thd->variables.option_bits & OPTION_KEEP_LOG) ||
+  else if (((thd->variables.option_bits & OPTION_BINLOG_THIS_TRX) ||
             thd->transaction->all.modified_non_trans_table) &&
            !thd->slave_thread)
     push_warning(thd, Sql_condition::WARN_LEVEL_WARN,
diff --git a/sql/xa.cc b/sql/xa.cc
index af7c7388c57..457aacfeb30 100644
--- a/sql/xa.cc
+++ b/sql/xa.cc
@@ -396,7 +396,7 @@ bool xa_trans_force_rollback(THD *thd)
     rc= true;
   }
   thd->variables.option_bits&=
-    ~(OPTION_BEGIN | OPTION_KEEP_LOG | OPTION_GTID_BEGIN);
+    ~(OPTION_BEGIN | OPTION_BINLOG_THIS_TRX | OPTION_GTID_BEGIN);
   thd->transaction->all.reset();
   thd->server_status&=
     ~(SERVER_STATUS_IN_TRANS | SERVER_STATUS_IN_TRANS_READONLY);
@@ -533,7 +533,7 @@ bool trans_xa_prepare(THD *thd)
     {
       if (!mdl_request.ticket)
         ha_rollback_trans(thd, TRUE);
-      thd->variables.option_bits&= ~(OPTION_BEGIN | OPTION_KEEP_LOG);
+      thd->variables.option_bits&= ~(OPTION_BEGIN | OPTION_BINLOG_THIS_TRX);
       thd->transaction->all.reset();
       thd->server_status&=
         ~(SERVER_STATUS_IN_TRANS | SERVER_STATUS_IN_TRANS_READONLY);
@@ -713,7 +713,7 @@ bool trans_xa_commit(THD *thd)
     DBUG_RETURN(TRUE);
   }
 
-  thd->variables.option_bits&= ~(OPTION_BEGIN | OPTION_KEEP_LOG);
+  thd->variables.option_bits&= ~(OPTION_BEGIN | OPTION_BINLOG_THIS_TRX);
   thd->transaction->all.reset();
   thd->server_status&=
     ~(SERVER_STATUS_IN_TRANS | SERVER_STATUS_IN_TRANS_READONLY);
@@ -1080,7 +1080,7 @@ bool mysql_xa_recover(THD *thd)
 
 static bool slave_applier_reset_xa_trans(THD *thd)
 {
-  thd->variables.option_bits&= ~(OPTION_BEGIN | OPTION_KEEP_LOG);
+  thd->variables.option_bits&= ~(OPTION_BEGIN | OPTION_BINLOG_THIS_TRX);
   thd->server_status&=
     ~(SERVER_STATUS_IN_TRANS | SERVER_STATUS_IN_TRANS_READONLY);
   DBUG_PRINT("info", ("clearing SERVER_STATUS_IN_TRANS"));