bk commit into 5.0 tree (aelkin:1.2439) BUG#27417

[Andrei Elkin]

Below is the list of changes that have just been committed into a local
5.0 repository of elkin. When elkin does a push these changes will
be propagated to the main repository and, within 24 hours after the
push, to the public repository.
For information on how to access the public repository
see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html

ChangeSet@stripped, 2007-06-06 16:55:00+03:00, aelkin@stripped
+9 -0
  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.
  
  Fixed with introducing Sub_statement_state::no_trans_update_stmt flag durable within
substatement life time.
  At the end of substatement the gained value contributes to thd->no_trans_update.stmt
so that on the bottom
  line are:
  
  1. Eventually thd->no_trans_update.stmt will be propagated out of the
     last substatement as the union of values worked out in all
     sub-statements                  -  fixed bug#27417
  
  2. Inside of any sub-statement thd->no_trans_update.stmt corresponds
     to the value yielded at the sub-statement to satisfy
     THD::really_abort_on_warnin() concern to get the status of the current
     substatement.
  
  The supplied test verifies limitely, mostly asserts. The ultimate testing is defered
  for bug_13270, bug_23333.

  mysql-test/r/stored_routines_side_effect.result@stripped, 2007-06-06 16:54:54+03:00,
aelkin@stripped +26 -0
    new result file

  mysql-test/r/stored_routines_side_effect.result@stripped, 2007-06-06 16:54:54+03:00,
aelkin@stripped +0 -0

  mysql-test/t/sp_trans.test@stripped, 2007-06-06 16:54:52+03:00,
aelkin@stripped +1 -0
    not-related test clean-up spotted and fixed

  mysql-test/t/stored_routines_side_effect.test@stripped, 2007-06-06 16:54:53+03:00,
aelkin@stripped +56 -0
    the test basicly to check asserts deployed with the patch.

  mysql-test/t/stored_routines_side_effect.test@stripped, 2007-06-06 16:54:53+03:00,
aelkin@stripped +0 -0

  sql/sql_class.cc@stripped, 2007-06-06 16:54:52+03:00,
aelkin@stripped +3 -0
    backing up and resetting .stmt flag for a new starting statement;
    restoring the flag as the union with the value yieded by the current sub-statement;

  sql/sql_class.h@stripped, 2007-06-06 16:54:53+03:00,
aelkin@stripped +5 -0
    adding a member Sub_statement_state

  sql/sql_delete.cc@stripped, 2007-06-06 16:54:53+03:00,
aelkin@stripped +23 -7
    correcting basic delete incl truncate branch and multi-delete queries to set and reset
    thd->no_trans_update.stmt;

  sql/sql_insert.cc@stripped, 2007-06-06 16:54:53+03:00,
aelkin@stripped +14 -8
    asserts regarding to the .stmt flag

  sql/sql_load.cc@stripped, 2007-06-06 16:54:53+03:00,
aelkin@stripped +4 -6
    asserts regarding to the flag;
    eliminating a separate issue where the flag was stored and re-stored after
write_record
    that could change it and the change would be lost but should remain permanent.

  sql/sql_update.cc@stripped, 2007-06-06 16:54:53+03:00,
aelkin@stripped +5 -1
    correcting update (multi-update part of the issues covered by other bug fix)
    to set and reset the flag;
    assert is added.

# This is a BitKeeper patch.  What follows are the unified diffs for the
# set of deltas contained in the patch.  The rest of the patch, the part
# that BitKeeper cares about, is below these diffs.
# User:	aelkin
# Host:	dsl-hkibras1-ff5dc300-70.dhcp.inet.fi
# Root:	/home/elkin/MySQL/TEAM/FIXES/5.0/bug27417-no_trans_update__stmt

