List:Commits« Previous MessageNext Message »
From:Andrei Elkin Date:May 14 2007 11:24am
Subject:bk commit into 5.0 tree (aelkin:1.2439) BUG#27716
View as plain text  
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-05-14 12:24:20+03:00, aelkin@stripped
+3 -0
  Bug #27716  	multi-update did partially and has not binlogged
  
  Implementation of mysql_multi_update did not call multi_update::send_error method in
some cases 
  (see the test reported on bug page and test cases in changeset).
  
  Fixed with deploying the method, ::send_error() is refined to get binlogging code which
works whenever 
  there is modified non-transactional table.
  thd->no_trans_update.stmt flag is set in to TRUE to ease testing though being the
beginning of relative 
  bug#27417 fix.

  mysql-test/r/multi_update.result@stripped, 2007-05-14 12:24:15+03:00,
aelkin@stripped +34 -0
    results changed

  mysql-test/t/multi_update.test@stripped, 2007-05-14 12:24:15+03:00,
aelkin@stripped +41 -0
    regression tests added - erred query must be binlogged

  sql/sql_update.cc@stripped, 2007-05-14 12:24:16+03:00,
aelkin@stripped +58 -20
    making a call to multi_update::send_error when mysql_select return an error;
    deploying binlogging inside of the method;
    refining multi_update::send_eof() to make binlogging work when ::do_updates() errs
    and the query must be binlogged with the error. ::send_error() will be called but
    do not do anything;
    thd->no_trans_update.stmt is set to TRUE according to the existed code pattern.
