Alter starter

4 files changed, 212 insertions, 78 deletions

diff --git a/sql/datadict.cc b/sql/datadict.cc
index 5ea4dacd9cb..34aec2ac356 100644
--- a/sql/datadict.cc
+++ b/sql/datadict.cc
@@ -651,6 +651,8 @@ bool FK_ddl_vector::install_shadow_frms(THD *thd)
 #endif
   for (FK_ddl_backup &bak: *this)
   {
+    if (!bak.update_frm)
+      continue;
     if (bak.fk_backup_frm(*this))
       goto error;
 #ifndef DBUG_OFF
@@ -662,6 +664,8 @@ bool FK_ddl_vector::install_shadow_frms(THD *thd)
 #endif
   for (FK_ddl_backup &bak: *this)
   {
+    if (!bak.update_frm)
+      continue;
     if (bak.fk_install_shadow_frm(*this))
       goto error;
 #ifndef DBUG_OFF
@@ -682,7 +686,7 @@ void FK_ddl_vector::rollback(THD *thd)
   for (FK_ddl_backup &bak: *this)
     bak.rollback(*this);
 
-  // NB: we might not fk_write_shadow_frm() at all in case rename table failed
+  // NB: we might not fk_write_shadow_frm() at all f.ex. when rename table failed
   if (!first_entry)
     return;
 
@@ -707,6 +711,8 @@ void FK_ddl_vector::drop_backup_frms(THD *thd)
 #endif
   for (FK_ddl_backup &bak: *this)
   {
+    if (!bak.update_frm)
+      continue;
     if (deactivate_ddl_log_entry(bak.restore_backup_entry->entry_pos))
     {
       // FIXME: test getting into here (and other deactivate_ddl_log_entry() failures)
@@ -723,10 +729,13 @@ void FK_ddl_vector::drop_backup_frms(THD *thd)
 #endif
   for (FK_ddl_backup &bak: *this)
   {
+    if (!bak.update_frm)
+      continue;
     bak.fk_drop_backup_frm(*this);
 #ifndef DBUG_OFF
     dbg_first= false;
 #endif
   }
-  write_log_finish();
+  if (first_entry)
+    write_log_finish();
 }
diff --git a/sql/sql_alter.h b/sql/sql_alter.h
index 08af28c9426..7ee27980244 100644
--- a/sql/sql_alter.h
+++ b/sql/sql_alter.h
@@ -57,15 +57,38 @@ public:
 };
 
 
-/* Backup for the table we refering or which referes us */
-class FK_ref_backup : public FK_table_backup
+class FK_ddl_backup2 : public FK_backup
 {
 public:
-  bool install_shadow;
-  FK_ref_backup() : install_shadow(false) {}
-  virtual ~FK_ref_backup()
+  TABLE_SHARE *share;
+
+  FK_ddl_backup2() : share(NULL) {}
+  virtual ~FK_ddl_backup2()
+  {
+    if (share)
+      rollback();
+  }
+  FK_ddl_backup2(Share_acquire&& _sa)
   {
-    commit();
+  }
+  bool init(TABLE_SHARE *)
+  {
+    return true;
+  }
+  void commit()
+  {
+    share= NULL;
+  }
+  void rollback()
+  {
+    DBUG_ASSERT(share);
+    share->foreign_keys= foreign_keys;
+    share->referenced_keys= referenced_keys;
+    share= NULL;
+  }
+  TABLE_SHARE *get_share() const
+  {
+    return share;
   }
 };
 
@@ -442,9 +465,9 @@ public:
   // Backup for the table we altering. NB: auto-rollback if not committed.
   FK_table_backup fk_table_backup;
   // NB: share is owned and released by fk_shares
-  mbd::map<TABLE_SHARE *, FK_ref_backup> fk_ref_backup;
+  mbd::map<TABLE_SHARE *, FK_share_backup> fk_ref_backup;
   // NB: backup is added only if not exists
-  FK_ref_backup* fk_add_backup(TABLE_SHARE *share);
+  FK_share_backup* fk_add_backup(TABLE_SHARE *share);
   void fk_rollback();
   bool fk_install_frms();
 
diff --git a/sql/sql_class.h b/sql/sql_class.h
index 89b98742604..4f77866d6a9 100644
--- a/sql/sql_class.h
+++ b/sql/sql_class.h
@@ -336,6 +336,27 @@ public:
     }
     return false;
   }
