MDEV-25644 UPDATE not working properly on transaction precise system versioned table

row_ins_duplicate_error_in_clust():

Restrict DB_FOREIGN_DUPLICATE_KEY to the better
conditions. VERSIONED_DELETE is used specifically to help lower stack
to understand what caused current insert. Related to MDEV-29813.

9 files changed, 96 insertions, 61 deletions

diff --git a/mysql-test/suite/versioning/r/delete.result b/mysql-test/suite/versioning/r/delete.result
index 7ca3dd16cd8..c293e923816 100644
--- a/mysql-test/suite/versioning/r/delete.result
+++ b/mysql-test/suite/versioning/r/delete.result
@@ -133,21 +133,41 @@ drop table t1;
 #
 # MDEV-21138 Assertion `col->ord_part' or `f.col->ord_part' failed in row_build_index_entry_low
 #
+# Check DELETE and multi-DELETE with foreign key
 create table t1 (
 f1 int, f2 text, f3 int, fulltext (f2), key(f1), key(f3),
-foreign key r (f3) references t1 (f1) on delete set null)
+foreign key r (f3) references t1 (f1) on delete set null,
+row_start SYS_TYPE as row start invisible,
+row_end SYS_TYPE as row end invisible,
+period for system_time (row_start, row_end))
 with system versioning engine innodb;
 insert into t1 values (1, repeat('a', 8193), 1), (1, repeat('b', 8193), 1);
-select f1, f3, check_row_ts(row_start, row_end) from t1;
-f1	f3	check_row_ts(row_start, row_end)
+insert into t1 select 2, f2, 2 from t1;
+select f1, f3, check_row(row_start, row_end) from t1;
+f1	f3	check_row(row_start, row_end)
 1	1	CURRENT ROW
 1	1	CURRENT ROW
-delete from t1;
-select f1, f3, check_row_ts(row_start, row_end) from t1 for system_time all;
-f1	f3	check_row_ts(row_start, row_end)
+2	2	CURRENT ROW
+2	2	CURRENT ROW
+delete from t1 where f1 = 1;
+select f1, f3, check_row(row_start, row_end) from t1 for system_time all order by f1, row_end;
+f1	f3	check_row(row_start, row_end)
 1	1	HISTORICAL ROW
 1	1	HISTORICAL ROW
-drop table t1;
+2	2	CURRENT ROW
+2	2	CURRENT ROW
+create table t2 (f1 int);
+insert into t2 values (2);
+# Multi-delelte
+delete t1, t2 from t1 join t2 where t1.f1 = t2.f1;
+select f1, f3, check_row(row_start, row_end) from t1 for system_time all order by f1, row_end;
+f1	f3	check_row(row_start, row_end)
+1	1	HISTORICAL ROW
+1	1	HISTORICAL ROW
+2	2	HISTORICAL ROW
+2	2	HISTORICAL ROW
+# Cleanup
+drop tables t1, t2;
 #
 # MDEV-30378 Versioned REPLACE succeeds with ON DELETE RESTRICT
 # constraint
diff --git a/mysql-test/suite/versioning/r/update.result b/mysql-test/suite/versioning/r/update.result
index cbb75a10cf5..c97bac58ac2 100644
--- a/mysql-test/suite/versioning/r/update.result
+++ b/mysql-test/suite/versioning/r/update.result
@@ -51,19 +51,22 @@ sys_trx_start SYS_DATATYPE as row start invisible,
 sys_trx_end SYS_DATATYPE as row end invisible,
 period for system_time (sys_trx_start, sys_trx_end))
 with system versioning;
+set timestamp= unix_timestamp('2000-01-01 00:00:00');
 insert into t1 values(1, 1, 1);
-set @ins_t= now(6);
 select sys_trx_start into @tmp1 from t1;
+set timestamp= unix_timestamp('2000-01-01 01:00:00');
 update t1 set x= 11, y= 11 where id = 1;
 select @tmp1 < sys_trx_start as A1, x, y from t1;
 A1	x	y
 1	11	11
 select sys_trx_start into @tmp1 from t1;
