Bug#27821060 "NEWDD FK: DROP TABLES/DATABASE SHOULD CHECK FOR FKS".

After we have started to store information about foreign keys in the
New Data Dictionary it became possible to move checks of foreign key
definition validity from the storage engine to SQL-layer and thus
reduce code duplication and simplify SE implementation.

This patch moves check that disallows dropping of parent table in
a foreign key without prior dropping of child table from InnoDB
SE to SQL-layer code implementing DROP TABLES/DROP DATABASE.

Such check now happens before trying to delete any tables mentioned
in DROP TABLES statement/belonging to schema to be dropped. So DROP
TABLES/DROP DATABASE no longer have any side-effect even on tables
in SEs which don't support atomic DDL when they fail due to this check.

The check was also relaxed to allow dropping of parent and child table
in arbitrary order as long as they are dropped by the same DROP TABLES
statement. This is possible thanks to the fact that both our engines
supporting foreign keys also support atomic DDL.

Finally, error message which is emitted when user attempts to drop
parent table without dropping child was changed to more verbose one.

New test coverage for this check was added and existing tests were
adjusted accordingly. Also, some test cases which previously used
foreign key dependency as a way to fail DROP TABLES/DATABASE in the
middle of their execution had to be adjusted to rely on error injection
instead.

Author: Dmitry Lenev

mysql-test/extra/binlog_tests/database.test

@@ -1,4 +1,5 @@
 source include/have_log_bin.inc;
+source include/have_debug.inc;
 
 # Checking that the drop of a database does not replicate anything in
 # addition to the drop of the database
@@ -48,20 +49,20 @@ CREATE DATABASE testing_1;
 USE testing_1;
 CREATE TABLE t1(c1 INT) ENGINE=MyISAM;
 CREATE TABLE t2(c1 INT PRIMARY KEY) ENGINE=InnoDB;
-CREATE TABLE test.t1 (c1 INT, FOREIGN KEY(c1) REFERENCES testing_1.t2(c1)) ENGINE=InnoDB;
 
 --echo #
 --echo # 'DROP DATABASE' will fail but will delete table t1.
 --echo #
 
---error ER_ROW_IS_REFERENCED
+SET SESSION DEBUG='+d,rm_table_no_locks_abort_after_atomic_tables';
+--error ER_UNKNOWN_ERROR
 DROP DATABASE testing_1;
+SET SESSION DEBUG='-d,rm_table_no_locks_abort_after_atomic_tables';
 let $wait_binlog_event= DROP TABLE IF EXIST;
 source include/wait_for_binlog_event.inc;
 let $MYSQLD_DATADIR= `SELECT @@datadir`;
 
-DROP TABLE test.t1;
---echo # Now we can drop the database.
+--echo # Cleanup.
 DROP DATABASE testing_1;
 
 
@@ -79,19 +80,18 @@ DROP TABLE IF EXISTS t3;
 CREATE DATABASE db1;
 CREATE TABLE db1.t1 (a INT) engine=innodb;
 CREATE TABLE db1.t2 (b INT, KEY(b)) engine=innodb;
-CREATE TABLE t3 (a INT, KEY (a), FOREIGN KEY(a) REFERENCES db1.t2(b))
-  engine=innodb;
 RESET MASTER;
 
---error ER_ROW_IS_REFERENCED
-DROP DATABASE db1;                      # Fails because of the fk
+SET SESSION DEBUG='+d,rm_table_no_locks_abort_after_atomic_tables';
+--error ER_UNKNOWN_ERROR
+DROP DATABASE db1;
+SET SESSION DEBUG='-d,rm_table_no_locks_abort_after_atomic_tables';
 # Both t1 and t2 remain as the whole statement is rolled back.
 SHOW TABLES FROM db1;
 --let $mask_binlog_commit_events= 1
 --source include/show_binlog_events.inc # Check that the binlog is empty.
 --let $mask_binlog_commit_events= 0
 
 # Cleanup
-DROP TABLE t3;
 DROP DATABASE db1;
 RESET MASTER;

mysql-test/extra/rpl_tests/rpl_split_statements.test

@@ -97,12 +97,10 @@
 #     according to GTID_MODE.
 #
 #     Third, we drop group of 4 tables, where 2 tables are in SE which