+  template <class... Args>
+  typename Base::iterator emplace(bool &inserted, Args&&... args) noexcept
+  {
+    try
+    {
+      auto ret= Base::emplace(args...);
+      inserted= ret.second;
+      return ret.first;
+    }
+    catch (std::bad_alloc())
+    {
+      my_error(ER_OUT_OF_RESOURCES, MYF(0));
+      return Base::end();
+    }
+    catch (...)
+    {
+      my_error(ER_INTERNAL_ERROR, MYF(0), "Unexpected exception");
+      return Base::end();
+    }
+    return Base::end();
+  }
 };
 
 
@@ -389,6 +410,18 @@ public:
       return NULL;
     return &*ret;
   }
+  template <class... Args>
+  const Key* emplace(bool *inserted, Args&&... args)
+  {
+    bool ins;
+    auto ret= exception_wrapper<std::set<Key, Compare, Allocator> >::
+      emplace(ins, args...);
+    if (inserted)
+      *inserted= ins;
+    if (ret == std::set<Key, Compare, Allocator>::end())
+      return NULL;
+    return &*ret;
+  }
 };
 
 
@@ -418,6 +451,18 @@ public:
       *inserted= ins;
     return &ret->second;
   }
+  template <class... Args>
+  T* emplace(bool *inserted, Args&&... args)
+  {
+    bool ins;
+    auto ret= exception_wrapper<std::map<Key, T, Compare, Allocator> >::
+      emplace(ins, args...);
+    if (inserted)
+      *inserted= ins;
+    if (ret == std::map<Key, T, Compare, Allocator>::end())
+      return NULL;
+    return &ret->second;
+  }
 };
 } // namespace mbd
 
@@ -937,9 +982,12 @@ public:
   DDL_LOG_MEMORY_ENTRY *restore_backup_entry;
 
 public:
+  bool update_frm;
+
   FK_backup() :
     delete_shadow_entry(NULL),
-    restore_backup_entry(NULL)
+    restore_backup_entry(NULL),
+    update_frm(false)
   {}
   virtual ~FK_backup()
   {}
@@ -954,35 +1002,65 @@ public:
 };
 
 
