List:Commits« Previous MessageNext Message »
From:guilhem Date:June 7 2006 3:26pm
Subject:bk commit into 5.1 tree (guilhem:1.2176) BUG#19630
View as plain text  
Below is the list of changes that have just been committed into a local
5.1 repository of guilhem. When guilhem 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
  1.2176 06/06/07 17:26:25 guilhem@stripped +5 -0
  For for BUG#19630 "stored function inserting into two auto_increment breaks statement-based binlog":
  a stored function inserting into two such tables may fail to replicate (inserting wrong data in the slave's copy 
  of the second table) if the slave's second table had an internal auto_increment counter different
  fro master's. Because the auto_increment value autogenerated by master for the 2nd table
  does not go into binlog, only the first does, so the slave lack information. To fix this,
  if running in mixed binlogging mode, if the stored function or trigger plans to update
  two different tables both having auto_increment columns, we switch to row-based for the whole function.
  Re-enabling rpl_switch_stm_row_mixed.

  sql/sql_class.h
    1.296 06/06/07 17:26:20 guilhem@stripped +10 -1
    In inside the execution of a stored function or trigger, we shouldn't change from row-based to statement-based
    or vice-versa. Indeed we may have determined at start of function that we need row-based because
    it will update two tables having auto_increment, that decision must hold until all substatements
    are finished (as they may be updating those tables).

  sql/sql_base.cc
    1.326 06/06/07 17:26:19 guilhem@stripped +56 -0
    When we come to locking tables, we have collected all tables used by functions, views and triggers,
    we detect if we're going to update two tables having auto_increment columns. If yes, statement-based
    binlogging won't work (Intvar_log_event records only one insert_id) so, if in mixed binlogging mode, switch to row-based.

  mysql-test/t/rpl_switch_stm_row_mixed.test
    1.5 06/06/07 17:26:19 guilhem@stripped +137 -7
    adding disable_ps_protocol, as running all queries using prep stmts leads to more Table_map
    log event thus a different SHOW BINLOG EVENTS. This test does run many queries as prep stmts
    (see the PREPARE commands in it), so it's ok that it does not run all of them this way.
    Test for BUG#19630.

  mysql-test/t/disabled.def
    1.159 06/06/07 17:26:19 guilhem@stripped +0 -1
    test passed for me in SBR and RBR; referred bug, 18590, was marked duplicate of 18492,
    fixed on May 2nd.

  mysql-test/r/rpl_switch_stm_row_mixed.result
    1.6 06/06/07 17:26:19 guilhem@stripped +230 -0
    testing for the bug; we test that it goes row-based, but only when it needs.
    Without the bugfix, slave and master differed.

# 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:	guilhem
# Host:	gbichot3.local
# Root:	/home/mysql_src/mysql-5.1-new-19630

--- 1.325/sql/sql_base.cc	2006-05-19 16:39:28 +02:00
+++ 1.326/sql/sql_base.cc	2006-06-07 17:26:19 +02:00
@@ -49,6 +49,9 @@
 static void close_old_data_files(THD *thd, TABLE *table, bool abort_locks,
                                  bool send_refresh);
 static bool reopen_table(TABLE *table);
