MDEV-17614: After-merge fix

MDEV-17614 flags INSERT…ON DUPLICATE KEY UPDATE unsafe for statement-based
replication when there are multiple unique indexes. This correctly fixes
something whose attempted fix in MySQL 5.7
in mysql/mysql-server@c93b0d9a972cb6f98fd445f2b69d924350f9128a
caused lock conflicts. That change was reverted in MySQL 5.7.26
in mysql/mysql-server@066b6fdd433aa6673622341f1a2f0a3a20018043
(with a substantial amount of other changes).

In MDEV-17073 we already disabled the unfortunate MySQL change when
statement-based replication was not being used. Now, thanks to MDEV-17614,
we can actually remove the change altogether.

This reverts commit 8a346f31 (MDEV-17073)
and mysql/mysql-server@c93b0d9a972cb6f98fd445f2b69d924350f9128a while
keeping the test cases.

mysql-test/suite/innodb/r/auto_increment_dup,skip-log-bin.rdiff

---- auto_increment_dup.result
-+++ auto_increment_dup,skip-log-bin.reject
-@@ -89,13 +89,14 @@
- SET DEBUG_SYNC='execute_command_after_close_tables SIGNAL continue';
- affected rows: 0
- INSERT INTO t1(k) VALUES (2), (4), (5) ON DUPLICATE KEY UPDATE c='2';
--ERROR HY000: Lock wait timeout exceeded; try restarting transaction
-+affected rows: 3
-+info: Records: 3  Duplicates: 0  Warnings: 0
- connection con1;
- #
- # 2 duplicates
- #
--affected rows: 3
--info: Records: 3  Duplicates: 0  Warnings: 0
-+affected rows: 4
-+info: Records: 3  Duplicates: 1  Warnings: 0
- connection default;
- #
- # 3 rows
-@@ -103,19 +104,21 @@
- SELECT * FROM t1 order by k;
- id	k	c
- 1	1	NULL
--2	2	NULL
--3	3	NULL
--affected rows: 3
-+4	2	1
-+2	3	NULL
-+5	4	NULL
-+6	5	NULL
-+affected rows: 5
- INSERT INTO t1(k) VALUES (2), (4), (5) ON DUPLICATE KEY UPDATE c='2';
--affected rows: 4
--info: Records: 3  Duplicates: 1  Warnings: 0
-+affected rows: 6
-+info: Records: 3  Duplicates: 3  Warnings: 0
- SELECT * FROM t1 order by k;
- id	k	c
- 1	1	NULL
--2	2	2
--3	3	NULL
--7	4	NULL
--8	5	NULL
-+4	2	2
-+2	3	NULL
-+5	4	2
-+6	5	2
- affected rows: 5
- disconnect con1;
- disconnect con2;

mysql-test/suite/innodb/r/auto_increment_dup.result

@@ -89,13 +89,14 @@ affected rows: 0
 SET DEBUG_SYNC='execute_command_after_close_tables SIGNAL continue';
 affected rows: 0
 INSERT INTO t1(k) VALUES (2), (4), (5) ON DUPLICATE KEY UPDATE c='2';
-ERROR HY000: Lock wait timeout exceeded; try restarting transaction
+affected rows: 3
+info: Records: 3  Duplicates: 0  Warnings: 0
 connection con1;
 #
 # 2 duplicates
 #
-affected rows: 3
-info: Records: 3  Duplicates: 0  Warnings: 0
+affected rows: 4
+info: Records: 3  Duplicates: 1  Warnings: 0
 connection default;
 #
 # 3 rows
@@ -103,19 +104,21 @@ connection default;
 SELECT * FROM t1 order by k;
 id	k	c
 1	1	NULL
-2	2	NULL
-3	3	NULL
-affected rows: 3
+4	2	1
+2	3	NULL
+5	4	NULL
+6	5	NULL
+affected rows: 5
 INSERT INTO t1(k) VALUES (2), (4), (5) ON DUPLICATE KEY UPDATE c='2';
