MDEV-17070 Table corruption or Assertion `table->file->stats.records > 0 ||...

MDEV-17070 Table corruption or Assertion `table->file->stats.records > 0 || error' or Assertion `!is_set() || (m_status == DA_OK_BULK && is_bulk_op())' failed upon actions on temporary table

This was caused by a combination of factors:
* MyISAM/Aria temporary tables historically never saved the state
  to disk (MYI/MAI), because the state never needed to persist
* certain ALTER TABLE operations modify the original TABLE structure
  and if they fail, the original table has to be reopened to
  revert all changes (m_needs_reopen=1)

as a result, when ALTER fails and MyISAM/Aria temp table gets reopened,
it reads the stale state from the disk.

As a fix, MyISAM/Aria tables now *always* write the state to disk
on close, *unless* HA_EXTRA_PREPARE_FOR_DROP was done first. And
the server now always does HA_EXTRA_PREPARE_FOR_DROP before dropping
a temporary table.

mysql-test/r/reopen_temp_table.result

@@ -190,3 +190,56 @@ NULL	NULL
 DROP TABLE t;
 # Cleanup
 DROP DATABASE temp_db;
+USE test;
+create temporary table t1 (f char(255), b int, index(b)) engine=MyISAM;
+replace into t1 values (null,1),(null,2);
+alter table t1 add fulltext key(f);
+alter table t1 change if exists a b int, algorithm=inplace;
+ERROR 0A000: ALGORITHM=INPLACE is not supported for this operation. Try ALGORITHM=COPY
+check table t1;
+Table	Op	Msg_type	Msg_text
+test.t1	check	status	OK
+select * from t1;
+f	b
+NULL	1
+NULL	2
+drop table t1;
+create temporary table t1 (f char(255), b int, index(b)) engine=aria transactional=1;
+replace into t1 values (null,1),(null,2);
+alter table t1 add fulltext key(f);
+alter table t1 change if exists a b int, algorithm=inplace;
+ERROR 0A000: ALGORITHM=INPLACE is not supported for this operation. Try ALGORITHM=COPY
+check table t1;
+Table	Op	Msg_type	Msg_text
+test.t1	check	status	OK
+select * from t1;
+f	b
+NULL	1
+NULL	2
+drop table t1;
+create temporary table t1 (f char(255), b int, index(b)) engine=aria transactional=0 row_format=page;
+replace into t1 values (null,1),(null,2);
+alter table t1 add fulltext key(f);
+alter table t1 change if exists a b int, algorithm=inplace;
+ERROR 0A000: ALGORITHM=INPLACE is not supported for this operation. Try ALGORITHM=COPY
+check table t1;
+Table	Op	Msg_type	Msg_text
+test.t1	check	status	OK
+select * from t1;
+f	b
+NULL	1
+NULL	2
+drop table t1;
+create temporary table t1 (f char(255), b int, index(b)) engine=aria transactional=0 row_format=dynamic;
+replace into t1 values (null,1),(null,2);
+alter table t1 add fulltext key(f);
+alter table t1 change if exists a b int, algorithm=inplace;
+ERROR 0A000: ALGORITHM=INPLACE is not supported for this operation. Try ALGORITHM=COPY
+check table t1;
+Table	Op	Msg_type	Msg_text
+test.t1	check	status	OK
+select * from t1;
+f	b
+NULL	1
+NULL	2
+drop table t1;

mysql-test/t/reopen_temp_table.test

@@ -182,3 +182,43 @@ DROP TABLE t;
 
 --echo # Cleanup
 DROP DATABASE temp_db;
