Bug #51240 ALTER TABLE of a locked MERGE table fails

The problem was that ALTER TABLE on a merge table which was locked 
using LOCK TABLE ... WRITE, by mistake gave 
ER_TABLE_NOT_LOCKED_FOR_WRITE.

During opening of the table to be ALTERed, open_table() tried to
get an upgradable metadata lock. In LOCK TABLEs mode, this lock
must already exist (i.e. taken by LOCK TABLE) as new locks of this
type cannot be acquired for fear of deadlock. So in LOCK TABLEs
mode, open_table() tried to find an existing upgradable lock for
the table to be altered.

The problem was that open_table() also tried to find upgradable
metadata locks for children of merge tables even if no such
locks are needed to execute ALTER TABLE on merge tables.

This patch fixes the problem by making sure that open tables code
only searches for upgradable metadata locks for the merge table
and not for the merge children tables. 

The patch also fixes a related bug where an upgradable metadata
lock was aquired outside of LOCK TABLEs mode even if the table in
question was temporary. This bug meant that LOCK TABLES or DDL on
temporary tables by mistake could be blocked/aborted by locks held
on base tables with the same table name by other connections.

Test cases added to merge.test and lock_multi.test.

5 files changed, 153 insertions, 24 deletions

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
@@ -2556,16 +2556,6 @@ bool open_table(THD *thd, TABLE_LIST *table_list, MEM_ROOT *mem_root,
                          (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
             table that has an equal or higher lock than requested,
@@ -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 :
@@ -4413,6 +4402,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
 
   @param[in]     thd      Thread context.
@@ -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;