-#     do not support atomic DDL and 2 in SE which support it and the
-#     last table from the latter group can't be dropped. This triggers
-#     different code path in DROP TABLE implementation. Again this is
-#     done for both GTID_MODE='AUTOMATIC' and GTID_MODE='UUID:NUMBER'
-#     to see difference in splitting/combining. Notice that with GTID
-#     assigned even such failed DROP TABLE will consume GTID.
+#     do not support atomic DDL and 2 in SE which support it and then
+#     fail. This triggers code path in DROP TABLE implementation
+#     which differs from case when there is no failure. This test has
+#     been moved into rpl_split_statements_debug.test.
 #
 #     Note that two last tests also provide coverage for changes to binary
 #     logging and GTID handling for normal and failed DROP TABLES, which
@@ -332,93 +330,11 @@ if (!$gtid_mode_on)
 --source include/gtid_step_assert.inc
 }
 
---echo ==== Case 2D: Failing DROP TABLES for base tables with and without atomic DDL support ====
-
-CREATE TABLE base_1_n (a INT) ENGINE = MyISAM;
-CREATE TABLE base_2_n (a INT) ENGINE = MyISAM;
-CREATE TABLE base_3_a (a INT) ENGINE = InnoDB;
-CREATE TABLE base_4_a (pk INT PRIMARY KEY) ENGINE = InnoDB;
-CREATE TABLE base_5_a (fk INT, FOREIGN KEY (fk) REFERENCES base_4_a(pk)) ENGINE = InnoDB;
-
---source include/rpl_sync.inc
-
---echo ---- GTID_MODE=AUTOMATIC ----
-
---source include/gtid_step_reset.inc
-
---error ER_ROW_IS_REFERENCED
-DROP TABLES base_1_n, base_2_n, base_3_a, base_4_a;
-
---echo # In AUTOMATIC mode the above statement should be split into
---echo # two statements for each of MyISAM tables.
---echo # There should be no statement for dropping base_3_a as its
---echo # removal is rolled back.
---let $gtid_step_count= 2.
---source include/gtid_step_assert.inc
-
---echo # base_3_a and base_4_a should still be there on master
-SHOW CREATE TABLE base_3_a;
-SHOW CREATE TABLE base_4_a;
-
---source include/rpl_sync.inc
-
---connection slave
-
---echo # base_3_a and base_4_a should still be there on master
-SHOW CREATE TABLE base_3_a;
-SHOW CREATE TABLE base_4_a;
-
---connection master
-
-CREATE TABLE base_1_n (a INT) ENGINE = MyISAM;
-CREATE TABLE base_2_n (a INT) ENGINE = MyISAM;
-
 --source include/rpl_sync.inc
 
---echo ---- GTID_NEXT=non-automatic ----
-
---source include/gtid_step_reset.inc
-
---source include/set_gtid_next_gtid_mode_agnostic.inc
-
---error ER_ROW_IS_REFERENCED
-DROP TABLES base_1_n, base_2_n, base_3_a, base_4_a;
-
-SET GTID_NEXT= 'AUTOMATIC';
-
-if ($gtid_mode_on)
-{
---echo # With GTID assigned the above statement should not be split.
---let $gtid_step_count= 1
---source include/gtid_step_assert.inc
-}
-if (!$gtid_mode_on)
-{
---echo # Without GTID assigned the above statement should be split into
---echo # two as in AUTOMATIC mode.
---let $gtid_step_count= 2
---source include/gtid_step_assert.inc
-}
-
---echo # base_3_a and base_4_a should still be there on master
-SHOW CREATE TABLE base_3_a;
-SHOW CREATE TABLE base_4_a;
-
---source include/rpl_sync.inc
-
---connection slave
-
---echo # base_3_a and base_4_a should still be there on master
-SHOW CREATE TABLE base_3_a;
-SHOW CREATE TABLE base_4_a;
-
---connection master
-
---echo ---- Clean up ----
-
-DROP TABLE base_5_a, base_4_a, base_3_a;
+--echo ==== Case 2D: Failing DROP TABLES for base tables with and without atomic DDL support ====
 
---source include/rpl_sync.inc
+--echo See rpl_split_statements_debug.test
 
 --echo ==== Case 3: DROP DATABASE ====
 

mysql-test/extra/rpl_tests/rpl_split_statements_debug.test

@@ -1,7 +1,24 @@
 # ==== Purpose ====
 #
 # This is part of rpl_split_statements.test which covers DROP DATABASE
