Bug #45677 Slave stops with Duplicate entry for key PRIMARY when using trigger

The problem is that there is only one autoinc value associated with 
the query when binlogging. If more than one autoinc values are used 
in the query, the autoinc values after the first one can be inserted 
wrongly on slave. So these autoinc values can become inconsistent on 
master and slave.

The problem is resolved by marking all the statements that invoke 
a trigger or call a function that updated autoinc fields as unsafe, 
and will switch to row-format in Mixed mode. Actually, the statement 
is safe if just one autoinc value is used in sub-statement, but it's 
impossible to check how many autoinc values are used in sub-statement.)

mysql-test/suite/rpl/r/rpl_auto_increment_update_failure.result:
  Test result for bug#45677
mysql-test/suite/rpl/t/rpl_auto_increment_update_failure.test:
  Added test to verify the following two properties:
  P1) insert/update in an autoinc column causes statement to 
  be logged in row format if binlog_format=mixed
  P2) if binlog_format=mixed, and a trigger or function contains 
      two or more inserts/updates in a table that has an autoinc 
      column, then the slave should not go out of sync, even if 
      there are concurrent transactions.
sql/sql_base.cc:
  Added function 'has_write_table_with_auto_increment' to check 
  if one (or more) write tables have auto_increment columns.
  
  Removed function 'has_two_write_locked_tables_with_auto_increment', 
  because the function is included in function 
  'has_write_table_with_auto_increment'.

mysql-test/extra/rpl_tests/rpl_auto_increment_insert_view.test

+#
+# This test verifies if inserting data into view that invokes a 
+# trigger will make the autoinc values become inconsistent on 
+# master and slave.
+#
+connection master; 
+CREATE TABLE t1(i1 int not null auto_increment, c1 INT, primary key(i1)) engine=innodb;
+CREATE TABLE t2(i1 int not null auto_increment, c2 INT, primary key(i1)) engine=innodb;
+CREATE TABLE t3(i1 int not null auto_increment, a int, primary key(i1)) engine=innodb;
+eval create trigger tr16 $insert_action on t1 for each row insert into t3(a) values(new.c1);
+eval create trigger tr17 $insert_action on t2 for each row insert into t3(a) values(new.c2);
+begin;
+INSERT INTO t1(c1) VALUES (11), (12);
+INSERT INTO t2(c2) VALUES (13), (14);
+
+CREATE VIEW v16 AS SELECT c1, c2 FROM t1, t2;
+
+INSERT INTO v16(c1) VALUES (15),(16);
+INSERT INTO v16(c2) VALUES (17),(18);
+
+connection master1;
+INSERT INTO v16(c1) VALUES (19),(20);
+INSERT INTO v16(c2) VALUES (21),(22);
+
+connection master;
+INSERT INTO v16(c1) VALUES (23), (24);
+INSERT INTO v16(c1) VALUES (25), (26);
+commit;
+sync_slave_with_master;
+--echo #Test if the results are consistent on master and slave
+--echo #for 'INSERT DATA INTO VIEW WHICH INVOKES TRIGGERS'
+let $diff_table_1=master:test.t3;
+let $diff_table_2=slave:test.t3;
+source include/diff_tables.inc;
+
+connection master;
+DROP TABLE t1;
+DROP TABLE t2;
+DROP TABLE t3;
+DROP VIEW  v16;
+sync_slave_with_master;
+
+
+

mysql-test/extra/rpl_tests/rpl_auto_increment_invoke_trigger.test

