MDEV-11412 Ensure that table is truly dropped when using DROP TABLE

The used code is largely based on code from Tencent

The problem is that in some rare cases there may be a conflict between .frm
files and the files in the storage engine. In this case the DROP TABLE
was not able to properly drop the table.

Some MariaDB/MySQL forks has solved this by adding a FORCE option to
DROP TABLE. After some discussion among MariaDB developers, we concluded
that users expects that DROP TABLE should always work, even if the
table would not be consistent. There should not be a need to use a
separate keyword to ensure that the table is really deleted.

The used solution is:
- If a .frm table doesn't exists, try dropping the table from all storage
  engines.
- If the .frm table exists but the table does not exist in the engine
  try dropping the table from all storage engines.
- Update storage engines using many table files (.CVS, MyISAM, Aria) to
  succeed with the drop even if some of the files are missing.
- Add HTON_AUTOMATIC_DELETE_TABLE to handlerton's where delete_table()
  is not needed and always succeed. This is used by ha_delete_table_force()
  to know which handlers to ignore when trying to drop a table without
  a .frm file.

The disadvantage of this solution is that a DROP TABLE on a non existing
table will be a bit slower as we have to ask all active storage engines
if they know anything about the table.

Other things:
- Added a new flag MY_IGNORE_ENOENT to my_delete() to not give an error
  if the file doesn't exist. This simplifies some of the code.
- Don't clear thd->error in ha_delete_table() if there was an active
  error. This is a bug fix.
- handler::delete_table() will not abort if first file doesn't exists.
  This is bug fix to handle the case when a drop table was aborted in
  the middle.
- Cleaned up mysql_rm_table_no_locks() to ensure that if_exists uses
  same code path as when it's not used.
- Use non_existing_Table_error() to detect if table didn't exists.
  Old code used different errors tests in different position.
- Table_triggers_list::drop_all_triggers() now drops trigger file if
  it can't be parsed instead of leaving it hanging around (bug fix)
- InnoDB doesn't anymore print error about .frm file out of sync with
  InnoDB directory if .frm file does not exists. This change was required
  to be able to try to drop an InnoDB file when .frm doesn't exists.
- Fixed bug in mi_delete_table() where the .MYD file would not be dropped
  if the .MYI file didn't exists.
- Fixed memory leak in Mroonga when deleting non existing table
- Fixed memory leak in Connect when deleting non existing table

Bugs fixed introduced by the original version of this commit:
MDEV-22826 Presence of Spider prevents tables from being force-deleted from
           other engines

1 files changed, 171 insertions, 83 deletions

diff --git a/sql/sql_table.cc b/sql/sql_table.cc
index 05bef5f4bda..2569665fc59 100644
--- a/sql/sql_table.cc
+++ b/sql/sql_table.cc
@@ -1107,7 +1107,7 @@ static int execute_ddl_log_action(THD *thd, DDL_LOG_ENTRY *ddl_log_entry)
   LEX_CSTRING handler_name;
   handler *file= NULL;
   MEM_ROOT mem_root;
-  int error= TRUE;
+  int error= 1;
   char to_path[FN_REFLEN];
   char from_path[FN_REFLEN];
   handlerton *hton;
@@ -1157,28 +1157,27 @@ static int execute_ddl_log_action(THD *thd, DDL_LOG_ENTRY *ddl_log_entry)
         {
           strxmov(to_path, ddl_log_entry->name, reg_ext, NullS);
           if (unlikely((error= mysql_file_delete(key_file_frm, to_path,
-                                                 MYF(MY_WME)))))
-          {
-            if (my_errno != ENOENT)
-              break;
-          }
+                                                 MYF(MY_WME |
+                                                     MY_IGNORE_ENOENT)))))
+            break;
 #ifdef WITH_PARTITION_STORAGE_ENGINE
           strxmov(to_path, ddl_log_entry->name, PAR_EXT, NullS);
-          (void) mysql_file_delete(key_file_partition_ddl_log, to_path, MYF(MY_WME));
+          (void) mysql_file_delete(key_file_partition_ddl_log, to_path,
+                                   MYF(0));
 #endif
         }
         else
         {
           if (unlikely((error= file->ha_delete_table(ddl_log_entry->name))))
           {
-            if (error != ENOENT && error != HA_ERR_NO_SUCH_TABLE)
+            if (!non_existing_table_error(error))
               break;
           }
         }
         if ((deactivate_ddl_log_entry_no_lock(ddl_log_entry->entry_pos)))
           break;
         (void) sync_ddl_log_no_lock();
