MDEV-10216: Assertion `strcmp(share->unique_file_name,filename) ||

.. share->last_version' failed in myisam/mi_open.c:67: test_if_reopen

During the RENAME operation since the renamed temporary table is also
opened and added to myisam_open_list/maria_open_list, resetting the
last_version at the end of operation (HA_EXTRA_PREPARE_FOR_RENAME)
will cause an assertion failure when a subsequent query tries to open
an additional temporary table instance and thus attempts to reuse it
from the open table list.

This commit fixes the issue by skipping flush/close operations executed
toward the end of ALTER for temporary tables. It also enables a shortcut
for simple ALTERs (like rename, disable/enable keys) on temporary
tables.

As safety checks, added some assertions at code points that should not
be hit for temporary tables.

mysql-test/r/reopen_temp_table.result

@@ -164,5 +164,29 @@ SELECT COUNT(*)=4 FROM t6;
 COUNT(*)=4
 1
 DROP TABLE t5, t6;
+#
+# MDEV-10216: Assertion `strcmp(share->unique_file_name,filename) ||
+# share->last_version' failed in myisam/mi_open.c:67: test_if_reopen
+#
+CREATE TEMPORARY TABLE t7 (i INT) ENGINE=MYISAM;
+INSERT INTO t7 VALUES(1);
+ALTER TABLE t7 RENAME TO t;
+SELECT * FROM t a, t b;
+i	i
+1	1
+DROP TABLE t;
+CREATE TEMPORARY TABLE t7 (i INT) ENGINE=ARIA;
+INSERT INTO t7 VALUES(1);
+ALTER TABLE t7 RENAME TO t;
+SELECT * FROM t a, t b;
+i	i
+1	1
+DROP TABLE t;
+CREATE TEMPORARY TABLE t8 (i INT) ENGINE=ARIA;
+ALTER TABLE t8 rename to t;
+SELECT (SELECT 1 FROM t a1, t a2 ) AS f1, ( SELECT 2 FROM t a3 ) AS f2 FROM DUAL;
+f1	f2
+NULL	NULL
+DROP TABLE t;
 # Cleanup
 DROP DATABASE temp_db;

mysql-test/r/temp_table.result

@@ -490,5 +490,50 @@ SELECT * FROM temp_t1;
 i
 1
 DROP TABLE temp_t1, t1;
+#
+# ALTER TABLE RENAME & ENABLE/DISABLE KEYS (shortcuts)
+#
+CREATE TEMPORARY TABLE t1(i INT PRIMARY KEY) ENGINE=MYISAM;
+INSERT INTO t1 VALUES(1);
+SELECT COUNT(*)=1 FROM t1;
+COUNT(*)=1
+1
+ALTER TABLE t1 RENAME t2;
+SELECT COUNT(*)=1 FROM t2;
+COUNT(*)=1
+1
+ALTER TABLE t2 RENAME t1;
+ALTER TABLE t1 DISABLE KEYS;
+ALTER TABLE t1 ENABLE KEYS;
+LOCK TABLES t1 WRITE;
+ALTER TABLE t1 RENAME t2;
+SELECT COUNT(*)=1 FROM t2;
+COUNT(*)=1
+1
+ALTER TABLE t2 RENAME t1;
+ALTER TABLE t1 DISABLE KEYS;
+ALTER TABLE t1 ENABLE KEYS;
+UNLOCK TABLES;
+LOCK TABLES t1 READ;
+ALTER TABLE t1 RENAME t2;
+SELECT COUNT(*)=1 FROM t2;
+COUNT(*)=1
+1
+ALTER TABLE t2 RENAME t1;
+ALTER TABLE t1 DISABLE KEYS;
+ALTER TABLE t1 ENABLE KEYS;
+UNLOCK TABLES;
+FLUSH TABLES WITH READ LOCK;
+ALTER TABLE t1 RENAME t2;
+SELECT COUNT(*)=1 FROM t2;
+COUNT(*)=1
+1
+ALTER TABLE t2 RENAME t1;
+ALTER TABLE t1 DISABLE KEYS;
+ALTER TABLE t1 ENABLE KEYS;
+UNLOCK TABLES;
+ALTER TABLE t1 RENAME t2, LOCK SHARED;
+ALTER TABLE t2 RENAME t1, LOCK EXCLUSIVE;
+DROP TABLE t1;
 # Cleanup
 DROP DATABASE temp_db;

