(pushing for Andrei)

Bug #27417 thd->no_trans_update.stmt lost value inside of SF-exec-stack
  
Once had been set the flag might later got reset inside of a stored routine 
execution stack.
The reason was in that there was no check if a new statement started at time 
of resetting.
The artifact affects most of binlogable DML queries. Notice, that multi-update 
is wrapped up within
  bug@27716 fix, multi-delete bug@29136.
  
Fixed with saving parent's statement flag of whether the statement modified 
non-transactional table, and unioning (merging) the value with that was gained 
in mysql_execute_command.
  
Resettling thd->no_trans_update members into thd->transaction.`member`;
Asserting code;
Effectively the following properties are held.
  
1. At the end of a substatement thd->transaction.stmt.modified_non_trans_table
   reflects the fact if such a table got modified by the substatement.
   That also respects THD::really_abort_on_warnin() requirements.
2. Eventually thd->transaction.stmt.modified_non_trans_table will be computed as
   the union of the values of all invoked sub-statements.
   That fixes this bug#27417;

Computing of thd->transaction.all.modified_non_trans_table is refined to base to 
the stmt's value for all the case including insert .. select statement which 
before the patch had an extra issue bug@28960.
Minor issues are covered with mysql_load, mysql_delete, and binloggin of insert in
to temp_table select. 
  
The supplied test verifies limitely, mostly asserts. The ultimate testing is defered
for bug@13270, bug@23333.

mysql-test/r/mix_innodb_myisam_binlog.result

@@ -280,3 +280,116 @@ select
 @a like "%#%error_code=0%ROLLBACK/*!*/;%ROLLBACK /* added by mysqlbinlog */;%"	@a not like "%#%error_code=%error_code=%"
 1	1
 drop table t1, t2;
+create temporary table tt (a int unique);
+create table ti (a int) engine=innodb;
+reset master;
+show master status;
+File	Position	Binlog_Do_DB	Binlog_Ignore_DB
+master-bin.000001	98		
+begin;
+insert into ti values (1);
+insert into ti values (2) ;
+insert into tt select * from ti;
+rollback;
+Warnings:
+Warning	1196	Some non-transactional changed tables couldn't be rolled back
+select count(*) from tt /* 2 */;
+count(*)
+2
+show master status;
+File	Position	Binlog_Do_DB	Binlog_Ignore_DB
+master-bin.000001	507		
+show binlog events from 98;
+Log_name	Pos	Event_type	Server_id	End_log_pos	Info
+master-bin.000001	#	Query	1	#	use `test`; BEGIN
+master-bin.000001	#	Query	1	#	use `test`; insert into ti values (1)
+master-bin.000001	#	Query	1	#	use `test`; insert into ti values (2)
+master-bin.000001	#	Query	1	#	use `test`; insert into tt select * from ti
+master-bin.000001	#	Query	1	#	use `test`; ROLLBACK
+select count(*) from ti /* zero */;
+count(*)
+0
+insert into ti select * from tt;
+select * from ti /* that is what slave would miss - a bug */;
+a
+1
+2
+delete from ti;
+delete from tt where a=1;
+reset master;
+show master status;
+File	Position	Binlog_Do_DB	Binlog_Ignore_DB
+master-bin.000001	98		
+begin;
+insert into ti values (1);
+insert into ti values (2) /* to make the dup error in the following */;
+insert into tt select * from ti /* one affected and error */;
+ERROR 23000: Duplicate entry '2' for key 1
+rollback;
+Warnings:
+Warning	1196	Some non-transactional changed tables couldn't be rolled back
+show master status;
+File	Position	Binlog_Do_DB	Binlog_Ignore_DB
+master-bin.000001	581		
+show binlog events from 98;
+Log_name	Pos	Event_type	Server_id	End_log_pos	Info
+master-bin.000001	#	Query	1	#	use `test`; BEGIN
+master-bin.000001	#	Query	1	#	use `test`; insert into ti values (1)
+master-bin.000001	#	Query	1	#	use `test`; insert into ti values (2) /* to make the dup error in the following */
+master-bin.000001	#	Query	1	#	use `test`; insert into tt select * from ti /* one affected and error */
+master-bin.000001	#	Query	1	#	use `test`; ROLLBACK
+select count(*) from ti /* zero */;
+count(*)
+0
+insert into ti select * from tt;
+select * from tt /* that is what otherwise slave missed - the bug */;
+a
+1
+2
+drop table ti;
+drop function if exists bug27417;
+drop table if exists t1,t2;
+CREATE TABLE t1 (a int NOT NULL auto_increment primary key) ENGINE=MyISAM;
+CREATE TABLE t2 (a int NOT NULL auto_increment, PRIMARY KEY (a));
+create function bug27417(n int) 
+RETURNS int(11)
+begin
+insert into t1 values (null);
+return n;
+end|
+reset master;
+insert into t2 values (bug27417(1));
+insert into t2 select bug27417(2);
+reset master;
+insert into t2 values (bug27417(2));
+ERROR 23000: Duplicate entry '2' for key 1
+show master status;
+File	Position	Binlog_Do_DB	Binlog_Ignore_DB
+master-bin.000001	98		
+/* only (!) with fixes for #23333 will show there is the query */;
+select count(*) from t1 /* must be 3 */;
+count(*)
+3
+reset master;
+select count(*) from t2;
+count(*)
+2
+delete from t2 where a=bug27417(3);
+select count(*) from t2 /* nothing got deleted */;
+count(*)
+2
+show master status;
+File	Position	Binlog_Do_DB	Binlog_Ignore_DB
+master-bin.000001	195		
+/* the query must be in regardless of #23333 */;
+select count(*) from t1 /* must be 5 */;
+count(*)
+5
+delete t2 from t2 where t2.a=bug27417(100) /* must not affect t2 */;
+affected rows: 0
+select count(*) from t1 /* must be 7 */;
+count(*)
+7
+drop function bug27417;
+drop table t1,t2;
+end of tests

