Bug#28523841: IMPLEMENT SECONDARY_LOAD/SECONDARY_UNLOAD AS STANDALONE SQL COMMANDS

Moves handling of loading into and unloading from secondary engine out
of mysql_alter_table. A new SQL command class,
Sql_cmd_secondary_load_unload, has been added instead, to follow the
same approach taken for IMPORT/DISCARD TABLESPACE.

Change-Id: If2940635b7ce691da0832d685a9a100d86873b81

Author: Havard Dybvik

mysql-test/suite/secondary_engine/r/define.result

@@ -135,8 +135,8 @@ DROP TABLE t1;
 #
 CREATE TABLE t1 (i INT);
 ALTER TABLE t1 SECONDARY_LOAD, ADD KEY idx(i);
-ERROR HY000: Secondary engine operation failed. No secondary engine defined.
+ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near ', ADD KEY idx(i)' at line 1
 ALTER TABLE t1 SECONDARY_ENGINE NONEXISTENT;
 ALTER TABLE t1 SECONDARY_LOAD, ADD KEY idx(i);
-ERROR HY000: DDLs on a table with a secondary engine defined are not allowed.
+ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near ', ADD KEY idx(i)' at line 1
 DROP TABLE t1;

mysql-test/suite/secondary_engine/r/load.result

@@ -36,17 +36,21 @@ DROP TABLE t1;
 #
 CREATE TABLE t1 (a INT) SECONDARY_ENGINE MOCK;
 ALTER TABLE t1 SECONDARY_LOAD, FORCE;
-ERROR HY000: DDLs on a table with a secondary engine defined are not allowed.
+ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near ', FORCE' at line 1
 DROP TABLE t1;
 #
 # Perform alterations related to secondary engine using differing
 # algorithms.
 #
 CREATE TABLE t1 (a INT) SECONDARY_ENGINE MOCK;
 ALTER TABLE t1 SECONDARY_LOAD, ALGORITHM=INPLACE;
+ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near ', ALGORITHM=INPLACE' at line 1
 ALTER TABLE t1 SECONDARY_UNLOAD, ALGORITHM=INPLACE;
+ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near ', ALGORITHM=INPLACE' at line 1
 ALTER TABLE t1 SECONDARY_LOAD, ALGORITHM=COPY;
+ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near ', ALGORITHM=COPY' at line 1
 ALTER TABLE t1 SECONDARY_UNLOAD, ALGORITHM=COPY;
+ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near ', ALGORITHM=COPY' at line 1
 ALTER TABLE t1 SECONDARY_ENGINE NULL, ALGORITHM=INPLACE;
 ALTER TABLE t1 SECONDARY_ENGINE MOCK, ALGORITHM=INPLACE;
 ALTER TABLE t1 SECONDARY_ENGINE NULL, ALGORITHM=COPY;
@@ -63,3 +67,23 @@ DROP TABLE t1;
 #
 CREATE TABLE t1 (a INT) SECONDARY_ENGINE MOCK;
 DROP TABLE t1;
+#
+# Attempt to LOCK TABLE before loading a table into secondary engine.
+#
+CREATE TABLE t1 (a INT) SECONDARY_ENGINE MOCK;
+LOCK TABLES t1 WRITE;
+ALTER TABLE t1 SECONDARY_LOAD;
+ALTER TABLE t1 SECONDARY_UNLOAD;
+UNLOCK TABLES;
+LOCK TABLES t1 READ;
+ALTER TABLE t1 SECONDARY_LOAD;
+ERROR HY000: Table 't1' was locked with a READ lock and can't be updated
+ALTER TABLE t1 SECONDARY_UNLOAD;
+ERROR HY000: Table 't1' was locked with a READ lock and can't be updated
+LOCK TABLES t1 WRITE;
+ALTER TABLE t1 SECONDARY_LOAD;
+ALTER TABLE t1 SECONDARY_UNLOAD;
+UNLOCK TABLES;
+ALTER TABLE t1 SECONDARY_LOAD;
+ALTER TABLE t1 SECONDARY_UNLOAD;
+DROP TABLE t1;

mysql-test/suite/secondary_engine/t/define.test

@@ -124,9 +124,9 @@ DROP TABLE t1;
 --echo # Attempt to use SECONDARY_LOAD in conjunction with another DDL.
 --echo #
 CREATE TABLE t1 (i INT);
---error ER_SECONDARY_ENGINE
+--error ER_PARSE_ERROR
 ALTER TABLE t1 SECONDARY_LOAD, ADD KEY idx(i);
 ALTER TABLE t1 SECONDARY_ENGINE NONEXISTENT;
---error ER_SECONDARY_ENGINE_DDL
+--error ER_PARSE_ERROR
 ALTER TABLE t1 SECONDARY_LOAD, ADD KEY idx(i);
 DROP TABLE t1;

mysql-test/suite/secondary_engine/t/load.test

@@ -38,7 +38,7 @@ DROP TABLE t1;
 --echo # operations.
 --echo #
 CREATE TABLE t1 (a INT) SECONDARY_ENGINE MOCK;