+#
+# This test verifies if concurrent transactions that invoke a 
+# trigger that inserts more than one values into one or more 
+# tables with an auto_increment column will make the autoinc 
+# values become inconsistent on master and slave.
+#
+
+connection master;
+create table t1(a int, b int) engine=innodb;
+create table t2(i1 int not null auto_increment, a int, primary key(i1)) engine=innodb;
+eval create trigger tr1 $trigger_action on t1 for each row insert into t2(a) values(6);
+
+create table t3(a int, b int) engine=innodb;
+create table t4(i1 int not null auto_increment, a int, primary key(i1)) engine=innodb;
+create table t5(a int) engine=innodb;
+delimiter |;
+eval create trigger tr2 $trigger_action on t3 for each row begin
+    insert into t4(a) values(f1_insert_triggered());
+    insert into t4(a) values(f1_insert_triggered());
+    insert into t5(a) values(8);
+end |
+delimiter ;|
+
+create table t6(i1 int not null auto_increment, a int, primary key(i1)) engine=innodb;
+delimiter //;
+CREATE FUNCTION f1_insert_triggered() RETURNS INTEGER
+BEGIN
+   INSERT INTO t6(a) values(2),(3);
+   RETURN 1;
+END//
+delimiter ;//
+
+begin;
+let $binlog_start= query_get_value("SHOW MASTER STATUS", Position, 1);
+insert into t1(a,b) values(1,1),(2,1);
+insert into t3(a,b) values(1,1),(2,1);
+update t1 set a = a + 5 where b = 1;
+update t3 set a = a + 5 where b = 1;
+delete from t1 where b = 1;
+delete from t3 where b = 1;
+
+connection master1;
+#The default autocommit is set to 1, so the statement is auto committed
+insert into t2(a) values(3);
+insert into t4(a) values(3);
+
+connection master;
+commit;
+insert into t1(a,b) values(4,2);
+insert into t3(a,b) values(4,2);
+update t1 set a = a + 5 where b = 2;
+update t3 set a = a + 5 where b = 2;
+delete from t1 where b = 2;
+delete from t3 where b = 2;
+--echo # To verify if insert/update in an autoinc column causes statement to be logged in row format
+source include/show_binlog_events.inc;
+commit;
+
+connection master;
+sync_slave_with_master;
+--echo #Test if the results are consistent on master and slave
+--echo #for 'INVOKES A TRIGGER with $trigger_action action'
+let $diff_table_1=master:test.t2;
+let $diff_table_2=slave:test.t2;
+source include/diff_tables.inc;
+let $diff_table_1=master:test.t4;
+let $diff_table_2=slave:test.t4;
+source include/diff_tables.inc;
+let $diff_table_1=master:test.t6;
+let $diff_table_2=slave:test.t6;
+source include/diff_tables.inc;
+
+connection master;
+DROP TABLE t1;
+DROP TABLE t2;
+DROP TABLE t3;
+DROP TABLE t4;
+DROP TABLE t5;
+DROP TABLE t6;
+DROP FUNCTION f1_insert_triggered;
+sync_slave_with_master;
+

mysql-test/extra/rpl_tests/rpl_autoinc_func_invokes_trigger.test

+#
+# This test verifies if concurrent transactions that call a 
+# function which invokes a 'after/before insert action' trigger 
+# that inserts more than one values into a table with autoinc 
+# column will make the autoinc values become inconsistent on 
+# master and slave.
+#
+
+connection master;
+create table t1(a int) engine=innodb;
+create table t2(i1 int not null auto_increment, a int, primary key(i1)) engine=innodb;
+create table t3(i1 int not null auto_increment, a int, primary key(i1)) engine=innodb;
+delimiter |;
+CREATE FUNCTION f1_two_inserts_trigger() RETURNS INTEGER
+BEGIN
+   INSERT INTO t2(a) values(2),(3);
+   INSERT INTO t2(a) values(2),(3);
+   RETURN 1;
+END |
+eval create trigger tr11 $insert_action on t2 for each row begin
+    insert into t3(a) values(new.a);
+    insert into t3(a) values(new.a);
+end |
+delimiter ;|
+begin;
+let $binlog_start= query_get_value("SHOW MASTER STATUS", Position, 1);
+insert into t1(a) values(f1_two_inserts_trigger());
+
+connection master1;
+#The default autocommit is set to 1, so the statement is auto committed
+insert into t2(a) values(4),(5);
+
+connection master;
+commit;
+insert into t1(a) values(f1_two_inserts_trigger());
+--echo # To verify if insert/update in an autoinc column causes statement to be logged in row format
+source include/show_binlog_events.inc;
+commit;
+
+connection master;
+sync_slave_with_master;
+--echo #Test if the results are consistent on master and slave
+--echo #for 'CALLS A FUNCTION which INVOKES A TRIGGER with $insert_action action'
+let $diff_table_1=master:test.t2;
+let $diff_table_2=slave:test.t2;
+source include/diff_tables.inc;
+let $diff_table_1=master:test.t3;
+let $diff_table_2=slave:test.t3;
+source include/diff_tables.inc;
+
+connection master;
+drop table t1;
+drop table t2;
+drop table t3;
+drop function f1_two_inserts_trigger;
+sync_slave_with_master;
+