+static void
+try_row_based_if_write_two_tables_with_auto_increment(THD *thd,
+                                                      TABLE_LIST *tables);
 
 
 extern "C" byte *table_cache_key(const byte *record,uint *length,
@@ -3323,6 +3326,7 @@
     {
       thd->in_lock_tables=1;
       thd->options|= OPTION_TABLE_LOCK;
+      try_row_based_if_write_two_tables_with_auto_increment(thd, tables);
     }
 
     if (! (thd->lock= mysql_lock_tables(thd, start, (uint) (ptr - start),
@@ -6365,3 +6369,55 @@
   DBUG_VOID_RETURN;
 }
 
+
+/*
+  If two tables have auto_increment columns and we want to lock those tables
+  with a write lock, switch to row-based binary logging if in mixed binary
+  logging.
+
+  SYNPOSIS
+    try_row_based_if_write_two_tables_with_auto_increment
+    thd		  Thread object
+    tables	  Table list
+
+ NOTES:
+    Call this function only when you have established the list of all tables
+    which you'll want to update (including stored functions, triggers, views
+    inside your statement).
+
+ RETURN
+   void
+*/
+
+static void
+try_row_based_if_write_two_tables_with_auto_increment(THD *thd,
+                                                      TABLE_LIST *tables)
+{
+  if (thd->variables.binlog_format == BINLOG_FORMAT_MIXED)
+  {
+    char *first_table_name= NULL, *first_db;
+    for (TABLE_LIST *table= tables; table; table= table->next_global)
+    {
+      if (table->table->found_next_number_field &&
+          (table->lock_type >= TL_WRITE_ALLOW_WRITE))
+      {
+        if (first_table_name == NULL)
+        {
+          first_table_name= table->table_name;
+          first_db= table->db ?  table->db : thd->db;
+          DBUG_ASSERT(first_db);
+        }
+        else if (strcmp(first_db, (table->db ?  table->db : thd->db)) ||
+                 strcmp(first_table_name, table->table_name))
+        {
+          /*
+            Found 2 different tables to update, each having auto_inc;
+            statement-based binlogging won't work:
+          */
+          thd->set_current_stmt_binlog_row_based_if_mixed();
+          break;
+        }
+      }
+    }
+  }
+}

--- 1.295/sql/sql_class.h	2006-05-23 20:26:11 +02:00
+++ 1.296/sql/sql_class.h	2006-06-07 17:26:20 +02:00
@@ -1408,7 +1408,16 @@
   }
   inline void reset_current_stmt_binlog_row_based()
   {
-    current_stmt_binlog_row_based= test(variables.binlog_format == BINLOG_FORMAT_ROW);
+    /*
+      If in a function/trigger, don't reset after substatement, wait until end
+      of function/trigger. Indeed we may have determined at start of function
+      that we need row-based because it will update two tables having
+      auto_increment, that decision must hold until all substatements are
+      finished (as they may be updating those tables).
+    */
+    if (!prelocked_mode)
+      current_stmt_binlog_row_based=
+        test(variables.binlog_format == BINLOG_FORMAT_ROW);
   }
 #endif /*HAVE_ROW_BASED_REPLICATION*/
 };

--- 1.5/mysql-test/r/rpl_switch_stm_row_mixed.result	2006-03-13 15:34:15 +01:00
+++ 1.6/mysql-test/r/rpl_switch_stm_row_mixed.result	2006-06-07 17:26:19 +02:00
@@ -157,6 +157,171 @@
 select count(*) from t5;
 count(*)
 58
+set binlog_format=mixed;
+drop table t1,t2;
+create table t1 (a int primary key auto_increment);
+create table t2 (a int primary key auto_increment);
+create function f (x int) returns int deterministic
+begin
+insert into t1 values(null);
+insert into t2 values(null);
+return 1;
+end|
+select f(0);
+f(0)
+1
+select * from t1;
+a
+1
+select * from t2;
+a
+1
+use mysqltest1;
+insert into t2 values(2),(3),(4);
+delete from t2 where a>=2;
+select * from t1;
+a
+1
+select * from t2;
+a
+1
+select f(0);
+f(0)
+1
+select * from t1;
+a
+1
+2
+select * from t2;
+a
+1
+2
+select * from t1;
+a
+1
+2
+select * from t2;
+a
+1
+2
+insert into t2 values(3),(4);
+delete from t2 where a>=3;
+prepare stmt1 from 'select f(?)';
+set @nombre=0;
+insert into t1 /* SBB */ values(null);
+execute stmt1 using @number;
+f(?)
+1
+deallocate prepare stmt1;
+select * from t1;
+a
+1
+2
+3
+4
+select * from t2;
+a
+1
+2
+3
+select * from t1;
+a
+1
+2
+3
+4
+select * from t2;
+a
+1
+2
+3
+drop table t1;
+create table t1 (a int, key(a));
+select f(0);
+f(0)
+1
+drop table t1;
+create table t1 (a int primary key auto_increment);
+drop function f;
+truncate table t1;
+truncate table t2;
+create function f1 (x int) returns int deterministic
+begin
+insert into t1 values(null);
+return 1;
+end|
+create function f2 (x int) returns int deterministic
+begin
+insert into t2 values(null);
+return 1;
+end|
+select f1(0),f2(0);
+f1(0)	f2(0)
+1	1
+select * from t1;
+a
+1
+select * from t2;
+a
+1
+insert into t2 values(2),(3),(4);
+delete from t2 where a>=2;
+select * from t1;
+a
+1
+select * from t2;
+a
+1
+select f1(0),f2(0);
+f1(0)	f2(0)
+1	1
+select * from t1;
+a
+1
+2
+select * from t2;
+a
+1
+2
+select * from t1;
+a
+1
+2
+select * from t2;
+a
+1
+2
+drop function f2;
+create function f2 (x int) returns int deterministic
+begin
+declare y int;
+insert into t1 values(null);
+set y = (select count(*) from t2);
+return y;
+end|
+select f1(0),f2(0);
+f1(0)	f2(0)
+1	2
+select * from t1;
+a
+1
+2
+3
+4
+select * from t2;
+a
+1
+2
+select * from t1;
+a
+1
+2
+3
+4
+select * from t2;
+a
+1
+2
 show binlog events from 102;
 Log_name	Pos	Event_type	Server_id	End_log_pos	Info
 master-bin.000001	#	Query	1	#	drop database if exists mysqltest1
@@ -269,4 +434,69 @@
 master-bin.000001	#	Write_rows	1	#	table_id: # flags: STMT_END_F
 master-bin.000001	#	Table_map	1	#	table_id: # (mysqltest1.t1)
 master-bin.000001	#	Write_rows	1	#	table_id: # flags: STMT_END_F
+master-bin.000001	#	Query	1	#	use `mysqltest1`; drop table t1,t2
+master-bin.000001	#	Query	1	#	use `mysqltest1`; create table t1 (a int primary key auto_increment)
+master-bin.000001	#	Query	1	#	use `mysqltest1`; create table t2 (a int primary key auto_increment)
+master-bin.000001	#	Query	1	#	use `mysqltest1`; CREATE DEFINER=`root`@`localhost` function f (x int) returns int deterministic
+begin
+insert into t1 values(null);
+insert into t2 values(null);
+return 1;
+end
+master-bin.000001	#	Table_map	1	#	table_id: # (mysqltest1.t2)
+master-bin.000001	#	Table_map	1	#	table_id: # (mysqltest1.t1)
+master-bin.000001	#	Write_rows	1	#	table_id: #
+master-bin.000001	#	Write_rows	1	#	table_id: # flags: STMT_END_F
+master-bin.000001	#	Table_map	1	#	table_id: # (mysqltest1.t2)
+master-bin.000001	#	Table_map	1	#	table_id: # (mysqltest1.t1)
+master-bin.000001	#	Write_rows	1	#	table_id: #
+master-bin.000001	#	Write_rows	1	#	table_id: # flags: STMT_END_F
+master-bin.000001	#	Table_map	1	#	table_id: # (mysqltest1.t2)
+master-bin.000001	#	Table_map	1	#	table_id: # (mysqltest1.t1)
+master-bin.000001	#	Write_rows	1	#	table_id: # flags: STMT_END_F
+master-bin.000001	#	Intvar	1	#	INSERT_ID=3
+master-bin.000001	#	Query	1	#	use `mysqltest1`; insert into t1 /* SBB */ values(null)
+master-bin.000001	#	Table_map	1	#	table_id: # (mysqltest1.t2)
+master-bin.000001	#	Table_map	1	#	table_id: # (mysqltest1.t1)
+master-bin.000001	#	Write_rows	1	#	table_id: #
+master-bin.000001	#	Write_rows	1	#	table_id: # flags: STMT_END_F
+master-bin.000001	#	Query	1	#	use `mysqltest1`; drop table t1
+master-bin.000001	#	Query	1	#	use `mysqltest1`; create table t1 (a int, key(a))
+master-bin.000001	#	Intvar	1	#	INSERT_ID=4
+master-bin.000001	#	Query	1	#	use `mysqltest1`; SELECT `f`(0)
+master-bin.000001	#	Query	1	#	use `mysqltest1`; drop table t1
+master-bin.000001	#	Query	1	#	use `mysqltest1`; create table t1 (a int primary key auto_increment)
+master-bin.000001	#	Query	1	#	use `mysqltest1`; drop function f
+master-bin.000001	#	Query	1	#	use `mysqltest1`; truncate table t1
+master-bin.000001	#	Query	1	#	use `mysqltest1`; truncate table t2
+master-bin.000001	#	Query	1	#	use `mysqltest1`; CREATE DEFINER=`root`@`localhost` function f1 (x int) returns int deterministic
+begin
+insert into t1 values(null);
+return 1;
+end
+master-bin.000001	#	Query	1	#	use `mysqltest1`; CREATE DEFINER=`root`@`localhost` function f2 (x int) returns int deterministic
+begin
+insert into t2 values(null);
+return 1;
+end
+master-bin.000001	#	Table_map	1	#	table_id: # (mysqltest1.t1)
+master-bin.000001	#	Table_map	1	#	table_id: # (mysqltest1.t2)
+master-bin.000001	#	Write_rows	1	#	table_id: #
+master-bin.000001	#	Write_rows	1	#	table_id: # flags: STMT_END_F
+master-bin.000001	#	Table_map	1	#	table_id: # (mysqltest1.t1)
+master-bin.000001	#	Table_map	1	#	table_id: # (mysqltest1.t2)
+master-bin.000001	#	Write_rows	1	#	table_id: #
+master-bin.000001	#	Write_rows	1	#	table_id: # flags: STMT_END_F
+master-bin.000001	#	Query	1	#	use `mysqltest1`; drop function f2
+master-bin.000001	#	Query	1	#	use `mysqltest1`; CREATE DEFINER=`root`@`localhost` function f2 (x int) returns int deterministic
+begin
+declare y int;
+insert into t1 values(null);
+set y = (select count(*) from t2);
+return y;
+end
+master-bin.000001	#	Intvar	1	#	INSERT_ID=3
+master-bin.000001	#	Query	1	#	use `mysqltest1`; SELECT `f1`(0)
+master-bin.000001	#	Intvar	1	#	INSERT_ID=4
+master-bin.000001	#	Query	1	#	use `mysqltest1`; SELECT `f2`(0)
 drop database mysqltest1;

--- 1.4/mysql-test/t/rpl_switch_stm_row_mixed.test	2006-03-13 15:34:15 +01:00
+++ 1.5/mysql-test/t/rpl_switch_stm_row_mixed.test	2006-06-07 17:26:19 +02:00
@@ -1,6 +1,7 @@
 -- source include/have_binlog_format_row.inc
 -- source include/master-slave.inc
 
+--disable_ps_protocol
 connection master;
 --disable_warnings
 drop database if exists mysqltest1;
@@ -196,21 +197,150 @@
   select count(*) from t9;
 }
 
