diff --git a/mysql-test/main/create_or_replace.result b/mysql-test/main/create_or_replace.result index 142856d603f..d9ca4102b06 100644 --- a/mysql-test/main/create_or_replace.result +++ b/mysql-test/main/create_or_replace.result @@ -847,3 +847,19 @@ t2 CREATE TABLE `t2` ( `a` varchar(2300) DEFAULT NULL ) ENGINE=Aria DEFAULT CHARSET=latin1 PAGE_CHECKSUM=1 drop tables t2, t1; +# +# MDEV-28956 Locking is broken if CREATE OR REPLACE fails under LOCK TABLES +# +# Atomic CREATE OR REPLACE part: +# +create table t1 (pk int primary key) engine=innodb; +create or replace table t2 (a int primary key references t1 (pk)) engine=innodb; +lock tables t1 write, t2 write; +create or replace table t2 (c1 int not null, c1 varchar(255) ) engine=innodb; +ERROR 42S21: Duplicate column name 'c1' +select * from t1; +pk +select * from t2; +a +unlock tables; +drop tables t2, t1; diff --git a/mysql-test/main/create_or_replace.test b/mysql-test/main/create_or_replace.test index c18c8b9eb7c..316d904e591 100644 --- a/mysql-test/main/create_or_replace.test +++ b/mysql-test/main/create_or_replace.test @@ -638,3 +638,20 @@ create or replace table t2 engine aria select * from t1; select * from t2; show create table t2; drop tables t2, t1; + +--echo # +--echo # MDEV-28956 Locking is broken if CREATE OR REPLACE fails under LOCK TABLES +--echo # +--echo # Atomic CREATE OR REPLACE part: +--echo # +create table t1 (pk int primary key) engine=innodb; +create or replace table t2 (a int primary key references t1 (pk)) engine=innodb; + +lock tables t1 write, t2 write; +--error ER_DUP_FIELDNAME +create or replace table t2 (c1 int not null, c1 varchar(255) ) engine=innodb; +select * from t1; +select * from t2; +unlock tables; + +drop tables t2, t1; diff --git a/mysql-test/main/create_or_replace2.result b/mysql-test/main/create_or_replace2.result index f7bbb7417e6..dd183441e42 100644 --- a/mysql-test/main/create_or_replace2.result +++ b/mysql-test/main/create_or_replace2.result @@ -72,3 +72,22 @@ ERROR 42000: Duplicate key name 'k' create or replace table t1 (a int, b int, key k (a), key k (b)); ERROR 42000: Duplicate key name 'k' drop table t1; +# +# MDEV-28956 Locking is broken if CREATE OR REPLACE fails under LOCK TABLES +# +# Non-atomic CREATE OR REPLACE part: +# +set @saved_debug_dbug= @@session.debug_dbug; +set @@debug_dbug="+d,ddl_log_expensive_rename"; +create table t1 (pk int primary key) engine=innodb; +create or replace table t2 (a int primary key references t1 (pk)) engine=innodb; +lock tables t1 write, t2 write; +create or replace table t2 (c1 int not null, c1 varchar(255) ) engine=innodb; +ERROR 42S21: Duplicate column name 'c1' +select * from t1; +pk +select * from t2; +ERROR HY000: Table 't2' was not locked with LOCK TABLES +unlock tables; +drop tables t1; +set @@debug_dbug= @saved_debug_dbug; diff --git a/mysql-test/main/create_or_replace2.test b/mysql-test/main/create_or_replace2.test index 227aff226d5..bd82d13f1d1 100644 --- a/mysql-test/main/create_or_replace2.test +++ b/mysql-test/main/create_or_replace2.test @@ -65,3 +65,24 @@ create or replace table t1 (a int, b int, key k (a), key k (b)); --error ER_DUP_KEYNAME create or replace table t1 (a int, b int, key k (a), key k (b)); drop table t1; + +--echo # +--echo # MDEV-28956 Locking is broken if CREATE OR REPLACE fails under LOCK TABLES +--echo # +--echo # Non-atomic CREATE OR REPLACE part: +--echo # +set @saved_debug_dbug= @@session.debug_dbug; +set @@debug_dbug="+d,ddl_log_expensive_rename"; +create table t1 (pk int primary key) engine=innodb; +create or replace table t2 (a int primary key references t1 (pk)) engine=innodb; + +lock tables t1 write, t2 write; +--error ER_DUP_FIELDNAME +create or replace table t2 (c1 int not null, c1 varchar(255) ) engine=innodb; +select * from t1; +--error ER_TABLE_NOT_LOCKED +select * from t2; +unlock tables; + +drop tables t1; +set @@debug_dbug= @saved_debug_dbug; diff --git a/sql/handler.h b/sql/handler.h index 1abe66448aa..d4ea7687f15 100644 --- a/sql/handler.h +++ b/sql/handler.h @@ -2382,7 +2382,7 @@ struct HA_CREATE_INFO: public Table_scope_and_contents_source_st, } bool finalize_atomic_replace(THD *thd, TABLE_LIST *orig_table); void finalize_ddl(THD *thd, bool roll_back); - bool finalize_locked_tables(THD *thd); + bool finalize_locked_tables(THD *thd, bool operation_failed); bool make_tmp_table_list(THD *thd, TABLE_LIST **create_table, int *create_table_mode); }; diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc index 6fc46e5818c..163dd45f50c 100644 --- a/sql/sql_insert.cc +++ b/sql/sql_insert.cc @@ -5414,7 +5414,8 @@ bool select_create::send_eof() mysql_unlock_tables(thd, lock); } else if (atomic_replace && create_info->pos_in_locked_tables && - create_info->finalize_locked_tables(thd)) + /* send_eof() is done on success, passing false here */ + create_info->finalize_locked_tables(thd, false)) DBUG_RETURN(true); send_ok_packet(); @@ -5526,6 +5527,7 @@ void select_create::abort_result_set() thd->locked_tables_list.unlock_locked_table(thd, create_info->mdl_ticket); } else if (atomic_replace && create_info->pos_in_locked_tables) - (void) create_info->finalize_locked_tables(thd); + /* abort_result_set() is done on error, passing true here */ + (void) create_info->finalize_locked_tables(thd, true); DBUG_VOID_RETURN; } diff --git a/sql/sql_table.cc b/sql/sql_table.cc index 0d76a50f4ce..5855c21bbb3 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -4509,27 +4509,47 @@ void HA_CREATE_INFO::finalize_ddl(THD *thd, bool roll_back) } -bool HA_CREATE_INFO::finalize_locked_tables(THD *thd) +/** + Finalize operation of LOCK TABLES mode for CREATE TABLE family of commands. + + @param operation_failed Notify if the callee CREATE fails the operation + @return true on error, false on success +*/ +bool HA_CREATE_INFO::finalize_locked_tables(THD *thd, bool operation_failed) { DBUG_ASSERT(pos_in_locked_tables); DBUG_ASSERT(thd->locked_tables_mode); DBUG_ASSERT(thd->variables.option_bits & OPTION_TABLE_LOCK); - /* - Add back the deleted table and re-created table as a locked table - This should always work as we have a meta lock on the table. - */ - thd->locked_tables_list.add_back_last_deleted_lock(pos_in_locked_tables); + if (!operation_failed) + { + /* + Add back the deleted table and re-created table as a locked table + This should always work as we have a meta lock on the table. + */ + thd->locked_tables_list.add_back_last_deleted_lock(pos_in_locked_tables); + } if (thd->locked_tables_list.reopen_tables(thd, false)) { thd->locked_tables_list.unlink_all_closed_tables(thd, NULL, 0); return true; } - /* - The lock was made exclusive in create_table_impl(). We have now - to bring it back to it's orginal state - */ - TABLE *table= pos_in_locked_tables->table; - table->mdl_ticket->downgrade_lock(MDL_SHARED_NO_READ_WRITE); + if (is_atomic_replace() || !operation_failed) + { + /* + The lock was made exclusive in create_table_impl(). We have now + to bring it back to it's orginal state. + */ + TABLE *table= pos_in_locked_tables->table; + table->mdl_ticket->downgrade_lock(MDL_SHARED_NO_READ_WRITE); + } + else + { + /* + In failed non-atomic we have nothing to downgrade: + original table was deleted and the lock was already removed. + */ + DBUG_ASSERT(!pos_in_locked_tables->table); + } return false; } @@ -5278,7 +5298,7 @@ err: */ if (thd->locked_tables_mode && pos_in_locked_tables && create_info->or_replace()) - result|= (int) create_info->finalize_locked_tables(thd); + result|= (int) create_info->finalize_locked_tables(thd, result); DBUG_RETURN(result); } @@ -6023,7 +6043,7 @@ err: */ if (thd->locked_tables_mode && pos_in_locked_tables && create_info->or_replace()) - res|= (int) local_create_info.finalize_locked_tables(thd); + res|= (int) local_create_info.finalize_locked_tables(thd, res); DBUG_RETURN(res != 0); }