+set timestamp= unix_timestamp('2000-01-01 02:00:00');
 update t1 set y= 1 where id = 1;
 select @tmp1 = sys_trx_start as A2, x from t1;
 A2	x
 1	11
 drop table t1;
+set timestamp= default;
 create table t1 (
 x int,
 y int,
diff --git a/mysql-test/suite/versioning/t/delete.test b/mysql-test/suite/versioning/t/delete.test
index 3db53ff93bc..bcefbb33a50 100644
--- a/mysql-test/suite/versioning/t/delete.test
+++ b/mysql-test/suite/versioning/t/delete.test
@@ -97,17 +97,27 @@ drop table t1;
 --echo #
 --echo # MDEV-21138 Assertion `col->ord_part' or `f.col->ord_part' failed in row_build_index_entry_low
 --echo #
-create table t1 (
+--echo # Check DELETE and multi-DELETE with foreign key
+replace_result $sys_datatype_expl SYS_TYPE;
+eval create table t1 (
   f1 int, f2 text, f3 int, fulltext (f2), key(f1), key(f3),
-  foreign key r (f3) references t1 (f1) on delete set null)
+  foreign key r (f3) references t1 (f1) on delete set null,
+  row_start $sys_datatype_expl as row start invisible,
+  row_end $sys_datatype_expl as row end invisible,
+  period for system_time (row_start, row_end))
 with system versioning engine innodb;
 insert into t1 values (1, repeat('a', 8193), 1), (1, repeat('b', 8193), 1);
-select f1, f3, check_row_ts(row_start, row_end) from t1;
-delete from t1;
-select f1, f3, check_row_ts(row_start, row_end) from t1 for system_time all;
-
-# cleanup
-drop table t1;
+insert into t1 select 2, f2, 2 from t1;
+select f1, f3, check_row(row_start, row_end) from t1;
+delete from t1 where f1 = 1;
+select f1, f3, check_row(row_start, row_end) from t1 for system_time all order by f1, row_end;
+create table t2 (f1 int);
+insert into t2 values (2);
+--echo # Multi-delelte
+delete t1, t2 from t1 join t2 where t1.f1 = t2.f1;
+select f1, f3, check_row(row_start, row_end) from t1 for system_time all order by f1, row_end;
+--echo # Cleanup
+drop tables t1, t2;
 
 --echo #
 --echo # MDEV-30378 Versioned REPLACE succeeds with ON DELETE RESTRICT
diff --git a/mysql-test/suite/versioning/t/update.test b/mysql-test/suite/versioning/t/update.test
index 56bbd909256..cdd11505f99 100644
--- a/mysql-test/suite/versioning/t/update.test
+++ b/mysql-test/suite/versioning/t/update.test
@@ -26,15 +26,18 @@ eval create table t1 (
   sys_trx_end $sys_datatype_expl as row end invisible,
   period for system_time (sys_trx_start, sys_trx_end))
 with system versioning;
+set timestamp= unix_timestamp('2000-01-01 00:00:00');
 insert into t1 values(1, 1, 1);
-set @ins_t= now(6);
 select sys_trx_start into @tmp1 from t1;
+set timestamp= unix_timestamp('2000-01-01 01:00:00');
 update t1 set x= 11, y= 11 where id = 1;
 select @tmp1 < sys_trx_start as A1, x, y from t1;
 select sys_trx_start into @tmp1 from t1;
+set timestamp= unix_timestamp('2000-01-01 02:00:00');
 update t1 set y= 1 where id = 1;
 select @tmp1 = sys_trx_start as A2, x from t1;
 drop table t1;
+set timestamp= default;
 
 replace_result $sys_datatype_expl SYS_DATATYPE;
 eval create table t1 (
diff --git a/sql/sql_update.cc b/sql/sql_update.cc
index a2980717018..46f86093c07 100644
--- a/sql/sql_update.cc
+++ b/sql/sql_update.cc
@@ -232,6 +232,11 @@ bool TABLE::vers_check_update(List<Item> &items)
       }
     }
   }
+  /*
+    Tell TRX_ID-versioning that it does not insert history row
+    (see calc_row_difference()).
+  */
+  vers_write= false;
   return false;
 }
 
diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc
index 6f781a1f291..972c0738f5b 100644
--- a/storage/innobase/handler/ha_innodb.cc
+++ b/storage/innobase/handler/ha_innodb.cc
@@ -8389,7 +8389,8 @@ calc_row_difference(
 	uchar*		upd_buff,
 	ulint		buff_len,
 	row_prebuilt_t*	prebuilt,
-	ib_uint64_t&	auto_inc)
+	ib_uint64_t&	auto_inc,
+	int		sql_command)
 {
 	uchar*		original_upd_buff = upd_buff;
 	Field*		field;
@@ -8412,6 +8413,7 @@ calc_row_difference(
 	trx_t* const	trx = prebuilt->trx;
 	doc_id_t	doc_id = FTS_NULL_DOC_ID;
 	ulint		num_v = 0;
+	prebuilt->versioned_write = table->versioned_write(VERS_TRX_ID);
 	const bool skip_virtual = ha_innobase::omits_virtual_cols(*table->s);
 
 	ut_ad(!srv_read_only_mode);
@@ -8563,9 +8565,15 @@ calc_row_difference(
 			}
 		}
 
+		const bool add_anyway= prebuilt->versioned_write
+					&& !field->vers_update_unversioned();
 		if (o_len != n_len || (o_len != 0 && o_len != UNIV_SQL_NULL
-				       && 0 != memcmp(o_ptr, n_ptr, o_len))) {
-			/* The field has changed */
+				       && (add_anyway ||
+				           0 != memcmp(o_ptr, n_ptr, o_len)))) {
+			/* The field has changed or it is trx-versioned write
+			which must write history for affects_versioned() rows
+			for UPDATE with versioned fields even when the value
+			was not changed (MDEV-23446) */
 
 			ufield = uvect->fields + n_changed;
 			MEM_UNDEFINED(ufield, sizeof *ufield);
@@ -8761,6 +8769,17 @@ calc_row_difference(
 
 	ut_a(buf <= (byte*) original_upd_buff + buff_len);
 
+	/* Used to avoid history in FK check on DELETE (see MDEV-16210). */
+	prebuilt->upd_node->is_delete =
+		((sql_command == SQLCOM_DELETE
+		  || sql_command == SQLCOM_DELETE_MULTI)
+		 && table->versioned(VERS_TIMESTAMP))
+		? VERSIONED_DELETE : NO_DELETE;
+
+	if (prebuilt->versioned_write && uvect->affects_versioned()) {
+		prebuilt->upd_node->vers_make_update(trx);
+	}
+
 	ut_ad(uvect->validate());
 	return(DB_SUCCESS);
 }
@@ -8904,7 +8923,7 @@ ha_innobase::update_row(
 
 	error = calc_row_difference(
 		uvect, old_row, new_row, table, m_upd_buf, m_upd_buf_size,
-		m_prebuilt, autoinc);
+		m_prebuilt, autoinc, thd_sql_command(m_user_thd));
 
 	if (error != DB_SUCCESS) {
 		goto func_exit;
@@ -8915,48 +8934,25 @@ ha_innobase::update_row(
 		MySQL that the row is not really updated and it
 		should not increase the count of updated rows.
 		This is fix for http://bugs.mysql.com/29157 */
-		if (m_prebuilt->versioned_write
-		    && thd_sql_command(m_user_thd) != SQLCOM_ALTER_TABLE
-		    /* Multiple UPDATE of same rows in single transaction create
-		       historical rows only once. */
-		    && trx->id != table->vers_start_id()) {
-			error = row_insert_for_mysql((byte*) old_row,
-						     m_prebuilt,
-						     ROW_INS_HISTORICAL);
-			if (error != DB_SUCCESS) {
-				goto func_exit;
-			}
-			innobase_srv_conc_exit_innodb(m_prebuilt);
-			innobase_active_small();
-		}
 		DBUG_RETURN(HA_ERR_RECORD_IS_THE_SAME);
 	} else {
-		const bool vers_set_fields = m_prebuilt->versioned_write
-			&& m_prebuilt->upd_node->update->affects_versioned();
-		const bool vers_ins_row = vers_set_fields
-			&& thd_sql_command(m_user_thd) != SQLCOM_ALTER_TABLE;
-
-                TABLE_LIST *tl= table->pos_in_table_list;
-                uint8 op_map= tl->trg_event_map | tl->slave_fk_event_map;
-		/* This is not a delete */
-		m_prebuilt->upd_node->is_delete =
-			(vers_set_fields && !vers_ins_row) ||
-			(op_map & trg2bit(TRG_EVENT_DELETE) &&
-				table->versioned(VERS_TIMESTAMP))
-			? VERSIONED_DELETE
-			: NO_DELETE;
-
 		innobase_srv_conc_enter_innodb(m_prebuilt);
 
 		if (m_prebuilt->upd_node->is_delete) {
 			trx->fts_next_doc_id = 0;
 		}
+		/* See vers_make_update() inside for versioned_write for how
+		row_start/row_end updated */
 		error = row_update_for_mysql(m_prebuilt);
 
-		if (error == DB_SUCCESS && vers_ins_row
+		if (error == DB_SUCCESS && m_prebuilt->versioned_write
+		    && uvect->affects_versioned()
 		    /* Multiple UPDATE of same rows in single transaction create
 		       historical rows only once. */
 		    && trx->id != table->vers_start_id()) {
+			/* UPDATE is not used by ALTER TABLE. Just precaution
+			as we don't need history generation for ALTER TABLE. */
+			ut_ad(thd_sql_command(m_user_thd) != SQLCOM_ALTER_TABLE);
 			error = row_insert_for_mysql((byte*) old_row,
 						     m_prebuilt,
 						     ROW_INS_HISTORICAL);
diff --git a/storage/innobase/row/row0ins.cc b/storage/innobase/row/row0ins.cc
index 81f662e7a15..6765ba546d5 100644
--- a/storage/innobase/row/row0ins.cc
+++ b/storage/innobase/row/row0ins.cc
@@ -2439,7 +2439,10 @@ row_ins_duplicate_error_in_clust(
 duplicate:
 				trx->error_info = cursor->index;
 				err = DB_DUPLICATE_KEY;
-				if (cursor->index->table->versioned()
+				if (thr->prebuilt
+				    && thr->prebuilt->upd_node
+				    && thr->prebuilt->upd_node->is_delete
+					== VERSIONED_DELETE
 				    && entry->vers_history_row())
 				{
 					ulint trx_id_len;
diff --git a/storage/innobase/row/row0mysql.cc b/storage/innobase/row/row0mysql.cc
index bb994a926eb..dc3d0ca2ade 100644
--- a/storage/innobase/row/row0mysql.cc
+++ b/storage/innobase/row/row0mysql.cc
@@ -1790,12 +1790,8 @@ row_update_for_mysql(row_prebuilt_t* prebuilt)
 
 	ut_ad(!prebuilt->versioned_write || node->table->versioned());
 
-	if (prebuilt->versioned_write) {
-		if (node->is_delete == VERSIONED_DELETE) {
-                  node->vers_make_delete(trx);
-                } else if (node->update->affects_versioned()) {
-                  node->vers_make_update(trx);
-                }
+	if (prebuilt->versioned_write && node->is_delete == VERSIONED_DELETE) {
+		node->vers_make_delete(trx);
 	}
 
 	for (;;) {
diff --git a/storage/innobase/row/row0upd.cc b/storage/innobase/row/row0upd.cc
index c7087559f47..26dd852e3d7 100644
--- a/storage/innobase/row/row0upd.cc
+++ b/storage/innobase/row/row0upd.cc
@@ -3472,9 +3472,8 @@ error_handling:
 void thd_get_query_start_data(THD *thd, char *buf);
 
 /** Appends row_start or row_end field to update vector and sets a
-CURRENT_TIMESTAMP/trx->id value to it.
-Supposed to be called only by make_versioned_update() and
-make_versioned_delete().
+CURRENT_TIMESTAMP/trx->id value to it. Called by vers_make_update() and
+vers_make_delete().
 @param[in]	trx	transaction
 @param[in]	vers_sys_idx	table->row_start or table->row_end */
 void upd_node_t::vers_update_fields(const trx_t *trx, ulint idx)