-// NB: FK_ddl_backup responds for share release unlike FK_table_backup
-class FK_ddl_backup : public FK_backup
+class FK_share_backup : public FK_backup
 {
+protected:
+  TABLE_SHARE *share;
+
 public:
+  // FIXME: remove (used in ALTER TABLE)
+  FK_share_backup() : share(NULL) {}
+  bool init(TABLE_SHARE *_share);
+
+  FK_share_backup(TABLE_SHARE *_share)
+  {
+    if (init(_share))
+      share= NULL;
+  }
+//   virtual ~FK_share_backup()
+//   {
+//     commit();
+//   }
+  void commit()
+  {
+    share= NULL;
+  }
+  TABLE_SHARE *get_share() const
+  {
+    return share;
+  }
+  void rollback(ddl_log_info& log_info);
+};
+
+
+// NB: FK_ddl_backup responds for share release unlike FK_table_backup
+class FK_ddl_backup : public FK_share_backup
+{
+  /* NB: if sa.share is not empty, share is auto-released on destructor */
   Share_acquire sa;
+  /*
+     NB: if sa.share is not empty, share == sa.share. ALTER algorithms are more
+     complex and shares are held and released in separate container alter_ctx.fk_shares.
+     To make DDL logging common for all commands we handle it via FK_ddl_vector interface, but
+     without templating and virtual interfaces (these are overcomplexity for only 2 variations)
+     we have to converge backup operations into single FK_ddl_backup.
+  */
 
+public:
   FK_ddl_backup(Share_acquire&& _sa);
-  FK_ddl_backup(const FK_ddl_backup&)= delete;
+  FK_ddl_backup(const FK_ddl_backup&)= delete; // (explict reminder for default)
   FK_ddl_backup(FK_ddl_backup&& src) :
-    FK_backup(std::move(src)),
-    sa(std::move(src.sa))
-  {}
+    FK_share_backup(std::move(src)),
+    sa(std::move(src.sa)) {}
 
   FK_ddl_backup& operator=(FK_ddl_backup&& src)
   {
-    *((FK_backup *) this)= std::move(src);
+    *((FK_share_backup *) this)= std::move(src);
     sa= std::move(src.sa);
     return *this;
   }
 
-  void rollback(ddl_log_info& log_info);
   bool backup_frm(ddl_log_info &log_info, Table_name table);
-  TABLE_SHARE *get_share() const
-  {
-    return sa.share;
-  }
-
-protected:
-  FK_ddl_backup() {}
 };
 
 
diff --git a/sql/sql_table.cc b/sql/sql_table.cc
index 514f8b5172b..0a7879ada49 100644
--- a/sql/sql_table.cc
+++ b/sql/sql_table.cc
@@ -12497,13 +12497,13 @@ bool TABLE_SHARE::fk_handle_create(THD *thd, FK_ddl_vector &shares, FK_list *fk_
       return true;
     }
     DBUG_ASSERT(!ref_sa.share);
-    if (!shares.back().sa.share)
+    if (!shares.back().get_share())
       return true; // ctor failed, share was released
   }
 
   for (FK_ddl_backup &ref: shares)
   {
-    TABLE_SHARE *ref_share= ref.sa.share;
+    TABLE_SHARE *ref_share= ref.get_share();
     for (const FK_info &fk: fkeys)
     {
       // Find keys referencing the acquired share and add them to referenced_keys
@@ -12724,7 +12724,7 @@ bool Alter_table_ctx::fk_handle_alter(THD *thd)
     if (!fk_table.share)
       return true;
     TABLE_SHARE *fk_share= fk_table.share;
-    FK_ref_backup *ref_bak= fk_add_backup(fk_share);
+    FK_share_backup *ref_bak= fk_add_backup(fk_share);
     if (!ref_bak)
       return true;
     for (FK_info &fk: fk_share->foreign_keys)
@@ -12740,7 +12740,7 @@ bool Alter_table_ctx::fk_handle_alter(THD *thd)
           my_error(ER_OUT_OF_RESOURCES, MYF(0));
           return true;
         }
-        ref_bak->install_shadow= true;
+        ref_bak->update_frm= true;
       }
     }
   }
@@ -12765,10 +12765,10 @@ bool Alter_table_ctx::fk_handle_alter(THD *thd)
     if (!ref_table.share)
       return true;
     TABLE_SHARE *ref_share= ref_table.share;
-    FK_ref_backup *ref_bak= fk_add_backup(ref_share);
+    FK_share_backup *ref_bak= fk_add_backup(ref_share);
     if (!ref_bak)
       return true;
-    ref_bak->install_shadow= true;
+    ref_bak->update_frm= true;
     // Find prepared FK in fk_list. If ID exists, use it.
     FK_info *fk;
     List_iterator<FK_info> fk_it(new_foreign_keys);
@@ -12841,7 +12841,7 @@ bool Alter_table_ctx::fk_handle_alter(THD *thd)
     if (!ref_table.share)
       return true;
     TABLE_SHARE *ref_share= ref_table.share;
