MDEV-26833 Missed statement rollback in case transaction drops or create temporary table

When transaction creates or drops temporary tables and afterward its statement
faces an error even the transactional table statement's cached ROW
format events get involved into binlog and are visible after the transaction's commit.

Fixed with proper analysis of whether the errored-out statement needs
to be rolled back in binlog.
For instance a fact of already cached CREATE or DROP for temporary
tables by previous statements alone
does not cause to retain the being errored-out statement events in the
cache.
Conversely, if the statement creates or drops a temporary table
itself it can't be rolled back - this rule remains.

mysql-test/extra/rpl_tests/rpl_drop_create_temp_table.inc

@@ -58,7 +58,7 @@ if (`SELECT HEX(@commands) = HEX('configure')`)
   --eval CREATE TEMPORARY TABLE nt_tmp_xx_1 ( id INT ) ENGINE = MyIsam
 
   #
-  # Creates a Temporary N-table that is never dropped.
+  # Creates a Temporary T-table that is never dropped.
   #
   --eval CREATE TEMPORARY TABLE tt_tmp_xx_1 ( id INT ) ENGINE = Innodb
 

mysql-test/extra/rpl_tests/rpl_drop_create_temp_table.test

@@ -483,6 +483,65 @@ SET @commands= 'B N N-Temp T-SELECT-T-Temp N-Temp Te R';
 SET @commands= 'B N N-Temp T-SELECT-T-Temp N-Temp NeT-trig R';
 --source extra/rpl_tests/rpl_drop_create_temp_table.inc
 
+--echo # > MDEV-26833
+--echo # Errored out and rolled back Te statement should not produce any event to binlog
+--echo #   in the following cases:
+
+SET @sav_var = @@session.binlog_direct_non_transactional_updates;
+
+SET @@session.binlog_direct_non_transactional_updates = ON;
+SET @commands= 'B T Drop-Temp-TT-Temp Te C';
+--source extra/rpl_tests/rpl_drop_create_temp_table.inc
+SET @commands= 'Drop-Temp-T-Temp B T Create-T-Temp Te C';
+--source extra/rpl_tests/rpl_drop_create_temp_table.inc
+SET @commands= 'B T Drop-Temp-TN-Temp Te C';
+--source extra/rpl_tests/rpl_drop_create_temp_table.inc
+SET @commands= 'B T N-Temp Te C';
+--source extra/rpl_tests/rpl_drop_create_temp_table.inc
+
+SET @commands= 'B T Drop-Temp-TT-Temp Te R';
+--source extra/rpl_tests/rpl_drop_create_temp_table.inc
+SET @commands= 'Drop-Temp-T-Temp B T Create-T-Temp Te R';
+--source extra/rpl_tests/rpl_drop_create_temp_table.inc
+SET @commands= 'B T Drop-Temp-TN-Temp Te R';
+--source extra/rpl_tests/rpl_drop_create_temp_table.inc
+SET @commands= 'B T N-Temp Te R';
+--source extra/rpl_tests/rpl_drop_create_temp_table.inc
+
+--echo # Non-transactional side effects.
+SET @commands= 'B T N-Temp Te Ne C';
+--source extra/rpl_tests/rpl_drop_create_temp_table.inc
+SET @commands= 'B T N-Temp Te Ne R';
+--source extra/rpl_tests/rpl_drop_create_temp_table.inc
+
+SET @@session.binlog_direct_non_transactional_updates = OFF;
+SET @commands= 'B T Drop-Temp-TT-Temp Te C';
+--source extra/rpl_tests/rpl_drop_create_temp_table.inc
+SET @commands= 'Drop-Temp-T-Temp B T Create-T-Temp Te C';
+--source extra/rpl_tests/rpl_drop_create_temp_table.inc
+SET @commands= 'B T Drop-Temp-TN-Temp Te C';
+--source extra/rpl_tests/rpl_drop_create_temp_table.inc
+SET @commands= 'B T N-Temp Te C';
+--source extra/rpl_tests/rpl_drop_create_temp_table.inc
+
+SET @commands= 'B T Drop-Temp-TT-Temp Te R';
+--source extra/rpl_tests/rpl_drop_create_temp_table.inc
+SET @commands= 'Drop-Temp-T-Temp B T Create-T-Temp Te R';
+--source extra/rpl_tests/rpl_drop_create_temp_table.inc
+SET @commands= 'B T Drop-Temp-TN-Temp Te R';
+--source extra/rpl_tests/rpl_drop_create_temp_table.inc
+SET @commands= 'B T N-Temp Te R';
+--source extra/rpl_tests/rpl_drop_create_temp_table.inc
+
+--echo # Non-transactional side effects.
+SET @commands= 'B T N-Temp Te Ne C';
+--source extra/rpl_tests/rpl_drop_create_temp_table.inc
+SET @commands= 'B T N-Temp Te Ne R';
+--source extra/rpl_tests/rpl_drop_create_temp_table.inc
+
+SET @@session.binlog_direct_non_transactional_updates = @sav_var;
+--echo # < MDEV-26833
+
 --echo ###################################################################################
 --echo #                               CHECK CONSISTENCY
 --echo ###################################################################################
