Simplified quick_rm_table() and mysql_rename_table()

Replaced obscure FRM_ONLY, NO_FRM_RENAME, NO_HA_TABLE, NO_PAR_TABLE with
straightforward explicit flags:

QRMT_FRM - [re]moves .frm
QRMT_PAR - [re]moves .par
QRMT_HANDLER - calls ha_delete_table()/ha_rename_table() and [re]moves
               high-level indexes
QRMT_DEFAULT - same as QRMT_FRM | QRMT_HANDLER, which is regular table
               drop/rename.

sql/ddl_log.cc

@@ -1985,7 +1985,7 @@ static int ddl_log_execute_action(THD *thd, MEM_ROOT *mem_root,
         the original name failed. Now we have to delete the temporary table
         and restore the backup.
       */
-      quick_rm_table(thd, hton, &db, &table, FN_IS_TMP);
+      quick_rm_table(thd, hton, &db, &table, QRMT_DEFAULT | FN_IS_TMP);
       if (!is_renamed)
       {
         execute_rename_table(ddl_log_entry, file,

sql/sql_base.cc

@@ -1461,7 +1461,7 @@ void drop_open_table(THD *thd, TABLE *table, const LEX_CSTRING *db_name,
     table->s->tdc->flush(thd, true);
     close_thread_table(thd, &thd->open_tables);
     /* Remove the table from the storage engine and rm the .frm. */
-    quick_rm_table(thd, table_type, db_name, table_name, 0);
+    quick_rm_table(thd, table_type, db_name, table_name, QRMT_DEFAULT);
  }
   DBUG_VOID_RETURN;
 }

sql/sql_insert.cc

@@ -4769,7 +4769,7 @@ TABLE *select_create::create_table_from_items(THD *thd, List<Item> *items,
       {
         quick_rm_table(thd, create_info->db_type, &table_list->db,
                        table_case_name(create_info, &table_list->table_name),
-                       0);
+                       QRMT_DEFAULT);
       }
       /* Restore */
       table_list->open_strategy= save_open_strategy;

sql/sql_rename.cc

@@ -299,7 +299,7 @@ check_rename(THD *thd, rename_param *param,
       Discovery will find the old table when it's accessed
      */
     tdc_remove_table(thd, ren_table->db.str, ren_table->table_name.str);
-    quick_rm_table(thd, 0, &ren_table->db, &param->old_alias, FRM_ONLY, 0);
+    quick_rm_table(thd, 0, &ren_table->db, &param->old_alias, QRMT_FRM);
     DBUG_RETURN(-1);
   }
 
@@ -380,7 +380,8 @@ do_rename(THD *thd, const rename_param *param, DDL_LOG_STATE *ddl_log_state,
 
     debug_crash_here("ddl_log_rename_before_rename_table");
     if (!(rc= mysql_rename_table(hton, &ren_table->db, old_alias,
-                                 new_db, new_alias, &param->old_version, 0)))
+                                 new_db, new_alias, &param->old_version,
+                                 QRMT_DEFAULT)))
     {
       /* Table rename succeded.
          It's safe to start recovery at rename trigger phase
@@ -415,7 +416,7 @@ do_rename(THD *thd, const rename_param *param, DDL_LOG_STATE *ddl_log_state,
         debug_crash_here("ddl_log_rename_after_failed_rename_trigger");
         (void) mysql_rename_table(hton, new_db, new_alias,
                                   &ren_table->db, old_alias, &param->old_version,
-                                  NO_FK_CHECKS);
+                                  QRMT_DEFAULT | NO_FK_CHECKS);
         debug_crash_here("ddl_log_rename_after_revert_rename_table");
         ddl_log_disable_entry(ddl_log_state);
         debug_crash_here("ddl_log_rename_after_disable_entry");

sql/sql_table.cc

@@ -2124,18 +2124,25 @@ bool quick_rm_table(THD *thd, handlerton *base, const LEX_CSTRING *db,
   int error= 0;
   DBUG_ENTER("quick_rm_table");
 
+  DBUG_ASSERT(flags & (QRMT_FRM | QRMT_PAR | QRMT_HANDLER));
   size_t path_length= table_path ?
     (strxnmov(path, pathmax, table_path, NullS) - path) :
     build_table_filename(path, pathmax, db->str, table_name->str, "", flags);
-  if ((flags & (NO_HA_TABLE | NO_PAR_TABLE)) == NO_HA_TABLE)
+  if (flags & QRMT_PAR)
   {
+    /*
+      Normally .par is removed by QRMT_HANDLER. Caller may want to remove it
+      explicitly along with .FRM in some cases.
+    */
+    DBUG_ASSERT(flags & QRMT_FRM);
+    DBUG_ASSERT(!(flags & QRMT_HANDLER));
     handler *file= get_new_handler((TABLE_SHARE*) 0, thd->mem_root, base);
     if (!file)
       DBUG_RETURN(true);
     (void) file->ha_create_partitioning_metadata(path, NULL, CHF_DELETE_FLAG);
     delete file;
   }