-affected rows: 4
-info: Records: 3  Duplicates: 1  Warnings: 0
+affected rows: 6
+info: Records: 3  Duplicates: 3  Warnings: 0
 SELECT * FROM t1 order by k;
 id	k	c
 1	1	NULL
-2	2	2
-3	3	NULL
-7	4	NULL
-8	5	NULL
+4	2	2
+2	3	NULL
+5	4	2
+6	5	2
 affected rows: 5
 disconnect con1;
 disconnect con2;

mysql-test/suite/innodb/t/auto_increment_dup.test

@@ -8,8 +8,6 @@
 --source include/have_debug_sync.inc
 --source include/innodb_binlog.inc
 
-let $stmt= `SELECT @@GLOBAL.log_bin`;
-
 set global transaction isolation level repeatable read;
 
 CREATE TABLE t1(
@@ -84,13 +82,7 @@ SET DEBUG_SYNC='ha_write_row_end SIGNAL write_row_done WAIT_FOR continue';
 --reap
 
 SET DEBUG_SYNC='execute_command_after_close_tables SIGNAL continue';
-if ($stmt) {
---error ER_LOCK_WAIT_TIMEOUT
-INSERT INTO t1(k) VALUES (2), (4), (5) ON DUPLICATE KEY UPDATE c='2';
-}
-if (!$stmt) {
 INSERT INTO t1(k) VALUES (2), (4), (5) ON DUPLICATE KEY UPDATE c='2';
-}
 
 --connection con1
 --echo #

mysql-test/suite/rpl/disabled.def

@@ -15,4 +15,3 @@ rpl_get_master_version_and_clock : Bug#11766137 Jan 05 2011 joro Valgrind warnin
 rpl_partition_archive     : MDEV-5077 2013-09-27 svoj Cannot exchange partition with archive table
 rpl_row_binlog_max_cache_size : MDEV-11092
 rpl_row_index_choice      : MDEV-11666
-rpl_mdev_17614            : MDEV-17614/MDEV-17073 Unexpected lock conflict

mysql-test/suite/rpl/r/rpl_mdev_17614.result

@@ -3,19 +3,24 @@ include/master-slave.inc
 call mtr.add_suppression("Unsafe statement written to the binary log using statement format");
 CREATE TABLE t1 (a INT NOT NULL PRIMARY KEY , b INT,
 UNIQUE(b),  c int) engine=innodb;
+connection slave;
+connection master;
 INSERT INTO t1 VALUES (1, 1, 1);
 BEGIN;
 INSERT INTO t1 VALUES (2, 1, 2) ON DUPLICATE KEY UPDATE b=VALUES(b), c=VALUES(c);
 Warnings:
 Note	1592	Unsafe statement written to the binary log using statement format since BINLOG_FORMAT = STATEMENT. INSERT... ON DUPLICATE KEY UPDATE  on a table with more than one UNIQUE KEY is unsafe
+connection master1;
 INSERT INTO t1 VALUES(2, 2, 3) ON DUPLICATE KEY UPDATE b=VALUES(b), c=VALUES(c);
 Warnings:
 Note	1592	Unsafe statement written to the binary log using statement format since BINLOG_FORMAT = STATEMENT. INSERT... ON DUPLICATE KEY UPDATE  on a table with more than one UNIQUE KEY is unsafe
+connection master;
 COMMIT;
 SELECT * FROM t1;
 a	b	c
 1	1	2
 2	2	3
+connection slave;
 include/wait_for_slave_sql_error.inc [errno=1062]
 Last_SQL_Error = 'Error 'Duplicate entry '1' for key 'b'' on query. Default database: 'test'. Query: 'INSERT INTO t1 VALUES (2, 1, 2) ON DUPLICATE KEY UPDATE b=VALUES(b), c=VALUES(c)''
 #Different value from server
@@ -26,61 +31,85 @@ a	b	c
 stop slave;
 include/wait_for_slave_to_stop.inc
 reset slave;
+connection master;
 reset master;
 drop table t1;
+connection slave;
 start slave;
 include/wait_for_slave_to_start.inc
+connection master;
 CREATE TABLE t1 (a INT NOT NULL PRIMARY KEY auto_increment, b INT,
 UNIQUE(b),  c int) engine=innodb;
+connection slave;
+connection master;
 INSERT INTO t1 VALUES (default, 1, 1);
 BEGIN;
 INSERT INTO t1 VALUES (default, 1, 2) ON DUPLICATE KEY UPDATE b=VALUES(b), c=VALUES(c);
+connection master1;
 INSERT INTO t1 VALUES(default, 2, 3) ON DUPLICATE KEY UPDATE b=VALUES(b), c=VALUES(c);
+connection master;
 COMMIT;
 SELECT * FROM t1;
 a	b	c
 1	1	2
 3	2	3
+connection slave;
 #same data as master
 SELECT * FROM t1;
 a	b	c
 1	1	2
 3	2	3
+connection master;
 drop table t1;
+connection slave;
+connection master;
 CREATE TABLE t1 (a INT NOT NULL PRIMARY KEY, b INT,
 UNIQUE(b),  c int, d int ) engine=innodb;
+connection slave;
+connection master;
 INSERT INTO t1 VALUES (1, 1, 1, 1);
 BEGIN;
 INSERT INTO t1 VALUES (2, NULL, 2, 2) ON DUPLICATE KEY UPDATE b=VALUES(b), c=VALUES(c);
 Warnings:
 Note	1592	Unsafe statement written to the binary log using statement format since BINLOG_FORMAT = STATEMENT. INSERT... ON DUPLICATE KEY UPDATE  on a table with more than one UNIQUE KEY is unsafe
+connection master1;
 INSERT INTO t1 VALUES(3, NULL, 2, 3) ON DUPLICATE KEY UPDATE b=VALUES(b), c=VALUES(c);
 Warnings:
 Note	1592	Unsafe statement written to the binary log using statement format since BINLOG_FORMAT = STATEMENT. INSERT... ON DUPLICATE KEY UPDATE  on a table with more than one UNIQUE KEY is unsafe
+connection master;
 COMMIT;
 SELECT * FROM t1;
 a	b	c	d
 1	1	1	1
 2	NULL	2	2
 3	NULL	2	3
+connection slave;
 #same data as master
 SELECT * FROM t1;
 a	b	c	d
 1	1	1	1
 2	NULL	2	2
 3	NULL	2	3
+connection master;
 drop table t1;
+connection slave;
+connection master;
 CREATE TABLE t1 (a INT NOT NULL PRIMARY KEY auto_increment, b INT,
 UNIQUE(b),  c int) engine=innodb;
+connection slave;
+connection master;
 INSERT INTO t1 VALUES (1, 1, 1);
 BEGIN;
 INSERT INTO t1 VALUES (2, 1, 2) ON DUPLICATE KEY UPDATE b=VALUES(b), c=VALUES(c);
+connection master1;
 INSERT INTO t1 VALUES(2, 2, 3) ON DUPLICATE KEY UPDATE b=VALUES(b), c=VALUES(c);
+connection master;
 COMMIT;
 SELECT * FROM t1;
 a	b	c
 1	1	2
 2	2	3
+connection slave;
 include/wait_for_slave_sql_error.inc [errno=1062]
 Last_SQL_Error = 'Error 'Duplicate entry '1' for key 'b'' on query. Default database: 'test'. Query: 'INSERT INTO t1 VALUES (2, 1, 2) ON DUPLICATE KEY UPDATE b=VALUES(b), c=VALUES(c)''
 #Different value from server
@@ -91,8 +120,10 @@ a	b	c
 stop slave;
 include/wait_for_slave_to_stop.inc
 reset slave;
+connection master;
 reset master;
 drop table t1;
+connection slave;
 start slave;
 include/wait_for_slave_to_start.inc
 include/rpl_end.inc

sql/sql_class.cc

@@ -4553,12 +4553,6 @@ extern "C" int thd_rpl_is_parallel(const MYSQL_THD thd)
   return thd->rgi_slave && thd->rgi_slave->is_parallel_exec;
 }
 