+USE test;
+
+#
+# MDEV-17070 Table corruption or Assertion `table->file->stats.records > 0 || error' or Assertion `!is_set() || (m_status == DA_OK_BULK && is_bulk_op())' failed upon actions on temporary table
+#
+create temporary table t1 (f char(255), b int, index(b)) engine=MyISAM;
+replace into t1 values (null,1),(null,2);
+alter table t1 add fulltext key(f);
+--error ER_ALTER_OPERATION_NOT_SUPPORTED
+alter table t1 change if exists a b int, algorithm=inplace;
+check table t1;
+select * from t1;
+drop table t1;
+
+create temporary table t1 (f char(255), b int, index(b)) engine=aria transactional=1;
+replace into t1 values (null,1),(null,2);
+alter table t1 add fulltext key(f);
+--error ER_ALTER_OPERATION_NOT_SUPPORTED
+alter table t1 change if exists a b int, algorithm=inplace;
+check table t1;
+select * from t1;
+drop table t1;
+
+create temporary table t1 (f char(255), b int, index(b)) engine=aria transactional=0 row_format=page;
+replace into t1 values (null,1),(null,2);
+alter table t1 add fulltext key(f);
+--error ER_ALTER_OPERATION_NOT_SUPPORTED
+alter table t1 change if exists a b int, algorithm=inplace;
+check table t1;
+select * from t1;
+drop table t1;
+
+create temporary table t1 (f char(255), b int, index(b)) engine=aria transactional=0 row_format=dynamic;
+replace into t1 values (null,1),(null,2);
+alter table t1 add fulltext key(f);
+--error ER_ALTER_OPERATION_NOT_SUPPORTED
+alter table t1 change if exists a b int, algorithm=inplace;
+check table t1;
+select * from t1;
+drop table t1;

sql/temporary_tables.cc

@@ -491,6 +491,7 @@ bool THD::close_temporary_tables()
     /* Traverse the table list. */
     while ((table= share->all_tmp_tables.pop_front()))
     {
+      table->file->extra(HA_EXTRA_PREPARE_FOR_DROP);
       free_temporary_table(table);
     }
   }
@@ -579,9 +580,7 @@ bool THD::rename_temporary_table(TABLE *table,
   @return false                       Table was dropped
           true                        Error
 */
-bool THD::drop_temporary_table(TABLE *table,
-                               bool *is_trans,
-                               bool delete_table)
+bool THD::drop_temporary_table(TABLE *table, bool *is_trans, bool delete_table)
 {
   DBUG_ENTER("THD::drop_temporary_table");
 
@@ -624,7 +623,8 @@ bool THD::drop_temporary_table(TABLE *table,
       parallel replication
     */
     tab->in_use= this;
-
+    if (delete_table)
+      tab->file->extra(HA_EXTRA_PREPARE_FOR_DROP);
     free_temporary_table(tab);
   }
 
@@ -1419,8 +1419,7 @@ bool THD::log_events_and_free_tmp_shares()
 
   @return void
 */
-void THD::free_tmp_table_share(TMP_TABLE_SHARE *share,
-                               bool delete_table)
+void THD::free_tmp_table_share(TMP_TABLE_SHARE *share, bool delete_table)
 {
   DBUG_ENTER("THD::free_tmp_table_share");
 

storage/maria/ma_blockrec.c

@@ -449,9 +449,7 @@ my_bool _ma_once_end_block_record(MARIA_SHARE *share)
   if (share->bitmap.file.file >= 0)
   {
     if (flush_pagecache_blocks(share->pagecache, &share->bitmap.file,
-                               ((share->temporary || share->deleting) ?
-                                FLUSH_IGNORE_CHANGED :
-                                FLUSH_RELEASE)))
+                       share->deleting ? FLUSH_IGNORE_CHANGED : FLUSH_RELEASE))
       res= 1;
     /*
       File must be synced as it is going out of the maria_open_list and so

storage/maria/ma_close.c

@@ -47,9 +47,7 @@ int maria_close(register MARIA_HA *info)
       a global mutex
     */
     if (flush_pagecache_blocks(share->pagecache, &share->kfile,
-                               ((share->temporary || share->deleting) ?
-                                FLUSH_IGNORE_CHANGED :
-                                FLUSH_RELEASE)))
+                       share->deleting ? FLUSH_IGNORE_CHANGED : FLUSH_RELEASE))
       error= my_errno;
   }
 
