MDEV-17187 table doesn't exist in engine after ALTER of FOREIGN KEY

ha_innobase::open(): Always ignore problems with FOREIGN KEY constraints
(pass DICT_ERR_IGNORE_FK_NOKEY), no matter whether foreign_key_checks
is enabled. Instead, we must report errors when enforcing the FOREIGN KEY
constraints. As a result of ignoring these errors, the tables will be
loaded with dict_foreign_t objects whose foreign_index or referenced_index
will be NULL.

Also, pass DICT_ERR_IGNORE_FK_NOKEY instead of DICT_ERR_IGNORE_NONE
to dict_table_open_on_id_low() in many other cases. Notably, on
CREATE TABLE and ALTER TABLE, we will keep validating the FOREIGN KEY
constraints as before.

dict_table_open_on_name(): If no other flags than
DICT_ERR_IGNORE_FK_NOKEY are set, refuse access to unreadable tables.
Some encryption tests rely on this code path.

For the DML code path, we used to have the problem that when
one of the indexes was missing in dict_foreign_t, we would ignore
the FOREIGN KEY constraint altogether. The following changes
address that.

row_ins_check_foreign_constraints(): Add the parameter pk.
For the primary key, consider also foreign key constraints for which
foreign->foreign_index=NULL (no underlying index is available).

row_ins_check_foreign_constraint(): Report errors also for !check_ref.
Remove a redundant check for srv_read_only_mode.

row_ins_foreign_report_add_err(): Tolerate foreign->foreign_index=NULL.

10 files changed, 175 insertions, 57 deletions

diff --git a/mysql-test/suite/innodb/r/foreign-keys.result b/mysql-test/suite/innodb/r/foreign-keys.result
index c4cf3a6a72d..5cbbb5298de 100644
--- a/mysql-test/suite/innodb/r/foreign-keys.result
+++ b/mysql-test/suite/innodb/r/foreign-keys.result
@@ -161,3 +161,62 @@ c	d
 6	30
 drop table t2, t1;
 drop user foo;
+#
+# MDEV-17187 table doesn't exist in engine after ALTER other tables
+# with CONSTRAINTs
+#
+set foreign_key_checks=on;
+create table t1 (id int not null primary key) engine=innodb;
+create table t2 (id int not null primary key, fid int not null,
+CONSTRAINT fk_fid FOREIGN KEY (fid) REFERENCES t1 (id))engine=innodb;
+insert into t1 values (1), (2), (3);
+insert into t2 values (1, 1), (2, 1), (3, 2);
+set foreign_key_checks=off;
+alter table t2 drop index fk_fid;
+set foreign_key_checks=on;
+delete from t1 where id=2;
+ERROR 23000: Cannot delete or update a parent row: a foreign key constraint fails (`test`.`t2`, CONSTRAINT `fk_fid` FOREIGN KEY (`fid`) REFERENCES `t1` (`id`))
+insert into t2 values(4, 99);
+ERROR 23000: Cannot add or update a child row: a foreign key constraint fails (`test`.`t2`, CONSTRAINT `fk_fid` FOREIGN KEY (`fid`) REFERENCES `t1` (`id`))
+select * from t1;
+id
+1
+2
+3
+select * from t2;
+id	fid
+1	1
+2	1
+3	2
+set foreign_key_checks=off;
+delete from t1 where id=2;
+insert into t2 values(4, 99);
+set foreign_key_checks=on;
+select * from t1;
+id
+1
+3
+select * from t2;
+id	fid
+1	1
+2	1
+3	2
+4	99
+show create table t1;
+Table	Create Table
+t1	CREATE TABLE `t1` (
+  `id` int(11) NOT NULL,
+  PRIMARY KEY (`id`)
+) ENGINE=InnoDB DEFAULT CHARSET=latin1
+show create table t2;
+Table	Create Table
+t2	CREATE TABLE `t2` (
+  `id` int(11) NOT NULL,
+  `fid` int(11) NOT NULL,
+  PRIMARY KEY (`id`),
+  CONSTRAINT `fk_fid` FOREIGN KEY (`fid`) REFERENCES `t1` (`id`)
+) ENGINE=InnoDB DEFAULT CHARSET=latin1
+drop table t1,t2;
+ERROR 23000: Cannot delete or update a parent row: a foreign key constraint fails
+drop table t1,t2;
+ERROR 42S02: Unknown table 'test.t2'
diff --git a/mysql-test/suite/innodb/r/innodb_bug68148.result b/mysql-test/suite/innodb/r/innodb_bug68148.result
index 88247053389..9da4ea80d08 100644
--- a/mysql-test/suite/innodb/r/innodb_bug68148.result
+++ b/mysql-test/suite/innodb/r/innodb_bug68148.result
@@ -19,7 +19,6 @@ main
 ref_table1
 ref_table2
 # restart and see if we can still access the main table
