From 2bb5981c202f85c8399485d6a57a69d8c6f627af Mon Sep 17 00:00:00 2001 From: Sergei Golubchik Date: Mon, 15 Jun 2020 12:13:44 +0200 Subject: [PATCH] MDEV-11412 Ensure that table is truly dropped when using DROP TABLE minor post-review fixes * remove duplicate tests * first file indicates table existance even with discovery * don't duplicate trigger dropping code --- mysql-test/main/drop_table_force.result | 13 +------- mysql-test/main/drop_table_force.test | 22 +------------- sql/handler.cc | 13 ++------ sql/sql_table.cc | 40 ++++++------------------- 4 files changed, 13 insertions(+), 75 deletions(-) diff --git a/mysql-test/main/drop_table_force.result b/mysql-test/main/drop_table_force.result index bb4ecc060b0..d3142887ade 100644 --- a/mysql-test/main/drop_table_force.result +++ b/mysql-test/main/drop_table_force.result @@ -8,7 +8,7 @@ db.opt # Test droping table without frm without super privilege create table t1(a int) engine=innodb; create user test identified by '123456'; -grant all privileges on test.t1 to 'test'@'%'identified by '123456' with grant option; +grant all privileges on test.t1 to 'test'@'%'identified by '123456'; connect con_test, localhost, test,'123456', ; connection con_test; drop table t1; @@ -18,10 +18,6 @@ connection default; disconnect con_test; drop user test; db.opt -#Test4: drop table can drop consistent table as well -create table t1(a int) engine=innodb; -drop table t1; -db.opt #Test5: drop table with triger, and with missing frm create table t1(a int)engine=innodb; create trigger t1_trg before insert on t1 for each row begin end; @@ -45,13 +41,6 @@ drop table if exists t1; Warnings: Note 1051 Unknown table 'test.t1' db.opt -#Test8: check compatibility with if exists -create table t1(a int)engine=innodb; -drop table t1; -db.opt -drop table if exists t1; -Warnings: -Note 1051 Unknown table 'test.t1' #Test9: check compatibility with restrict/cascade CREATE TABLE parent (id INT NOT NULL, PRIMARY KEY (id)) ENGINE=INNODB; CREATE TABLE child (id INT, parent_id INT, INDEX par_ind (parent_id), FOREIGN KEY (parent_id) REFERENCES parent(id) ON DELETE CASCADE) ENGINE=INNODB; diff --git a/mysql-test/main/drop_table_force.test b/mysql-test/main/drop_table_force.test index bb735309167..518b4e754c3 100644 --- a/mysql-test/main/drop_table_force.test +++ b/mysql-test/main/drop_table_force.test @@ -32,7 +32,7 @@ create table t1(a int) engine=innodb; # create test user create user test identified by '123456'; -grant all privileges on test.t1 to 'test'@'%'identified by '123456' with grant option; +grant all privileges on test.t1 to 'test'@'%'identified by '123456'; # connect as test connect (con_test, localhost, test,'123456', ); @@ -52,13 +52,6 @@ drop user test; # check files in datadir about t1 --list_files $DATADIR/test/ ---echo #Test4: drop table can drop consistent table as well -create table t1(a int) engine=innodb; -drop table t1; - -# check files in datadir about t1 ---list_files $DATADIR/test/ - --echo #Test5: drop table with triger, and with missing frm # create table t1 with triger and rm frm create table t1(a int)engine=innodb; @@ -107,19 +100,6 @@ drop table if exists t1; # check files in datadir about t1 --list_files $DATADIR/test/ ---echo #Test8: check compatibility with if exists -create table t1(a int)engine=innodb; ---remove_file $DATADIR/test/t1.frm - -# first drop will success -drop table t1; - -# check files in datadir about t1 ---list_files $DATADIR/test/ - -# second drop with if exists will success -drop table if exists t1; - --echo #Test9: check compatibility with restrict/cascade # create table with foreign key reference and rm frm CREATE TABLE parent (id INT NOT NULL, PRIMARY KEY (id)) ENGINE=INNODB; diff --git a/sql/handler.cc b/sql/handler.cc index 8642c773007..28588235477 100644 --- a/sql/handler.cc +++ b/sql/handler.cc @@ -4457,21 +4457,12 @@ int handler::delete_table(const char *name) // For discovery tables, it's ok if first file doesn't exists if (ht->discover_table) - { - abort_if_first_file_error= 0; saved_error= 0; - if (!bas_ext()) - { - DBUG_ASSERT(ht->flags & HTON_AUTOMATIC_DELETE_TABLE); - DBUG_RETURN(0); // Drop succeded - } - } for (const char **ext= bas_ext(); *ext ; ext++) { - int error; - if ((error= mysql_file_delete_with_symlink(key_file_misc, name, *ext, - MYF(0)))) + int err= mysql_file_delete_with_symlink(key_file_misc, name, *ext, MYF(0)); + if (err) { if (my_errno != ENOENT) { diff --git a/sql/sql_table.cc b/sql/sql_table.cc index de1ebb4f191..0d6e2c69695 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -2308,7 +2308,7 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists, for (table= tables; table; table= table->next_local) { bool is_trans= 0, frm_was_deleted= 0, temporary_table_was_dropped= 0; - bool table_creation_was_logged= 0, trigger_drop_executed= 0; + bool table_creation_was_logged= 0; bool local_non_tmp_error= 0, frm_exists= 0, wrong_drop_sequence= 0; bool table_dropped= 0; LEX_CSTRING db= table->db; @@ -2363,14 +2363,6 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists, */ if (!dont_log_query && table_creation_was_logged) { - /* - DROP TEMPORARY succeded. For the moment when we only come - here on success (error == 0) - - If there is an error, we don't know the type of the engine - at this point. So, we keep it in the trx-cache. - */ - is_trans= error ? TRUE : is_trans; if (is_trans) trans_tmp_table_deleted= TRUE; else @@ -2416,7 +2408,6 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists, } DEBUG_SYNC(thd, "rm_table_no_locks_before_delete_table"); - error= 0; if (drop_temporary) { /* "DROP TEMPORARY" but a temporary table was not found */ @@ -2551,16 +2542,6 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists, table_dropped= 1; } } - if (likely(!error) || non_existing_table_error(error)) - { - trigger_drop_executed= 1; - - if (Table_triggers_list::drop_all_triggers(thd, &db, - &table->table_name, - MYF(MY_WME | - MY_IGNORE_ENOENT))) - error= error ? error : -1; - } local_non_tmp_error|= MY_TEST(error); } @@ -2568,14 +2549,9 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists, If there was no .frm file and the table is not temporary, scan all engines try to drop the table from there. This is to ensure we don't have any partial table files left. - - We check for trigger_drop_executed to ensure we don't again try - to drop triggers when it failed above (after sucecssfully dropping - the table). */ if (non_existing_table_error(error) && !drop_temporary && - table_type != view_pseudo_hton && !trigger_drop_executed && - !wrong_drop_sequence) + table_type != view_pseudo_hton && !wrong_drop_sequence) { int ferror= 0; @@ -2601,16 +2577,18 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists, MYF(MY_WME | MY_IGNORE_ENOENT))) ferror= my_errno; } - if (Table_triggers_list::drop_all_triggers(thd, &db, - &table->table_name, - MYF(MY_WME | - MY_IGNORE_ENOENT))) - ferror= -1; } if (!error) error= ferror; } + if (likely(!error) || non_existing_table_error(error)) + { + if (Table_triggers_list::drop_all_triggers(thd, &db, &table->table_name, + MYF(MY_WME | MY_IGNORE_ENOENT))) + error= error ? error : -1; + } + if (error) { char buff[FN_REFLEN];