---error ER_SECONDARY_ENGINE_DDL
+--error ER_PARSE_ERROR
 ALTER TABLE t1 SECONDARY_LOAD, FORCE;
 DROP TABLE t1;
 
@@ -47,9 +47,13 @@ DROP TABLE t1;
 --echo # algorithms.
 --echo #
 CREATE TABLE t1 (a INT) SECONDARY_ENGINE MOCK;
+--error ER_PARSE_ERROR
 ALTER TABLE t1 SECONDARY_LOAD, ALGORITHM=INPLACE;
+--error ER_PARSE_ERROR
 ALTER TABLE t1 SECONDARY_UNLOAD, ALGORITHM=INPLACE;
+--error ER_PARSE_ERROR
 ALTER TABLE t1 SECONDARY_LOAD, ALGORITHM=COPY;
+--error ER_PARSE_ERROR
 ALTER TABLE t1 SECONDARY_UNLOAD, ALGORITHM=COPY;
 ALTER TABLE t1 SECONDARY_ENGINE NULL, ALGORITHM=INPLACE;
 ALTER TABLE t1 SECONDARY_ENGINE MOCK, ALGORITHM=INPLACE;
@@ -86,6 +90,27 @@ connection default;
 --enable_result_log
 DROP TABLE t1;
 
+--echo #
+--echo # Attempt to LOCK TABLE before loading a table into secondary engine.
+--echo #
+CREATE TABLE t1 (a INT) SECONDARY_ENGINE MOCK;
+LOCK TABLES t1 WRITE;
+ALTER TABLE t1 SECONDARY_LOAD;
+ALTER TABLE t1 SECONDARY_UNLOAD;
+UNLOCK TABLES;
+LOCK TABLES t1 READ;
+--error ER_TABLE_NOT_LOCKED_FOR_WRITE
+ALTER TABLE t1 SECONDARY_LOAD;
+--error ER_TABLE_NOT_LOCKED_FOR_WRITE
+ALTER TABLE t1 SECONDARY_UNLOAD;
+LOCK TABLES t1 WRITE;
+ALTER TABLE t1 SECONDARY_LOAD;
+ALTER TABLE t1 SECONDARY_UNLOAD;
+UNLOCK TABLES;
+ALTER TABLE t1 SECONDARY_LOAD;
+ALTER TABLE t1 SECONDARY_UNLOAD;
+DROP TABLE t1;
+
 --disable_query_log
 UNINSTALL PLUGIN mock;
 --enable_query_log

sql/handler.cc