-extern "C" int thd_rpl_stmt_based(const MYSQL_THD thd)
-{
-  return thd &&
-    !thd->is_current_stmt_binlog_format_row() &&
-    !thd->is_current_stmt_binlog_disabled();
-}
 
 /* Returns high resolution timestamp for the start
   of the current query. */

storage/innobase/include/ha_prototypes.h

@@ -121,9 +121,6 @@ thd_is_replication_slave_thread(
 /*============================*/
 	THD*	thd);	/*!< in: thread handle */
 
-/** @return whether statement-based replication is active */
-extern "C" int thd_rpl_stmt_based(const THD* thd);
-
 /******************************************************************//**
 Returns true if the transaction this thread is processing has edited
 non-transactional tables. Used by the deadlock detector when deciding

storage/innobase/include/row0ins.h

@@ -196,10 +196,6 @@ struct ins_node_t{
 				entry_list and sys fields are stored here;
 				if this is NULL, entry list should be created
 				and buffers for sys fields in row allocated */
-	dict_index_t*   duplicate;
-				/* This is the first index that reported
-				DB_DUPLICATE_KEY.  Used in the case of REPLACE
-				or INSERT ... ON DUPLICATE UPDATE. */
 	ulint		magic_n;
 };
 

storage/innobase/que/que0que.cc