mysql-test/suite/rpl/r/rpl_temporary.result

@@ -183,5 +183,52 @@ disconnect cont43748;
 -- throw out test-user on slave.
 connection slave;
 DROP USER user43748@127.0.0.1;
+#
+# MDEV-10216: Assertion `strcmp(share->unique_file_name,filename) ||
+# share->last_version' failed in myisam/mi_open.c:67: test_if_reopen
+#
+connection master;
+CREATE TEMPORARY TABLE t1(i INT PRIMARY KEY) ENGINE=MYISAM;
+INSERT INTO t1 VALUES(1);
+SELECT COUNT(*)=1 FROM t1;
+COUNT(*)=1
+1
+ALTER TABLE t1 RENAME t2;
+SELECT COUNT(*)=1 FROM t2;
+COUNT(*)=1
+1
+ALTER TABLE t2 RENAME t1;
+ALTER TABLE t1 DISABLE KEYS;
+ALTER TABLE t1 ENABLE KEYS;
+LOCK TABLES t1 WRITE;
+ALTER TABLE t1 RENAME t2;
+SELECT COUNT(*)=1 FROM t2;
+COUNT(*)=1
+1
+ALTER TABLE t2 RENAME t1;
+ALTER TABLE t1 DISABLE KEYS;
+ALTER TABLE t1 ENABLE KEYS;
+UNLOCK TABLES;
+LOCK TABLES t1 READ;
+ALTER TABLE t1 RENAME t2;
+SELECT COUNT(*)=1 FROM t2;
+COUNT(*)=1
+1
+ALTER TABLE t2 RENAME t1;
+ALTER TABLE t1 DISABLE KEYS;
+ALTER TABLE t1 ENABLE KEYS;
+UNLOCK TABLES;
+FLUSH TABLES WITH READ LOCK;
+ALTER TABLE t1 RENAME t2;
+SELECT COUNT(*)=1 FROM t2;
+COUNT(*)=1
+1
+ALTER TABLE t2 RENAME t1;
+ALTER TABLE t1 DISABLE KEYS;
+ALTER TABLE t1 ENABLE KEYS;
+UNLOCK TABLES;
+ALTER TABLE t1 RENAME t2, LOCK SHARED;
+ALTER TABLE t2 RENAME t1, LOCK EXCLUSIVE;
+DROP TABLE t1;
 End of 5.1 tests
 include/rpl_end.inc

mysql-test/suite/rpl/t/rpl_temporary.test

@@ -354,6 +354,52 @@ connection slave;
 
 DROP USER user43748@127.0.0.1;
 
+--echo #
+--echo # MDEV-10216: Assertion `strcmp(share->unique_file_name,filename) ||
+--echo # share->last_version' failed in myisam/mi_open.c:67: test_if_reopen
+--echo #
+
+connection master;
+CREATE TEMPORARY TABLE t1(i INT PRIMARY KEY) ENGINE=MYISAM;
+INSERT INTO t1 VALUES(1);
+SELECT COUNT(*)=1 FROM t1;
+
+ALTER TABLE t1 RENAME t2;
+SELECT COUNT(*)=1 FROM t2;
+ALTER TABLE t2 RENAME t1;
+
+ALTER TABLE t1 DISABLE KEYS;
+ALTER TABLE t1 ENABLE KEYS;
+
+# LOCK TABLES is ignored for temporary tables.
+LOCK TABLES t1 WRITE;
+ALTER TABLE t1 RENAME t2;
+SELECT COUNT(*)=1 FROM t2;
+ALTER TABLE t2 RENAME t1;
+ALTER TABLE t1 DISABLE KEYS;
+ALTER TABLE t1 ENABLE KEYS;
+UNLOCK TABLES;
+
+LOCK TABLES t1 READ;
+ALTER TABLE t1 RENAME t2;
+SELECT COUNT(*)=1 FROM t2;
+ALTER TABLE t2 RENAME t1;
+ALTER TABLE t1 DISABLE KEYS;
+ALTER TABLE t1 ENABLE KEYS;
+UNLOCK TABLES;
+
+FLUSH TABLES WITH READ LOCK;
+ALTER TABLE t1 RENAME t2;
+SELECT COUNT(*)=1 FROM t2;
+ALTER TABLE t2 RENAME t1;
+ALTER TABLE t1 DISABLE KEYS;
+ALTER TABLE t1 ENABLE KEYS;
+UNLOCK TABLES;
+
+ALTER TABLE t1 RENAME t2, LOCK SHARED;
+ALTER TABLE t2 RENAME t1, LOCK EXCLUSIVE;
+
+DROP TABLE t1;
 
 --echo End of 5.1 tests
 --let $rpl_only_running_threads= 1

