diff --git a/mysql-test/r/lock_multi.result b/mysql-test/r/lock_multi.result index c440dafb228..99e1f54e762 100644 --- a/mysql-test/r/lock_multi.result +++ b/mysql-test/r/lock_multi.result @@ -462,3 +462,17 @@ ERROR 70100: Query execution was interrupted unlock tables; # Switching to connection 'default' drop table t3; +# +# Test for the bug where upgradable metadata locks was acquired +# even if the table to altered was temporary. +# Bug found while working on the related bug #51240. +# +DROP TABLE IF EXISTS t1; +CREATE TABLE t1 (id INT); +LOCK TABLE t1 WRITE; +# Connection con1 +CREATE TEMPORARY TABLE t1 (id INT); +ALTER TABLE t1 ADD COLUMN j INT; +# Connection default +UNLOCK TABLES; +DROP TABLE t1; diff --git a/mysql-test/r/merge.result b/mysql-test/r/merge.result index 7a97ced5c1f..a77f3a75f16 100644 --- a/mysql-test/r/merge.result +++ b/mysql-test/r/merge.result @@ -2605,4 +2605,14 @@ ERROR 42000: FUNCTION test.f1 does not exist execute stmt; ERROR 42000: FUNCTION test.f1 does not exist drop table t4, t3, t2, t1; +# +# Bug#51240 ALTER TABLE of a locked MERGE table fails +# +DROP TABLE IF EXISTS m1, t1; +CREATE TABLE t1 (c1 INT); +CREATE TABLE m1 (c1 INT) ENGINE=MRG_MyISAM UNION=(t1); +LOCK TABLE m1 WRITE; +ALTER TABLE m1 ADD INDEX (c1); +UNLOCK TABLES; +DROP TABLE m1, t1; End of 6.0 tests diff --git a/mysql-test/t/lock_multi.test b/mysql-test/t/lock_multi.test index c34dcb05dd4..6983947d1c4 100644 --- a/mysql-test/t/lock_multi.test +++ b/mysql-test/t/lock_multi.test @@ -1092,5 +1092,31 @@ disconnect con2; drop table t3; +--echo # +--echo # Test for the bug where upgradable metadata locks was acquired +--echo # even if the table to altered was temporary. +--echo # Bug found while working on the related bug #51240. +--echo # + +--disable_warnings +DROP TABLE IF EXISTS t1; +--enable_warnings + +CREATE TABLE t1 (id INT); +LOCK TABLE t1 WRITE; + +--echo # Connection con1 +connect (con1, localhost, root); +CREATE TEMPORARY TABLE t1 (id INT); +# This alter should not block and timeout. +ALTER TABLE t1 ADD COLUMN j INT; + +--echo # Connection default +connection default; +disconnect con1; +UNLOCK TABLES; +DROP TABLE t1; + + # Wait till all disconnects are completed --source include/wait_until_count_sessions.inc diff --git a/mysql-test/t/merge.test b/mysql-test/t/merge.test index f0d8960322e..4035752b72d 100644 --- a/mysql-test/t/merge.test +++ b/mysql-test/t/merge.test @@ -2089,5 +2089,24 @@ execute stmt; execute stmt; drop table t4, t3, t2, t1; + +--echo # +--echo # Bug#51240 ALTER TABLE of a locked MERGE table fails +--echo # + +--disable_warnings +DROP TABLE IF EXISTS m1, t1; +--enable_warnings + +CREATE TABLE t1 (c1 INT); +CREATE TABLE m1 (c1 INT) ENGINE=MRG_MyISAM UNION=(t1); +LOCK TABLE m1 WRITE; +# This used to cause an error. +ALTER TABLE m1 ADD INDEX (c1); + +UNLOCK TABLES; +DROP TABLE m1, t1; + + --echo End of 6.0 tests diff --git a/sql/sql_base.cc b/sql/sql_base.cc index 0f572350647..0dcfafed882 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -2555,16 +2555,6 @@ bool open_table(THD *thd, TABLE_LIST *table_list, MEM_ROOT *mem_root, int distance= ((int) table->reginfo.lock_type - (int) table_list->lock_type); - /* - If we are performing DDL operation we also should ensure - that we will find TABLE instance with upgradable metadata - lock, - */ - if ((flags & MYSQL_OPEN_TAKE_UPGRADABLE_MDL) && - table_list->lock_type >= TL_WRITE_ALLOW_WRITE && - ! table->mdl_ticket->is_upgradable_or_exclusive()) - distance= -1; - /* Find a table that either has the exact lock type requested, or has the best suitable lock. In case there is no locked @@ -2598,13 +2588,6 @@ bool open_table(THD *thd, TABLE_LIST *table_list, MEM_ROOT *mem_root, } if (best_table) { - if ((flags & MYSQL_OPEN_TAKE_UPGRADABLE_MDL) && - table_list->lock_type >= TL_WRITE_ALLOW_WRITE && - ! best_table->mdl_ticket->is_upgradable_or_exclusive()) - { - my_error(ER_TABLE_NOT_LOCKED_FOR_WRITE, MYF(0), alias); - DBUG_RETURN(TRUE); - } table= best_table; table->query_id= thd->query_id; DBUG_PRINT("info",("Using locked table")); @@ -4354,7 +4337,8 @@ end: /** Acquire upgradable (SNW, SNRW) metadata locks on tables to be opened - for LOCK TABLES or a DDL statement. + for LOCK TABLES or a DDL statement. Under LOCK TABLES, we can't take + new locks, so use open_tables_check_upgradable_mdl() instead. @param thd Thread context. @param tables_start Start of list of tables on which upgradable locks @@ -4374,10 +4358,15 @@ open_tables_acquire_upgradable_mdl(THD *thd, TABLE_LIST *tables_start, MDL_request_list mdl_requests; TABLE_LIST *table; + DBUG_ASSERT(!thd->locked_tables_mode); + for (table= tables_start; table && table != tables_end; table= table->next_global) { - if (table->lock_type >= TL_WRITE_ALLOW_WRITE) + if (table->lock_type >= TL_WRITE_ALLOW_WRITE && + !(table->open_type == OT_TEMPORARY_ONLY || + (table->open_type != OT_BASE_ONLY && + find_temporary_table(thd, table)))) { table->mdl_request.set_type(table->lock_type > TL_WRITE_ALLOW_READ ? MDL_SHARED_NO_READ_WRITE : @@ -4412,6 +4401,58 @@ open_tables_acquire_upgradable_mdl(THD *thd, TABLE_LIST *tables_start, } +/** + Check for upgradable (SNW, SNRW) metadata locks on tables to be opened + for a DDL statement. Under LOCK TABLES, we can't take new locks, so we + must check if appropriate locks were pre-acquired. + + @param thd Thread context. + @param tables_start Start of list of tables on which upgradable locks + should be searched for. + @param tables_end End of list of tables. + + @retval FALSE Success. + @retval TRUE Failure (e.g. connection was killed) +*/ + +static bool +open_tables_check_upgradable_mdl(THD *thd, TABLE_LIST *tables_start, + TABLE_LIST *tables_end) +{ + TABLE_LIST *table; + + DBUG_ASSERT(thd->locked_tables_mode); + + for (table= tables_start; table && table != tables_end; + table= table->next_global) + { + if (table->lock_type >= TL_WRITE_ALLOW_WRITE && + !(table->open_type == OT_TEMPORARY_ONLY || + (table->open_type != OT_BASE_ONLY && + find_temporary_table(thd, table)))) + { + /* + We don't need to do anything about the found TABLE instance as it + will be handled later in open_tables(), we only need to check that + an upgradable lock is already acquired. When we enter LOCK TABLES + mode, SNRW locks are acquired before all other locks. So if under + LOCK TABLES we find that there is TABLE instance with upgradeable + lock, all other instances of TABLE for the same table will have the + same ticket. + + Note that find_table_for_mdl_upgrade() will report an error if a + ticket is not found. + */ + if (!find_table_for_mdl_upgrade(thd->open_tables, table->db, + table->table_name, FALSE)) + return TRUE; + } + } + + return FALSE; +} + + /** Open all tables in list @@ -4498,12 +4539,31 @@ restart: lock will be reused (thanks to the fact that in recursive case metadata locks are acquired without waiting). */ - if ((flags & MYSQL_OPEN_TAKE_UPGRADABLE_MDL) && - ! thd->locked_tables_mode) + if (flags & MYSQL_OPEN_TAKE_UPGRADABLE_MDL) { - if (open_tables_acquire_upgradable_mdl(thd, *start, - thd->lex->first_not_own_table(), - &ot_ctx)) + /* + open_tables_acquire_upgradable_mdl() does not currenly handle + these two flags. At this point, that does not matter as they + are not used together with MYSQL_OPEN_TAKE_UPGRADABLE_MDL. + */ + DBUG_ASSERT(!(flags & (MYSQL_OPEN_SKIP_TEMPORARY | + MYSQL_OPEN_TEMPORARY_ONLY))); + if (thd->locked_tables_mode) + { + /* + Under LOCK TABLES, we can't acquire new locks, so we instead + need to check if appropriate locks were pre-acquired. + */ + if (open_tables_check_upgradable_mdl(thd, *start, + thd->lex->first_not_own_table())) + { + error= TRUE; + goto err; + } + } + else if (open_tables_acquire_upgradable_mdl(thd, *start, + thd->lex->first_not_own_table(), + &ot_ctx)) { error= TRUE; goto err;