-  if (!(flags & (FRM_ONLY|NO_HA_TABLE)))
+  if (flags & QRMT_HANDLER)
   {
     uint keys, total_keys;
     int hlindex_error= get_hlindex_keys(thd, db, table_name, path, &keys,
@@ -2156,7 +2163,7 @@ bool quick_rm_table(THD *thd, handlerton *base, const LEX_CSTRING *db,
       error= 1;
   }
 
-  if (!(flags & NO_FRM_RENAME))
+  if (flags & QRMT_FRM)
   {
     memcpy(path + path_length, reg_ext, reg_ext_length + 1);
     if (mysql_file_delete(key_file_frm, path, MYF(0)))
@@ -5327,9 +5334,6 @@ bool operator!=(const MYSQL_TIME &lhs, const MYSQL_TIME &rhs)
   @param flags     flags
                    FN_FROM_IS_TMP old_name is temporary.
                    FN_TO_IS_TMP   new_name is temporary.
-                   NO_FRM_RENAME  Don't rename the FRM file
-                                  but only the table in the storage engine.
-                   NO_HA_TABLE    Don't rename table in engine.
                    NO_FK_CHECKS   Don't check FK constraints during rename.
   @return false    OK
   @return true     Error
@@ -5350,6 +5354,7 @@ mysql_rename_table(handlerton *base, const LEX_CSTRING *old_db,
   bool log_query= 0;
   DBUG_ENTER("mysql_rename_table");
   DBUG_ASSERT(base);
+  DBUG_ASSERT(flags & (QRMT_FRM | QRMT_PAR | QRMT_HANDLER));
   DBUG_PRINT("enter", ("old: '%s'.'%s'  new: '%s'.'%s'",
                        old_db->str, old_name->str, new_db->str,
                        new_name->str));
@@ -5381,17 +5386,22 @@ mysql_rename_table(handlerton *base, const LEX_CSTRING *old_db,
     to_base=   lc_to;
   }
 
-  if (flags & NO_HA_TABLE)
+  if (!(flags & QRMT_HANDLER))
   {
+    /*
+      This code expects callers to set QRMT_FRM if QRMT_HANDLER is omitted.
+      Otherwise this invariant is not strictly required.
+    */
+    DBUG_ASSERT(flags & QRMT_FRM);
     if (rename_file_ext(from,to,reg_ext))
       error= my_errno;
     log_query= true;
-    if (file && !(flags & NO_PAR_TABLE))
+    if (file && (flags & QRMT_PAR))
       (void) file->ha_create_partitioning_metadata(to, from, CHF_RENAME_FLAG);
   }
   else if (!file || likely(!(error=file->ha_rename_table(from_base, to_base))))
   {
-    if (!(flags & NO_FRM_RENAME) && unlikely(rename_file_ext(from,to,reg_ext)))
+    if ((flags & QRMT_FRM) && unlikely(rename_file_ext(from, to, reg_ext)))
     {
       error=my_errno;
       if (file)
@@ -8118,7 +8128,7 @@ static bool mysql_inplace_alter_table(THD *thd,
   if (mysql_rename_table(db_type, &alter_ctx->new_db, &alter_ctx->tmp_name,
                          &alter_ctx->db, &alter_ctx->alias,
                          &alter_ctx->tmp_id,
-                         FN_FROM_IS_TMP | NO_HA_TABLE) ||
+                         QRMT_FRM | QRMT_PAR | FN_FROM_IS_TMP) ||
                          thd->is_error())
   {
     // Since changes were done in-place, we can't revert them.
@@ -8133,7 +8143,7 @@ static bool mysql_inplace_alter_table(THD *thd,
                                      alter_ctx->table_name.str));
     if (mysql_rename_table(db_type, &alter_ctx->db, &alter_ctx->table_name,
                            &alter_ctx->new_db, &alter_ctx->new_alias,
-                           &alter_ctx->tmp_id, 0))
+                           &alter_ctx->tmp_id, QRMT_DEFAULT))
     {
       /*
         If the rename fails we will still have a working table
@@ -8157,7 +8167,7 @@ static bool mysql_inplace_alter_table(THD *thd,
                                 &alter_ctx->new_db, &alter_ctx->new_alias,
                                 &alter_ctx->db, &alter_ctx->alias,
                                 &alter_ctx->id,
-                                NO_FK_CHECKS);
+                                QRMT_DEFAULT | NO_FK_CHECKS);
       ddl_log_disable_entry(ddl_log_state);
       DBUG_RETURN(true);
     }
@@ -9889,7 +9899,7 @@ simple_rename_or_index_change(THD *thd, TABLE_LIST *table_list,
                                 &alter_ctx->new_db, &alter_ctx->new_alias);
     if (mysql_rename_table(old_db_type, &alter_ctx->db, &alter_ctx->table_name,
                            &alter_ctx->new_db, &alter_ctx->new_alias,
-                           &table_version, 0))
+                           &table_version, QRMT_DEFAULT))
       error= -1;
     if (!error)
       ddl_log_update_phase(&ddl_log_state, DDL_RENAME_PHASE_TRIGGER);
@@ -9906,7 +9916,7 @@ simple_rename_or_index_change(THD *thd, TABLE_LIST *table_list,
                                 &alter_ctx->new_db, &alter_ctx->new_alias,
                                 &alter_ctx->db, &alter_ctx->table_name,
                                 &table_version,
-                                NO_FK_CHECKS);
+                                QRMT_DEFAULT | NO_FK_CHECKS);
       ddl_log_disable_entry(&ddl_log_state);
       error= -1;
     }
@@ -10601,8 +10611,7 @@ bool mysql_alter_table(THD *thd, const LEX_CSTRING *new_db,
                                              thd->variables.lock_wait_timeout))
       DBUG_RETURN(1);
     quick_rm_table(thd, table->file->ht, &table_list->db,
-                   &table_list->table_name,
-                   NO_HA_TABLE, 0);
+                   &table_list->table_name, QRMT_FRM | QRMT_PAR);
     goto end_inplace;
   }
   if (!if_exists &&
@@ -11759,11 +11768,11 @@ do_continue:;
     if (mysql_rename_table(old_db_type, &alter_ctx.db, &alter_ctx.table_name,
                            &alter_ctx.db, &backup_name, &alter_ctx.id,
                            FN_TO_IS_TMP |
-                           (engine_changed ? NO_HA_TABLE | NO_PAR_TABLE : 0)))
+                           (engine_changed ? QRMT_FRM : QRMT_DEFAULT)))
     {
       // Rename to temporary name failed, delete the new table, abort ALTER.
       (void) quick_rm_table(thd, new_db_type, &alter_ctx.new_db,
-                            &alter_ctx.tmp_name, FN_IS_TMP);
+                            &alter_ctx.tmp_name, QRMT_DEFAULT | FN_IS_TMP);
       goto err_with_mdl;
     }
   }
@@ -11791,12 +11800,12 @@ do_continue:;
   if (mysql_rename_table(new_db_type, &alter_ctx.new_db, &alter_ctx.tmp_name,
                          &alter_ctx.new_db, &alter_ctx.new_alias,
                          &alter_ctx.tmp_id,
-                         FN_FROM_IS_TMP))
+                         QRMT_DEFAULT | FN_FROM_IS_TMP))
   {
     // Rename failed, delete the temporary table.
     ddl_log_update_phase(&ddl_log_state, DDL_ALTER_TABLE_PHASE_RENAME_FAILED);
     (void) quick_rm_table(thd, new_db_type, &alter_ctx.new_db,
-                          &alter_ctx.tmp_name, FN_IS_TMP);
+                          &alter_ctx.tmp_name, QRMT_DEFAULT | FN_IS_TMP);
 
     if (!alter_ctx.is_table_renamed() || alter_ctx.fk_error_if_delete_row)
     {
@@ -11804,8 +11813,7 @@ do_continue:;
       (void) mysql_rename_table(old_db_type, &alter_ctx.db, &backup_name,
                                 &alter_ctx.db, &alter_ctx.alias, &alter_ctx.id,
                                 FN_FROM_IS_TMP | NO_FK_CHECKS |
-                                (engine_changed ? NO_HA_TABLE | NO_PAR_TABLE :
-                                 0));
+                                (engine_changed ? QRMT_FRM : QRMT_DEFAULT));
     }
     goto err_with_mdl;
   }
@@ -11823,14 +11831,13 @@ do_continue:;
                                                &alter_ctx.new_alias))
     {
       // Rename succeeded, delete the new table.
-      (void) quick_rm_table(thd, new_db_type,
-                            &alter_ctx.new_db, &alter_ctx.new_alias, 0);
+      (void) quick_rm_table(thd, new_db_type, &alter_ctx.new_db,
+                            &alter_ctx.new_alias, QRMT_DEFAULT);
       // Restore the backup of the original table to the old name.
       (void) mysql_rename_table(old_db_type, &alter_ctx.db, &backup_name,
                                 &alter_ctx.db, &alter_ctx.alias, &alter_ctx.id,
                                 FN_FROM_IS_TMP | NO_FK_CHECKS |
-                                (engine_changed ? NO_HA_TABLE | NO_PAR_TABLE :
-                                 0));
+                                (engine_changed ? QRMT_FRM : QRMT_DEFAULT));
       goto err_with_mdl;
     }
     rename_table_in_stat_tables(thd, &alter_ctx.db, &alter_ctx.alias,
@@ -11847,13 +11854,13 @@ do_continue:;
   {
     /* the .frm file was removed but not the original table */
     quick_rm_table(thd, old_db_type, &alter_ctx.db, &alter_ctx.table_name,
-                           NO_FRM_RENAME | (engine_changed ? 0 : FN_IS_TMP));
+                   QRMT_HANDLER | (engine_changed ? 0 : FN_IS_TMP));
   }
 
   debug_crash_here("ddl_log_alter_after_delete_backup");
 
   quick_rm_table(thd, old_db_type, &alter_ctx.db, &backup_name,
-                        FN_IS_TMP | (engine_changed ? NO_HA_TABLE | NO_PAR_TABLE: 0));
+                 FN_IS_TMP | (engine_changed ? QRMT_FRM : QRMT_DEFAULT));
 
   debug_crash_here("ddl_log_alter_after_drop_original_table");
   if (binlog_as_create_select)
@@ -11992,7 +11999,8 @@ do_continue:;
   else
     (void) quick_rm_table(thd, new_db_type,
                           &alter_ctx.new_db, &alter_ctx.tmp_name,
-                          (FN_IS_TMP | (no_ha_table ? NO_HA_TABLE : 0)),
+                          FN_IS_TMP | (no_ha_table ?
+                                       QRMT_FRM | QRMT_PAR : QRMT_DEFAULT),
                           alter_ctx.get_tmp_path());
   DEBUG_SYNC(thd, "alter_table_after_temp_table_drop");
 err_cleanup:

sql/sql_table.h

@@ -66,16 +66,18 @@ enum enum_explain_filename_mode
 static const uint FN_FROM_IS_TMP=  1 << 0;
 static const uint FN_TO_IS_TMP=    1 << 1;
 static const uint FN_IS_TMP=       FN_FROM_IS_TMP | FN_TO_IS_TMP;
-static const uint NO_FRM_RENAME=   1 << 2;
-static const uint FRM_ONLY=        1 << 3;
-/** Don't remove table in engine. Remove only .FRM and maybe .PAR files. */
-static const uint NO_HA_TABLE=     1 << 4;
+/* Remove .frm table metadata. */
+static constexpr uint QRMT_FRM=       1 << 2;
+/* Remove .par partitioning metadata. */
+static constexpr uint QRMT_PAR=       1 << 3;
+/* Remove handler files and high-level indexes. */
+static constexpr uint QRMT_HANDLER=   1 << 4;
+/* Default behaviour is to drop .FRM and handler, but not .par. */
+static constexpr uint QRMT_DEFAULT=   QRMT_FRM | QRMT_HANDLER;
 /** Don't resolve MySQL's fake "foo.sym" symbolic directory names. */
 static const uint SKIP_SYMDIR_ACCESS= 1 << 5;
 /** Don't check foreign key constraints while renaming table */
 static const uint NO_FK_CHECKS=    1 << 6;
-/* Don't delete .par table in quick_rm_table() */
-static const uint NO_PAR_TABLE=   1 << 7;
 
 uint filename_to_tablename(const char *from, char *to, size_t to_length,
                            bool stay_quiet = false);