WL#15129 - Enable DDL Statements in HeatWave

Enh#34350907 - [Nvidia] Allow DDLs when tables are loaded to HeatWave
Bug#34433145 - WL#15129: mysqld crash Assertion `column_count == static_cast<int64_t>(cp_table-
Bug#34446287 - WL#15129: mysqld crash at rapid::data::RapidNetChunkCtx::consolidateEncodingsDic
Bug#34520634 - MYSQLD CRASH : Sql_cmd_secondary_load_unload::mysql_secondary_load_or_unload
Bug#34520630 - Failed Condition: "table_id != InvalidTableId"

Currently, DDL statements such as ALTER TABLE*, RENAME TABLE, and
TRUNCATE TABLE are not allowed if a table has a secondary engine
defined. The statements fail with the following error: "DDLs on a table
with a secondary engine defined are not allowed."

This worklog lifts this restriction for tables whose secondary engine is
RAPID.

A secondary engine hook is called in the beginning (pre-hook) and in the
end (post-hook) of a DDL statement execution. If the DDL statement
succeeds, the post-hook will direct the recovery framework to reload the
table in order to reflect that change in HeatWave.

Currently all DDL statements that were previously disallowed will
trigger a reload. This can be improved in the future by checking whether
the DDL operation has an impact on HeatWave or not. However detecting
all edge-cases in this behavior is not straightforward so this
improvement has been left as a future improvement.

Additionally, if a DDL modifies the table schema in a way that makes it
incompatible with HeatWave (e.g., dropping a primary key column) the
reload will fail silently. There is no easy way to recognize whether the
table schema will become incompatible with HeatWave in a pre-hook.

List of changes:
  1) [MySQL] Add new HTON_SECONDARY_ENGINE_SUPPORTS_DDL flag to indicate
whether a secondary engine supports DDLs.
  2) [MySQL] Add RAII hooks for RENAME TABLE and TRUNCATE TABLE, modeled
on the ALTER TABLE hook.
  3) Define HeatWave hooks for ALTER TABLE, RENAME TABLE, and TRUNCATE
TABLE statements.
  4) If a table reload is necessary, trigger it by marking the table as
stale (WL#14914).
  4) Move all change propagation & DDL hooks to ha_rpd_hooks.cc.
  5) Adjust existing tests to support table reload upon DDL execution.
  6) Extract code related to RapidOpSyncCtx in ha_rpd_sync_ctx.cc, and
the PluginState enum to ha_rpd_fsm.h.

* Note: ALTER TABLE statements related to secondary engine setting and
loading were allowed before:
    - ALTER TABLE <TABLE> SECONDARY_UNLOAD,
    - ALTER TABLE SECONDARY_ENGINE = NULL.

-- Bug#34433145 --
-- Bug#34446287 --

--Problem #1--
Crashes in Change Propagation when the CP thread tries to apply DMLs of
tables with new schema to the not-yet-reloaded table in HeatWave.

--Solution #1--
Remove table from Change Propagation before marking it as stale and
revert the original change from rpd_binlog_parser.cc where we were
checking if the table was stale before continuing with binlog parsing.
The original change is no longer necessary since the table is removed
from CP before being marked as stale.

--Problem #2--
In case of a failed reload, tables are not removed from Global State.

--Solution #2--
Keep track of whether the table was reloaded because it was marked as
STALE. In that case we do not want the Recovery Framework to retry the
reload and therefore we can remove the table from the Global State.

-- Bug#34520634 --

Problem:
Allowing the change of primary engine for tables with a defined
secondary engine hits an assertion in mysql_secondary_load_or_unload().

Example:
    CREATE TABLE t1 (col1 INT PRIMARY KEY) SECONDARY_ENGINE = RAPID;
    ALTER TABLE t1 ENGINE = BLACKHOLE;
    ALTER TABLE t1 SECONDARY_LOAD; <- assertion hit here

Solution:
Disallow changing the primary engine for tables with a defined secondary
engine.

-- Bug#34520630 --

Problem:
A debug assert is being hit in rapid_gs_is_table_reloading_from_stale
because the table was dropped in the meantime.

Solution:
Instead of asserting, just return false if table is not present in the
Global State.

This patch also changes rapid_gs_is_table_reloading_from_stale to a more
specific check (inlined the logic in load_table()). This check now also
covers the case when a table was dropped/unloaded before the Recovery
Framework marked it as INRECOVERY. In that case, if the reload fails we
should not have an entry for that table in the Global State.

The patch also adjusts dict_types MTR test, where we no longer expect
for tables to be in UNAVAIL state after a failed reload. Additionally,
recovery2_ddls.test is adjusted to not try to offload queries running on
Performance Schema.

Change-Id: I6ee390b1f418120925f5359d5e9365f0a6a415ee

sql/handler.cc