mysql-test/t/reopen_temp_table.test

@@ -159,5 +159,26 @@ SELECT COUNT(*)=6 FROM t5;
 SELECT COUNT(*)=4 FROM t6;
 DROP TABLE t5, t6;
 
+--echo #
+--echo # MDEV-10216: Assertion `strcmp(share->unique_file_name,filename) ||
+--echo # share->last_version' failed in myisam/mi_open.c:67: test_if_reopen
+--echo #
+CREATE TEMPORARY TABLE t7 (i INT) ENGINE=MYISAM;
+INSERT INTO t7 VALUES(1);
+ALTER TABLE t7 RENAME TO t;
+SELECT * FROM t a, t b;
+DROP TABLE t;
+
+CREATE TEMPORARY TABLE t7 (i INT) ENGINE=ARIA;
+INSERT INTO t7 VALUES(1);
+ALTER TABLE t7 RENAME TO t;
+SELECT * FROM t a, t b;
+DROP TABLE t;
+
+CREATE TEMPORARY TABLE t8 (i INT) ENGINE=ARIA;
+ALTER TABLE t8 rename to t;
+SELECT (SELECT 1 FROM t a1, t a2 ) AS f1, ( SELECT 2 FROM t a3 ) AS f2 FROM DUAL;
+DROP TABLE t;
+
 --echo # Cleanup
 DROP DATABASE temp_db;

mysql-test/t/temp_table.test

@@ -535,6 +535,50 @@ CREATE TEMPORARY TABLE temp_t1 AS SELECT * FROM t1;
 SELECT * FROM temp_t1;
 DROP TABLE temp_t1, t1;
 
+--echo #
+--echo # ALTER TABLE RENAME & ENABLE/DISABLE KEYS (shortcuts)
+--echo #
+CREATE TEMPORARY TABLE t1(i INT PRIMARY KEY) ENGINE=MYISAM;
+INSERT INTO t1 VALUES(1);
+SELECT COUNT(*)=1 FROM t1;
+
+ALTER TABLE t1 RENAME t2;
+SELECT COUNT(*)=1 FROM t2;
+ALTER TABLE t2 RENAME t1;
+
+ALTER TABLE t1 DISABLE KEYS;
+ALTER TABLE t1 ENABLE KEYS;
+
+# LOCK TABLES is ignored for temporary tables.
+LOCK TABLES t1 WRITE;
+ALTER TABLE t1 RENAME t2;
+SELECT COUNT(*)=1 FROM t2;
+ALTER TABLE t2 RENAME t1;
+ALTER TABLE t1 DISABLE KEYS;
+ALTER TABLE t1 ENABLE KEYS;
+UNLOCK TABLES;
+
+LOCK TABLES t1 READ;
+ALTER TABLE t1 RENAME t2;
+SELECT COUNT(*)=1 FROM t2;
+ALTER TABLE t2 RENAME t1;
+ALTER TABLE t1 DISABLE KEYS;
+ALTER TABLE t1 ENABLE KEYS;
+UNLOCK TABLES;
+
+FLUSH TABLES WITH READ LOCK;
+ALTER TABLE t1 RENAME t2;
+SELECT COUNT(*)=1 FROM t2;
+ALTER TABLE t2 RENAME t1;
+ALTER TABLE t1 DISABLE KEYS;
+ALTER TABLE t1 ENABLE KEYS;
+UNLOCK TABLES;
+
+ALTER TABLE t1 RENAME t2, LOCK SHARED;
+ALTER TABLE t2 RENAME t1, LOCK EXCLUSIVE;
+
+DROP TABLE t1;
+
 --echo # Cleanup
 DROP DATABASE temp_db;
 

sql/sql_base.cc