mysql-test/t/mix_innodb_myisam_binlog.test

@@ -233,8 +233,6 @@ source include/show_binlog_events.inc;
 do release_lock("lock1");
 drop table t0,t2;
 
-# End of 4.1 tests
-
 # Test for BUG#16559 (ROLLBACK should always have a zero error code in
 # binlog). Has to be here and not earlier, as the SELECTs influence
 # XIDs differently between normal and ps-protocol (and SHOW BINLOG
@@ -267,3 +265,123 @@ eval select
 @a like "%#%error_code=0%ROLLBACK/*!*/;%ROLLBACK /* added by mysqlbinlog */;%",
 @a not like "%#%error_code=%error_code=%";
 drop table t1, t2;
+
+#
+# Bug #27417  	thd->no_trans_update.stmt lost value inside of SF-exec-stack
+# bug #28960    non-trans temp table changes with insert .. select
+#               not binlogged after rollback
+#
+# testing appearence of insert into temp_table in binlog.
+# There are two branches of execution that require different setup.
+
+## send_eof() branch
+
+# prepare
+
+create temporary table tt (a int unique);
+create table ti (a int) engine=innodb;
+reset master;
+show master status;
+
+# action
+
+begin; 
+insert into ti values (1); 
+insert into ti values (2) ; 
+insert into tt select * from ti;
+rollback;
+
+# check
+
+select count(*) from tt /* 2 */;
+show master status;
+--replace_column 2 # 5 #
+show binlog events from 98;
+select count(*) from ti /* zero */;
+insert into ti select * from tt;
+select * from ti /* that is what slave would miss - a bug */;
+
+
+## send_error() branch
+delete from ti;
+delete from tt where a=1;
+reset master;
+show master status;
+
+# action
+
+begin; 
+insert into ti values (1); 
+insert into ti values (2) /* to make the dup error in the following */; 
+--error ER_DUP_ENTRY
+insert into tt select * from ti /* one affected and error */;
+rollback;
+
+# check
+
+show master status;
+--replace_column 2 # 5 #
+show binlog events from 98;
+select count(*) from ti /* zero */;
+insert into ti select * from tt;
+select * from tt /* that is what otherwise slave missed - the bug */;
+
+drop table ti;
+
+
+#
+# Bug #27417 thd->no_trans_update.stmt lost value inside of SF-exec-stack
+#
+# Testing asserts: if there is a side effect of modifying non-transactional
+# table thd->no_trans_update.stmt must be TRUE;
+# the assert is active with debug build
+#
+
+--disable_warnings
+drop function if exists bug27417;
+drop table if exists t1,t2;
+--enable_warnings
+# side effect table
+CREATE TABLE t1 (a int NOT NULL auto_increment primary key) ENGINE=MyISAM;
+# target tables
+CREATE TABLE t2 (a int NOT NULL auto_increment, PRIMARY KEY (a));
+
+delimiter |;
+create function bug27417(n int) 
+RETURNS int(11)
+begin
+  insert into t1 values (null);
+  return n;
+end|
+delimiter ;|
+
+reset master;
+
+# execute
+
+insert into t2 values (bug27417(1));
+insert into t2 select bug27417(2);
+reset master;
+
+--error ER_DUP_ENTRY
+insert into t2 values (bug27417(2)); 
+show master status; /* only (!) with fixes for #23333 will show there is the query */;
+select count(*) from t1 /* must be 3 */;
+
+reset master;
+select count(*) from t2;
+delete from t2 where a=bug27417(3);
+select count(*) from t2 /* nothing got deleted */; 
+show master status; /* the query must be in regardless of #23333 */;
+select count(*) from t1 /* must be 5 */;
+
+--enable_info
+delete t2 from t2 where t2.a=bug27417(100) /* must not affect t2 */;
+--disable_info
+select count(*) from t1 /* must be 7 */;
+
+drop function bug27417;
+drop table t1,t2;
+
+--echo end of tests
+

sql/ha_ndbcluster.cc

@@ -3732,7 +3732,7 @@ int ha_ndbcluster::external_lock(THD *thd, int lock_type)
     {
       m_transaction_on= FALSE;
       /* Would be simpler if has_transactions() didn't always say "yes" */
-      thd->no_trans_update.all= thd->no_trans_update.stmt= TRUE;
+      thd->transaction.all.modified_non_trans_table= thd->transaction.stmt.modified_non_trans_table= TRUE;
     }
     else if (!thd->transaction.on)
       m_transaction_on= FALSE;

sql/handler.cc

@@ -830,7 +830,7 @@ int ha_rollback_trans(THD *thd, bool all)
     the error log; but we don't want users to wonder why they have this
     message in the error log, so we don't send it.
   */