---replace_column 2 # 5 #
---replace_regex /table_id: [0-9]+/table_id: #/
-show binlog events from 102;
 sync_slave_with_master;
 # as we're using UUID we don't SELECT but use "diff" like in rpl_row_UUID
 --exec $MYSQL_DUMP --compact --order-by-primary --skip-extended-insert --no-create-info mysqltest1 > $MYSQLTEST_VARDIR/tmp/rpl_switch_stm_row_mixed_master.sql
 --exec $MYSQL_DUMP_SLAVE --compact --order-by-primary --skip-extended-insert --no-create-info mysqltest1 > $MYSQLTEST_VARDIR/tmp/rpl_switch_stm_row_mixed_slave.sql
 
-connection master;
-drop database mysqltest1;
-sync_slave_with_master;
-
 # Let's compare. Note: If they match test will pass, if they do not match
 # the test will show that the diff statement failed and not reject file
 # will be created. You will need to go to the mysql-test dir and diff
 # the files your self to see what is not matching
 
 --exec diff $MYSQLTEST_VARDIR/tmp/rpl_switch_stm_row_mixed_master.sql $MYSQLTEST_VARDIR/tmp/rpl_switch_stm_row_mixed_slave.sql;
+
+# This tests the fix to
+# BUG#19630 stored function inserting into two auto_increment breaks statement-based binlog
+# We verify that under the mixed binlog mode, a stored function
+# modifying at least two tables having an auto_increment column,
+# is binlogged row-based. Indeed in statement-based binlogging,
+# only the auto_increment value generated for the first table
+# is recorded in the binlog, the value generated for the 2nd table
+# lacking.
+
+connection master;
+set binlog_format=mixed;
+drop table t1,t2;
+create table t1 (a int primary key auto_increment);
+create table t2 (a int primary key auto_increment);
+delimiter |;
+create function f (x int) returns int deterministic
+begin
+ insert into t1 values(null);
+ insert into t2 values(null);
+ return 1;
+end|
+delimiter ;|
+select f(0);
+select * from t1;
+select * from t2;
+# Two operations which compensate each other except that their net
+# effect is that they advance the auto_increment counter of t2 on slave:
+sync_slave_with_master;
+use mysqltest1;
+insert into t2 values(2),(3),(4);
+delete from t2 where a>=2;
+select * from t1;
+select * from t2;
+
+connection master;
+select f(0);
+select * from t1;
+select * from t2;
+sync_slave_with_master;
+select * from t1;
+select * from t2;
+
+# now use prepared statement and test again, just to see that the RBB
+# mode isn't set at PREPARE but at EXECUTE.
+
+insert into t2 values(3),(4);
+delete from t2 where a>=3;
+
+connection master;
+prepare stmt1 from 'select f(?)';
+set @nombre=0;
+insert into t1 /* SBB */ values(null); # should be SBB
+execute stmt1 using @number; # should be RBB
+deallocate prepare stmt1;
+select * from t1;
+select * from t2;
+sync_slave_with_master;
+select * from t1;
+select * from t2;
+
+# verify that if only one table has auto_inc, it does not trigger RBB
+# (we'll check in binlog further below)
+
+connection master;
+drop table t1;
+create table t1 (a int, key(a));
+select f(0);
+
+# restore table's key
+drop table t1;
+create table t1 (a int primary key auto_increment);
+
+# now test if it's two functions, each of them inserts in one table
+
+drop function f;
+truncate table t1;
+truncate table t2;
+delimiter |;
+create function f1 (x int) returns int deterministic
+begin
+ insert into t1 values(null);
+ return 1;
+end|
+create function f2 (x int) returns int deterministic
+begin
+ insert into t2 values(null);
+ return 1;
+end|
+delimiter ;|
+select f1(0),f2(0);
+select * from t1;
+select * from t2;
+sync_slave_with_master;
+insert into t2 values(2),(3),(4);
+delete from t2 where a>=2;
+select * from t1;
+select * from t2;
+
+connection master;
+select f1(0),f2(0);
+select * from t1;
+select * from t2;
+sync_slave_with_master;
+select * from t1;
+select * from t2;
+
+# verify that if f2 does only read on an auto_inc table, this does not
+# switch to RBB
+connection master;
+drop function f2;
+delimiter |;
+create function f2 (x int) returns int deterministic
+begin
+ declare y int;
+ insert into t1 values(null);
+ set y = (select count(*) from t2);
+ return y;
+end|
+delimiter ;|
+select f1(0),f2(0);
+select * from t1;
+select * from t2;
+sync_slave_with_master;
+select * from t1;
+select * from t2;
+
+connection master;
+--replace_column 2 # 5 #
+--replace_regex /table_id: [0-9]+/table_id: #/
+show binlog events from 102;
+
+connection master;
+drop database mysqltest1;
+sync_slave_with_master;
+--enable_ps_protocol

--- 1.158/mysql-test/t/disabled.def	2006-05-22 21:57:16 +02:00
+++ 1.159/mysql-test/t/disabled.def	2006-06-07 17:26:19 +02:00
@@ -30,7 +30,6 @@
 rpl_ndb_innodb2ndb       : Bug #19710  	Cluster replication to partition table fails on DELETE FROM statement
 rpl_ndb_log              : BUG#18947 2006-03-21 tomas CRBR: order in binlog of create table and insert (on different table) not determ
 rpl_ndb_myisam2ndb       : Bug #19710  	Cluster replication to partition table fails on DELETE FROM statement
-rpl_switch_stm_row_mixed : BUG#18590 2006-03-28 brian
 rpl_row_blob_innodb      : BUG#18980 2006-04-10 kent    Test fails randomly
 rpl_row_func003 	 : BUG#19074 2006-13-04 andrei  test failed
 rpl_row_inexist_tbl      : BUG#18948 2006-03-09 mats    Disabled since patch makes this test wait forever
Thread
bk commit into 5.1 tree (guilhem:1.2176) BUG#19630guilhem7 Jun