@@ -684,11 +684,6 @@ que_thr_stop(
 		trx->lock.wait_thr = thr;
 		thr->state = QUE_THR_LOCK_WAIT;
 
-	} else if (trx->duplicates && trx->error_state == DB_DUPLICATE_KEY
-		   && thd_rpl_stmt_based(trx->mysql_thd)) {
-
-		return(FALSE);
-
 	} else if (trx->error_state != DB_SUCCESS
 		   && trx->error_state != DB_LOCK_WAIT) {
 

storage/innobase/row/row0ins.cc

@@ -88,7 +88,6 @@ ins_node_create(
 	node->select = NULL;
 
 	node->trx_id = 0;
-	node->duplicate = NULL;
 
 	node->entry_sys_heap = mem_heap_create(128);
 
@@ -191,7 +190,6 @@ ins_node_set_new_row(
 	node->state = INS_NODE_SET_IX_LOCK;
 	node->index = NULL;
 	node->entry = NULL;
-	node->duplicate = NULL;
 
 	node->row = row;
 
@@ -2320,12 +2318,6 @@ row_ins_duplicate_error_in_clust(
 						  true,
 						  ULINT_UNDEFINED, &heap);
 
-			ulint lock_type =
-				trx->isolation_level <= TRX_ISO_READ_COMMITTED
-				|| (trx->mysql_thd
-				    && !thd_rpl_stmt_based(trx->mysql_thd))
-				? LOCK_REC_NOT_GAP : LOCK_ORDINARY;
-
 			/* We set a lock on the possible duplicate: this
 			is needed in logical logging of MySQL to make
 			sure that in roll-forward we get the same duplicate
@@ -2342,13 +2334,13 @@ row_ins_duplicate_error_in_clust(
 				INSERT ON DUPLICATE KEY UPDATE). */
 
 				err = row_ins_set_exclusive_rec_lock(
-					lock_type,
+					LOCK_REC_NOT_GAP,
 					btr_cur_get_block(cursor),
 					rec, cursor->index, offsets, thr);
 			} else {
 
 				err = row_ins_set_shared_rec_lock(
-					lock_type,
+					LOCK_REC_NOT_GAP,
 					btr_cur_get_block(cursor), rec,
 					cursor->index, offsets, thr);
 			}
@@ -2363,7 +2355,10 @@ row_ins_duplicate_error_in_clust(
 
 			if (row_ins_dupl_error_with_rec(
 				    rec, entry, cursor->index, offsets)) {
-				goto duplicate;
+duplicate:
+				trx->error_info = cursor->index;
+				err = DB_DUPLICATE_KEY;
+				goto func_exit;
 			}
 		}
 	}
@@ -2406,10 +2401,7 @@ row_ins_duplicate_error_in_clust(
 
 			if (row_ins_dupl_error_with_rec(
 				    rec, entry, cursor->index, offsets)) {
-duplicate:
-				trx->error_info = cursor->index;
-				err = DB_DUPLICATE_KEY;
-				goto func_exit;
+				goto duplicate;
 			}
 		}
 
@@ -3023,46 +3015,6 @@ row_ins_sec_index_entry_low(
 			&cursor, 0, __FILE__, __LINE__, &mtr);
 	}
 