-SET FOREIGN_KEY_CHECKS=0;
 ALTER TABLE `main` ADD INDEX `idx_1` (`ref_id1`);
 SHOW CREATE TABLE `main`;
 Table	Create Table
diff --git a/mysql-test/suite/innodb/t/foreign-keys.test b/mysql-test/suite/innodb/t/foreign-keys.test
index be2c891771b..58a71d11a6a 100644
--- a/mysql-test/suite/innodb/t/foreign-keys.test
+++ b/mysql-test/suite/innodb/t/foreign-keys.test
@@ -204,3 +204,49 @@ connection default;
 select * from t2;
 drop table t2, t1;
 drop user foo;
+
+--echo #
+--echo # MDEV-17187 table doesn't exist in engine after ALTER other tables
+--echo # with CONSTRAINTs
+--echo #
+
+set foreign_key_checks=on;
+create table t1 (id int not null primary key) engine=innodb;
+create table t2 (id int not null primary key, fid int not null,
+CONSTRAINT fk_fid FOREIGN KEY (fid) REFERENCES t1 (id))engine=innodb;
+
+insert into t1 values (1), (2), (3);
+insert into t2 values (1, 1), (2, 1), (3, 2);
+
+set foreign_key_checks=off;
+alter table t2 drop index fk_fid;
+set foreign_key_checks=on;
+
+--error ER_ROW_IS_REFERENCED_2
+delete from t1 where id=2;
+--error ER_NO_REFERENCED_ROW_2
+insert into t2 values(4, 99);
+
+select * from t1;
+select * from t2;
+
+set foreign_key_checks=off;
+delete from t1 where id=2;
+insert into t2 values(4, 99);
+set foreign_key_checks=on;
+
+select * from t1;
+select * from t2;
+
+show create table t1;
+show create table t2;
+
+# Optional: test DROP TABLE without any prior ha_innobase::open().
+# This was tested manually, but it would cause --embedded to skip the test,
+# and the restart would significantly increase the running time.
+# --source include/restart_mysqld.inc
+
+--error ER_ROW_IS_REFERENCED_2
+drop table t1,t2;
+--error ER_BAD_TABLE_ERROR
+drop table t1,t2;
diff --git a/mysql-test/suite/innodb/t/innodb_bug68148.test b/mysql-test/suite/innodb/t/innodb_bug68148.test
index 531baa30e48..2741c3cba3d 100644
--- a/mysql-test/suite/innodb/t/innodb_bug68148.test
+++ b/mysql-test/suite/innodb/t/innodb_bug68148.test
@@ -31,8 +31,6 @@ SHOW TABLES;
 --echo # restart and see if we can still access the main table
 --source include/restart_mysqld.inc
 
-# This is required to access the table
-SET FOREIGN_KEY_CHECKS=0;
 ALTER TABLE `main` ADD INDEX `idx_1` (`ref_id1`);
 SHOW CREATE TABLE `main`;
 
diff --git a/storage/innobase/dict/dict0dict.cc b/storage/innobase/dict/dict0dict.cc
index 3e5e8ca2fec..32fcfe68d32 100644
--- a/storage/innobase/dict/dict0dict.cc
+++ b/storage/innobase/dict/dict0dict.cc
@@ -432,7 +432,7 @@ dict_table_try_drop_aborted(
 
 	if (table == NULL) {
 		table = dict_table_open_on_id_low(
-			table_id, DICT_ERR_IGNORE_NONE, FALSE);
+			table_id, DICT_ERR_IGNORE_FK_NOKEY, FALSE);
 	} else {
 		ut_ad(table->id == table_id);
 	}