-    FK_ref_backup *ref_bak= fk_add_backup(ref_share);
+    FK_share_backup *ref_bak= fk_add_backup(ref_share);
     if (!ref_bak)
       return true;
     FK_info *rk;
@@ -12854,7 +12854,7 @@ bool Alter_table_ctx::fk_handle_alter(THD *thd)
       break;
     }
     if (rk)
-      ref_bak->install_shadow= true;
+      ref_bak->update_frm= true;
   }
 
   /* Handle table rename. FRM write is required. */
@@ -12871,7 +12871,7 @@ bool Alter_table_ctx::fk_handle_alter(THD *thd)
     if (!ref_table.share)
       return true;
     TABLE_SHARE *ref_share= ref_table.share;
-    FK_ref_backup *ref_bak= fk_add_backup(ref_share);
+    FK_share_backup *ref_bak= fk_add_backup(ref_share);
     if (!ref_bak)
       return true;
     // Update foreign_table of referenced_keys.
@@ -12894,7 +12894,7 @@ bool Alter_table_ctx::fk_handle_alter(THD *thd)
         }
       }
     }
-    ref_bak->install_shadow= true;
+    ref_bak->update_frm= true;
   } // for (const Table_name &ref: fk_renamed_table)
 
   for (const Table_name &ref: rk_renamed_table)
@@ -12910,7 +12910,7 @@ bool Alter_table_ctx::fk_handle_alter(THD *thd)
     if (!fk_table.share)
       return true;
     TABLE_SHARE *fk_share= fk_table.share;
-    FK_ref_backup *ref_bak= fk_add_backup(fk_share);
+    FK_share_backup *ref_bak= fk_add_backup(fk_share);
     if (!ref_bak)
       return true;
     // Update referenced_table of foreign_keys.
@@ -12933,14 +12933,14 @@ bool Alter_table_ctx::fk_handle_alter(THD *thd)
         }
       }
     }
-    ref_bak->install_shadow= true;
+    ref_bak->update_frm= true;
   } // for (const Table_name &ref: rk_renamed_table)
 
   /* Update EXTRA2_FOREIGN_KEY_INFO section in FRM files. */
   for (auto &key_val: fk_ref_backup)
   {
-    FK_ref_backup *ref_bak= const_cast<FK_ref_backup *>(&key_val.second);
-    if (ref_bak->install_shadow && ref_bak->fk_write_shadow_frm(fk_ddl_info))
+    FK_share_backup *ref_bak= const_cast<FK_share_backup *>(&key_val.second);
+    if (ref_bak->update_frm && ref_bak->fk_write_shadow_frm(fk_ddl_info))
       return true;
   }
 
@@ -12990,9 +12990,10 @@ bool Alter_table_ctx::fk_check_foreign_id(THD *thd)
 }
 
 
-FK_ref_backup* Alter_table_ctx::fk_add_backup(TABLE_SHARE *share)
+#if 0
+FK_share_backup* Alter_table_ctx::fk_add_backup(TABLE_SHARE *share)
 {
-  FK_ref_backup fk_bak;
+  FK_share_backup fk_bak;
   if (fk_bak.init(share))
     return NULL;
   auto found= fk_ref_backup.find(share);
@@ -13000,16 +13001,25 @@ FK_ref_backup* Alter_table_ctx::fk_add_backup(TABLE_SHARE *share)
     return &found->second;
   return fk_ref_backup.insert(share, fk_bak);
 }
+#endif
+
+
+FK_share_backup* Alter_table_ctx::fk_add_backup(TABLE_SHARE *share)
+{
+  bool inserted;
+  FK_share_backup *fk_bak= fk_ref_backup.emplace(&inserted, share, share);
+  return fk_bak;
+}
 
 
 void Alter_table_ctx::fk_rollback()
 {
   for (auto &key_val: fk_ref_backup)
   {
-    FK_ref_backup *ref_bak= const_cast<FK_ref_backup *>(&key_val.second);
-    if (ref_bak->install_shadow)
+    FK_share_backup *ref_bak= const_cast<FK_share_backup *>(&key_val.second);
+    if (ref_bak->update_frm)
       ref_bak->fk_drop_shadow_frm(fk_ddl_info);
-    ref_bak->rollback();
+    ref_bak->rollback(fk_ddl_info);
   }
 }
 