-# statement.
+# statement and one scenario for DROP TABLES statement.
+#
+# ==============================================================================
+#
+# DROP TABLES: See detailed description of its behavior in
+# rpl_split_statements.test file.
+#
+# This file contains coverage for the following scenario:
+#
+#   We drop group of 4 tables, where 2 tables are in SE which
+#   do not support atomic DDL and 2 in SE which support it and
+#   then fail. This triggers code path in DROP TABLE implementation
+#   which differs from case when there is no failure.
+#   This is done for both GTID_MODE='AUTOMATIC' and GTID_MODE='UUID:NUMBER'
+#   to see difference in splitting/combining. Notice that with GTID
+#   assigned even such failed DROP TABLE will consume GTID.
+#
+# ==============================================================================
 #
 # DROP DATABASE: this statement will first drop all non-temporary
 # tables in the database, then stored routines and events in it,
@@ -52,6 +69,7 @@
 # This last scenario and scenario 3.6 also provide coverage for changes to
 # DROP DATABASE binary logging which were implemented as part of WL#7743
 # "New data dictionary: changes to DDL-related parts of SE API".
+#
 
 # We use DBUG_EXECUTE_IF to simulate errors during DROP DATABASE.
 --source include/have_debug.inc
@@ -77,6 +95,99 @@ CALL mtr.add_suppression("Slave SQL for channel '': .* Error_code: MY-001105");
 
 --let $gtid_step_gtid_mode_agnostic= 1
 
+--echo ==== Case 2: DROP TABLES ====
+
+--echo ==== Case 2D: Failing DROP TABLES for base tables with and without atomic DDL support ====
+
+CREATE TABLE base_1_n (a INT) ENGINE = MyISAM;
+CREATE TABLE base_2_n (a INT) ENGINE = MyISAM;
+CREATE TABLE base_3_a (a INT) ENGINE = InnoDB;
+CREATE TABLE base_4_a (pk INT PRIMARY KEY) ENGINE = InnoDB;
+
+--source include/rpl_sync.inc
+
+--echo ---- GTID_MODE=AUTOMATIC ----
+
+--source include/gtid_step_reset.inc
+
+SET SESSION DEBUG='+d,rm_table_no_locks_abort_after_atomic_tables';
+--error ER_UNKNOWN_ERROR
+DROP TABLES base_1_n, base_2_n, base_3_a, base_4_a;
+SET SESSION DEBUG='-d,rm_table_no_locks_abort_after_atomic_tables';
+
+--echo # In AUTOMATIC mode the above statement should be split into
+--echo # two statements for each of MyISAM tables.
+--echo # There should be no statement for dropping base_3_a or
+--echo # base_4_a as their removal is rolled back.
+--let $gtid_step_count= 2.
+--source include/gtid_step_assert.inc
+
+--echo # base_3_a and base_4_a should still be there on master
+SHOW CREATE TABLE base_3_a;
+SHOW CREATE TABLE base_4_a;
+
+--source include/rpl_sync.inc
+
+--connection slave
+
+--echo # base_3_a and base_4_a should still be there on slave
+SHOW CREATE TABLE base_3_a;
+SHOW CREATE TABLE base_4_a;
+
+--connection master
+
+CREATE TABLE base_1_n (a INT) ENGINE = MyISAM;
+CREATE TABLE base_2_n (a INT) ENGINE = MyISAM;
+
+--source include/rpl_sync.inc
+
+--echo ---- GTID_NEXT=non-automatic ----
+
+--source include/gtid_step_reset.inc
+
+--source include/set_gtid_next_gtid_mode_agnostic.inc
+
+SET SESSION DEBUG='+d,rm_table_no_locks_abort_after_atomic_tables';
+--error ER_UNKNOWN_ERROR
+DROP TABLES base_1_n, base_2_n, base_3_a, base_4_a;
+SET SESSION DEBUG='-d,rm_table_no_locks_abort_after_atomic_tables';
+
+SET GTID_NEXT= 'AUTOMATIC';
+
+if ($gtid_mode_on)
+{
+--echo # With GTID assigned the above statement should not be split.
+--let $gtid_step_count= 1
+--source include/gtid_step_assert.inc
+}
+if (!$gtid_mode_on)
+{
+--echo # Without GTID assigned the above statement should be split into
+--echo # two as in AUTOMATIC mode.
+--let $gtid_step_count= 2
+--source include/gtid_step_assert.inc
+}
+
+--echo # base_3_a and base_4_a should still be there on master
+SHOW CREATE TABLE base_3_a;
+SHOW CREATE TABLE base_4_a;
+
+--source include/rpl_sync.inc
+
+--connection slave
+
+--echo # base_3_a and base_4_a should still be there on slave
+SHOW CREATE TABLE base_3_a;
+SHOW CREATE TABLE base_4_a;
+
+--connection master
+
+--echo ---- Clean up ----
+
+DROP TABLE base_4_a, base_3_a;
+
+--source include/rpl_sync.inc
+
 --echo ==== Case 3: DROP DATABASE ====
 
 --echo ---- Initialize ----

