drop_backup_frms()

sql/datadict.cc

@@ -646,6 +646,36 @@ bool FK_ddl_vector::install_shadow_frms(THD *thd)
     dbg_first= false;
 #endif
   }
+
+  return false;
+
+error:
+  rollback(thd);
+  return true;
+}
+
+
+void FK_ddl_vector::rollback(THD *thd)
+{
+  // FIXME: push warning
+  for (FK_ddl_backup &bak: *this)
+    bak.rollback(*this);
+
+  if (execute_ddl_log_entry(thd, first_entry->entry_pos))
+  {
+    write_log_finish();
+    // FIXME: push warning
+  }
+  else
+  {
+    Mutex_lock lock_gdl(&LOCK_gdl);
+    release();
+  }
+}
+
+
+void FK_ddl_vector::drop_backup_frms(THD *thd)
+{
 #ifndef DBUG_OFF
   dbg_first= true;
 #endif
@@ -653,8 +683,8 @@ bool FK_ddl_vector::install_shadow_frms(THD *thd)
   {
     if (deactivate_ddl_log_entry(bak.restore_backup_entry->entry_pos))
     {
-      // FIXME: return backup FRMs, test
-      goto error;
+      // TODO: must be atomic
+      // FIXME: push warning
     }
 #ifndef DBUG_OFF
     dbg_first= false;
@@ -671,23 +701,4 @@ bool FK_ddl_vector::install_shadow_frms(THD *thd)
 #endif
   }
   write_log_finish();
-
-  return false;
-
-error:
-  // FIXME: push warning
-  for (FK_ddl_backup &bak: *this)
-    bak.rollback(*this);
-
-  if (execute_ddl_log_entry(thd, first_entry->entry_pos))
-  {
-    write_log_finish();
-    // FIXME: push warning
-  }
-  else
-  {
-    Mutex_lock lock_gdl(&LOCK_gdl);
-    release();
-  }
-  return true;
 }

sql/handler.cc

@@ -5571,7 +5571,8 @@ int ha_create_table(THD *thd, const char *path,
 
   if (fk_update_refs)
   {
-    fk_shares.install_shadow_frms(thd);
+    fk_shares.install_shadow_frms(thd); // FIXME: move to beginning, handle return status
+    fk_shares.drop_backup_frms(thd);
   }
 
   free_table_share(&share);

sql/sql_class.h

@@ -982,6 +982,8 @@ class FK_ddl_vector: public mbd::vector<FK_ddl_backup>, public ddl_log_info
 {
 public:
   bool install_shadow_frms(THD *thd);
+  void drop_backup_frms(THD *thd);
+  void rollback(THD *thd);
   ~FK_ddl_vector()
   {
 

sql/sql_rename.cc

@@ -178,14 +178,15 @@ bool mysql_rename_tables(THD *thd, TABLE_LIST *table_list, bool silent,
     /* Revert the table list (for prepared statements) */
     table_list= reverse_table_list(table_list);
 
-    for (FK_ddl_backup &bak: fk_rename_backup)
-      bak.rollback(fk_rename_backup);
+    fk_rename_backup.rollback(thd);
 
     error= 1;
   }
   else
   {
-    error= fk_rename_backup.install_shadow_frms(thd);
+    error= fk_rename_backup.install_shadow_frms(thd); // FIXME: move to beginning
+    if (!error)
+      fk_rename_backup.drop_backup_frms(thd);
   }
 
   if (likely(!silent && !error))

sql/sql_table.cc

@@ -2485,8 +2485,9 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists,
       }
       bool enoent_warning;
       FK_ddl_vector shares;
-      if ((error= fk_handle_drop(thd, table, shares, drop_db)))
-        goto fk_error;
+      error= fk_handle_drop(thd, table, shares, drop_db);
+      if (unlikely(error))
+        goto err;
 
       if (thd->locked_tables_mode == LTM_LOCK_TABLES ||
           thd->locked_tables_mode == LTM_PRELOCKED_UNDER_LOCK_TABLES)
@@ -2494,9 +2495,7 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists,
         if (wait_while_table_is_used(thd, table->table, HA_EXTRA_NOT_USED))
         {
           error= -1;
-          for (FK_ddl_backup &bak: shares)
-            if (bak.sa.share)
-              bak.rollback(shares);
+          shares.rollback(thd);
           goto err;
         }
         close_all_tables_for_name(thd, table->table->s,
@@ -2510,6 +2509,11 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists,
       DBUG_ASSERT(thd->mdl_context.is_lock_owner(MDL_key::TABLE, db.str,
                                                 table_name.str, MDL_EXCLUSIVE));
 
+
+      error= shares.install_shadow_frms(thd);
+      if (unlikely(error))
+        goto err;
+
       // Remove extension for delete
       *path_end= '\0';
 
@@ -2526,13 +2530,10 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists,
         error= 0;                            // Table didn't exists
       else if (error)
       {
-fk_error:
+        shares.rollback(thd);
         if (drop_db || thd->is_killed())
         {
           error= -1;
-          for (FK_ddl_backup &bak: shares)
-            if (bak.sa.share)
-              bak.rollback(shares);
           goto err;
         }
       }
@@ -2580,23 +2581,12 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists,
           table_dropped= 1;
         }
       }
-      if (likely(!error))
-      {
-        /*
-           TODO: recover dropped table if install_shadow_frms() fails.
 
-           NB: We cannot do install_shadow_frms() before ha_delete_table() because
-           frms must be oroginal if DROP fails. OTOH we should fail DROP if we
-           failed to change frms.
-        */
-        error= shares.install_shadow_frms(thd);
-      }
+      if (likely(!error))
+        shares.drop_backup_frms(thd);
       else
-      {
-        for (FK_ddl_backup &bak: shares)
-          if (bak.sa.share)
-            bak.rollback(shares);
-      }
+        shares.rollback(thd);
+
       local_non_tmp_error|= MY_TEST(error);
     }
 
@@ -9996,12 +9986,13 @@ simple_rename_or_index_change(THD *thd, TABLE_LIST *table_list,
                                          &alter_ctx->table_name,
                                          &alter_ctx->new_db,
                                          &alter_ctx->new_name);
-      error= fk_rename_backup.install_shadow_frms(thd);
+      error= fk_rename_backup.install_shadow_frms(thd); // FIXME: move to beginning
+      if (!error)
+        fk_rename_backup.drop_backup_frms(thd);
     }
     else
     {
-      for (FK_ddl_backup &bak: fk_rename_backup)
-        bak.rollback(fk_rename_backup);
+      fk_rename_backup.rollback(thd);
     }
   }
 
@@ -13161,7 +13152,10 @@ bool fk_handle_drop(THD *thd, TABLE_LIST *table, FK_ddl_vector &shares,
     if (err)
     {
       if (err > 2)
+      {
+        shares.rollback(thd);
         return true;
+      }
       // ignore non-existent frm (main.drop_table_force, Test6)
       thd->clear_error();
       ref.sa.release();
@@ -13342,11 +13336,12 @@ void
 FK_ddl_backup::rollback(ddl_log_info& log_info)
 {
   // FIXME: add test case
-//   DBUG_ASSERT(0);
-  DBUG_ASSERT(sa.share);
-  sa.share->foreign_keys= foreign_keys;
-  sa.share->referenced_keys= referenced_keys;
-  delete_shadow_entry= NULL;
+  if (sa.share)
+  {
+    sa.share->foreign_keys= foreign_keys;
+    sa.share->referenced_keys= referenced_keys;
+    delete_shadow_entry= NULL;
+  }
 }