@@ -13032,31 +13042,31 @@ void Alter_table_ctx::fk_release_locks(THD* thd)
 bool Alter_table_ctx::fk_install_frms()
 {
   // FIXME: remake via FK_ddl_vector::install_shadow_frms()
-  // converge FK_ref_backup and FK_ddl_backup
+  // converge FK_share_backup and FK_ddl_backup
   for (auto &key_val: fk_ref_backup)
   {
-    FK_ref_backup *ref_bak= const_cast<FK_ref_backup *>(&key_val.second);
-    DBUG_ASSERT(ref_bak->share);
-    if (ref_bak->install_shadow && ref_bak->fk_backup_frm(fk_ddl_info))
+    FK_share_backup *ref_bak= const_cast<FK_share_backup *>(&key_val.second);
+    DBUG_ASSERT(ref_bak->get_share());
+    if (ref_bak->update_frm && ref_bak->fk_backup_frm(fk_ddl_info))
       return true;
   }
   for (auto &key_val: fk_ref_backup)
   {
-    FK_ref_backup *ref_bak= const_cast<FK_ref_backup *>(&key_val.second);
-    if (ref_bak->install_shadow && ref_bak->fk_install_shadow_frm(fk_ddl_info))
+    FK_share_backup *ref_bak= const_cast<FK_share_backup *>(&key_val.second);
+    if (ref_bak->update_frm && ref_bak->fk_install_shadow_frm(fk_ddl_info))
       // FIXME: test rollback
       return true;
   }
   for (auto &key_val: fk_ref_backup)
   {
-    FK_ref_backup *ref_bak= const_cast<FK_ref_backup *>(&key_val.second);
-    if (ref_bak->install_shadow && deactivate_ddl_log_entry(ref_bak->restore_backup_entry->entry_pos))
+    FK_share_backup *ref_bak= const_cast<FK_share_backup *>(&key_val.second);
+    if (ref_bak->update_frm && deactivate_ddl_log_entry(ref_bak->restore_backup_entry->entry_pos))
       return true;
   }
   for (auto &key_val: fk_ref_backup)
   {
-    FK_ref_backup *ref_bak= const_cast<FK_ref_backup *>(&key_val.second);
-    if (ref_bak->install_shadow)
+    FK_share_backup *ref_bak= const_cast<FK_share_backup *>(&key_val.second);
+    if (ref_bak->update_frm)
       ref_bak->fk_drop_backup_frm(fk_ddl_info);
   }
   fk_ddl_info.write_log_finish();
@@ -13145,7 +13155,7 @@ bool fk_handle_drop(THD *thd, TABLE_LIST *table, FK_ddl_vector &shares,
       return true;
     }
     DBUG_ASSERT(!ref_sa.share);
-    if (!shares.back().sa.share)
+    if (!shares.back().get_share())
       return true; // ctor failed, share was released
   }
 