mysql-test/r/drop_debug.result

@@ -77,20 +77,17 @@ ERROR 42S02: Table 'test.tt_m' doesn't exist
 SELECT * FROM tt_i;
 ERROR 42S02: Table 'test.tt_i' doesn't exist
 #
-# 1.c) DROP TABLES which fails due to foreign key error might
-#      have side effect. This might change once WL#6049 is
-#      implemented and we support smarter checks while dropping
-#      tables with foreign keys. Until this side-effect is
-#      limited to non-atomic SEs only.
+# 1.c) DROP TABLES which fails due to foreign key error does
+#      not have side effect.
 CREATE TABLE t_m (t_m INT) ENGINE=MyISAM;
 CREATE TABLE t_i_1 (t_i_1 INT) ENGINE=InnoDB;
 CREATE TABLE t_i_2 (t_i_2 INT PRIMARY KEY) ENGINE=InnoDB;
 CREATE TABLE t_i_3 (t_i_3 INT, FOREIGN KEY(t_i_3) REFERENCES t_i_2(t_i_2)) ENGINE=InnoDB;
 DROP TABLES t_m, t_i_1, t_i_2;
-ERROR 23000: Cannot delete or update a parent row: a foreign key constraint fails
-# Table t_m is gone, t_i_1 and t_i_2 are still there.
+ERROR HY000: Cannot drop table 't_i_2' referenced by a foreign key constraint 't_i_3_ibfk_1' on table 't_i_3'.
+# All tables are still there.
 SELECT * FROM t_m;
-ERROR 42S02: Table 'test.t_m' doesn't exist
+t_m
 SELECT * FROM t_i_1;
 t_i_1
 SELECT * FROM t_i_2;
@@ -100,7 +97,6 @@ t_i_2
 #      effect. Tables in engines which do not support atomic DDL
 #      which we have managed to drop before error stay dropped.
 #      Removal of InnoDB tables should be rolled back.
-CREATE TABLE t_m (t_m INT) ENGINE=MyISAM;
 SET SESSION DEBUG='+d,rm_table_no_locks_abort_after_atomic_tables';
 DROP TABLES t_m, t_i_1;
 ERROR HY000: Unknown error
@@ -157,9 +153,8 @@ Warning	1017	Can't find file: 't1' (errno: 2 - No such file or directory)
 # 4) Error handling by DROP DATABASE.
 #
 #
-# 4.a) DROP DATABASE which fails due to foreign key error might
-#      have side effect. This  side-effect is limited to non-atomic
-#      SEs only.
+# 4.a) DROP DATABASE which fails due to foreign key error should
+#      not have side effect.
 CREATE DATABASE mysqltest;
 CREATE TABLE mysqltest.t_m (t_m INT) ENGINE=MyISAM;
 CREATE TABLE mysqltest.t_i_1 (t_i_1 INT) ENGINE= InnoDB;
@@ -169,11 +164,10 @@ CREATE TABLE t1 (fk INT,
 FOREIGN KEY (fk) REFERENCES mysqltest.t_i_2(t_i_2))
 ENGINE=InnoDB;
 DROP DATABASE mysqltest;
-ERROR 23000: Cannot delete or update a parent row: a foreign key constraint fails
-# Table t_m is gone.
+ERROR HY000: Cannot drop table 't_i_2' referenced by a foreign key constraint 't1_ibfk_1' on table 't1'.
+# Database and all tables are still there.
 SELECT * FROM mysqltest.t_m;
-ERROR 42S02: Table 'mysqltest.t_m' doesn't exist
-# Database and tables t_i_1, t_i_2 are still there.
+t_m
 SELECT * FROM mysqltest.t_i_1;
 t_i_1
 SELECT * FROM mysqltest.t_i_2;
@@ -188,7 +182,6 @@ mysqltest.f1()
 #      which we have managed to drop before error stay dropped.
 #      Removal of InnoDB tables should be rolled back.
 DROP TABLE t1;
-CREATE TABLE mysqltest.t_m (t_m INT) ENGINE=MyISAM;
 SET SESSION DEBUG='+d,rm_db_fail_after_dropping_tables';
 DROP DATABASE mysqltest;
 ERROR HY000: Unknown error