-        error= FALSE;
+        error= 0;
         if (ddl_log_entry->action_type == DDL_LOG_DELETE_ACTION)
           break;
       }
@@ -2308,11 +2307,14 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists,
 
   for (table= tables; table; table= table->next_local)
   {
-    bool is_trans= 0;
-    bool table_creation_was_logged= 0;
+    bool is_trans= 0, frm_was_deleted= 0, temporary_table_was_dropped= 0;
+    bool table_creation_was_logged= 0, trigger_drop_executed= 0;
+    bool local_non_tmp_error= 0, frm_exists= 0, wrong_drop_sequence= 0;
+    bool drop_table_not_done= 0;
     LEX_CSTRING db= table->db;
     handlerton *table_type= 0;
 
+    error= 0;
     DBUG_PRINT("table", ("table_l: '%s'.'%s'  table: %p  s: %p",
                          table->db.str, table->table_name.str,  table->table,
                          table->table ?  table->table->s : NULL));
@@ -2327,36 +2329,42 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists,
                   thd->find_temporary_table(table) &&
                   table->mdl_request.ticket != NULL));
 
-    if (table->open_type == OT_BASE_ONLY || !is_temporary_table(table) ||
-        (drop_sequence && table->table->s->table_type != TABLE_TYPE_SEQUENCE))
-      error= 1;
-    else
+    /* First try to delete temporary tables and temporary sequences */
+    if ((table->open_type != OT_BASE_ONLY && is_temporary_table(table)) &&
+        (!drop_sequence || table->table->s->table_type == TABLE_TYPE_SEQUENCE))
     {
       table_creation_was_logged= table->table->s->table_creation_was_logged;
       if (thd->drop_temporary_table(table->table, &is_trans, true))
       {
+        /*
+          This is a very unlikely scenaro as dropping a temporary table
+          should always work. Would be better if we tried to drop all
+          temporary tables before giving the error.
+        */
         error= 1;
         goto err;
       }
-      error= 0;
       table->table= 0;
+      temporary_table_was_dropped= 1;
     }
 