-  if (is_real_trans && thd->no_trans_update.all &&
+  if (is_real_trans && thd->transaction.all.modified_non_trans_table &&
       !thd->slave_thread)
     push_warning(thd, MYSQL_ERROR::WARN_LEVEL_WARN,
                  ER_WARNING_NOT_COMPLETE_ROLLBACK,

sql/handler.h

@@ -420,6 +420,35 @@ typedef struct st_thd_trans
   bool        no_2pc;
   /* storage engines that registered themselves for this transaction */
   handlerton *ht[MAX_HA];
+  /* 
+    The purpose of this flag is to keep track of non-transactional
+    tables that were modified in scope of:
+    - transaction, when the variable is a member of
+    THD::transaction.all
+    - top-level statement or sub-statement, when the variable is a
+    member of THD::transaction.stmt
+    This member has the following life cycle:
+    * stmt.modified_non_trans_table is used to keep track of
+    modified non-transactional tables of top-level statements. At
+    the end of the previous statement and at the beginning of the session,
+    it is reset to FALSE.  If such functions
+    as mysql_insert, mysql_update, mysql_delete etc modify a
+    non-transactional table, they set this flag to TRUE.  At the
+    end of the statement, the value of stmt.modified_non_trans_table 
+    is merged with all.modified_non_trans_table and gets reset.
+    * all.modified_non_trans_table is reset at the end of transaction
+    
+    * Since we do not have a dedicated context for execution of a
+    sub-statement, to keep track of non-transactional changes in a
+    sub-statement, we re-use stmt.modified_non_trans_table. 
+    At entrance into a sub-statement, a copy of the value of
+    stmt.modified_non_trans_table (containing the changes of the
+    outer statement) is saved on stack. Then 
+    stmt.modified_non_trans_table is reset to FALSE and the
+    substatement is executed. Then the new value is merged with the
+    saved value.
+  */
+  bool modified_non_trans_table;
 } THD_TRANS;
 
 enum enum_tx_isolation { ISO_READ_UNCOMMITTED, ISO_READ_COMMITTED,

sql/log.cc

@@ -162,7 +162,7 @@ static int binlog_rollback(THD *thd, bool all)
     table. Such cases should be rare (updating a
     non-transactional table inside a transaction...)
   */
-  if (unlikely(thd->no_trans_update.all))
+  if (unlikely(thd->transaction.all.modified_non_trans_table))
   {
     Query_log_event qev(thd, STRING_WITH_LEN("ROLLBACK"), TRUE, FALSE);
     qev.error_code= 0; // see comment in MYSQL_LOG::write(THD, IO_CACHE)
@@ -217,7 +217,7 @@ static int binlog_savepoint_rollback(THD *thd, void *sv)
     non-transactional table. Otherwise, truncate the binlog cache starting
     from the SAVEPOINT command.
   */
-  if (unlikely(thd->no_trans_update.all))
+  if (unlikely(thd->transaction.all.modified_non_trans_table))
   {
     Query_log_event qinfo(thd, thd->query, thd->query_length, TRUE, FALSE);
     DBUG_RETURN(mysql_bin_log.write(&qinfo));

sql/set_var.cc

@@ -2901,14 +2901,14 @@ static bool set_option_autocommit(THD *thd, set_var *var)
     {
       /* We changed to auto_commit mode */
       thd->options&= ~(ulong) OPTION_BEGIN;
-      thd->no_trans_update.all= FALSE;
+      thd->transaction.all.modified_non_trans_table= FALSE;
       thd->server_status|= SERVER_STATUS_AUTOCOMMIT;
       if (ha_commit(thd))
 	return 1;
     }
     else
     {
-      thd->no_trans_update.all= FALSE;
+      thd->transaction.all.modified_non_trans_table= FALSE;
       thd->server_status&= ~SERVER_STATUS_AUTOCOMMIT;
     }
   }

sql/sp_head.cc

@@ -337,13 +337,13 @@ sp_eval_expr(THD *thd, Field *result_field, Item **expr_item_ptr)
   
   enum_check_fields save_count_cuted_fields= thd->count_cuted_fields;
   bool save_abort_on_warning= thd->abort_on_warning;
-  bool save_no_trans_update_stmt= thd->no_trans_update.stmt;
+  bool save_stmt_modified_non_trans_table= thd->transaction.stmt.modified_non_trans_table;
 
   thd->count_cuted_fields= CHECK_FIELD_ERROR_FOR_NULL;
   thd->abort_on_warning=
     thd->variables.sql_mode &
     (MODE_STRICT_TRANS_TABLES | MODE_STRICT_ALL_TABLES);
-  thd->no_trans_update.stmt= FALSE;
+  thd->transaction.stmt.modified_non_trans_table= FALSE;
 
   /* Save the value in the field. Convert the value if needed. */
 
@@ -351,7 +351,7 @@ sp_eval_expr(THD *thd, Field *result_field, Item **expr_item_ptr)
 
   thd->count_cuted_fields= save_count_cuted_fields;
   thd->abort_on_warning= save_abort_on_warning;
-  thd->no_trans_update.stmt= save_no_trans_update_stmt;
+  thd->transaction.stmt.modified_non_trans_table= save_stmt_modified_non_trans_table;
 
   if (thd->net.report_error)
   {
@@ -2400,7 +2400,13 @@ sp_lex_keeper::reset_lex_and_exec_core(THD *thd, uint *nextp,
                                        bool open_tables, sp_instr* instr)
 {
   int res= 0;
-
+  /* 
+    The flag is saved at the entry to the following substatement.
+    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;
+  thd->transaction.stmt.modified_non_trans_table= FALSE;
   DBUG_ASSERT(!thd->derived_tables);
   DBUG_ASSERT(thd->change_list.is_empty());
   /*
@@ -2467,7 +2473,11 @@ sp_lex_keeper::reset_lex_and_exec_core(THD *thd, uint *nextp,
   /* Update the state of the active arena. */
   thd->stmt_arena->state= Query_arena::EXECUTED;
 
-
+  /*
+    Merge here with the saved parent's values
+    what is needed from the substatement gained
+  */
+  thd->transaction.stmt.modified_non_trans_table |= parent_modified_non_trans_table;
   /*
     Unlike for PS we should not call Item's destructors for newly created
     items after execution of each instruction in stored routine. This is

sql/sql_class.cc

@@ -339,7 +339,7 @@ void THD::init(void)
   if (variables.sql_mode & MODE_NO_BACKSLASH_ESCAPES)
     server_status|= SERVER_STATUS_NO_BACKSLASH_ESCAPES;
   options= thd_startup_options;
-  no_trans_update.stmt= no_trans_update.all= FALSE;
+  transaction.all.modified_non_trans_table= transaction.stmt.modified_non_trans_table= FALSE;
   open_options=ha_open_options;
   update_lock_default= (variables.low_priority_updates ?
 			TL_WRITE_LOW_PRIORITY :

sql/sql_class.h

@@ -1460,10 +1460,6 @@ public:
   bool	     charset_is_system_charset, charset_is_collation_connection;
   bool       charset_is_character_set_filesystem;
   bool       enable_slow_log;   /* enable slow log for current statement */
-  struct {
-    bool all:1;
-    bool stmt:1;
-  } no_trans_update;
   bool	     abort_on_warning;
   bool 	     got_warning;       /* Set on call to push_warning() */
   bool	     no_warnings_for_error; /* no warnings on call to my_error() */
@@ -1714,7 +1710,7 @@ public:
   inline bool really_abort_on_warning()
   {
     return (abort_on_warning &&
-            (!no_trans_update.stmt ||
+            (!transaction.stmt.modified_non_trans_table ||
              (variables.sql_mode & MODE_STRICT_ALL_TABLES)));
   }
   void set_status_var_init();

sql/sql_delete.cc

@@ -315,6 +315,9 @@ cleanup:
 
   delete select;
   transactional_table= table->file->has_transactions();
+  if (!transactional_table && deleted > 0)
+    thd->transaction.stmt.modified_non_trans_table= TRUE;
+  
   /* See similar binlogging code in sql_update.cc, for comments */
   if ((error < 0) || (deleted && !transactional_table))
   {
@@ -327,9 +330,10 @@ cleanup:
       if (mysql_bin_log.write(&qinfo) && transactional_table)
 	error=1;
     }
-    if (!transactional_table)
-      thd->no_trans_update.all= TRUE;
+    if (thd->transaction.stmt.modified_non_trans_table)
+      thd->transaction.all.modified_non_trans_table= TRUE;
   }
+  DBUG_ASSERT(transactional_table || !deleted || thd->transaction.stmt.modified_non_trans_table);
   free_underlaid_joins(thd, select_lex);
   if (transactional_table)
   {
@@ -642,20 +646,22 @@ bool multi_delete::send_data(List<Item> &values)
       if (table->triggers &&
           table->triggers->process_triggers(thd, TRG_EVENT_DELETE,
                                             TRG_ACTION_BEFORE, FALSE))
-	DBUG_RETURN(1);
+        DBUG_RETURN(1);
       table->status|= STATUS_DELETED;
       if (!(error=table->file->delete_row(table->record[0])))
       {
-	deleted++;
+        deleted++;
+        if (!table->file->has_transactions())
+          thd->transaction.stmt.modified_non_trans_table= TRUE;
         if (table->triggers &&
             table->triggers->process_triggers(thd, TRG_EVENT_DELETE,
                                               TRG_ACTION_AFTER, FALSE))
-	  DBUG_RETURN(1);
+          DBUG_RETURN(1);
       }
       else
       {
-	table->file->print_error(error,MYF(0));
-	DBUG_RETURN(1);
+        table->file->print_error(error,MYF(0));
+        DBUG_RETURN(1);
       }
     }
     else
@@ -705,6 +711,7 @@ void multi_delete::send_error(uint errcode,const char *err)
     error= 1;
     send_eof();
   }
+  DBUG_ASSERT(!normal_tables || !deleted || thd->transaction.stmt.modified_non_trans_table);
   DBUG_VOID_RETURN;
 }
 
@@ -732,6 +739,7 @@ int multi_delete::do_deletes()
   for (; table_being_deleted;
        table_being_deleted= table_being_deleted->next_local, counter++)
   { 
+    ha_rows last_deleted= deleted;
     TABLE *table = table_being_deleted->table;
     if (tempfiles[counter]->get(table))
     {
@@ -769,6 +777,8 @@ int multi_delete::do_deletes()
         break;
       }
     }
+    if (last_deleted != deleted && !table->file->has_transactions())
+      thd->transaction.stmt.modified_non_trans_table= TRUE;
     end_read_record(&info);
     if (thd->killed && !local_error)
       local_error= 1;
@@ -807,7 +817,6 @@ bool multi_delete::send_eof()
   {
     query_cache_invalidate3(thd, delete_tables, 1);
   }
-
   if ((local_error == 0) || (deleted && normal_tables))
   {
     if (mysql_bin_log.is_open())
@@ -819,9 +828,11 @@ bool multi_delete::send_eof()
       if (mysql_bin_log.write(&qinfo) && !normal_tables)
 	local_error=1;  // Log write failed: roll back the SQL statement
     }
-    if (!transactional_tables)
-      thd->no_trans_update.all= TRUE;
+    if (thd->transaction.stmt.modified_non_trans_table)
+      thd->transaction.all.modified_non_trans_table= TRUE;
   }
+  DBUG_ASSERT(!normal_tables || !deleted || thd->transaction.stmt.modified_non_trans_table);
+
   /* Commit or rollback the current SQL statement */
   if (transactional_tables)
     if (ha_autocommit_or_rollback(thd,local_error > 0))

sql/sql_insert.cc

@@ -733,7 +733,6 @@ bool mysql_insert(THD *thd,TABLE_LIST *table_list,
   if (lock_type != TL_WRITE_DELAYED && !thd->prelocked_mode)
     table->file->start_bulk_insert(values_list.elements);
 
-  thd->no_trans_update.stmt= FALSE;
   thd->abort_on_warning= (!ignore && (thd->variables.sql_mode &
                                        (MODE_STRICT_TRANS_TABLES |
                                         MODE_STRICT_ALL_TABLES)));
@@ -907,10 +906,12 @@ bool mysql_insert(THD *thd,TABLE_LIST *table_list,
           if (mysql_bin_log.write(&qinfo) && transactional_table)
             error=1;
         }
-        if (!transactional_table && changed)
-          thd->no_trans_update.all= TRUE;
+        if (thd->transaction.stmt.modified_non_trans_table)
+          thd->transaction.all.modified_non_trans_table= TRUE;
       }
     }
+    DBUG_ASSERT(transactional_table || !changed || 
+                thd->transaction.stmt.modified_non_trans_table);
     if (transactional_table)
       error=ha_autocommit_or_rollback(thd,error);
 
@@ -1311,7 +1312,7 @@ static int last_uniq_key(TABLE *table,uint keynr)
     then both on update triggers will work instead. Similarly both on
     delete triggers will be invoked if we will delete conflicting records.
 
-    Sets thd->no_trans_update.stmt to TRUE if table which is updated didn't have
+    Sets thd->transaction.stmt.modified_non_trans_table to TRUE if table which is updated didn't have
     transactions.
 
   RETURN VALUE
@@ -1478,7 +1479,7 @@ int write_record(THD *thd, TABLE *table,COPY_INFO *info)
             goto err;
           info->deleted++;
           if (!table->file->has_transactions())
-            thd->no_trans_update.stmt= TRUE;
+            thd->transaction.stmt.modified_non_trans_table= TRUE;
           if (table->triggers &&
               table->triggers->process_triggers(thd, TRG_EVENT_DELETE,
                                                 TRG_ACTION_AFTER, TRUE))
@@ -1510,7 +1511,7 @@ ok_or_after_trg_err:
   if (key)
     my_safe_afree(key,table->s->max_unique_length,MAX_KEY_LENGTH);
   if (!table->file->has_transactions())
-    thd->no_trans_update.stmt= TRUE;
+    thd->transaction.stmt.modified_non_trans_table= TRUE;
   DBUG_RETURN(trg_error);
 
 err:
@@ -2790,7 +2791,6 @@ select_insert::prepare(List<Item> &values, SELECT_LEX_UNIT *u)
   }
   if (info.handle_duplicates == DUP_UPDATE)
     table->file->extra(HA_EXTRA_INSERT_WITH_UPDATE);