@@ -1005,7 +1005,7 @@ dict_table_open_on_id(
 		table_id,
 		table_op == DICT_TABLE_OP_LOAD_TABLESPACE
 		? DICT_ERR_IGNORE_RECOVER_LOCK
-		: DICT_ERR_IGNORE_NONE,
+		: DICT_ERR_IGNORE_FK_NOKEY,
 		table_op == DICT_TABLE_OP_OPEN_ONLY_IF_CACHED);
 
 	if (table != NULL) {
@@ -1167,7 +1167,7 @@ dict_table_open_on_name(
 	if (table != NULL) {
 
 		/* If table is encrypted or corrupted */
-		if (ignore_err == DICT_ERR_IGNORE_NONE
+		if (!(ignore_err & ~DICT_ERR_IGNORE_FK_NOKEY)
 		    && !table->is_readable()) {
 			/* Make life easy for drop table. */
 			dict_table_prevent_eviction(table);
diff --git a/storage/innobase/dict/dict0load.cc b/storage/innobase/dict/dict0load.cc
index a8ed8dd3e29..6900d62b225 100644
--- a/storage/innobase/dict/dict0load.cc
+++ b/storage/innobase/dict/dict0load.cc
@@ -3121,7 +3121,7 @@ func_exit:
 	mem_heap_free(heap);
 
 	ut_ad(!table
-	      || ignore_err != DICT_ERR_IGNORE_NONE
+	      || (ignore_err & ~DICT_ERR_IGNORE_FK_NOKEY)
 	      || !table->is_readable()
 	      || !table->corrupted);
 
diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc
index 21639fd63b6..7c9e4148886 100644
--- a/storage/innobase/handler/ha_innodb.cc
+++ b/storage/innobase/handler/ha_innodb.cc
@@ -3185,7 +3185,7 @@ static bool innobase_query_caching_table_check(
 	const char*	norm_name)
 {
 	dict_table_t*   table = dict_table_open_on_name(
-		norm_name, FALSE, FALSE, DICT_ERR_IGNORE_NONE);
+		norm_name, FALSE, FALSE, DICT_ERR_IGNORE_FK_NOKEY);
 
 	if (table == NULL) {
 		return false;
@@ -6209,9 +6209,7 @@ initialize_auto_increment(dict_table_t* table, const Field* field)
 int
 ha_innobase::open(const char* name, int, uint)
 {
-	dict_table_t*		ib_table;
 	char			norm_name[FN_REFLEN];
-	dict_err_ignore_t	ignore_err = DICT_ERR_IGNORE_NONE;
 
 	DBUG_ENTER("ha_innobase::open");
 
@@ -6225,15 +6223,8 @@ ha_innobase::open(const char* name, int, uint)
 
 	char*	is_part = is_partition(norm_name);
 	THD*	thd = ha_thd();
-
-	/* Check whether FOREIGN_KEY_CHECKS is set to 0. If so, the table
-	can be opened even if some FK indexes are missing. If not, the table
-	can't be opened in the same situation */
-	if (thd_test_options(thd, OPTION_NO_FOREIGN_KEY_CHECKS)) {
-		ignore_err = DICT_ERR_IGNORE_FK_NOKEY;
-	}
-
-	ib_table = open_dict_table(name, norm_name, is_part, ignore_err);
+	dict_table_t* ib_table = open_dict_table(name, norm_name, is_part,
+						 DICT_ERR_IGNORE_FK_NOKEY);
 
 	DEBUG_SYNC(thd, "ib_open_after_dict_open");
 
@@ -13404,8 +13395,8 @@ innobase_rename_table(
 		row_mysql_lock_data_dictionary(trx);
 	}
 
-	dict_table_t*   table = dict_table_open_on_name(norm_from, TRUE, FALSE,
-							DICT_ERR_IGNORE_NONE);
+	dict_table_t*   table = dict_table_open_on_name(
+		norm_from, TRUE, FALSE, DICT_ERR_IGNORE_FK_NOKEY);
 
 	/* Since DICT_BG_YIELD has sleep for 250 milliseconds,
 	Convert lock_wait_timeout unit from second to 250 milliseconds */
@@ -14582,7 +14573,7 @@ ha_innobase::defragment_table(
 	normalize_table_name(norm_name, name);
 
 	table = dict_table_open_on_name(norm_name, FALSE,
-		FALSE, DICT_ERR_IGNORE_NONE);
+		FALSE, DICT_ERR_IGNORE_FK_NOKEY);
 
 	for (index = dict_table_get_first_index(table); index;
 	     index = dict_table_get_next_index(index)) {
diff --git a/storage/innobase/include/dict0types.h b/storage/innobase/include/dict0types.h
index 39addf697e8..93c2f570e54 100644
--- a/storage/innobase/include/dict0types.h
+++ b/storage/innobase/include/dict0types.h
@@ -59,11 +59,11 @@ Note: please define the IGNORE_ERR_* as bits, so their value can
 be or-ed together */
 enum dict_err_ignore_t {
 	DICT_ERR_IGNORE_NONE = 0,	/*!< no error to ignore */
-	DICT_ERR_IGNORE_INDEX_ROOT = 1,	/*!< ignore error if index root
-					page is FIL_NULL or incorrect value */
-	DICT_ERR_IGNORE_CORRUPT = 2,	/*!< skip corrupted indexes */
-	DICT_ERR_IGNORE_FK_NOKEY = 4,	/*!< ignore error if any foreign
+	DICT_ERR_IGNORE_FK_NOKEY = 1,	/*!< ignore error if any foreign
 					key is missing */
+	DICT_ERR_IGNORE_INDEX_ROOT = 2,	/*!< ignore error if index root
+					page is FIL_NULL or incorrect value */
+	DICT_ERR_IGNORE_CORRUPT = 4,	/*!< skip corrupted indexes */
 	DICT_ERR_IGNORE_RECOVER_LOCK = 8,
 					/*!< Used when recovering table locks
 					for resurrected transactions.
diff --git a/storage/innobase/row/row0ins.cc b/storage/innobase/row/row0ins.cc
index ece291378b6..2cda0e3d03c 100644
--- a/storage/innobase/row/row0ins.cc
+++ b/storage/innobase/row/row0ins.cc
@@ -875,8 +875,12 @@ row_ins_foreign_report_add_err(
 	fk_str = dict_print_info_on_foreign_key_in_create_format(trx, foreign,
 							TRUE);
 	fputs(fk_str.c_str(), ef);
-	fprintf(ef, " in parent table, in index %s",
-		foreign->foreign_index->name());
+	if (foreign->foreign_index) {
+		fprintf(ef, " in parent table, in index %s",
+			foreign->foreign_index->name());
+	} else {
+		fputs(" in parent table", ef);
+	}
 	if (entry) {
 		fputs(" tuple:\n", ef);
 		/* TODO: DB_TRX_ID and DB_ROLL_PTR may be uninitialized.
@@ -1628,34 +1632,51 @@ row_ins_check_foreign_constraint(
 	    || check_index == NULL
 	    || fil_space_get(check_table->space)->is_being_truncated) {
 
-		if (!srv_read_only_mode && check_ref) {
-			FILE*	ef = dict_foreign_err_file;
-			std::string fk_str;
-
-			row_ins_set_detailed(trx, foreign);
-
-			row_ins_foreign_trx_print(trx);
-
-			fputs("Foreign key constraint fails for table ", ef);
-			ut_print_name(ef, trx,
-				      foreign->foreign_table_name);
-			fputs(":\n", ef);
-			fk_str = dict_print_info_on_foreign_key_in_create_format(
-				trx, foreign, TRUE);
-			fputs(fk_str.c_str(), ef);
-			fprintf(ef, "\nTrying to add to index %s tuple:\n",
-				foreign->foreign_index->name());
+		FILE*	ef = dict_foreign_err_file;
+		std::string fk_str;
+
+		row_ins_set_detailed(trx, foreign);
+		row_ins_foreign_trx_print(trx);
+
+		fputs("Foreign key constraint fails for table ", ef);
+		ut_print_name(ef, trx, check_ref
+			      ? foreign->foreign_table_name
+			      : foreign->referenced_table_name);
+		fputs(":\n", ef);
+		fk_str = dict_print_info_on_foreign_key_in_create_format(
+			trx, foreign, TRUE);
+		fputs(fk_str.c_str(), ef);
+		if (check_ref) {
+			if (foreign->foreign_index) {
+				fprintf(ef, "\nTrying to add to index %s"
+					" tuple:\n",
+					foreign->foreign_index->name());
+			} else {
+				fputs("\nTrying to add tuple:\n", ef);
+			}
 			dtuple_print(ef, entry);
 			fputs("\nBut the parent table ", ef);
-			ut_print_name(ef, trx,
-				      foreign->referenced_table_name);
-			fputs("\nor its .ibd file does"
+			ut_print_name(ef, trx, foreign->referenced_table_name);
+			fputs("\nor its .ibd file or the required index does"
 			      " not currently exist!\n", ef);
-			mutex_exit(&dict_foreign_err_mutex);
-
 			err = DB_NO_REFERENCED_ROW;
+		} else {
+			if (foreign->referenced_index) {
+				fprintf(ef, "\nTrying to modify index %s"
+					" tuple:\n",
+					foreign->referenced_index->name());
+			} else {
+				fputs("\nTrying to modify tuple:\n", ef);
+			}
+			dtuple_print(ef, entry);
+			fputs("\nBut the referencing table ", ef);
+			ut_print_name(ef, trx, foreign->foreign_table_name);
+			fputs("\nor its .ibd file or the required index does"
+			      " not currently exist!\n", ef);
+			err = DB_ROW_IS_REFERENCED;
 		}
 
+		mutex_exit(&dict_foreign_err_mutex);
 		goto exit_func;
 	}
 
@@ -1923,6 +1944,7 @@ row_ins_check_foreign_constraints(
 /*==============================*/
 	dict_table_t*	table,	/*!< in: table */
 	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 */
 {
@@ -1931,6 +1953,8 @@ row_ins_check_foreign_constraints(
 	trx_t*		trx;
 	ibool		got_s_lock	= FALSE;
 
+	DBUG_ASSERT(index->is_primary() == pk);
+
 	trx = thr_get_trx(thr);
 
 	DEBUG_SYNC_C_IF_THD(thr_get_trx(thr)->mysql_thd,
@@ -1942,7 +1966,8 @@ row_ins_check_foreign_constraints(
 
 		foreign = *it;
 
-		if (foreign->foreign_index == index) {
+		if (foreign->foreign_index == index
+		    || (pk && !foreign->foreign_index)) {
 			dict_table_t*	ref_table = NULL;
 			dict_table_t*	referenced_table
 						= foreign->referenced_table;
@@ -3119,7 +3144,7 @@ row_ins_clust_index_entry(
 
 	if (!index->table->foreign_set.empty()) {
 		err = row_ins_check_foreign_constraints(
-			index->table, index, entry, thr);
+			index->table, index, true, entry, thr);
 		if (err != DB_SUCCESS) {
 
 			DBUG_RETURN(err);
@@ -3193,7 +3218,7 @@ row_ins_sec_index_entry(
 
 	if (!index->table->foreign_set.empty()) {
 		err = row_ins_check_foreign_constraints(index->table, index,
-							entry, thr);
+							false, entry, thr);
 		if (err != DB_SUCCESS) {
 
 			return(err);
diff --git a/storage/innobase/row/row0mysql.cc b/storage/innobase/row/row0mysql.cc
index f3d48edfb80..4b82fb9c2c4 100644
--- a/storage/innobase/row/row0mysql.cc
+++ b/storage/innobase/row/row0mysql.cc
@@ -2799,7 +2799,7 @@ row_discard_tablespace_begin(
 	dict_table_t*	table;
 
 	table = dict_table_open_on_name(
-		name, TRUE, FALSE, DICT_ERR_IGNORE_NONE);
+		name, TRUE, FALSE, DICT_ERR_IGNORE_FK_NOKEY);
 
 	if (table) {
 		dict_stats_wait_bg_to_stop_using_table(table, trx);
@@ -3199,7 +3199,7 @@ row_drop_table_from_cache(
 
 	dict_table_remove_from_cache(table);
 
-	if (dict_load_table(tablename, true, DICT_ERR_IGNORE_NONE)) {
+	if (dict_load_table(tablename, true, DICT_ERR_IGNORE_FK_NOKEY)) {
 		ib::error() << "Not able to remove table "
 			<< ut_get_name(trx, tablename)
 			<< " from the dictionary cache!";
@@ -4164,7 +4164,7 @@ row_rename_table_for_mysql(
 	dict_locked = trx->dict_operation_lock_mode == RW_X_LATCH;
 
 	table = dict_table_open_on_name(old_name, dict_locked, FALSE,
-					DICT_ERR_IGNORE_NONE);
+					DICT_ERR_IGNORE_FK_NOKEY);
 
 	/* We look for pattern #P# to see if the table is partitioned
 	MySQL table. */
@@ -4212,7 +4212,7 @@ row_rename_table_for_mysql(
 			par_case_name, old_name, FALSE);
 #endif
 		table = dict_table_open_on_name(par_case_name, dict_locked, FALSE,
-					DICT_ERR_IGNORE_NONE);
+						DICT_ERR_IGNORE_FK_NOKEY);
 	}
 
 	if (!table) {