mysql-test/suite/rpl/t/rpl_auto_increment_update_failure.test

+#
+# Bug45677
+# This test verifies the following two properties:
+# P1) insert/update in an autoinc column causes statement to 
+#     be logged in row format if binlog_format=mixed.
+# P2) if binlog_format=mixed, and a trigger or function contains 
+#     two or more inserts/updates in a table that has an autoinc 
+#     column, then the slave should not go out of sync, even if 
+#     there are concurrent transactions.
+# 
+# Property (P1) is tested by executing an insert and an update on 
+# a table that has an autoinc column, and verifying that these 
+# statements result in row events in the binlog.
+# Property (P2) is tested by setting up the test scenario and 
+# verifying that the tables are identical on master and slave.
+#
+
+source include/have_binlog_format_mixed.inc;
+source include/have_innodb.inc;
+source include/master-slave.inc;
+
+--echo # Test case1: INVOKES A TRIGGER with after insert action
+let $trigger_action = after insert;
+source extra/rpl_tests/rpl_auto_increment_invoke_trigger.test;
+
+--echo # Test case2: INVOKES A TRIGGER with before insert action
+let $trigger_action = before insert;
+source extra/rpl_tests/rpl_auto_increment_invoke_trigger.test;
+
+--echo # Test case3: INVOKES A TRIGGER with after update action
+let $trigger_action = after update;
+source extra/rpl_tests/rpl_auto_increment_invoke_trigger.test;
+
+--echo # Test case4: INVOKES A TRIGGER with before update action
+let $trigger_action = before update;
+source extra/rpl_tests/rpl_auto_increment_invoke_trigger.test;
+
+--echo # Test case5: INVOKES A TRIGGER with after delete action
+let $trigger_action = after delete;
+source extra/rpl_tests/rpl_auto_increment_invoke_trigger.test;
+
+--echo # Test case6: INVOKES A TRIGGER with before delete action
+let $trigger_action = before delete;
+source extra/rpl_tests/rpl_auto_increment_invoke_trigger.test;
+
+--echo # Test case7: CALLS A FUNCTION which INVOKES A TRIGGER with after insert action
+let $insert_action = after insert;
+source extra/rpl_tests/rpl_autoinc_func_invokes_trigger.test;
+
+--echo # Test case8: CALLS A FUNCTION which INVOKES A TRIGGER with before insert action
+let $insert_action = before insert;
+source extra/rpl_tests/rpl_autoinc_func_invokes_trigger.test;
+
+--echo # Test case9: INSERT DATA INTO VIEW WHICH INVOKES TRIGGERS with after insert action
+let $insert_action = after insert;
+source extra/rpl_tests/rpl_auto_increment_insert_view.test;
+
+--echo # Test case10: INSERT DATA INTO VIEW WHICH INVOKES TRIGGERS with before insert action
+let $insert_action = before insert;
+source extra/rpl_tests/rpl_auto_increment_insert_view.test;
+
+--echo # Test case11: INVOKES A FUNCTION TO INSERT TWO OR MORE VALUES INTO A TABLE WITH AUTOINC COLUMN
+connection master;
+create table t1(a int) engine=innodb;
+create table t2(i1 int not null auto_increment, a int, primary key(i1)) engine=innodb;
+delimiter //;
+CREATE FUNCTION f1_two_inserts() RETURNS INTEGER
+BEGIN
+   INSERT INTO t2(a) values(2);
+   INSERT INTO t2(a) values(2);
+   RETURN 1;
+END//
+delimiter ;//
+begin;
+insert into t1(a) values(f1_two_inserts());
+
+connection master1;
+#The default autocommit is set to 1, so the statement is auto committed
+insert into t2(a) values(4),(5);
+
+connection master;
+commit;
+insert into t1(a) values(f1_two_inserts());
+commit;
+
+connection master;
+--echo #Test result for INVOKES A FUNCTION TO INSERT TWO OR MORE VALUES on master
+select * from t2 ORDER BY i1;
+
+sync_slave_with_master;
+connection slave;
+--echo #Test result for INVOKES A FUNCTION TO INSERT TWO OR MORE VALUES on slave
+select * from t2 ORDER BY i1;
+
+connection master;
+drop table t1;
+drop table t2;
+drop function f1_two_inserts;
+sync_slave_with_master;
+
+--echo # Test case12: INVOKES A FUNCTION TO UPDATE TWO OR MORE VALUES OF A TABLE WITH AUTOINC COLUMN
+connection master;
+create table t1(a int) engine=innodb;
+create table t2(i1 int not null auto_increment, a int, b int, primary key(i1)) engine=innodb;
+delimiter //;
+CREATE FUNCTION f1_two_updates() RETURNS INTEGER
+BEGIN
+   update t2 set a = a + 5 where b = 1;
+   update t2 set a = a + 5 where b = 2;
+   update t2 set a = a + 5 where b = 3;
+   update t2 set a = a + 5 where b = 4;
+   RETURN 1;
+END//
+delimiter ;//
+
+connection master1;
+#The default autocommit is set to 1, so the statement is auto committed
+insert into t2(a,b) values(1,1);
+insert into t2(a,b) values(2,2);
+insert into t2(a,b) values(3,3);
+insert into t2(a,b) values(4,4);
+insert into t1(a) values(f1_two_updates());
+
+connection master;
+begin;
+insert into t1(a) values(f1_two_updates());
+commit;
+
+connection master;
+--echo #Test result for INVOKES A FUNCTION TO UPDATE TWO OR MORE VALUES on master
+select * from t2 ORDER BY i1;
+
+sync_slave_with_master;
+connection slave;
+--echo #Test result for INVOKES A FUNCTION TO UPDATE TWO OR MORE VALUES on slave
+select * from t2 ORDER BY i1;
+
+connection master;
+drop table t1;
+drop table t2;
+drop function f1_two_updates;
+sync_slave_with_master;
+
+--echo # Test case13: UPDATE MORE THAN ONE TABLES ON TOP-STATEMENT
+connection master;
+create table t1(i1 int not null auto_increment, a int, b int, primary key(i1)) engine=innodb;
+create table t2(i1 int not null auto_increment, a int, b int, primary key(i1)) engine=innodb;
+begin;
+let $binlog_start= query_get_value(SHOW MASTER STATUS, Position, 1);
+insert into t1(a,b) values(1,1),(2,2);
+insert into t2(a,b) values(1,1),(2,2);
+update t1,t2 set t1.a=t1.a+5, t2.a=t2.a+5 where t1.b=t2.b;
+insert into t1(a,b) values(3,3);
+insert into t2(a,b) values(3,3);
+commit;
+--echo # To verify if it works fine when these statements are not be marked as unsafe
+source include/show_binlog_events.inc;
+
+sync_slave_with_master;
+--echo #Test if the results are consistent on master and slave
+--echo #for 'UPDATE MORE THAN ONE TABLES ON TOP-STATEMENT'
+let $diff_table_1=master:test.t1;
+let $diff_table_2=slave:test.t1;
+source include/diff_tables.inc;
+let $diff_table_1=master:test.t2;
+let $diff_table_2=slave:test.t2;
+source include/diff_tables.inc;
+
+connection master;
+drop table t1;
+drop table t2;
+sync_slave_with_master;
+
+--echo # Test case14: INSERT DATA INTO VIEW WHICH INVOLVED MORE THAN ONE TABLES
+connection master;
+CREATE TABLE t1(i1 int not null auto_increment, c1 INT, primary key(i1)) engine=innodb;
+CREATE TABLE t2(i1 int not null auto_increment, c2 INT, primary key(i1)) engine=innodb;
+let $binlog_start= query_get_value(SHOW MASTER STATUS, Position, 1);
+begin;
+INSERT INTO t1(c1) VALUES (11), (12);
+INSERT INTO t2(c2) VALUES (13), (14);
+
+CREATE VIEW v15 AS SELECT c1, c2 FROM t1, t2;
+
+INSERT INTO v15(c1) VALUES (15),(16);
+INSERT INTO v15(c2) VALUES (17),(18);
+
+connection master1;
+INSERT INTO v15(c1) VALUES (19),(20);
+INSERT INTO v15(c2) VALUES (21),(22);
+
+connection master;
+INSERT INTO v15(c1) VALUES (23), (24);
+INSERT INTO v15(c2) VALUES (25), (26);
+commit;
+--echo # To verify if it works fine when these statements are not be marked as unsafe
+source include/show_binlog_events.inc;
+
+sync_slave_with_master;
+--echo #Test if the results are consistent on master and slave
+--echo #for 'INSERT DATA INTO VIEW WHICH INVOLVED MORE THAN ONE TABLES'
+let $diff_table_1=master:test.t1;
+let $diff_table_2=slave:test.t1;
+source include/diff_tables.inc;
+let $diff_table_1=master:test.t2;
+let $diff_table_2=slave:test.t2;
+source include/diff_tables.inc;
+
+connection master;
+drop table t1;
+drop table t2;
+drop view  v15;
+sync_slave_with_master;
+