-  thd->no_trans_update.stmt= FALSE;
   thd->abort_on_warning= (!info.ignore &&
                           (thd->variables.sql_mode &
                            (MODE_STRICT_TRANS_TABLES |
@@ -2927,7 +2927,8 @@ void select_insert::send_error(uint errcode,const char *err)
 
 bool select_insert::send_eof()
 {
-  int error,error2;
+  int error, error2;
+  bool changed, transactional_table= table->file->has_transactions();
   DBUG_ENTER("select_insert::send_eof");
 
   error= (!thd->prelocked_mode) ? table->file->end_bulk_insert():0;
@@ -2939,12 +2940,14 @@ bool select_insert::send_eof()
     and ha_autocommit_or_rollback
   */
 
-  if (info.copied || info.deleted || info.updated)
+  if (changed= (info.copied || info.deleted || info.updated))
   {
     query_cache_invalidate3(thd, table, 1);
-    if (!(table->file->has_transactions() || table->s->tmp_table))
-      thd->no_trans_update.all= TRUE;
+    if (thd->transaction.stmt.modified_non_trans_table)
+      thd->transaction.all.modified_non_trans_table= TRUE;
   }
+  DBUG_ASSERT(transactional_table || !changed || 
+              thd->transaction.stmt.modified_non_trans_table);
 
   if (last_insert_id)
     thd->insert_id(info.copied ? last_insert_id : 0);		// For binary log
@@ -2954,7 +2957,7 @@ bool select_insert::send_eof()
     if (!error)
       thd->clear_error();
     Query_log_event qinfo(thd, thd->query, thd->query_length,
-			  table->file->has_transactions(), FALSE);
+			  transactional_table, FALSE);
     mysql_bin_log.write(&qinfo);
   }
   if ((error2=ha_autocommit_or_rollback(thd,error)) && ! error)
@@ -2980,6 +2983,7 @@ bool select_insert::send_eof()
 
 void select_insert::abort()
 {
+  bool changed, transactional_table;
   DBUG_ENTER("select_insert::abort");
 
   if (!table)
@@ -2990,6 +2994,7 @@ void select_insert::abort()
     */
     DBUG_VOID_RETURN;
   }
+  transactional_table= table->file->has_transactions();
   if (!thd->prelocked_mode)
     table->file->end_bulk_insert();
   /*
@@ -2998,21 +3003,22 @@ void select_insert::abort()
     error while inserting into a MyISAM table) we must write to the binlog (and
     the error code will make the slave stop).
   */
-  if ((info.copied || info.deleted || info.updated) &&
-      !table->file->has_transactions())
+  if ((changed= info.copied || info.deleted || info.updated) &&
+      !transactional_table)
   {
     if (last_insert_id)
       thd->insert_id(last_insert_id);		// For binary log
     if (mysql_bin_log.is_open())
     {
       Query_log_event qinfo(thd, thd->query, thd->query_length,
-                            table->file->has_transactions(), FALSE);
+                            transactional_table, FALSE);
       mysql_bin_log.write(&qinfo);
     }
-    if (!table->s->tmp_table)
-      thd->no_trans_update.all= TRUE;
+    if (thd->transaction.stmt.modified_non_trans_table)
+      thd->transaction.all.modified_non_trans_table= TRUE;
   }
-  if (info.copied || info.deleted || info.updated)
+  DBUG_ASSERT(transactional_table || !changed || thd->transaction.stmt.modified_non_trans_table);
+  if (changed)
   {
     query_cache_invalidate3(thd, table, 1);
   }
@@ -3259,7 +3265,6 @@ select_create::prepare(List<Item> &values, SELECT_LEX_UNIT *u)
     table->file->extra(HA_EXTRA_INSERT_WITH_UPDATE);
   if (!thd->prelocked_mode)
     table->file->start_bulk_insert((ha_rows) 0);
-  thd->no_trans_update.stmt= FALSE;
   thd->abort_on_warning= (!info.ignore &&
                           (thd->variables.sql_mode &
                            (MODE_STRICT_TRANS_TABLES |

sql/sql_load.cc

@@ -377,7 +377,6 @@ bool mysql_load(THD *thd,sql_exchange *ex,TABLE_LIST *table_list,
       table->file->start_bulk_insert((ha_rows) 0);
     table->copy_blobs=1;
 
-    thd->no_trans_update.stmt= FALSE;
     thd->abort_on_warning= (!ignore &&
                             (thd->variables.sql_mode &
                              (MODE_STRICT_TRANS_TABLES |
@@ -411,7 +410,6 @@ bool mysql_load(THD *thd,sql_exchange *ex,TABLE_LIST *table_list,
     ha_autocommit_...
   */
   query_cache_invalidate3(thd, table_list, 0);
-
   if (error)
   {
     if (read_file_from_client)
@@ -466,8 +464,8 @@ bool mysql_load(THD *thd,sql_exchange *ex,TABLE_LIST *table_list,
   sprintf(name, ER(ER_LOAD_INFO), (ulong) info.records, (ulong) info.deleted,
 	  (ulong) (info.records - info.copied), (ulong) thd->cuted_fields);
 
-  if (!transactional_table)
-    thd->no_trans_update.all= TRUE;
+  if (thd->transaction.stmt.modified_non_trans_table)
+    thd->transaction.all.modified_non_trans_table= TRUE;
 #ifndef EMBEDDED_LIBRARY
   if (mysql_bin_log.is_open())
   {
@@ -488,6 +486,8 @@ bool mysql_load(THD *thd,sql_exchange *ex,TABLE_LIST *table_list,
   /* ok to client sent only after binlog write and engine commit */
   send_ok(thd, info.copied + info.deleted, 0L, name);
 err:
+  DBUG_ASSERT(transactional_table || !(info.copied || info.deleted) ||
+              thd->transaction.stmt.modified_non_trans_table);
   if (thd->lock)
   {
     mysql_unlock_tables(thd, thd->lock);
@@ -532,7 +532,7 @@ read_fixed_length(THD *thd, COPY_INFO &info, TABLE_LIST *table_list,
   Item_field *sql_field;
   TABLE *table= table_list->table;
   ulonglong id;
-  bool no_trans_update_stmt, err;
+  bool err;
   DBUG_ENTER("read_fixed_length");
 
   id= 0;
@@ -560,7 +560,6 @@ read_fixed_length(THD *thd, COPY_INFO &info, TABLE_LIST *table_list,
 #ifdef HAVE_purify
     read_info.row_end[0]=0;
 #endif
-    no_trans_update_stmt= !table->file->has_transactions();
 
     restore_record(table, s->default_values);
     /*
@@ -630,7 +629,6 @@ read_fixed_length(THD *thd, COPY_INFO &info, TABLE_LIST *table_list,
     table->auto_increment_field_not_null= FALSE;
     if (err)
       DBUG_RETURN(1);
-    thd->no_trans_update.stmt= no_trans_update_stmt;
    
     /*
       If auto_increment values are used, save the first one for
@@ -673,12 +671,11 @@ read_sep_field(THD *thd, COPY_INFO &info, TABLE_LIST *table_list,
   TABLE *table= table_list->table;
   uint enclosed_length;
   ulonglong id;
-  bool no_trans_update_stmt, err;
+  bool err;
   DBUG_ENTER("read_sep_field");
 
   enclosed_length=enclosed.length();
   id= 0;
-  no_trans_update_stmt= !table->file->has_transactions();
 
   for (;;it.rewind())
   {
@@ -821,7 +818,6 @@ read_sep_field(THD *thd, COPY_INFO &info, TABLE_LIST *table_list,
       We don't need to reset auto-increment field since we are restoring
       its default value at the beginning of each loop iteration.
     */
-    thd->no_trans_update.stmt= no_trans_update_stmt;
     if (read_info.next_line())			// Skip to next line
       break;
     if (read_info.line_cuted)

sql/sql_parse.cc

@@ -149,7 +149,7 @@ static bool end_active_trans(THD *thd)
     if (ha_commit(thd))
       error=1;
     thd->options&= ~(ulong) OPTION_BEGIN;
-    thd->no_trans_update.all= FALSE;
+    thd->transaction.all.modified_non_trans_table= FALSE;
   }
   DBUG_RETURN(error);
 }
@@ -173,7 +173,7 @@ static bool begin_trans(THD *thd)
   else
   {
     LEX *lex= thd->lex;
-    thd->no_trans_update.all= FALSE;
+    thd->transaction.all.modified_non_trans_table= FALSE;
     thd->options|= (ulong) OPTION_BEGIN;
     thd->server_status|= SERVER_STATUS_IN_TRANS;
     if (lex->start_transaction_opt & MYSQL_START_TRANS_OPT_WITH_CONS_SNAPSHOT)
@@ -1471,7 +1471,7 @@ int end_trans(THD *thd, enum enum_mysql_completiontype completion)
     thd->server_status&= ~SERVER_STATUS_IN_TRANS;
     res= ha_commit(thd);
     thd->options&= ~(ulong) OPTION_BEGIN;
-    thd->no_trans_update.all= FALSE;
+    thd->transaction.all.modified_non_trans_table= FALSE;
     break;
   case COMMIT_RELEASE:
     do_release= 1; /* fall through */
@@ -1489,7 +1489,7 @@ int end_trans(THD *thd, enum enum_mysql_completiontype completion)
     if (ha_rollback(thd))
       res= -1;
     thd->options&= ~(ulong) OPTION_BEGIN;
-    thd->no_trans_update.all= FALSE;
+    thd->transaction.all.modified_non_trans_table= FALSE;
     if (!res && (completion == ROLLBACK_AND_CHAIN))
       res= begin_trans(thd);
     break;
@@ -2600,6 +2600,8 @@ mysql_execute_command(THD *thd)
     statistic_increment(thd->status_var.com_stat[lex->sql_command],
                         &LOCK_status);
 
+  DBUG_ASSERT(thd->transaction.stmt.modified_non_trans_table == FALSE);
+  
   switch (lex->sql_command) {
   case SQLCOM_SELECT:
   {
@@ -2937,7 +2939,7 @@ mysql_execute_command(THD *thd)
     else 
     {
       /* So that CREATE TEMPORARY TABLE gets to binlog at commit/rollback */
-      thd->no_trans_update.all= TRUE;
+      thd->transaction.all.modified_non_trans_table= TRUE;
     }
     DBUG_ASSERT(first_table == all_tables && first_table != 0);
     bool link_to_local;
@@ -3720,7 +3722,7 @@ end_with_restore_list:
         lex->drop_if_exists= 1;
 
       /* So that DROP TEMPORARY TABLE gets to binlog at commit/rollback */
-      thd->no_trans_update.all= TRUE;
+      thd->transaction.all.modified_non_trans_table= TRUE;
     }
     /* DDL and binlog write order protected by LOCK_open */
     res= mysql_rm_table(thd, first_table, lex->drop_if_exists,
@@ -4322,7 +4324,7 @@ end_with_restore_list:
         res= TRUE; // cannot happen
       else
       {
-        if (thd->no_trans_update.all &&
+        if (thd->transaction.all.modified_non_trans_table &&
             !thd->slave_thread)
           push_warning(thd, MYSQL_ERROR::WARN_LEVEL_WARN,
                        ER_WARNING_NOT_COMPLETE_ROLLBACK,
@@ -4969,7 +4971,7 @@ create_sp_error:
     thd->transaction.xid_state.xa_state=XA_ACTIVE;
     thd->transaction.xid_state.xid.set(thd->lex->xid);
     xid_cache_insert(&thd->transaction.xid_state);
-    thd->no_trans_update.all= FALSE;
+    thd->transaction.all.modified_non_trans_table= FALSE;
     thd->options|= (ulong) OPTION_BEGIN;
     thd->server_status|= SERVER_STATUS_IN_TRANS;
     send_ok(thd);
@@ -5064,7 +5066,7 @@ create_sp_error:
       break;
     }
     thd->options&= ~(ulong) OPTION_BEGIN;
-    thd->no_trans_update.all= FALSE;
+    thd->transaction.all.modified_non_trans_table= FALSE;
     thd->server_status&= ~SERVER_STATUS_IN_TRANS;
     xid_cache_delete(&thd->transaction.xid_state);
     thd->transaction.xid_state.xa_state=XA_NOTR;
@@ -5095,7 +5097,7 @@ create_sp_error:
     else
       send_ok(thd);
     thd->options&= ~(ulong) OPTION_BEGIN;
-    thd->no_trans_update.all= FALSE;
+    thd->transaction.all.modified_non_trans_table= FALSE;
     thd->server_status&= ~SERVER_STATUS_IN_TRANS;
     xid_cache_delete(&thd->transaction.xid_state);
     thd->transaction.xid_state.xa_state=XA_NOTR;

sql/sql_table.cc

@@ -4091,7 +4091,6 @@ copy_data_between_tables(TABLE *from,TABLE *to,
   alter_table_manage_keys(to, from->file->indexes_are_disabled(), keys_onoff);
 
   /* We can abort alter table for any table type */
-  thd->no_trans_update.stmt= FALSE;
   thd->abort_on_warning= !ignore && test(thd->variables.sql_mode &
                                          (MODE_STRICT_TRANS_TABLES |
                                           MODE_STRICT_ALL_TABLES));

sql/sql_update.cc

@@ -430,7 +430,6 @@ int mysql_update(THD *thd,
   query_id=thd->query_id;
 
   transactional_table= table->file->has_transactions();
-  thd->no_trans_update.stmt= FALSE;
   thd->abort_on_warning= test(!ignore &&
                               (thd->variables.sql_mode &
                                (MODE_STRICT_TRANS_TABLES |
@@ -487,7 +486,6 @@ int mysql_update(THD *thd,
                                             (byte*) table->record[0])))
         {
           updated++;
-          thd->no_trans_update.stmt= !transactional_table;
           
           if (table->triggers &&
               table->triggers->process_triggers(thd, TRG_EVENT_UPDATE,
@@ -522,6 +520,10 @@ int mysql_update(THD *thd,
     thd->row_count++;
   }
 
+  if (!transactional_table && updated > 0)
+    thd->transaction.stmt.modified_non_trans_table= TRUE;
+
+
   /*
     todo bug#27571: to avoid asynchronization of `error' and
     `error_code' of binlog event constructor
@@ -589,9 +591,10 @@ int mysql_update(THD *thd,
       if (mysql_bin_log.write(&qinfo) && transactional_table)
 	error=1;				// Rollback update
     }
-    if (!transactional_table)
-      thd->no_trans_update.all= TRUE;
+    if (thd->transaction.stmt.modified_non_trans_table)
+      thd->transaction.all.modified_non_trans_table= TRUE;
   }
+  DBUG_ASSERT(transactional_table || !updated || thd->transaction.stmt.modified_non_trans_table);
   free_underlaid_joins(thd, select_lex);
   if (transactional_table)
   {
@@ -955,7 +958,6 @@ bool mysql_multi_update(THD *thd,
 				 handle_duplicates, ignore)))
     DBUG_RETURN(TRUE);
 
-  thd->no_trans_update.stmt= FALSE;
   thd->abort_on_warning= test(thd->variables.sql_mode &
                               (MODE_STRICT_TRANS_TABLES |
                                MODE_STRICT_ALL_TABLES));
@@ -1331,9 +1333,7 @@ multi_update::~multi_update()
   if (copy_field)
     delete [] copy_field;
   thd->count_cuted_fields= CHECK_FIELD_IGNORE;		// Restore this setting
-  if (!trans_safe)  // todo: remove since redundant
-    thd->no_trans_update.all= TRUE;
-  DBUG_ASSERT(trans_safe || thd->no_trans_update.all);
+  DBUG_ASSERT(trans_safe || thd->transaction.all.modified_non_trans_table);
 }
 
 
@@ -1426,7 +1426,7 @@ bool multi_update::send_data(List<Item> &not_used_values)
           else
           {
             trans_safe= 0;
-            thd->no_trans_update.stmt= TRUE;
+            thd->transaction.stmt.modified_non_trans_table= TRUE;
           }
           if (table->triggers &&
               table->triggers->process_triggers(thd, TRG_EVENT_UPDATE,
@@ -1489,7 +1489,6 @@ void multi_update::send_error(uint errcode,const char *err)
 
   /* Something already updated so we have to invalidate cache */
   query_cache_invalidate3(thd, update_tables, 1);
-
   /*
     If all tables that has been updated are trans safe then just do rollback.
     If not attempt to do remaining updates.
@@ -1502,7 +1501,7 @@ void multi_update::send_error(uint errcode,const char *err)
   }
   else
   { 
-    DBUG_ASSERT(thd->no_trans_update.stmt);
+    DBUG_ASSERT(thd->transaction.stmt.modified_non_trans_table);
     if (do_update && table_count > 1)
     {
       /* Add warning here */
@@ -1513,7 +1512,7 @@ void multi_update::send_error(uint errcode,const char *err)
       VOID(do_updates(0));
     }
   }
-  if (thd->no_trans_update.stmt)
+  if (thd->transaction.stmt.modified_non_trans_table)
   {
     /*
       The query has to binlog because there's a modified non-transactional table
@@ -1526,9 +1525,9 @@ void multi_update::send_error(uint errcode,const char *err)
       mysql_bin_log.write(&qinfo);
     }
     if (!trans_safe)
-      thd->no_trans_update.all= TRUE;
+      thd->transaction.all.modified_non_trans_table= TRUE;
   }
-  DBUG_ASSERT(trans_safe || !updated || thd->no_trans_update.stmt);
+  DBUG_ASSERT(trans_safe || !updated || thd->transaction.stmt.modified_non_trans_table);
   
   if (transactional_tables)
   {
@@ -1664,7 +1663,7 @@ int multi_update::do_updates(bool from_send_error)
       else
       {
         trans_safe= 0;				// Can't do safe rollback
-        thd->no_trans_update.stmt= TRUE;
+        thd->transaction.stmt.modified_non_trans_table= TRUE;
       }
     }
     (void) table->file->ha_rnd_end();
@@ -1697,7 +1696,7 @@ err2:
     else
     {
       trans_safe= 0;
-      thd->no_trans_update.stmt= TRUE;
+      thd->transaction.stmt.modified_non_trans_table= TRUE;
     }
   }
   DBUG_RETURN(1);
@@ -1722,7 +1721,6 @@ bool multi_update::send_eof()
   {
     query_cache_invalidate3(thd, update_tables, 1);
   }
-
   /*
     Write the SQL statement to the binlog if we updated
     rows and we succeeded or if we updated some non
@@ -1732,8 +1730,9 @@ bool multi_update::send_eof()
     either from the query's list or via a stored routine: bug#13270,23333
   */
 
-  DBUG_ASSERT(trans_safe || !updated || thd->no_trans_update.stmt);
-  if (local_error == 0 || thd->no_trans_update.stmt)
+  DBUG_ASSERT(trans_safe || !updated || 
+              thd->transaction.stmt.modified_non_trans_table);
+  if (local_error == 0 || thd->transaction.stmt.modified_non_trans_table)
   {
     if (mysql_bin_log.is_open())
     {
@@ -1746,8 +1745,8 @@ bool multi_update::send_eof()
       if (mysql_bin_log.write(&qinfo) && trans_safe)
         local_error= 1;				// Rollback update
     }
-    if (!trans_safe)
-      thd->no_trans_update.all= TRUE;
+    if (thd->transaction.stmt.modified_non_trans_table)
+      thd->transaction.all.modified_non_trans_table= TRUE;
   }
 
   if (transactional_tables)