Bug#13982017: ALTER TABLE RENAME ENDS UP WITH ERROR 1050 (42S01)

Fixed by backport of:
    ------------------------------------------------------------
    revno: 3402.50.156
    committer: Jon Olav Hauglid <jon.hauglid@oracle.com>
    branch nick: mysql-trunk-test
    timestamp: Wed 2012-02-08 14:10:23 +0100
    message:
      Bug#13417754 ASSERT IN ROW_DROP_DATABASE_FOR_MYSQL DURING DROP SCHEMA
      
      This assert could be triggered if an InnoDB table was being moved
      to a different database using ALTER TABLE ... RENAME, while this
      database concurrently was being dropped by DROP DATABASE.
      
      The reason for the problem was that no metadata lock was taken
      on the target database by ALTER TABLE ... RENAME.
      DROP DATABASE was therefore not blocked and could remove
      the database while ALTER TABLE ... RENAME was executing. This
      could cause the assert in InnoDB to be triggered.
      
      This patch fixes the problem by taking a IX metadata lock on
      the target database before ALTER TABLE ... RENAME starts
      moving a table to a different database.
      
      Note that this problem did not occur with RENAME TABLE which
      already takes the correct metadata locks.
      
      Also note that this patch slightly changes the behavior of
      ALTER TABLE ... RENAME. Before, the statement would abort and
      return an error if a lock on the target table name could not
      be taken immediately. With this patch, ALTER TABLE ... RENAME
      will instead block and wait until the lock can be taken 
      (or until we get a lock timeout). This also means that it is
      possible to get ER_LOCK_DEADLOCK errors in this situation
      since we allow ALTER TABLE ... RENAME to wait and not just
      abort immediately.

mysql-test/r/create-big.result

@@ -53,8 +53,8 @@ set debug_sync='create_table_select_before_create SIGNAL parked WAIT_FOR go';
 create table t1 select 1 as i;;
 set debug_sync='now WAIT_FOR parked';
 alter table t3 rename to t1;
-ERROR 42S01: Table 't1' already exists
 set debug_sync='now SIGNAL go';
+ERROR 42S01: Table 't1' already exists
 show create table t1;
 Table	Create Table
 t1	CREATE TABLE `t1` (
@@ -65,8 +65,8 @@ set debug_sync='create_table_select_before_create SIGNAL parked WAIT_FOR go';
 create table t1 select 1 as i;;
 set debug_sync='now WAIT_FOR parked';
 alter table t3 rename to t1, add k int;
-ERROR 42S01: Table 't1' already exists
 set debug_sync='now SIGNAL go';
+ERROR 42S01: Table 't1' already exists
 show create table t1;
 Table	Create Table
 t1	CREATE TABLE `t1` (

mysql-test/t/create-big.test

@@ -132,11 +132,20 @@ set debug_sync='create_table_select_before_create SIGNAL parked WAIT_FOR go';
 --send create table t1 select 1 as i;
 connection addconroot1;
 set debug_sync='now WAIT_FOR parked';
---error ER_TABLE_EXISTS_ERROR
-alter table t3 rename to t1;
+--send alter table t3 rename to t1
+connection addconroot2;
+# Wait until the above ALTER TABLE RENAME is blocked due to CREATE
+let $wait_condition=
+    select count(*) = 1 from information_schema.processlist
+    where state = "Waiting for table metadata lock" and
+          info = "alter table t3 rename to t1";
+--source include/wait_condition.inc
 set debug_sync='now SIGNAL go';
 connection default;
 --reap
+connection addconroot1;
+--error ER_TABLE_EXISTS_ERROR
+--reap
 connection default;
 show create table t1;
 drop table t1;
@@ -146,11 +155,21 @@ set debug_sync='create_table_select_before_create SIGNAL parked WAIT_FOR go';
 --send create table t1 select 1 as i;
 connection addconroot1;
 set debug_sync='now WAIT_FOR parked';
---error ER_TABLE_EXISTS_ERROR
-alter table t3 rename to t1, add k int;
+--send alter table t3 rename to t1, add k int
+connection addconroot2;
+# Wait until the above ALTER TABLE RENAME is blocked due to CREATE
+let $wait_condition=
+    select count(*) = 1 from information_schema.processlist
+    where state = "Waiting for table metadata lock" and
+          info = "alter table t3 rename to t1, add k int";
+--source include/wait_condition.inc
 set debug_sync='now SIGNAL go';
 connection default;
 --reap
+connection addconroot1;
+--error ER_TABLE_EXISTS_ERROR
+--reap
+connection default;
 show create table t1;
 drop table t1,t3;
 

sql/sql_table.cc

@@ -5872,8 +5872,26 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
       }
       else
       {
+        MDL_request_list mdl_requests;
+        MDL_request target_db_mdl_request;
+
         target_mdl_request.init(MDL_key::TABLE, new_db, new_name,
                                 MDL_EXCLUSIVE, MDL_TRANSACTION);
+        mdl_requests.push_front(&target_mdl_request);
+
+        /*
+          If we are moving the table to a different database, we also
+          need IX lock on the database name so that the target database
+          is protected by MDL while the table is moved.
+        */
+        if (new_db != db)
+        {
+          target_db_mdl_request.init(MDL_key::SCHEMA, new_db, "",
+                                     MDL_INTENTION_EXCLUSIVE,
+                                     MDL_TRANSACTION);
+          mdl_requests.push_front(&target_db_mdl_request);
+        }
+
         /*
           Global intention exclusive lock must have been already acquired when
           table to be altered was open, so there is no need to do it here.
@@ -5882,14 +5900,10 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
                                                    "", "",
                                                    MDL_INTENTION_EXCLUSIVE));
 
-        if (thd->mdl_context.try_acquire_lock(&target_mdl_request))
-          DBUG_RETURN(TRUE);
-        if (target_mdl_request.ticket == NULL)
-        {
-          /* Table exists and is locked by some thread. */
-	  my_error(ER_TABLE_EXISTS_ERROR, MYF(0), new_alias);
+        if (thd->mdl_context.acquire_locks(&mdl_requests,
+                                           thd->variables.lock_wait_timeout))
           DBUG_RETURN(TRUE);
-        }
+
         DEBUG_SYNC(thd, "locked_table_name");
         /*
           Table maybe does not exist, but we got an exclusive lock

storage/innobase/handler/ha_innodb.cc

@@ -47,6 +47,7 @@ this program; if not, write to the Free Software Foundation, Inc.,
 
 #include <sql_acl.h>	// PROCESS_ACL
 #include <m_ctype.h>
+#include <debug_sync.h> // DEBUG_SYNC
 #include <mysys_err.h>
 #include <mysql/plugin.h>
 #include <mysql/innodb_priv.h>
@@ -7691,6 +7692,8 @@ ha_innobase::rename_table(
 
 	error = innobase_rename_table(trx, from, to, TRUE);
 
+	DEBUG_SYNC(thd, "after_innobase_rename_table");
+
 	/* Tell the InnoDB server that there might be work for
 	utility threads: */