-	if (!(flags & BTR_NO_LOCKING_FLAG)
-	    && dict_index_is_unique(index)
-	    && thr_get_trx(thr)->duplicates
-	    && thr_get_trx(thr)->isolation_level >= TRX_ISO_REPEATABLE_READ
-	    && thd_rpl_stmt_based(thr_get_trx(thr)->mysql_thd)) {
-
-		/* In statement-based replication, when replicating a
-		REPLACE statement or ON DUPLICATE KEY UPDATE clause, a
-		gap lock is taken on the position of the to-be-inserted record,
-		to avoid other concurrent transactions from inserting the same
-		record. */
-
-		dberr_t	err;
-		const rec_t* rec = page_rec_get_next_const(
-			btr_cur_get_rec(&cursor));
-
-		ut_ad(!page_rec_is_infimum(rec));
-
-		offsets = rec_get_offsets(rec, index, offsets, true,
-					  ULINT_UNDEFINED, &offsets_heap);
-
-		err = row_ins_set_exclusive_rec_lock(
-			LOCK_GAP, btr_cur_get_block(&cursor), rec,
-			index, offsets, thr);
-
-		switch (err) {
-		case DB_SUCCESS:
-		case DB_SUCCESS_LOCKED_REC:
-			if (thr_get_trx(thr)->error_state != DB_DUPLICATE_KEY) {
-				break;
-			}
-			/* Fall through (skip actual insert) after we have
-			successfully acquired the gap lock. */
-		default:
-			goto func_exit;
-		}
-	}
-
-	ut_ad(thr_get_trx(thr)->error_state == DB_SUCCESS);
-
 	if (dup_chk_only) {
 		goto func_exit;
 	}
