MDEV-20812 Unexpected ER_ROW_IS_REFERENCED_2 upon DELETE from versioned table with FK

MDEV-16210 original case was wrongly allowed versioned DELETE from
referenced table where reference is by non-primary key. InnoDB UPDATE
has optimization for new rows not changing its clustered index
position. In this case InnoDB doesn't update all secondary indexes and
misses the one holding the referenced key. The fix was to disable this
optimization for versioned DELETE. In case of versioned DELETE we
forcely update all secondary indexes and therefore check them for
constraints.

But the above fix raised another problem with versioned DELETE on
foreign table side. In case when there was no corresponding record in
referenced table (illegal foreign reference can be done with "set
foreign_key_checks=off") there was spurious constraint check (because
versioned DELETE is actually UPDATE) and hence the operation failed
with constraint error.

MDEV-16210 tried to fix the above problem by checking foreign table
instead of referenced table and that at least was illegal.

Constraint check is done by row_ins_check_foreign_constraint() no
matter what kind of table is checked, referenced or foreign
(controlled by check_ref argument).

Referenced table is checked by row_upd_check_references_constraints().
Foreign table is checked by row_ins_check_foreign_constraints().

Current fix rolls back the wrong fix for the above problem and
disables referenced table check for DELETE on foreign side by
introducing `check_foreign` argument which when set to *false* skips
row_ins_check_foreign_constraints() call.

5 files changed, 71 insertions, 17 deletions

diff --git a/mysql-test/suite/versioning/r/foreign.result b/mysql-test/suite/versioning/r/foreign.result
index 6a9746b62e2..32b5e5cf3d8 100644
--- a/mysql-test/suite/versioning/r/foreign.result
+++ b/mysql-test/suite/versioning/r/foreign.result
@@ -398,3 +398,32 @@ Warning	1265	Data truncated for column 'f12' at row 7
 SET timestamp = 9;
 REPLACE INTO t2 SELECT * FROM t2;
 DROP TABLE t1, t2;
+#
+# MDEV-16210 FK constraints on versioned tables use historical rows, which may cause constraint violation
+#
+create or replace table t1 (a int, key(a)) engine innodb with system versioning;
+create or replace table t2 (b int, foreign key (b) references t1(a)) engine innodb;
+insert into t1 values (1),(2);
+insert into t2 values (1);
+# DELETE from referenced table is not allowed
+delete from t1 where a = 1;
+ERROR 23000: Cannot delete or update a parent row: a foreign key constraint fails (`test`.`t2`, CONSTRAINT `t2_ibfk_1` FOREIGN KEY (`b`) REFERENCES `t1` (`a`))
+drop tables t2, t1;
+#
+# MDEV-20812 Unexpected ER_ROW_IS_REFERENCED_2 or server crash in row_ins_foreign_report_err upon DELETE from versioned table with FK
+#
+create or replace table t1 (x int primary key) engine innodb;
+create or replace table t2 (x int, foreign key (x) references t1(x)) engine innodb with system versioning;
+set foreign_key_checks= off;
+insert into t2 values (1), (1);
+set foreign_key_checks= on;
+# DELETE from foreign table is allowed
+delete from t2;
+drop tables t2, t1;
+create or replace table t1 (a int, key(a)) engine innodb;
+insert into t1 values (1);
+create or replace table t2 (b int, foreign key (b) references t1(a)) engine innodb with system versioning;
+insert into t2 values (1), (1);
+# DELETE from foreign table is allowed
+delete from t2;
+drop tables t2, t1;
diff --git a/mysql-test/suite/versioning/t/foreign.test b/mysql-test/suite/versioning/t/foreign.test
index 9f015630e0a..7493f99cba7 100644
--- a/mysql-test/suite/versioning/t/foreign.test
+++ b/mysql-test/suite/versioning/t/foreign.test
@@ -426,4 +426,36 @@ DROP TABLE t1, t2;
 --remove_file $datadir/test/t1.data.2
 --remove_file $datadir/test/t2.data
 
+--echo #
+--echo # MDEV-16210 FK constraints on versioned tables use historical rows, which may cause constraint violation
+--echo #
+create or replace table t1 (a int, key(a)) engine innodb with system versioning;
+create or replace table t2 (b int, foreign key (b) references t1(a)) engine innodb;
+insert into t1 values (1),(2);
+insert into t2 values (1);
+--echo # DELETE from referenced table is not allowed
+--error ER_ROW_IS_REFERENCED_2
+delete from t1 where a = 1;
+drop tables t2, t1;
+
+--echo #
+--echo # MDEV-20812 Unexpected ER_ROW_IS_REFERENCED_2 or server crash in row_ins_foreign_report_err upon DELETE from versioned table with FK
+--echo #
+create or replace table t1 (x int primary key) engine innodb;
+create or replace table t2 (x int, foreign key (x) references t1(x)) engine innodb with system versioning;
+set foreign_key_checks= off;
+insert into t2 values (1), (1);
+set foreign_key_checks= on;
+--echo # DELETE from foreign table is allowed
+delete from t2;
+drop tables t2, t1;
+
+create or replace table t1 (a int, key(a)) engine innodb;
+insert into t1 values (1);
+create or replace table t2 (b int, foreign key (b) references t1(a)) engine innodb with system versioning;
+insert into t2 values (1), (1);
+--echo # DELETE from foreign table is allowed
+delete from t2;
+drop tables t2, t1;
+
 --source suite/versioning/common_finish.inc