@@ -495,8 +554,7 @@ if (`select @@session.binlog_format != 'STATEMENT'`)
 {
   --exec $MYSQL_DUMP --compact --order-by-primary --skip-extended-insert --no-create-info test > $MYSQLD_DATADIR/test-temporary-master.sql
   --exec $MYSQL_DUMP_SLAVE --compact --order-by-primary --skip-extended-insert --no-create-info test > $MYSQLD_DATADIR/test-temporary-slave.sql
-  # uncomment when anders.song@greatopensource.com-20110105052107-zoab0bsf5a6xxk2y from mysql-5.6 is merged
-  #--diff_files $MYSQLD_DATADIR/test-temporary-master.sql $MYSQLD_DATADIR/test-temporary-slave.sql
+  --diff_files $MYSQLD_DATADIR/test-temporary-master.sql $MYSQLD_DATADIR/test-temporary-slave.sql
 }
 
 --echo #########################################################################

sql/handler.h

@@ -1477,6 +1477,9 @@ struct THD_TRANS
  /*
     Define the type of statemens which cannot be rolled back safely.
     Each type occupies one bit in m_unsafe_rollback_flags.
+    MODIFIED_NON_TRANS_TABLE is limited to mark only the temporary
+    non-transactional table *when* it's cached along with the transactional
+    events; the regular table is covered by the "namesake" bool var.
   */
   static unsigned int const MODIFIED_NON_TRANS_TABLE= 0x01;
   static unsigned int const CREATED_TEMP_TABLE= 0x02;
@@ -1485,6 +1488,14 @@ struct THD_TRANS
   static unsigned int const DID_DDL= 0x10;
   static unsigned int const EXECUTED_TABLE_ADMIN_CMD= 0x20;
 