@@ -638,6 +638,8 @@ close_all_tables_for_name(THD *thd, TABLE_SHARE *share,
                           ha_extra_function extra,
                           TABLE *skip_table)
 {
+  DBUG_ASSERT(!share->tmp_table);
+
   char key[MAX_DBKEY_LENGTH];
   uint key_length= share->table_cache_key.length;
   const char *db= key;
@@ -1173,6 +1175,7 @@ bool wait_while_table_is_used(THD *thd, TABLE *table,
                               enum ha_extra_function function)
 {
   DBUG_ENTER("wait_while_table_is_used");
+  DBUG_ASSERT(!table->s->tmp_table);
   DBUG_PRINT("enter", ("table: '%s'  share: 0x%lx  db_stat: %u  version: %lu",
                        table->s->table_name.str, (ulong) table->s,
                        table->db_stat, table->s->tdc->version));

sql/sql_rename.cc

@@ -27,7 +27,6 @@
 #include "sql_table.h"                         // build_table_filename
 #include "sql_view.h"             // mysql_frm_type, mysql_rename_view
 #include "sql_trigger.h"
-#include "lock.h"       // MYSQL_OPEN_SKIP_TEMPORARY
 #include "sql_base.h"   // tdc_remove_table, lock_table_names,
 #include "sql_handler.h"                        // mysql_ha_rm_tables
 #include "sql_statistics.h" 

sql/sql_table.cc

@@ -8254,6 +8254,72 @@ static bool fk_prepare_copy_alter_table(THD *thd, TABLE *table,
   DBUG_RETURN(false);
 }
 
+/**
+  Rename temporary table and/or turn indexes on/off without touching .FRM.
+  Its a variant of simple_rename_or_index_change() to be used exclusively
+  for temporary tables.
+
+  @param thd            Thread handler
+  @param table_list     TABLE_LIST for the table to change
+  @param keys_onoff     ENABLE or DISABLE KEYS?
+  @param alter_ctx      ALTER TABLE runtime context.
+
+  @return Operation status
+    @retval false           Success
+    @retval true            Failure
+*/
+static bool
+simple_tmp_rename_or_index_change(THD *thd, TABLE_LIST *table_list,
+                                  Alter_info::enum_enable_or_disable keys_onoff,
+                                  Alter_table_ctx *alter_ctx)
+{
+  DBUG_ENTER("simple_tmp_rename_or_index_change");
+
+  TABLE *table= table_list->table;
+  bool error= false;
+
+  DBUG_ASSERT(table->s->tmp_table);
+
+  if (keys_onoff != Alter_info::LEAVE_AS_IS)
+  {
+    THD_STAGE_INFO(thd, stage_manage_keys);
+    error= alter_table_manage_keys(table, table->file->indexes_are_disabled(),
+                                   keys_onoff);
+  }
+
+  if (!error && alter_ctx->is_table_renamed())
+  {
+    THD_STAGE_INFO(thd, stage_rename);
+
+    /*
+      If THD::rename_temporary_table() fails, there is no need to rename it
+      back to the original name (unlike the case for non-temporary tables),
+      as it was an allocation error and the table was not renamed.
+    */
+    error= thd->rename_temporary_table(table, alter_ctx->new_db,
+                                       alter_ctx->new_alias);
+  }
+
+  if (!error)
+  {
+    int res= 0;
+    /*
+      We do not replicate alter table statement on temporary tables under
+      ROW-based replication.
+    */
+    if (!thd->is_current_stmt_binlog_format_row())
+    {
+      res= write_bin_log(thd, true, thd->query(), thd->query_length());
+    }
+    if (res != 0)
+      error= true;
+    else
+      my_ok(thd);
+  }
+
+  DBUG_RETURN(error);
+}
+
 
 /**
   Rename table and/or turn indexes on/off without touching .FRM
@@ -8290,6 +8356,7 @@ simple_rename_or_index_change(THD *thd, TABLE_LIST *table_list,
     if (lock_tables(thd, table_list, alter_ctx->tables_opened, 0))
       DBUG_RETURN(true);
 
+    THD_STAGE_INFO(thd, stage_manage_keys);
     error= alter_table_manage_keys(table,
                                    table->file->indexes_are_disabled(),
                                    keys_onoff);
@@ -8674,23 +8741,37 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
     DBUG_RETURN(false);
   }
 
+  /*
+     Test if we are only doing RENAME or KEYS ON/OFF. This works
+     as we are testing if flags == 0 above.
+  */
   if (!(alter_info->flags & ~(Alter_info::ALTER_RENAME |
                               Alter_info::ALTER_KEYS_ONOFF)) &&
       alter_info->requested_algorithm !=
-      Alter_info::ALTER_TABLE_ALGORITHM_COPY &&
+      Alter_info::ALTER_TABLE_ALGORITHM_COPY)   // No need to touch frm.
-      !table->s->tmp_table) // no need to touch frm
   {
-    // This requires X-lock, no other lock levels supported.
+    bool res;
-    if (alter_info->requested_lock != Alter_info::ALTER_TABLE_LOCK_DEFAULT &&
+
-        alter_info->requested_lock != Alter_info::ALTER_TABLE_LOCK_EXCLUSIVE)
+    if (!table->s->tmp_table)
     {
-      my_error(ER_ALTER_OPERATION_NOT_SUPPORTED, MYF(0),
+      // This requires X-lock, no other lock levels supported.
-               "LOCK=NONE/SHARED", "LOCK=EXCLUSIVE");
+      if (alter_info->requested_lock != Alter_info::ALTER_TABLE_LOCK_DEFAULT &&
-      DBUG_RETURN(true);
+          alter_info->requested_lock != Alter_info::ALTER_TABLE_LOCK_EXCLUSIVE)
+      {
+        my_error(ER_ALTER_OPERATION_NOT_SUPPORTED, MYF(0),
+                 "LOCK=NONE/SHARED", "LOCK=EXCLUSIVE");
+        DBUG_RETURN(true);
+      }
+      res= simple_rename_or_index_change(thd, table_list,
+                                         alter_info->keys_onoff,
+                                         &alter_ctx);
+    }
+    else
+    {
+      res= simple_tmp_rename_or_index_change(thd, table_list,
+                                             alter_info->keys_onoff,
+                                             &alter_ctx);
     }
-    bool res= simple_rename_or_index_change(thd, table_list,
-                                            alter_info->keys_onoff,
-                                            &alter_ctx);
     DBUG_RETURN(res);
   }
 
@@ -9421,6 +9502,9 @@ err_new_table_cleanup:
 
 err_with_mdl_after_alter:
   /* the table was altered. binlog the operation */
+  DBUG_ASSERT(!(mysql_bin_log.is_open() &&
+                thd->is_current_stmt_binlog_format_row() &&
+                (create_info->tmp_table())));
   write_bin_log(thd, true, thd->query(), thd->query_length());
 
 err_with_mdl:
@@ -9737,7 +9821,7 @@ copy_data_between_tables(THD *thd, TABLE *from, TABLE *to,
   THD_STAGE_INFO(thd, stage_enabling_keys);
   thd_progress_next_stage(thd);
 
-  if (error > 0)
+  if (error > 0 && !from->s->tmp_table)
   {
     /* We are going to drop the temporary table */
     to->file->extra(HA_EXTRA_PREPARE_FOR_DROP);
@@ -9766,7 +9850,8 @@ copy_data_between_tables(THD *thd, TABLE *from, TABLE *to,
   to->file->ha_release_auto_increment();
   if (to->file->ha_external_lock(thd,F_UNLCK))
     error=1;
-  if (error < 0 && to->file->extra(HA_EXTRA_PREPARE_FOR_RENAME))
+  if (error < 0 && !from->s->tmp_table &&
+      to->file->extra(HA_EXTRA_PREPARE_FOR_RENAME))
     error= 1;
   thd_progress_end(thd);
   DBUG_RETURN(error > 0 ? -1 : 0);

storage/maria/ma_extra.c

@@ -316,6 +316,7 @@ int maria_extra(MARIA_HA *info, enum ha_extra_function function,
     /* Fall trough */
   case HA_EXTRA_PREPARE_FOR_RENAME:
   {
+    DBUG_ASSERT(!share->temporary);
     my_bool do_flush= MY_TEST(function != HA_EXTRA_PREPARE_FOR_DROP);
     my_bool save_global_changed;
     enum flush_type type;

storage/myisam/mi_extra.c

@@ -264,6 +264,7 @@ int mi_extra(MI_INFO *info, enum ha_extra_function function, void *extra_arg)
     _mi_mark_file_changed(info);
     /* Fall trough */
   case HA_EXTRA_PREPARE_FOR_RENAME:
+    DBUG_ASSERT(!share->temporary);
     mysql_mutex_lock(&THR_LOCK_myisam);
     share->last_version= 0L;			/* Impossible version */
     mysql_mutex_lock(&share->intern_lock);