-    if ((drop_temporary && if_exists) || !error)
+    if ((drop_temporary && if_exists) || temporary_table_was_dropped)
     {
       /*
         This handles the case of temporary tables. We have the following cases:
 
-          . "DROP TEMPORARY" was executed and a temporary table was affected
-          (i.e. drop_temporary && !error) or the if_exists was specified (i.e.
-          drop_temporary && if_exists).
-
-          . "DROP" was executed but a temporary table was affected (.i.e
-          !error).
+          - "DROP TEMPORARY" was executed and table was dropped
+            temporary_table_was_dropped == 1
+          - "DROP TEMPORARY IF EXISTS" was specified but no temporary table
+            existed
+            temporary_table_was_dropped == 0
       */
       if (!dont_log_query && table_creation_was_logged)
       {
         /*
+          DROP TEMPORARY succeded. For the moment when we only come
+          here on success (error == 0)
+
           If there is an error, we don't know the type of the engine
           at this point. So, we keep it in the trx-cache.
         */
@@ -2387,7 +2395,8 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists,
         is no need to proceed with the code that tries to drop a regular
         table.
       */
-      if (!error) continue;
+      if (temporary_table_was_dropped)
+        continue;
     }
     else if (!drop_temporary)
     {
@@ -2402,48 +2411,33 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists,
       path_length= build_table_filename(path, sizeof(path) - 1, db.str,
                                         alias.str, reg_ext, 0);
     }
+
     DEBUG_SYNC(thd, "rm_table_no_locks_before_delete_table");
     error= 0;
-    if (drop_temporary ||
-        (ha_table_exists(thd, &db, &alias, &table_type, &is_sequence) == 0 &&
-         table_type == 0) ||
-        (!drop_view && (was_view= (table_type == view_pseudo_hton))) ||
-        (drop_sequence && !is_sequence))
+    if (drop_temporary)
+    {
+      /* "DROP TEMPORARY" but a temporary table was not found */
+      error= ENOENT;
+    }
+    else if (((frm_exists= ha_table_exists(thd, &db, &alias, &table_type,
+                                           &is_sequence)) == 0 &&
+              table_type == 0) ||
+             (!drop_view && (was_view= (table_type == view_pseudo_hton))) ||
+             (drop_sequence && !is_sequence))
     {
       /*
         One of the following cases happened:
-          . "DROP TEMPORARY" but a temporary table was not found.
           . "DROP" but table was not found
           . "DROP TABLE" statement, but it's a view.
           . "DROP SEQUENCE", but it's not a sequence
       */
-      was_table= drop_sequence && table_type;
-      if (if_exists)
-      {
-        char buff[FN_REFLEN];
-        int err= (drop_sequence ? ER_UNKNOWN_SEQUENCES :
-                  ER_BAD_TABLE_ERROR);
-        String tbl_name(buff, sizeof(buff), system_charset_info);
-        tbl_name.length(0);
-        tbl_name.append(&db);
-        tbl_name.append('.');
-        tbl_name.append(&table->table_name);
-        push_warning_printf(thd, Sql_condition::WARN_LEVEL_NOTE,
-                            err, ER_THD(thd, err),
-                            tbl_name.c_ptr_safe());
-
-        /*
-          Our job is done here. This statement was added to avoid executing
-          unnecessary code farther below which in some strange corner cases
-          caused the server to crash (see MDEV-17896).
-        */
-        goto log_query;
-      }
-      else
-      {
-        non_tmp_error = (drop_temporary ? non_tmp_error : TRUE);
-        error= 1;
-      }
+      wrong_drop_sequence= drop_sequence && table_type;
+      was_table|= wrong_drop_sequence;
+      local_non_tmp_error= 1;
+      error= -1;
+      if ((!frm_exists && !table_type) ||       // no .frm
+          if_exists)
+        error= ENOENT;
     }
     else
     {
@@ -2500,8 +2494,12 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists,
         log_if_exists= 1;
 
       thd->replication_flags= 0;
-      if ((error= ha_delete_table(thd, table_type, path, &db,
-                                  &table->table_name, !dont_log_query)))
+      error= ha_delete_table(thd, table_type, path, &db,
+                             &table->table_name, !dont_log_query);
+
+      if (error < 0)                            // Table didn't exists
+        error= 0;
+      if (error)
       {
         if (thd->is_killed())
         {
@@ -2509,49 +2507,137 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists,
           goto err;
         }
       }
-      else
+
+      /*
+        Delete the .frm file if we managed to delete the table from the
+        engine or the table didn't exists in the engine
+      */
+      if (likely(!error) || non_existing_table_error(error))
       {
         /* Delete the table definition file */
         strmov(end,reg_ext);
         if (table_type && table_type != view_pseudo_hton &&
-            table_type->discover_table)
+            (table_type->discover_table || error))
         {
           /*
-            Table type is using discovery and may not need a .frm file.
+            Table type is using discovery and may not need a .frm file
+            or the .frm file existed but no table in engine.
             Delete it silently if it exists
           */
-          (void) mysql_file_delete(key_file_frm, path, MYF(0));
+          if (mysql_file_delete(key_file_frm, path,
+                                MYF(MY_WME | MY_IGNORE_ENOENT)))
+            error= my_errno;
         }
         else if (unlikely(mysql_file_delete(key_file_frm, path,
-                                            MYF(MY_WME))))
+                                            !error ? MYF(MY_WME) :
+                                            MYF(MY_WME | MY_IGNORE_ENOENT))))
         {
           frm_delete_error= my_errno;
           DBUG_ASSERT(frm_delete_error);
         }
       }
+      frm_was_deleted= 1;
       if (thd->replication_flags & OPTION_IF_EXISTS)
         log_if_exists= 1;
 