+  void mark_modified_non_trans_temp_table()
+  {
+    m_unsafe_rollback_flags|= MODIFIED_NON_TRANS_TABLE;
+  }
+  bool has_modified_non_trans_temp_table() const
+  {
+    return (m_unsafe_rollback_flags & MODIFIED_NON_TRANS_TABLE) != 0;
+  }
   void mark_executed_table_admin_cmd()
   {
     DBUG_PRINT("debug", ("mark_executed_table_admin_cmd"));

sql/log.cc

@@ -252,8 +252,9 @@ void make_default_log_name(char **out, const char* log_ext, bool once)
 class binlog_cache_data
 {
 public:
-  binlog_cache_data(): m_pending(0), before_stmt_pos(MY_OFF_T_UNDEF),
-  incident(FALSE), changes_to_non_trans_temp_table_flag(FALSE),
+  binlog_cache_data(): m_pending(0),
+  before_stmt_pos(MY_OFF_T_UNDEF),
+  incident(FALSE),
   saved_max_binlog_cache_size(0), ptr_binlog_cache_use(0),
   ptr_binlog_cache_disk_use(0)
   { }
@@ -289,16 +290,6 @@ class binlog_cache_data
     return(incident);
   }
 
-  void set_changes_to_non_trans_temp_table()
-  {
-    changes_to_non_trans_temp_table_flag= TRUE;    
-  }
-
-  bool changes_to_non_trans_temp_table()
-  {
-    return (changes_to_non_trans_temp_table_flag);    
-  }
-
   void reset()
   {
     compute_statistics();
@@ -306,7 +297,6 @@ class binlog_cache_data
     if(cache_log.file != -1)
       my_chsize(cache_log.file, 0, 0, MYF(MY_WME));
 
-    changes_to_non_trans_temp_table_flag= FALSE;
     incident= FALSE;
     before_stmt_pos= MY_OFF_T_UNDEF;
     /*
@@ -401,12 +391,6 @@ class binlog_cache_data
   */ 
   bool incident;
 
-  /*
-    This flag indicates if the cache has changes to temporary tables.
-    @TODO This a temporary fix and should be removed after BUG#54562.
-  */
-  bool changes_to_non_trans_temp_table_flag;
-
   /**
     This function computes binlog cache and disk usage.
   */
@@ -1942,13 +1926,12 @@ static int binlog_prepare(handlerton *hton, THD *thd, bool all)
 */
 static bool trans_cannot_safely_rollback(THD *thd, bool all)
 {
-  binlog_cache_mngr *const cache_mngr=
-    (binlog_cache_mngr*) thd_get_ha_data(thd, binlog_hton);
+  DBUG_ASSERT(ending_trans(thd, all));
 
   return ((thd->variables.option_bits & OPTION_KEEP_LOG) ||
           (trans_has_updated_non_trans_table(thd) &&
            thd->wsrep_binlog_format() == BINLOG_FORMAT_STMT) ||
-          (cache_mngr->trx_cache.changes_to_non_trans_temp_table() &&
+          (thd->transaction.all.has_modified_non_trans_temp_table() &&
            thd->wsrep_binlog_format() == BINLOG_FORMAT_MIXED) ||
           (trans_has_updated_non_trans_table(thd) &&
            ending_single_stmt_trans(thd,all) &&
@@ -2092,17 +2075,19 @@ static int binlog_rollback(handlerton *hton, THD *thd, bool all)
     /*
       Truncate the cache if:
         . aborting a single or multi-statement transaction or;
-        . the OPTION_KEEP_LOG is not active and;
+        . the current statement created or dropped a temporary table
+          while having actual STATEMENT format;
         . the format is not STMT or no non-trans table was
           updated and;
         . the format is not MIXED or no temporary non-trans table
           was updated.
     */
     else if (ending_trans(thd, all) ||
-             (!(thd->variables.option_bits & OPTION_KEEP_LOG) &&
+             (!(thd->transaction.stmt.has_created_dropped_temp_table() &&
+                !thd->is_current_stmt_binlog_format_row()) &&
               (!stmt_has_updated_non_trans_table(thd) ||
                thd->wsrep_binlog_format() != BINLOG_FORMAT_STMT) &&
-              (!cache_mngr->trx_cache.changes_to_non_trans_temp_table() ||
+              (!thd->transaction.stmt.has_modified_non_trans_temp_table() ||
                thd->wsrep_binlog_format() != BINLOG_FORMAT_MIXED)))
       error= binlog_truncate_trx_cache(thd, cache_mngr, all);
   }
@@ -6276,9 +6261,8 @@ bool MYSQL_BIN_LOG::write(Log_event *event_info, my_bool *with_annotate)
       file= cache_mngr->get_binlog_cache_log(is_trans_cache);
       cache_data= cache_mngr->get_binlog_cache_data(is_trans_cache);
 
-      if (thd->lex->stmt_accessed_non_trans_temp_table())
-        cache_data->set_changes_to_non_trans_temp_table();
-
+      if (thd->lex->stmt_accessed_non_trans_temp_table() && is_trans_cache)
+        thd->transaction.stmt.mark_modified_non_trans_temp_table();
       thd->binlog_start_trans_and_stmt();
     }
     DBUG_PRINT("info",("event type: %d",event_info->get_type_code()));

sql/sp_head.cc

@@ -3040,8 +3040,13 @@ sp_lex_keeper::reset_lex_and_exec_core(THD *thd, uint *nextp,
     It's reset further in the common code part.
     It's merged with the saved parent's value at the exit of this func.
   */
-  bool parent_modified_non_trans_table= thd->transaction.stmt.modified_non_trans_table;
+  bool parent_modified_non_trans_table=
+    thd->transaction.stmt.modified_non_trans_table;
+  unsigned int parent_unsafe_rollback_flags=
+    thd->transaction.stmt.m_unsafe_rollback_flags;
   thd->transaction.stmt.modified_non_trans_table= FALSE;
+  thd->transaction.stmt.m_unsafe_rollback_flags= 0;
+
   DBUG_ASSERT(!thd->derived_tables);
   DBUG_ASSERT(thd->Item_change_list::is_empty());
   /*
@@ -3162,6 +3167,7 @@ sp_lex_keeper::reset_lex_and_exec_core(THD *thd, uint *nextp,
     what is needed from the substatement gained
   */
   thd->transaction.stmt.modified_non_trans_table |= parent_modified_non_trans_table;
+  thd->transaction.stmt.m_unsafe_rollback_flags |= parent_unsafe_rollback_flags;
 
   TRANSACT_TRACKER(add_trx_state_from_thd(thd));
 

sql/sql_class.h

@@ -4524,7 +4524,8 @@ class THD :public Statement,
       transaction.all.modified_non_trans_table= TRUE;
     transaction.all.m_unsafe_rollback_flags|=
       (transaction.stmt.m_unsafe_rollback_flags &
-       (THD_TRANS::DID_WAIT | THD_TRANS::CREATED_TEMP_TABLE |
+       (THD_TRANS::MODIFIED_NON_TRANS_TABLE |
+        THD_TRANS::DID_WAIT | THD_TRANS::CREATED_TEMP_TABLE |
         THD_TRANS::DROPPED_TEMP_TABLE | THD_TRANS::DID_DDL |
         THD_TRANS::EXECUTED_TABLE_ADMIN_CMD));
   }