@@ -13154,7 +13164,7 @@ bool fk_handle_drop(THD *thd, TABLE_LIST *table, FK_ddl_vector &shares,
   // NB: another loop separates share acquisition which may fail
   for (auto ref= std::begin(shares); ref != std::end(shares); )
   {
-    ref_it.init(ref->sa.share->referenced_keys);
+    ref_it.init(ref->get_share()->referenced_keys);
     while (FK_info *rk= ref_it++)
     {
       if (0 == cmp_table(rk->foreign_db, share->db) &&
@@ -13212,7 +13222,7 @@ bool fk_handle_rename(THD *thd, TABLE_LIST *old_table, const LEX_CSTRING *new_db
   for (FK_ddl_backup &bak: fk_rename_backup)
   {
     // NB: exception_wrapper prints error message
-    if (!already.insert(Table_name(bak.sa.share->db, bak.sa.share->table_name)))
+    if (!already.insert(Table_name(bak.get_share()->db, bak.get_share()->table_name)))
       return true;
   }
   // NB: we do not allow same share twice in fk_rename_backup
@@ -13303,13 +13313,13 @@ bool fk_handle_rename(THD *thd, TABLE_LIST *old_table, const LEX_CSTRING *new_db
     if (fk_rename_backup.push_back(std::move(ref_sa)))
       return true;
     DBUG_ASSERT(!ref_sa.share);
-    if (!fk_rename_backup.back().sa.share)
+    if (!fk_rename_backup.back().get_share())
       return true; // ctor failed, share was released
   }
 
   for (FK_ddl_backup &ref: fk_rename_backup)
   {
-    TABLE_SHARE *ref_share= ref.sa.share;
+    TABLE_SHARE *ref_share= ref.get_share();
     if (!ref_share)
       continue; // renamed table backup
     for (FK_info &fk: ref_share->foreign_keys)
@@ -13345,39 +13355,40 @@ mem_error:
 FK_ddl_backup::FK_ddl_backup(Share_acquire&& _sa) :
   sa(std::move(_sa))
 {
-  if (foreign_keys.copy(&sa.share->foreign_keys, &sa.share->mem_root) ||
-      list_copy_and_replace_each_value(foreign_keys, &sa.share->mem_root))
-  {
-    my_error(ER_OUT_OF_RESOURCES, MYF(0));
+  if (init(sa.share))
     sa.release();
-    return;
-  }
-  if (referenced_keys.copy(&sa.share->referenced_keys, &sa.share->mem_root) ||
-      list_copy_and_replace_each_value(referenced_keys, &sa.share->mem_root))
+  else
   {
-    my_error(ER_OUT_OF_RESOURCES, MYF(0));
-    sa.release();
-    return;
+    share= sa.share;
+    update_frm= true;
   }
 }
 
 
-void
-FK_ddl_backup::rollback(ddl_log_info& log_info)
+bool
+FK_share_backup::init(TABLE_SHARE *_share)
 {
-  if (sa.share)
+  if (foreign_keys.copy(&_share->foreign_keys, &_share->mem_root) ||
+      list_copy_and_replace_each_value(foreign_keys, &_share->mem_root))
   {
-    sa.share->foreign_keys= foreign_keys;
-    sa.share->referenced_keys= referenced_keys;
-    delete_shadow_entry= NULL;
+    my_error(ER_OUT_OF_RESOURCES, MYF(0));
+    return true;
   }
+  if (referenced_keys.copy(&_share->referenced_keys, &_share->mem_root) ||
+      list_copy_and_replace_each_value(referenced_keys, &_share->mem_root))
+  {
+    my_error(ER_OUT_OF_RESOURCES, MYF(0));
+    return true;
+  }
+  share= _share;
+  return false;
 }
 
 
+// FIXME: remove FK_table_backup
 bool
 FK_table_backup::init(TABLE_SHARE *_share)
 {
-  DBUG_ASSERT(_share);
   if (foreign_keys.copy(&_share->foreign_keys, &_share->mem_root) ||
       list_copy_and_replace_each_value(foreign_keys, &_share->mem_root))
   {
@@ -13393,3 +13404,16 @@ FK_table_backup::init(TABLE_SHARE *_share)
   share= _share;
   return false;
 }
+
+
+void
+FK_share_backup::rollback(ddl_log_info& log_info)
+{
+  if (share)
+  {
+    share->foreign_keys= foreign_keys;
+    share->referenced_keys= referenced_keys;
+    share= NULL;
+  }
+  delete_shadow_entry= NULL;
+}