@@ -3578,13 +3530,6 @@ row_ins(
 
 	DBUG_PRINT("row_ins", ("table: %s", node->table->name.m_name));
 
-	trx_t* trx = thr_get_trx(thr);
-
-	if (node->duplicate) {
-		ut_ad(thd_rpl_stmt_based(trx->mysql_thd));
-		trx->error_state = DB_DUPLICATE_KEY;
-	}
-
 	if (node->state == INS_NODE_ALLOC_ROW_ID) {
 
 		row_ins_alloc_row_id_step(node);
@@ -3610,91 +3555,7 @@ row_ins(
 		if (node->index->type != DICT_FTS) {
 			dberr_t err = row_ins_index_entry_step(node, thr);
 
-			switch (err) {
-			case DB_SUCCESS:
-				break;
-			case DB_NO_REFERENCED_ROW:
-				if (!dict_index_is_unique(node->index)) {
-					DBUG_RETURN(err);
-				}
-				/* fall through */
-			case DB_DUPLICATE_KEY:
-				ut_ad(dict_index_is_unique(node->index));
-
-				if (trx->isolation_level
-				    >= TRX_ISO_REPEATABLE_READ
-				    && trx->duplicates
-				    && !node->table->is_temporary()
-				    && thd_rpl_stmt_based(trx->mysql_thd)) {
-
-					/* When we are in REPLACE statement or
-					INSERT ..  ON DUPLICATE UPDATE
-					statement, we process all the
-					unique secondary indexes, even after we
-					encounter a duplicate error. This is
-					done to take necessary gap locks in
-					secondary indexes to block concurrent
-					transactions from inserting the
-					searched records. */
-					if (err == DB_NO_REFERENCED_ROW
-					    && node->duplicate) {
-						/* A foreign key check on a
-						unique index may fail to
-						find the record.
-
-						Consider as a example
-						following:
-						create table child(a int not null
-						primary key, b int not null,
-						c int,
-						unique key (b),
-						foreign key (b) references
-						parent (id)) engine=innodb;
-
-						insert into child values
-						(1,1,2);
-
-						insert into child(a) values
-						(1) on duplicate key update
-						c = 3;
-
-						Now primary key value 1
-						naturally causes duplicate
-						key error that will be
-						stored on node->duplicate.
-						If there was no duplicate
-						key error, we should return
-						the actual no referenced
-						row error.
-
-						As value for
-						column b used in both unique
-						key and foreign key is not
-						provided, server uses 0 as a
-						search value. This is
-						naturally, not found leading
-						to DB_NO_REFERENCED_ROW.
-						But, we should update the
-						row with primay key value 1
-						anyway.
-
-						Return the
-						original  DB_DUPLICATE_KEY
-						error after
-						placing all gaplocks. */
-						err = DB_DUPLICATE_KEY;
-						break;
-					} else if (!node->duplicate) {
-						/* Save 1st dup error. Ignore
-						subsequent dup errors. */
-						node->duplicate = node->index;
-						trx->error_state
-							= DB_DUPLICATE_KEY;
-					}
-					break;
-				}
-				// fall through
-			default:
+			if (err != DB_SUCCESS) {
 				DBUG_RETURN(err);
 			}
 		}
@@ -3711,31 +3572,13 @@ row_ins(
 			node->index = dict_table_get_next_index(node->index);
 			node->entry = UT_LIST_GET_NEXT(tuple_list, node->entry);
 		}
-
-		/* After encountering a duplicate key error, we process
-		remaining indexes just to place gap locks and no actual
-		insertion will take place.  These gap locks are needed
-		only for unique indexes.  So skipping non-unique indexes. */
-		if (node->duplicate) {
-			ut_ad(thd_rpl_stmt_based(trx->mysql_thd));
-			while (node->index
-			       && !dict_index_is_unique(node->index)) {
-
-				node->index = dict_table_get_next_index(
-					node->index);
-				node->entry = UT_LIST_GET_NEXT(tuple_list,
-							       node->entry);
-			}
-			trx->error_state = DB_DUPLICATE_KEY;
-		}
 	}
 
 	ut_ad(node->entry == NULL);
 
-	trx->error_info = node->duplicate;
 	node->state = INS_NODE_ALLOC_ROW_ID;
 
-	DBUG_RETURN(node->duplicate ? DB_DUPLICATE_KEY : DB_SUCCESS);
+	DBUG_RETURN(DB_SUCCESS);
 }
 
 /***********************************************************//**

storage/innobase/row/row0mysql.cc

@@ -1435,7 +1435,6 @@ row_insert_for_mysql(
 			goto run_again;
 		}
 
-		node->duplicate = NULL;
 		trx->op_info = "";
 
 		if (blob_heap != NULL) {
@@ -1445,8 +1444,6 @@ row_insert_for_mysql(
 		return(err);
 	}
 
-	node->duplicate = NULL;
-
 	if (dict_table_has_fts_index(table)) {
 		doc_id_t	doc_id;