@@ -434,6 +434,22 @@ plugin_ref ha_resolve_by_name(THD *thd, const LEX_CSTRING *name,
   return nullptr;
 }
 
+bool ha_secondary_engine_supports_ddl(
+    THD *thd, const LEX_CSTRING &secondary_engine) noexcept {
+  bool ret = false;
+  auto *plugin = ha_resolve_by_name_raw(thd, secondary_engine);
+
+  if (plugin != nullptr) {
+    const auto *se_hton = plugin_data<const handlerton *>(plugin);
+    if (se_hton != nullptr) {
+      ret = secondary_engine_supports_ddl(se_hton);
+    }
+
+    plugin_unlock(thd, plugin);
+  }
+  return ret;
+}
+
 /**
   Read a comma-separated list of storage engine names. Look up each in the
   known list of canonical and legacy names. In case of a match; add both the
@@ -8390,16 +8406,32 @@ Temp_table_handle::~Temp_table_handle() {
 */
 
 struct HTON_NOTIFY_PARAMS {
-  HTON_NOTIFY_PARAMS(const MDL_key *mdl_key, ha_notification_type mdl_type)
+  HTON_NOTIFY_PARAMS(const MDL_key *mdl_key, ha_notification_type mdl_type,
+                     ha_ddl_type ddl_type = HA_INVALID_DDL,
+                     const char *old_db_name = nullptr,
+                     const char *old_table_name = nullptr,
+                     const char *new_db_name = nullptr,
+                     const char *new_table_name = nullptr)
       : key(mdl_key),
         notification_type(mdl_type),
+        ddl_type{ddl_type},
         some_htons_were_notified(false),
-        victimized(false) {}
+        victimized(false),
+        m_old_db_name(old_db_name),
+        m_old_table_name(old_table_name),
+        m_new_db_name(new_db_name),
+        m_new_table_name(new_table_name) {}
 
   const MDL_key *key;
   const ha_notification_type notification_type;
+  const ha_ddl_type ddl_type;
   bool some_htons_were_notified;
   bool victimized;
+  /* Only used in RENAME TABLE */
+  const char *m_old_db_name;
+  const char *m_old_table_name;
+  const char *m_new_db_name;
+  const char *m_new_table_name;
 };
 
 static bool notify_exclusive_mdl_helper(THD *thd, plugin_ref plugin,
@@ -8463,12 +8495,51 @@ bool ha_notify_exclusive_mdl(THD *thd, const MDL_key *mdl_key,
   return false;
 }
 
-static bool notify_alter_table_helper(THD *thd, plugin_ref plugin, void *arg) {
+static bool notify_table_ddl_helper(THD *thd, plugin_ref plugin, void *arg) {
   handlerton *hton = plugin_data<handlerton *>(plugin);
-  if (hton->state == SHOW_OPTION_YES && hton->notify_alter_table) {
+  if (hton->state == SHOW_OPTION_YES &&
+      (hton->notify_alter_table || hton->notify_rename_table ||
+       hton->notify_truncate_table)) {
     HTON_NOTIFY_PARAMS *params = reinterpret_cast<HTON_NOTIFY_PARAMS *>(arg);
 
-    if (hton->notify_alter_table(thd, params->key, params->notification_type)) {
+    bool notify_ret{false};
+
+    /* If the DDL is ALTER or TRUNCATE, it shouldn't have the names set. */
+    assert(((params->ddl_type == HA_ALTER_DDL ||
+             params->ddl_type == HA_TRUNCATE_DDL) &&
+            (params->m_old_db_name == nullptr &&
+             params->m_old_table_name == nullptr &&
+             params->m_new_db_name == nullptr &&
+             params->m_new_table_name == nullptr)) ||
+           (params->ddl_type == HA_RENAME_DDL));
+
+    switch (params->ddl_type) {
+      case HA_ALTER_DDL:
+        if (hton->notify_alter_table) {
+          notify_ret = hton->notify_alter_table(thd, params->key,
+                                                params->notification_type);
+        }
+        break;
+      case HA_TRUNCATE_DDL:
+        if (hton->notify_truncate_table) {
+          notify_ret = hton->notify_truncate_table(thd, params->key,
+                                                   params->notification_type);
+        }
+        break;
+      case HA_RENAME_DDL:
+        if (hton->notify_rename_table) {
+          notify_ret = hton->notify_rename_table(
+              thd, params->key, params->notification_type,
+              params->m_old_db_name, params->m_old_table_name,
+              params->m_new_db_name, params->m_new_table_name);
+        }
+        break;
+      default:
+        assert(0);
+        return true;
+    }
+
+    if (notify_ret) {
       // Ignore failures from post event notification.
       if (params->notification_type == HA_NOTIFY_PRE_EVENT) return true;
     } else
@@ -8478,38 +8549,41 @@ static bool notify_alter_table_helper(THD *thd, plugin_ref plugin, void *arg) {
 }
 
 /**
-  Notify/get permission from all interested storage engines before
-  or after executed ALTER TABLE on the table identified by key.
+  Notify/get permission from all interested storage engines before or after
+  executed DDL (ALTER TABLE, RENAME TABLE, TRUNCATE TABLE) on the table
+  identified by key.
 
   @param thd                Thread context.
   @param mdl_key            MDL key identifying table.
-  @param notification_type  Indicates whether this is pre-ALTER or
-                            post-ALTER notification.
-
-  See @sa handlerton::notify_alter_table for rationale,
-  details about calling convention and error reporting.
-
-  @return False - if notification was successful/ALTER TABLE can
-                  proceed.
-          True -  if it has failed/ALTER TABLE should fail.
-*/
-
-bool ha_notify_alter_table(THD *thd, const MDL_key *mdl_key,
-                           ha_notification_type notification_type) {
-  HTON_NOTIFY_PARAMS params(mdl_key, notification_type);
-
-  if (plugin_foreach(thd, notify_alter_table_helper,
-                     MYSQL_STORAGE_ENGINE_PLUGIN, &params)) {
-    /*
-      If some SE hasn't given its permission to do ALTER TABLE and some SEs
-      has given their permissions, we need to notify the latter group about
-      failed attemopt. We do this by calling post-ALTER TABLE notification
-      for all interested SEs unconditionally.
-    */
+  @param notification_type  Indicates whether this is pre-DDL or post-DDL
+  notification.
+  @param old_db_name        Old db name, used in RENAME DDL
+  @param old_table_name     Old table name, used in RENAME DDL
+  @param new_db_name        New db name, used in RENAME DDL
+  @param new_table_name     New table name, used in RENAME DDL
+
+  See @sa handlerton::notify_alter_table for rationale, details about calling
+  convention and error reporting.
+
+  @return False - if notification was successful/DDL can proceed.
+          True -  if it has failed/DDL should fail.
+*/
+bool ha_notify_table_ddl(THD *thd, const MDL_key *mdl_key,
+                         ha_notification_type notification_type,
+                         ha_ddl_type ddl_type, const char *old_db_name,
+                         const char *old_table_name, const char *new_db_name,
+                         const char *new_table_name) {
+  HTON_NOTIFY_PARAMS params(mdl_key, notification_type, ddl_type, old_db_name,
+                            old_table_name, new_db_name, new_table_name);
+
+  if (plugin_foreach(thd, notify_table_ddl_helper, MYSQL_STORAGE_ENGINE_PLUGIN,
+                     &params)) {
     if (notification_type == HA_NOTIFY_PRE_EVENT &&
         params.some_htons_were_notified) {
-      HTON_NOTIFY_PARAMS rollback_params(mdl_key, HA_NOTIFY_POST_EVENT);
-      (void)plugin_foreach(thd, notify_alter_table_helper,
+      HTON_NOTIFY_PARAMS rollback_params(mdl_key, HA_NOTIFY_POST_EVENT,
+                                         ddl_type, old_db_name, old_table_name,
+                                         new_db_name, new_table_name);
+      (void)plugin_foreach(thd, notify_table_ddl_helper,
                            MYSQL_STORAGE_ENGINE_PLUGIN, &rollback_params);
     }
     return true;

sql/handler.h

@@ -929,6 +929,12 @@ enum enum_schema_tables : int {
 
 enum ha_stat_type { HA_ENGINE_STATUS, HA_ENGINE_LOGS, HA_ENGINE_MUTEX };
 enum ha_notification_type : int { HA_NOTIFY_PRE_EVENT, HA_NOTIFY_POST_EVENT };
+enum ha_ddl_type : int {
+  HA_INVALID_DDL,
+  HA_ALTER_DDL,
+  HA_TRUNCATE_DDL,
+  HA_RENAME_DDL
+};
 
 /** Clone start operation mode */
 enum Ha_clone_mode {
@@ -2028,6 +2034,40 @@ typedef bool (*notify_exclusive_mdl_t)(THD *thd, const MDL_key *mdl_key,
 typedef bool (*notify_alter_table_t)(THD *thd, const MDL_key *mdl_key,
                                      ha_notification_type notification_type);
 
+/**
+  Notify/get permission from storage engine before or after execution of
+  RENAME TABLE operation on the table identified by the MDL key.
+
+  @param thd                Thread context.
+  @param mdl_key            MDL key identifying table which is going to be
+                            or was RENAMEd.
+  @param notification_type  Indicates whether this is pre-RENAME TABLE or
+                            post-RENAME TABLE notification.
+  @param old_db_name
+  @param old_table_name
+  @param new_db_name
+  @param new_table_name
+*/
+typedef bool (*notify_rename_table_t)(THD *thd, const MDL_key *mdl_key,
+                                      ha_notification_type notification_type,
+                                      const char *old_db_name,
+                                      const char *old_table_name,
+                                      const char *new_db_name,
+                                      const char *new_table_name);
+
+/**
+  Notify/get permission from storage engine before or after execution of
+  TRUNCATE TABLE operation on the table identified by the MDL key.
+
+  @param thd                Thread context.
+  @param mdl_key            MDL key identifying table which is going to be
+                            or was TRUNCATEd.
+  @param notification_type  Indicates whether this is pre-TRUNCATE TABLE or
+                            post-TRUNCATE TABLE notification.
+*/
+typedef bool (*notify_truncate_table_t)(THD *thd, const MDL_key *mdl_key,
+                                        ha_notification_type notification_type);
+
 /**
   @brief
   Initiate master key rotation
@@ -2685,6 +2725,8 @@ struct handlerton {
   replace_native_transaction_in_thd_t replace_native_transaction_in_thd;
   notify_exclusive_mdl_t notify_exclusive_mdl;
   notify_alter_table_t notify_alter_table;
+  notify_rename_table_t notify_rename_table;
+  notify_truncate_table_t notify_truncate_table;
   rotate_encryption_master_key_t rotate_encryption_master_key;
   redo_log_set_state_t redo_log_set_state;
 
@@ -2844,6 +2886,16 @@ constexpr const decltype(handlerton::flags) HTON_SUPPORTS_ENGINE_ATTRIBUTE{
 constexpr const decltype(
     handlerton::flags) HTON_SUPPORTS_GENERATED_INVISIBLE_PK{1 << 18};
 
+/** Whether the secondary engine supports DDLs. No meaning if the engine is not
+ * secondary. */
+#define HTON_SECONDARY_ENGINE_SUPPORTS_DDL (1 << 19)
+
+inline bool secondary_engine_supports_ddl(const handlerton *hton) {
+  assert(hton->flags & HTON_IS_SECONDARY_ENGINE);
+
+  return (hton->flags & HTON_SECONDARY_ENGINE_SUPPORTS_DDL) != 0;
+}
+
 inline bool ddl_is_atomic(const handlerton *hton) {
   return (hton->flags & HTON_SUPPORTS_ATOMIC_DDL) != 0;
 }
@@ -7029,6 +7081,9 @@ handler *get_new_handler(TABLE_SHARE *share, bool partitioned, MEM_ROOT *alloc,
 handlerton *ha_checktype(THD *thd, enum legacy_db_type database_type,
                          bool no_substitute, bool report_error);
 
+bool ha_secondary_engine_supports_ddl(
+    THD *thd, const LEX_CSTRING &secondary_engine) noexcept;
+
 /**
   Get default handlerton, if handler supplied is null.
 
@@ -7268,8 +7323,11 @@ bool ha_is_storage_engine_disabled(handlerton *se_engine);
 bool ha_notify_exclusive_mdl(THD *thd, const MDL_key *mdl_key,
                              ha_notification_type notification_type,
                              bool *victimized);
-bool ha_notify_alter_table(THD *thd, const MDL_key *mdl_key,
-                           ha_notification_type notification_type);
+bool ha_notify_table_ddl(THD *thd, const MDL_key *mdl_key,
+                         ha_notification_type notification_type,
+                         ha_ddl_type ddl_type, const char *old_db_name,
+                         const char *old_table_name, const char *new_db_name,
+                         const char *new_table_name);
 
 std::pair<int, bool> commit_owned_gtids(THD *thd, bool all);
 bool set_tx_isolation(THD *thd, enum_tx_isolation tx_isolation, bool one_shot);

sql/sql_partition_admin.cc

@@ -656,12 +656,20 @@ bool Sql_cmd_alter_table_truncate_partition::execute(THD *thd) {
   /* Table was successfully opened above. */
   assert(table_def != nullptr);
 
+  Table_ddl_hton_notification_guard notification_guard{
+      thd, &first_table->mdl_request.key, ha_ddl_type::HA_TRUNCATE_DDL};
+
+  if (notification_guard.notify()) return true;
+
   if (table_def->options().exists("secondary_engine")) {
-    /* Truncate operation is not allowed for tables with secondary engine
-     * since it's not currently supported by change propagation
-     */
-    my_error(ER_SECONDARY_ENGINE_DDL, MYF(0));
-    return true;
+    LEX_CSTRING secondary_engine;
+    table_def->options().get("secondary_engine", &secondary_engine,
+                             thd->mem_root);
+
+    if (!ha_secondary_engine_supports_ddl(thd, secondary_engine)) {
+      my_error(ER_SECONDARY_ENGINE_DDL, MYF(0));
+      return true;
+    }
   }
 
   if (hton->partition_flags() & HA_TRUNCATE_PARTITION_PRECLOSE) {