rollback() cleanup

4 files changed, 25 insertions, 19 deletions

diff --git a/mysql-test/suite/innodb/t/f.test b/mysql-test/suite/innodb/t/f.test
index 61d3376f191..000d8ce342e 100644
--- a/mysql-test/suite/innodb/t/f.test
+++ b/mysql-test/suite/innodb/t/f.test
@@ -56,7 +56,7 @@ let $debug_dbug="+d,crash_fk_install_shadow_frm";
 --source include/foreign_fail.inc
 call drop_tables;
 
-# FIXME: crash statements
+# FIXME: crash statements, fail_fk_write_shadow
 
 --echo # CREATE TABLE
 create table t1 (x int primary key);
diff --git a/sql/datadict.cc b/sql/datadict.cc
index 15481997c87..07f67d25fd0 100644
--- a/sql/datadict.cc
+++ b/sql/datadict.cc
@@ -667,7 +667,7 @@ bool FK_ddl_vector::install_shadow_frms(THD *thd)
       dbg_fail= true;
 #endif
     if (bak.second.fk_backup_frm(*this))
-      goto error;
+      return true;
   }
 #ifndef DBUG_OFF
   dbg_fail= false;
@@ -681,14 +681,10 @@ bool FK_ddl_vector::install_shadow_frms(THD *thd)
       dbg_fail= true;
 #endif
     if (bak.second.fk_install_shadow_frm(*this))
-      goto error;
+      return true;
   }
 
   return false;
-
-error:
-  rollback(thd);
-  return true;
 }
 
 
diff --git a/sql/handler.cc b/sql/handler.cc
index 042e959e105..1a35251871d 100644
--- a/sql/handler.cc
+++ b/sql/handler.cc
@@ -5539,12 +5539,10 @@ int ha_create_table(THD *thd, const char *path,
       goto err;
   }
 
-  if (fk_update_refs)
-  {
-    if (share.fk_handle_create(thd, fk_shares) ||
-        fk_shares.install_shadow_frms(thd))
-      goto err;
-  }
+  if (fk_update_refs &&
+      (share.fk_handle_create(thd, fk_shares) ||
+      fk_shares.install_shadow_frms(thd)))
+    goto err;
 
   share.m_psi= PSI_CALL_get_table_share(temp_table, &share);
 
diff --git a/sql/sql_table.cc b/sql/sql_table.cc
index e867d3d9429..9587c09bd35 100644
--- a/sql/sql_table.cc
+++ b/sql/sql_table.cc
@@ -2549,7 +2549,10 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists,
 
       error= shares.install_shadow_frms(thd);
       if (unlikely(error))
+      {
+        shares.rollback(thd);
         goto err;
+      }
 
       // Remove extension for delete
       *path_end= '\0';
@@ -8275,7 +8278,10 @@ static bool mysql_inplace_alter_table(THD *thd,
 
   // FIXME: do right after fk_handle_alter?
   if (alter_ctx->fk_ref_backup.install_shadow_frms(thd))
+  {
+    alter_ctx->fk_ref_backup.rollback(thd);
     DBUG_RETURN(true);
+  }
 
   alter_ctx->fk_ref_backup.drop_backup_frms(thd);
 
@@ -10002,14 +10008,22 @@ simple_rename_or_index_change(THD *thd, TABLE_LIST *table_list,
       DBUG_RETURN(true);
 
     for (auto &ref: fk_rename_backup)
+    {
       if (ref.second.fk_write_shadow_frm(fk_rename_backup))
+      {
+        fk_rename_backup.rollback(thd);
         DBUG_RETURN(true);
+      }
+    }
 
     close_all_tables_for_name(thd, table->s, HA_EXTRA_PREPARE_FOR_RENAME,
                               NULL);
 
     if (fk_rename_backup.install_shadow_frms(thd))
+    {
+      fk_rename_backup.rollback(thd);
       DBUG_RETURN(true);
+    }
 
     if (mysql_rename_table(old_db_type, &alter_ctx->db, &alter_ctx->table_name,
                            &alter_ctx->new_db, &alter_ctx->new_name, 0))
@@ -13347,11 +13361,9 @@ FK_share_backup::init(TABLE_SHARE *_share)
 void
 FK_share_backup::rollback(ddl_log_info& log_info)
 {
-  if (share)
-  {
-    share->foreign_keys= foreign_keys;
-    share->referenced_keys= referenced_keys;
-    share= NULL;
-  }
+  DBUG_ASSERT(share);
+  share->foreign_keys= foreign_keys;
+  share->referenced_keys= referenced_keys;
+  share= NULL;
   delete_shadow_entry= NULL;
 }