--- 1.269/sql/sql_class.cc	2007-04-20 11:35:22 +03:00
+++ 1.270/sql/sql_class.cc	2007-06-06 16:54:52 +03:00
@@ -2098,6 +2098,8 @@ void THD::reset_sub_statement_state(Sub_
   backup->cuted_fields=     cuted_fields;
   backup->client_capabilities= client_capabilities;
   backup->savepoints= transaction.savepoints;
+  backup->no_trans_update_stmt= no_trans_update.stmt;
+  no_trans_update.stmt= 0;
 
   if (!lex->requires_prelocking() || is_update_query(lex->sql_command))
     options&= ~OPTION_BIN_LOG;
@@ -2160,6 +2162,7 @@ void THD::restore_sub_statement_state(Su
   */
   examined_row_count+= backup->examined_row_count;
   cuted_fields+=       backup->cuted_fields;
+  no_trans_update.stmt= backup->no_trans_update_stmt ||  no_trans_update.stmt;
 }
 
 

--- 1.328/sql/sql_class.h	2007-04-03 14:55:14 +03:00
+++ 1.329/sql/sql_class.h	2007-06-06 16:54:53 +03:00
@@ -1086,6 +1086,11 @@ public:
   bool last_insert_id_used;
   my_bool no_send_ok;
   SAVEPOINT *savepoints;
+  /* 
+     the flag resets at the beginning of substatement and restores as
+     the union with the gained value at the end of the substatement
+  */
+  bool no_trans_update_stmt;
 };
 
 /**

--- 1.197/sql/sql_delete.cc	2007-04-17 16:51:56 +03:00
+++ 1.198/sql/sql_delete.cc	2007-06-06 16:54:53 +03:00
@@ -91,7 +91,7 @@ bool mysql_delete(THD *thd, TABLE_LIST *
   }
 
   select_lex->no_error= thd->lex->ignore;
-
+  transactional_table= table->file->has_transactions();
   /*
     Test if the user wants to delete all rows and deletion doesn't have
     any side-effects (because of triggers), so we can use optimized
@@ -109,8 +109,11 @@ bool mysql_delete(THD *thd, TABLE_LIST *
     if (!(error=table->file->delete_all_rows()))
     {
       error= -1;				// ok
+      if (!transactional_table && deleted > 0)
+        thd->no_trans_update.stmt= TRUE;
       goto cleanup;
     }
+    deleted= 0;
     if (error != HA_ERR_WRONG_COMMAND)
     {
       table->file->print_error(error,MYF(0));
@@ -216,6 +219,8 @@ bool mysql_delete(THD *thd, TABLE_LIST *
   init_ftfuncs(thd, select_lex, 1);
   thd->proc_info="updating";
 
+  thd->no_trans_update.stmt= FALSE;
+
   if (table->triggers)
   {
     table->triggers->mark_fields_used(thd, TRG_EVENT_DELETE);
@@ -280,6 +285,9 @@ bool mysql_delete(THD *thd, TABLE_LIST *
     else
       table->file->unlock_row();  // Row failed selection, release lock on it
   }
+  if (!transactional_table && deleted > 0)
+    thd->no_trans_update.stmt= TRUE;
+
   if (thd->killed && !error)
     error= 1;					// Aborted
   thd->proc_info="end";
@@ -314,7 +322,6 @@ cleanup:
   }
 
   delete select;
-  transactional_table= table->file->has_transactions();
   /* See similar binlogging code in sql_update.cc, for comments */
   if ((error < 0) || (deleted && !transactional_table))
   {
@@ -330,6 +337,7 @@ cleanup:
     if (!transactional_table)
       thd->no_trans_update.all= TRUE;
   }
+  DBUG_ASSERT(transactional_table || !deleted || thd->no_trans_update.stmt);
   free_underlaid_joins(thd, select_lex);
   if (transactional_table)
   {
@@ -526,6 +534,7 @@ multi_delete::initialize_tables(JOIN *jo
   Unique **tempfiles_ptr;
   DBUG_ENTER("initialize_tables");
 
+  thd->no_trans_update.stmt= FALSE;
   if ((thd->options & OPTION_SAFE_UPDATES) && error_if_full_join(join))
     DBUG_RETURN(1);
 
@@ -644,20 +653,22 @@ bool multi_delete::send_data(List<Item> 
       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->no_trans_update.stmt= 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
@@ -734,6 +745,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))
     {
@@ -771,6 +783,8 @@ int multi_delete::do_deletes()
         break;
       }
     }
+    if (last_deleted != deleted && !table->file->has_transactions())
+      thd->no_trans_update.stmt= TRUE;
     end_read_record(&info);
     if (thd->killed && !local_error)
       local_error= 1;
@@ -824,6 +838,8 @@ bool multi_delete::send_eof()
     if (!transactional_tables)
       thd->no_trans_update.all= TRUE;
   }
+  DBUG_ASSERT(!normal_tables || !deleted || thd->no_trans_update.stmt);
+
   /* Commit or rollback the current SQL statement */
   if (transactional_tables)
     if (ha_autocommit_or_rollback(thd,local_error > 0))

--- 1.230/sql/sql_insert.cc	2007-04-12 12:46:07 +03:00
+++ 1.231/sql/sql_insert.cc	2007-06-06 16:54:53 +03:00
@@ -779,6 +779,7 @@ bool mysql_insert(THD *thd,TABLE_LIST *t
           thd->no_trans_update.all= TRUE;
       }
     }
+    DBUG_ASSERT(transactional_table || !changed || thd->no_trans_update.stmt);
     if (transactional_table)
       error=ha_autocommit_or_rollback(thd,error);
 