sql/sql_base.cc

@@ -106,7 +106,7 @@ static bool open_new_frm(THD *thd, TABLE_SHARE *share, const char *alias,
 static void close_old_data_files(THD *thd, TABLE *table, bool morph_locks,
                                  bool send_refresh);
 static bool
-has_two_write_locked_tables_with_auto_increment(TABLE_LIST *tables);
+has_write_table_with_auto_increment(TABLE_LIST *tables);
 
 
 extern "C" uchar *table_cache_key(const uchar *record, size_t *length,
@@ -5277,15 +5277,17 @@ int lock_tables(THD *thd, TABLE_LIST *tables, uint count, bool *need_reopen)
       thd->in_lock_tables=1;
       thd->options|= OPTION_TABLE_LOCK;
       /*
-        If we have >= 2 different tables to update with auto_inc columns,
-        statement-based binlogging won't work. We can solve this problem in
-        mixed mode by switching to row-based binlogging:
+        A query that modifies autoinc column in sub-statement can make the 
+        master and slave inconsistent.
+        We can solve these problems in mixed mode by switching to binlogging 
+        if at least one updated table is used by sub-statement
       */
-      if (thd->variables.binlog_format == BINLOG_FORMAT_MIXED &&
-          has_two_write_locked_tables_with_auto_increment(tables))
+      /* The BINLOG_FORMAT_MIXED judgement is saved for suppressing 
+         warnings, but it will be removed by fixing bug#45827 */
+      if (thd->variables.binlog_format == BINLOG_FORMAT_MIXED && tables && 
+          has_write_table_with_auto_increment(thd->lex->first_not_own_table()))
       {
         thd->lex->set_stmt_unsafe();
-        thd->set_current_stmt_binlog_row_based_if_mixed();
       }
     }
 
@@ -8815,47 +8817,31 @@ void mysql_wait_completed_table(ALTER_PARTITION_PARAM_TYPE *lpt, TABLE *my_table
 
 
 /*
-  Tells if two (or more) tables have auto_increment columns and we want to
-  lock those tables with a write lock.
+  Check if one (or more) write tables have auto_increment columns.
 
-  SYNOPSIS
-    has_two_write_locked_tables_with_auto_increment
-      tables        Table list
+  @param[in] tables Table list
+
+  @retval 0 if at least one write tables has an auto_increment column
+  @retval 1 otherwise
 
   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
-    0  No
-    1  Yes
 */
 
 static bool
-has_two_write_locked_tables_with_auto_increment(TABLE_LIST *tables)
+has_write_table_with_auto_increment(TABLE_LIST *tables)
 {
-  char *first_table_name= NULL, *first_db;
-  LINT_INIT(first_db);
-
   for (TABLE_LIST *table= tables; table; table= table->next_global)
   {
     /* we must do preliminary checks as table->table may be NULL */
     if (!table->placeholder() &&
         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;
-        DBUG_ASSERT(first_db);
-      }
-      else if (strcmp(first_db, table->db) ||
-               strcmp(first_table_name, table->table_name))
-        return 1;
-    }
+      return 1;
   }
+
   return 0;
 }