Although this part relates to
    another bugs (bug#27417 etc), it is needed here for testing.
    

# 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-hkibras-fe31f900-164.dhcp.inet.fi
# Root:	/home/elkin/MySQL/TEAM/FIXES/5.0/bug27716-multi_upd_no_binlog

--- 1.215/sql/sql_update.cc	2007-04-12 12:46:07 +03:00
+++ 1.216/sql/sql_update.cc	2007-05-14 12:24:16 +03:00
@@ -914,6 +914,7 @@ bool mysql_multi_update(THD *thd,
                         SELECT_LEX_UNIT *unit, SELECT_LEX *select_lex)
 {
   multi_update *result;
+  bool res;
   DBUG_ENTER("mysql_multi_update");
 
   if (!(result= new multi_update(table_list,
@@ -928,7 +929,7 @@ bool mysql_multi_update(THD *thd,
                                MODE_STRICT_ALL_TABLES));
 
   List<Item> total_list;
-  (void) mysql_select(thd, &select_lex->ref_pointer_array,
+  res= mysql_select(thd, &select_lex->ref_pointer_array,
                       table_list, select_lex->with_wild,
                       total_list,
                       conds, 0, (ORDER *) NULL, (ORDER *)NULL, (Item *) NULL,
@@ -936,6 +937,15 @@ bool mysql_multi_update(THD *thd,
                       options | SELECT_NO_JOIN_CACHE | SELECT_NO_UNLOCK |
                       OPTION_SETUP_TABLES_DONE,
                       result, unit, select_lex);
+  DBUG_PRINT("info",("res: %d  report_error: %d", res,
+                     thd->net.report_error));
+  res|= thd->net.report_error;
+  if (unlikely(res))
+  {
+    /* If we had a another error reported earlier then this will be ignored */
+    result->send_error(ER_UNKNOWN_ERROR, ER(ER_UNKNOWN_ERROR));
+    result->abort();
+  }
   delete result;
   thd->abort_on_warning= 0;
   DBUG_RETURN(FALSE);
@@ -1249,7 +1259,7 @@ multi_update::~multi_update()
   if (copy_field)
     delete [] copy_field;
   thd->count_cuted_fields= CHECK_FIELD_IGNORE;		// Restore this setting
-  if (!trans_safe)
+  if (!trans_safe)  // todo: remove since redundant
     thd->no_trans_update.all= TRUE;
 }
 
@@ -1337,7 +1347,12 @@ bool multi_update::send_data(List<Item> 
         else
         {
           if (!table->file->has_transactions())
+          {
+            trans_safe= 0;
             thd->no_trans_update.stmt= TRUE;
+          }
+          else
+            transactional_tables= 1;
           if (table->triggers &&
               table->triggers->process_triggers(thd, TRG_EVENT_UPDATE,
                                                 TRG_ACTION_AFTER, TRUE))
@@ -1379,8 +1394,8 @@ void multi_update::send_error(uint errco
   my_error(errcode, MYF(0), err);
 
   /* If nothing updated return */
-  if (!updated)
-    return;
+  if (!updated) /* the flag might be dropped in send_eof */
+    return;     /* and then the query has been binlogged */
 
   /* Something already updated so we have to invalidate cache */
   query_cache_invalidate3(thd, update_tables, 1);
@@ -1392,10 +1407,26 @@ void multi_update::send_error(uint errco
 
   if (trans_safe)
     ha_rollback_stmt(thd);
-  else if (do_update && table_count > 1)
-  {
-    /* Add warning here */
-    VOID(do_updates(0));
+  else
+  { 
+    if (do_update && table_count > 1)
+    {
+      /* Add warning here */
+      VOID(do_updates(0));
+    }
+    if (mysql_bin_log.is_open())
+    {
+      Query_log_event qinfo(thd, thd->query, thd->query_length,
+                            transactional_tables, FALSE);
+      mysql_bin_log.write(&qinfo);
+    }
+    thd->no_trans_update.all= TRUE;
+    if (transactional_tables)
+    {
+      if (ha_autocommit_or_rollback(thd, true))
+        my_message(ER_UNKNOWN_ERROR, "An error occured in multi-table update",
+                   MYF(0));
+    }
   }
 }
 
@@ -1484,14 +1515,13 @@ int multi_update::do_updates(bool from_s
           else if (error == VIEW_CHECK_ERROR)
             goto err;
         }
-	if ((local_error=table->file->update_row(table->record[1],
-						 table->record[0])))
-	{
-	  if (!ignore || local_error != HA_ERR_FOUND_DUPP_KEY)
-	    goto err;
-	}
-	updated++;
-
+        if ((local_error=table->file->update_row(table->record[1],
+                                                 table->record[0])))
+        {
+          if (!ignore || local_error != HA_ERR_FOUND_DUPP_KEY)
+            goto err;
+        }
+        updated++;
         if (table->triggers &&
             table->triggers->process_triggers(thd, TRG_EVENT_UPDATE,
                                               TRG_ACTION_AFTER, TRUE))
@@ -1502,9 +1532,12 @@ int multi_update::do_updates(bool from_s
     if (updated != org_updated)
     {
       if (table->file->has_transactions())
-	transactional_tables= 1;
+        transactional_tables= 1;
       else
-	trans_safe= 0;				// Can't do safe rollback
+      {
+        trans_safe= 0;				// Can't do safe rollback
+        thd->no_trans_update.stmt= TRUE;
+      }
     }
     (void) table->file->ha_rnd_end();
     (void) tmp_table->file->ha_rnd_end();
@@ -1527,7 +1560,10 @@ err2:
     if (table->file->has_transactions())
       transactional_tables= 1;
     else
+    {
       trans_safe= 0;
+      thd->no_trans_update.stmt= TRUE;
+    }
   }
   DBUG_RETURN(1);
 }
@@ -1558,12 +1594,14 @@ bool multi_update::send_eof()
     transactional tables.
   */
 
-  if ((local_error == 0) || (updated && !trans_safe))
+  if (local_error == 0 || (updated && !trans_safe) ||
thd->no_trans_update.stmt)
   {
     if (mysql_bin_log.is_open())
     {
       if (local_error == 0)
         thd->clear_error();
+      else
+        updated= 0; /* binlog with an error here, not in ::send_error */
       Query_log_event qinfo(thd, thd->query, thd->query_length,
 			    transactional_tables, FALSE);
       if (mysql_bin_log.write(&qinfo) && trans_safe)
@@ -1581,7 +1619,7 @@ bool multi_update::send_eof()
 
   if (local_error > 0) // if the above log write did not fail ...
   {
-    /* Safety: If we haven't got an error before (should not happen) */
+    /* Safety: If we haven't got an error before (can happen in do_updates) */
     my_message(ER_UNKNOWN_ERROR, "An error occured in multi-table update",
 	       MYF(0));
     return TRUE;

--- 1.45/mysql-test/r/multi_update.result	2006-10-04 14:09:34 +03:00
+++ 1.46/mysql-test/r/multi_update.result	2007-05-14 12:24:15 +03:00
@@ -524,3 +524,37 @@ a
 30
 drop view v1;
 drop table t1, t2;
+CREATE TABLE `t1` (
+`a` int(11) NOT NULL auto_increment,
+`b` int(11) default NULL,
+PRIMARY KEY  (`a`)
+) ENGINE=MyISAM DEFAULT CHARSET=latin1 ;
+CREATE TABLE `t2` (
+`a` int(11) NOT NULL auto_increment,
+`b` int(11) default NULL,
+PRIMARY KEY  (`a`)
+) ENGINE=MyISAM DEFAULT CHARSET=latin1 ;
+insert into t1 values (1,1),(2,2);
+insert into t2 values (1,1),(4,4);
+reset master;
+UPDATE t2,t1 SET t2.a=t1.a+2;
+ERROR 23000: Duplicate entry '3' for key 1
+select * from t2 /* must be (3,1), (4,4) */;
+a	b
+3	1
+4	4
+show master status /* there must be the UPDATE query event */;
+File	Position	Binlog_Do_DB	Binlog_Ignore_DB
+master-bin.000001	189		
+delete from t1;
+delete from t2;
+insert into t1 values (1,2),(3,4),(4,4);
+insert into t2 values (1,2),(3,4),(4,4);
+reset master;
+UPDATE t2,t1  SET t2.a=t2.b where t2.a=t1.a;
+ERROR 23000: Duplicate entry '4' for key 1
+show master status /* there must be the UPDATE query event */;
+File	Position	Binlog_Do_DB	Binlog_Ignore_DB
+master-bin.000001	204		
+drop table t1, t2;
+end of tests

--- 1.49/mysql-test/t/multi_update.test	2006-05-29 18:02:18 +03:00
+++ 1.50/mysql-test/t/multi_update.test	2007-05-14 12:24:15 +03:00
@@ -534,3 +534,44 @@ select * from t1;
 select * from t2;
 drop view v1;
 drop table t1, t2;
+
+#
+# Bug#27716  	multi-update did partially and has not binlogged
+#
+
+CREATE TABLE `t1` (
+  `a` int(11) NOT NULL auto_increment,
+  `b` int(11) default NULL,
+  PRIMARY KEY  (`a`)
+) ENGINE=MyISAM DEFAULT CHARSET=latin1 ;
+
+CREATE TABLE `t2` (
+  `a` int(11) NOT NULL auto_increment,
+  `b` int(11) default NULL,
+  PRIMARY KEY  (`a`)
+) ENGINE=MyISAM DEFAULT CHARSET=latin1 ;
+
+# A. testing multi_update::send_eof() execution branch
+insert into t1 values (1,1),(2,2);
+insert into t2 values (1,1),(4,4);
+reset master;
+--error ER_DUP_ENTRY
+UPDATE t2,t1 SET t2.a=t1.a+2;
+# check
+select * from t2 /* must be (3,1), (4,4) */;
+show master status /* there must be the UPDATE query event */;
+
+# B. testing multi_update::send_error() execution branch
+delete from t1;
+delete from t2;
+insert into t1 values (1,2),(3,4),(4,4);
+insert into t2 values (1,2),(3,4),(4,4);
+reset master;
+--error ER_DUP_ENTRY
+UPDATE t2,t1  SET t2.a=t2.b where t2.a=t1.a;
+show master status /* there must be the UPDATE query event */;
+
+# cleanup bug#27716
+drop table t1, t2;
+
+--echo end of tests
Thread
bk commit into 5.0 tree (aelkin:1.2439) BUG#27716Andrei Elkin14 May