@@ -2691,6 +2692,7 @@ void select_insert::store_values(List<It
 
 void select_insert::send_error(uint errcode,const char *err)
 {
+  bool changed, transactional_table;
   DBUG_ENTER("select_insert::send_error");
 
   my_message(errcode, err, MYF(0));
@@ -2703,6 +2705,7 @@ void select_insert::send_error(uint errc
     */
     DBUG_VOID_RETURN;
   }
+  transactional_table= table->file->has_transactions();
   if (!thd->prelocked_mode)
     table->file->end_bulk_insert();
   /*
@@ -2711,21 +2714,22 @@ void select_insert::send_error(uint errc
     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 (info.copied || info.deleted || info.updated)
+  DBUG_ASSERT(transactional_table || !changed || thd->no_trans_update.stmt);
+  if (changed)
   {
     query_cache_invalidate3(thd, table, 1);
   }
@@ -2736,7 +2740,8 @@ void select_insert::send_error(uint errc
 
 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;
@@ -2748,12 +2753,13 @@ 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))
+    if (!(transactional_table || table->s->tmp_table))
       thd->no_trans_update.all= TRUE;
   }
+  DBUG_ASSERT(transactional_table || !changed || thd->no_trans_update.stmt);
 
   if (last_insert_id)
     thd->insert_id(info.copied ? last_insert_id : 0);		// For binary log
@@ -2763,7 +2769,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)

--- 1.113/sql/sql_load.cc	2007-05-08 03:43:16 +03:00
+++ 1.114/sql/sql_load.cc	2007-06-06 16:54:53 +03:00
@@ -488,6 +488,8 @@ bool mysql_load(THD *thd,sql_exchange *e
   /* 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->no_trans_update.stmt);
   if (thd->lock)
   {
     mysql_unlock_tables(thd, thd->lock);
@@ -532,7 +534,7 @@ read_fixed_length(THD *thd, COPY_INFO &i
   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 +562,6 @@ read_fixed_length(THD *thd, COPY_INFO &i
 #ifdef HAVE_purify
     read_info.row_end[0]=0;
 #endif
-    no_trans_update_stmt= !table->file->has_transactions();
 
     restore_record(table, s->default_values);
     /*
@@ -628,7 +629,6 @@ read_fixed_length(THD *thd, COPY_INFO &i
     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
@@ -671,12 +671,11 @@ read_sep_field(THD *thd, COPY_INFO &info
   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())
   {
@@ -817,7 +816,6 @@ read_sep_field(THD *thd, COPY_INFO &info
       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)

--- 1.215/sql/sql_update.cc	2007-04-12 12:46:07 +03:00
+++ 1.216/sql/sql_update.cc	2007-06-06 16:54:53 +03:00
@@ -486,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,
@@ -520,6 +519,10 @@ int mysql_update(THD *thd,
       table->file->unlock_row();
     thd->row_count++;
   }
+
+  if (!transactional_table && updated > 0)
+    thd->no_trans_update.stmt= TRUE;
+
   if (thd->killed && !error)
     error= 1;					// Aborted
   end_read_record(&info);
@@ -560,6 +563,7 @@ int mysql_update(THD *thd,
     if (!transactional_table)
       thd->no_trans_update.all= TRUE;
   }
+  DBUG_ASSERT(transactional_table || !updated || thd->no_trans_update.stmt);
   free_underlaid_joins(thd, select_lex);
   if (transactional_table)
   {
--- New file ---
+++ mysql-test/r/stored_routines_side_effect.result	07/06/06 16:54:54
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)
DETERMINISTIC
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		
/* with fixes for #23333 will show there is the query */;
select count(*) from t1 /* must be 3 */;
count(*)
3
drop table t1,t2;
end of tests

--- New file ---
+++ mysql-test/t/stored_routines_side_effect.test	07/06/06 16:54:53
#
# Bug #27417 thd->no_trans_update.stmt lost value inside of SF-exec-stack
#
# Testing the assertion: 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)
DETERMINISTIC
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 table t1,t2;

--echo end of tests


--- 1.12/mysql-test/t/sp_trans.test	2007-03-24 19:19:58 +02:00
+++ 1.13/mysql-test/t/sp_trans.test	2007-06-06 16:54:52 +03:00
@@ -583,6 +583,7 @@ insert into t2 values (bug23333(),1)| 
 --replace_column 2 # 5 # 6 #
 show binlog events from 98 /* with fixes for #23333 will show there is the query */|
 select count(*),@a from t1 /* must be 1,1 */|
+drop function bug23333|
 
 #
 # BUG#NNNN: New bug synopsis