-      if (likely(!error))
+      if (frm_delete_error)
       {
-        int trigger_drop_error= 0;
+        /*
+          Remember error if unexpected error from dropping the .frm file
+          or we got an error from ha_delete_table()
+        */
+        if (frm_delete_error != ENOENT)
+          error= frm_delete_error;
+        else if (if_exists && ! error)
+          thd->clear_error();
+      }
+      if (likely(!error) || !frm_delete_error)
+        non_tmp_table_deleted= TRUE;
+
+      if (likely(!error) || non_existing_table_error(error))
+      {
+        trigger_drop_executed= 1;
+
+        if (Table_triggers_list::drop_all_triggers(thd, &db,
+                                                   &table->table_name,
+                                                   MYF(MY_WME |
+                                                       MY_IGNORE_ENOENT)))
+          error= error ? error : -1;
+      }
+      local_non_tmp_error|= MY_TEST(error);
+    }
+
+    /*
+      If there was no .frm file and the table is not temporary,
+      scan all engines try to drop the table from there.
+      This is to ensure we don't have any partial table files left.
+
+      We check for trigger_drop_executed to ensure we don't again try
+      to drop triggers when it failed above (after sucecssfully dropping
+      the table).
+    */
+    if (non_existing_table_error(error) && !drop_temporary &&
+        table_type != view_pseudo_hton && !trigger_drop_executed &&
+        !wrong_drop_sequence)
+    {
+      char *end;
+      int ferror= 0;
+
+      /* Remove extension for delete */
+      *(end = path + path_length - reg_ext_length) = '\0';
+      ferror= ha_delete_table_force(thd, path, &db, &table->table_name);
+      if (!ferror)
+      {
+        /* Table existed and was deleted */
+        non_tmp_table_deleted= TRUE;
+        local_non_tmp_error= 0;
+        error= 0;
+      }
+      if (ferror <= 0)
+      {
+        ferror= 0;                              // Ignore table not found
 
-        if (likely(!frm_delete_error))
+        /* Delete the table definition file */
+        if (!frm_was_deleted)
         {
-          non_tmp_table_deleted= TRUE;
-          trigger_drop_error=
-            Table_triggers_list::drop_all_triggers(thd, &db,
-                                                   &table->table_name);
+          strmov(end, reg_ext);
+          if (mysql_file_delete(key_file_frm, path,
+                                MYF(MY_WME | MY_IGNORE_ENOENT)))
+            ferror= my_errno;
         }
-
-        if (unlikely(trigger_drop_error) ||
-            (frm_delete_error && frm_delete_error != ENOENT))
-          error= 1;
-        else if (frm_delete_error && if_exists)
-          thd->clear_error();
+        if (Table_triggers_list::drop_all_triggers(thd, &db,
+                                                   &table->table_name,
+                                                   MYF(MY_WME |
+                                                       MY_IGNORE_ENOENT)))
+          ferror= -1;
       }
-      non_tmp_error|= MY_TEST(error);
+      if (!error)
+        error= ferror;
+    }
+
+    /*
+      Don't give an error if we are using IF EXISTS for a table that
+      didn't exists
+    */
+
+    if (if_exists && non_existing_table_error(error))
+    {
+      char buff[FN_REFLEN];
+      int err= (drop_sequence ? ER_UNKNOWN_SEQUENCES :
+                ER_BAD_TABLE_ERROR);
+      String tbl_name(buff, sizeof(buff), system_charset_info);
+      tbl_name.length(0);
+      tbl_name.append(&db);
+      tbl_name.append('.');
+      tbl_name.append(&table->table_name);
+      push_warning_printf(thd, Sql_condition::WARN_LEVEL_NOTE,
+                          err, ER_THD(thd, err),
+                          tbl_name.c_ptr_safe());
+      error= 0;
+      local_non_tmp_error= 0;
+      drop_table_not_done= 1;
     }
+    non_tmp_error|= local_non_tmp_error;
 
     if (error)
     {
@@ -2562,14 +2648,14 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists,
       wrong_tables.append(&table->table_name);
       errors++;
     }
-    else
+    else if (!drop_table_not_done)
     {
-      PSI_CALL_drop_table_share(false, table->db.str, (uint)table->db.length,
+      PSI_CALL_drop_table_share(temporary_table_was_dropped,
+                                table->db.str, (uint)table->db.length,
                                 table->table_name.str, (uint)table->table_name.length);
       mysql_audit_drop_table(thd, table);
     }
 
-log_query:
     if (!dont_log_query && !drop_temporary)
     {
       non_tmp_table_deleted= (if_exists ? TRUE : non_tmp_table_deleted);
@@ -2727,9 +2813,10 @@ err:
   }
 
 end:
-  DBUG_RETURN(error);
+  DBUG_RETURN(error || thd->is_error());
 }
 
+
 /**
   Log the drop of a table.
 
@@ -2809,7 +2896,8 @@ bool quick_rm_table(THD *thd, handlerton *base, const LEX_CSTRING *db,
     delete file;
   }
   if (!(flags & (FRM_ONLY|NO_HA_TABLE)))
-    error|= ha_delete_table(thd, base, path, db, table_name, 0);
+    if (ha_delete_table(thd, base, path, db, table_name, 0) > 0)
+    error= 1;
 
   if (likely(error == 0))
   {