@@ -4732,8 +4732,7 @@ enum_alter_inplace_result handler::check_if_supported_inplace_alter(
       Alter_inplace_info::ALTER_RENAME | Alter_inplace_info::RENAME_INDEX |
       Alter_inplace_info::ALTER_INDEX_COMMENT |
       Alter_inplace_info::CHANGE_INDEX_OPTION |
-      Alter_inplace_info::ALTER_COLUMN_INDEX_LENGTH |
-      Alter_inplace_info::SECONDARY_LOAD | Alter_inplace_info::SECONDARY_UNLOAD;
+      Alter_inplace_info::ALTER_COLUMN_INDEX_LENGTH;
 
   /* Is there at least one operation that requires copy algorithm? */
   if (ha_alter_info->handler_flags & ~inplace_offline_operations)
@@ -8624,7 +8623,7 @@ void ha_post_recover(void) {
 }
 
 void handler::ha_set_primary_handler(handler *primary_handler) {
-  DBUG_ASSERT((ht->flags & HTON_SUPPORTS_SECONDARY) != 0);
+  DBUG_ASSERT((ht->flags & HTON_IS_SECONDARY_ENGINE) != 0);
   DBUG_ASSERT(primary_handler->table->s->has_secondary());
   m_primary_handler = primary_handler;
 }

sql/handler.h

@@ -2159,8 +2159,11 @@ struct handlerton {
 /* Engine supports packed keys. */
 #define HTON_SUPPORTS_PACKED_KEYS (1 << 13)
 
-/** Engine supports being set as secondary storage engine. */
-#define HTON_SUPPORTS_SECONDARY (1 << 14)
+/** Engine is a secondary storage engine. */
+#define HTON_IS_SECONDARY_ENGINE (1 << 14)
+
+/** Engine supports secondary storage engines. */
+#define HTON_SUPPORTS_SECONDARY_ENGINE (1 << 15)
 
 inline bool ddl_is_atomic(const handlerton *hton) {
   return (hton->flags & HTON_SUPPORTS_ATOMIC_DDL) != 0;
@@ -2518,12 +2521,6 @@ class Alter_inplace_info {
   // Rebuild partition
   static const HA_ALTER_FLAGS ALTER_REBUILD_PARTITION = 1ULL << 42;
 
-  // Load into secondary engine.
-  static const HA_ALTER_FLAGS SECONDARY_LOAD = 1ULL << 43;
-
-  // Unload from secondary engine.
-  static const HA_ALTER_FLAGS SECONDARY_UNLOAD = 1ULL << 44;
-
   /**
     Change in index length such that it does not require index rebuild.
     For example, change in index length due to column expansion like

sql/parse_tree_nodes.h

@@ -3986,21 +3986,6 @@ class PT_alter_table_remove_partitioning : public PT_alter_table_action {
       : super(Alter_info::ALTER_REMOVE_PARTITIONING) {}
 };
 
-class PT_alter_table_secondary_load : public PT_alter_table_action {
-  using super = PT_alter_table_action;
-
- public:
-  PT_alter_table_secondary_load() : super(Alter_info::ALTER_SECONDARY_LOAD) {}
-};
-
-class PT_alter_table_secondary_unload : public PT_alter_table_action {
-  using super = PT_alter_table_action;
-
- public:
-  PT_alter_table_secondary_unload()
-      : super(Alter_info::ALTER_SECONDARY_UNLOAD) {}
-};
-
 class PT_alter_table_standalone_action : public PT_alter_table_action {
   typedef PT_alter_table_action super;
 
@@ -4403,6 +4388,32 @@ class PT_alter_table_exchange_partition final
   const Alter_info::enum_with_validation m_validation;
 };
 
+class PT_alter_table_secondary_load final
+    : public PT_alter_table_standalone_action {
+  using super = PT_alter_table_standalone_action;
+
+ public:
+  explicit PT_alter_table_secondary_load()
+      : super(Alter_info::ALTER_SECONDARY_LOAD) {}
+
+  Sql_cmd *make_cmd(Table_ddl_parse_context *pc) override {
+    return new (pc->mem_root) Sql_cmd_secondary_load_unload(pc->alter_info);
+  }
+};
+
+class PT_alter_table_secondary_unload final
+    : public PT_alter_table_standalone_action {
+  using super = PT_alter_table_standalone_action;
+
+ public:
+  explicit PT_alter_table_secondary_unload()
+      : super(Alter_info::ALTER_SECONDARY_UNLOAD) {}
+
+  Sql_cmd *make_cmd(Table_ddl_parse_context *pc) override {
+    return new (pc->mem_root) Sql_cmd_secondary_load_unload(pc->alter_info);
+  }
+};
+
 class PT_alter_table_discard_partition_tablespace final
     : public PT_alter_table_partition_list_or_all {
   typedef PT_alter_table_partition_list_or_all super;

sql/sql_alter.cc

@@ -402,3 +402,25 @@ bool Sql_cmd_discard_import_tablespace::execute(THD *thd) {
 
   return mysql_discard_or_import_tablespace(thd, table_list);
 }
+
+bool Sql_cmd_secondary_load_unload::execute(THD *thd) {
+  // One of the SECONDARY_LOAD/SECONDARY_UNLOAD flags must have been set.
+  DBUG_ASSERT(
+      ((m_alter_info->flags & Alter_info::ALTER_SECONDARY_LOAD) == 0) !=
+      ((m_alter_info->flags & Alter_info::ALTER_SECONDARY_UNLOAD) == 0));
+
+  // No other flags should've been set.
+  DBUG_ASSERT(!(m_alter_info->flags & ~(Alter_info::ALTER_SECONDARY_LOAD |
+                                        Alter_info::ALTER_SECONDARY_UNLOAD)));
+
+  TABLE_LIST *table_list = thd->lex->select_lex->get_table_list();
+
+  if (check_access(thd, ALTER_ACL, table_list->db, &table_list->grant.privilege,
+                   &table_list->grant.m_internal, 0, 0))
+    return true;
+
+  if (check_grant(thd, ALTER_ACL, table_list, false, UINT_MAX, false))
+    return true;
+
+  return mysql_secondary_load_or_unload(thd, table_list);
+}

sql/sql_alter.h

@@ -272,10 +272,10 @@ class Alter_info {
     /// Means that the visibility of an index is changed.
     ALTER_INDEX_VISIBILITY = 1UL << 29,
 
-    /// Set for SECONDARY LOAD
+    /// Set for SECONDARY_LOAD
     ALTER_SECONDARY_LOAD = 1UL << 30,
 
-    /// Set for SECONDARY UNLOAD
+    /// Set for SECONDARY_UNLOAD
     ALTER_SECONDARY_UNLOAD = 1UL << 31,
   };
 
@@ -555,4 +555,18 @@ class Sql_cmd_discard_import_tablespace : public Sql_cmd_common_alter_table {
   bool mysql_discard_or_import_tablespace(THD *thd, TABLE_LIST *table_list);
 };
 
+/**
+  Represents ALTER TABLE SECONDARY_LOAD/SECONDARY_UNLOAD statements.
+*/
+class Sql_cmd_secondary_load_unload final : public Sql_cmd_common_alter_table {
+ public:
+  // Inherit the constructors from the parent class.
+  using Sql_cmd_common_alter_table::Sql_cmd_common_alter_table;
+
+  bool execute(THD *thd) override;
+
+ private:
+  bool mysql_secondary_load_or_unload(THD *thd, TABLE_LIST *table_list);
+};
+
 #endif