@@ -113,23 +111,14 @@ int maria_close(register MARIA_HA *info)
         since the start of the function (very unlikely)
       */
       if (flush_pagecache_blocks(share->pagecache, &share->kfile,
-                                 ((share->temporary || share->deleting) ?
-                                  FLUSH_IGNORE_CHANGED :
-                                  FLUSH_RELEASE)))
+                        share->deleting ? FLUSH_IGNORE_CHANGED : FLUSH_RELEASE))
         error= my_errno;
 #ifdef HAVE_MMAP
       if (share->file_map)
         _ma_unmap_file(info);
 #endif
-      /*
-        If we are crashed, we can safely flush the current state as it will
-        not change the crashed state.
-        We can NOT write the state in other cases as other threads
-        may be using the file at this point
-        IF using --external-locking, which does not apply to Maria.
-      */
       if (((share->changed && share->base.born_transactional) ||
-           maria_is_crashed(info)))
+           maria_is_crashed(info) || (share->temporary && !share->deleting)))
       {
         if (save_global_changed)
         {

storage/maria/ma_extra.c

@@ -314,6 +314,8 @@ int maria_extra(MARIA_HA *info, enum ha_extra_function function,
     share->state.open_count= 1;
     share->changed= 1;
     _ma_mark_file_changed_now(share);
+    if (share->temporary)
+      break;
     /* fall through */
   case HA_EXTRA_PREPARE_FOR_RENAME:
   {

storage/maria/ma_open.c

@@ -1374,7 +1374,7 @@ uint _ma_state_info_write(MARIA_SHARE *share, uint pWrite)
 
   if (pWrite & MA_STATE_INFO_WRITE_LOCK)
     mysql_mutex_lock(&share->intern_lock);
-  else if (maria_multi_threaded)
+  else if (maria_multi_threaded && !share->temporary)
     mysql_mutex_assert_owner(&share->intern_lock);
   if (share->base.born_transactional && translog_status == TRANSLOG_OK &&
       !maria_in_recovery)

storage/myisam/mi_close.c

@@ -65,11 +65,8 @@ int mi_close(register MI_INFO *info)
     DBUG_EXECUTE_IF("crash_before_flush_keys",
                     if (share->kfile >= 0) DBUG_ABORT(););
     if (share->kfile >= 0 &&
-	flush_key_blocks(share->key_cache, share->kfile,
-                         &share->dirty_part_map,
-                         ((share->temporary || share->deleting) ?
-                          FLUSH_IGNORE_CHANGED :
-                          FLUSH_RELEASE)))
+	flush_key_blocks(share->key_cache, share->kfile, &share->dirty_part_map,
+                         share->deleting ? FLUSH_IGNORE_CHANGED : FLUSH_RELEASE))
       error=my_errno;
     if (share->kfile >= 0)
     {
@@ -77,10 +74,14 @@ int mi_close(register MI_INFO *info)
         If we are crashed, we can safely flush the current state as it will
         not change the crashed state.
         We can NOT write the state in other cases as other threads
-        may be using the file at this point
-        IF using --external-locking.
+        may be using the file at this point IF using --external-locking.
+
+        Also, write the state if a temporary table is not being dropped
+        (the server might want to reopen it, and mi_lock_database() only
+        writes the state for non-temp ones)
       */
-      if (share->mode != O_RDONLY && mi_is_crashed(info))
+      if (share->mode != O_RDONLY &&
+          (mi_is_crashed(info) || (share->temporary && !share->deleting)))
 	mi_state_info_write(share->kfile, &share->state, 1);
       /* Decrement open count must be last I/O on this file. */
       _mi_decrement_open_count(info);

storage/myisam/mi_extra.c

@@ -260,9 +260,11 @@ int mi_extra(MI_INFO *info, enum ha_extra_function function, void *extra_arg)
     break;
   case HA_EXTRA_PREPARE_FOR_DROP:
     /* Signals about intent to delete this table */
-    //share->deleting= TRUE;
+    share->deleting= TRUE;
     share->global_changed= FALSE;     /* force writing changed flag */
     _mi_mark_file_changed(info);
+    if (share->temporary)
+      break;
     /* fall through */
   case HA_EXTRA_PREPARE_FOR_RENAME:
     DBUG_ASSERT(!share->temporary);