diff --git a/storage/innobase/include/row0ins.h b/storage/innobase/include/row0ins.h
index 87a72d88eb6..9b6aac9c548 100644
--- a/storage/innobase/include/row0ins.h
+++ b/storage/innobase/include/row0ins.h
@@ -146,9 +146,8 @@ row_ins_sec_index_entry(
 	dict_index_t*	index,	/*!< in: secondary index */
 	dtuple_t*	entry,	/*!< in/out: index entry to insert */
 	que_thr_t*	thr,	/*!< in: query thread */
-	bool		check_ref) /*!< in: TRUE if we want to check that
-				the referenced table is ok, FALSE if we
-				want to check the foreign key table */
+	bool		check_foreign = true) /*!< in: true if check
+				foreign table is needed, false otherwise */
 	MY_ATTRIBUTE((warn_unused_result));
 /***********************************************************//**
 Inserts a row to a table. This is a high-level function used in
diff --git a/storage/innobase/row/row0ins.cc b/storage/innobase/row/row0ins.cc
index 88deacbc823..4760960a441 100644
--- a/storage/innobase/row/row0ins.cc
+++ b/storage/innobase/row/row0ins.cc
@@ -1988,10 +1988,7 @@ row_ins_check_foreign_constraints(
 	dict_index_t*	index,	/*!< in: index */
 	bool		pk,	/*!< in: index->is_primary() */
 	dtuple_t*	entry,	/*!< in: index entry for index */
-	que_thr_t*	thr,	/*!< in: query thread */
-	bool		check_ref = true) /*!< in: TRUE if we want to check that
-				the referenced table is ok, FALSE if we
-				want to check the foreign key table */
+	que_thr_t*	thr)	/*!< in: query thread */
 {
 	dict_foreign_t*	foreign;
 	dberr_t		err;
@@ -2040,7 +2037,7 @@ row_ins_check_foreign_constraints(
 			table from being dropped while the check is running. */
 
 			err = row_ins_check_foreign_constraint(
-				check_ref, foreign, table, entry, thr);
+				TRUE, foreign, table, entry, thr);
 
 			if (referenced_table) {
 				foreign->foreign_table->dec_fk_checks();
@@ -3271,9 +3268,8 @@ row_ins_sec_index_entry(
 	dict_index_t*	index,	/*!< in: secondary index */
 	dtuple_t*	entry,	/*!< in/out: index entry to insert */
 	que_thr_t*	thr,	/*!< in: query thread */
-	bool		check_ref) /*!< in: true if we want to check that
-				the referenced table is ok, false if we
-				want to check the foreign key table */
+	bool		check_foreign) /*!< in: true if check
+				foreign table is needed, false otherwise */
 {
 	dberr_t		err;
 	mem_heap_t*	offsets_heap;
@@ -3284,10 +3280,9 @@ row_ins_sec_index_entry(
 			DBUG_SET("-d,row_ins_sec_index_entry_timeout");
 			return(DB_LOCK_WAIT);});
 
-	if (!index->table->foreign_set.empty()) {
+	if (check_foreign && !index->table->foreign_set.empty()) {
 		err = row_ins_check_foreign_constraints(index->table, index,
-							false, entry, thr,
-							check_ref);
+							false, entry, thr);
 		if (err != DB_SUCCESS) {
 
 			return(err);
@@ -3362,7 +3357,7 @@ row_ins_index_entry(
 	if (index->is_primary()) {
 		return row_ins_clust_index_entry(index, entry, thr, 0);
 	} else {
-		return(row_ins_sec_index_entry(index, entry, thr, true));
+		return row_ins_sec_index_entry(index, entry, thr);
 	}
 }
 
diff --git a/storage/innobase/row/row0upd.cc b/storage/innobase/row/row0upd.cc
index 64021b48ee2..ad94b649397 100644
--- a/storage/innobase/row/row0upd.cc
+++ b/storage/innobase/row/row0upd.cc
@@ -2529,8 +2529,7 @@ row_upd_sec_index_entry(
 	ut_a(entry);
 
 	/* Insert new index entry */
-	err = row_ins_sec_index_entry(index, entry, thr,
-				      node->is_delete != VERSIONED_DELETE);
+	err = row_ins_sec_index_entry(index, entry, thr, !node->is_delete);
 
 func_exit:
 	mem_heap_free(heap);