MDEV-28854 Disallow INSERT DELAYED on Spider table

h-michael · web-flow · commit 2c1aaa666401 · 2022-06-27T14:58:18.000+09:00
Spider supports (or at least allows) INSERT DELAYED but the
documentation does not specify spider as a storage engine that supports
"INSERT DELAYED".
Also, although not mentioned in the documentation, "INSERT DELAYED" is
not intended to be executed inside a transaction, as can be seen from
the list of supported storage engines.
The current implementation allows executing a delayed insert on a
remote transactional table and this breaks the consistency ensured by
the transaction.

We too remove "internal_delayed", one of the Spider table parameters.
Documentation says,

&gt; Whether to transmit existence of delay to remote servers when
&gt; executing an INSERT DELAYED statement on local server.

This table parameter is only used for "INSERT DELAYED".

Reviewed by: Nayuta Yanagisawa
diff --git a/storage/spider/ha_spider.cc b/storage/spider/ha_spider.cc
@@ -1110,9 +1110,6 @@ THR_LOCK_DATA **ha_spider::store_lock(
     case TL_READ_HIGH_PRIORITY:
       high_priority = TRUE;
       break;
-    case TL_WRITE_DELAYED:
-      insert_delayed = TRUE;
-      break;
     case TL_WRITE_LOW_PRIORITY:
       low_priority = TRUE;
       break;
@@ -1222,7 +1219,6 @@ THR_LOCK_DATA **ha_spider::store_lock(
         lock_type = TL_READ;
       if (
         lock_type >= TL_WRITE_CONCURRENT_INSERT && lock_type <= TL_WRITE &&
-        lock_type != TL_WRITE_DELAYED &&
         !thd->in_lock_tables && !thd_tablespace_op(thd)
       )
         lock_type = TL_WRITE_ALLOW_WRITE;
@@ -1804,7 +1800,6 @@ int ha_spider::reset()
   insert_with_update = FALSE;
   low_priority = FALSE;
   high_priority = FALSE;
-  insert_delayed = FALSE;
   use_pre_call = FALSE;
   use_pre_records = FALSE;
   pre_bitmap_checked = FALSE;
@@ -9556,7 +9551,6 @@ ulonglong ha_spider::table_flags() const
     HA_CAN_FULLTEXT |
     HA_CAN_SQL_HANDLER |
     HA_FILE_BASED |
-    HA_CAN_INSERT_DELAYED |
     HA_CAN_BIT_FIELD |
     HA_NO_COPY_ON_ALTER |
     HA_BINLOG_ROW_CAPABLE |
@@ -13955,7 +13949,6 @@ int ha_spider::sync_from_clone_source(
     update_request = spider->update_request;
     lock_mode = spider->lock_mode;
     high_priority = spider->high_priority;
-    insert_delayed = spider->insert_delayed;
     low_priority = spider->low_priority;
     memcpy(conns, spider->conns,
       sizeof(SPIDER_CONN *) * share->link_count);
@@ -13997,7 +13990,6 @@ int ha_spider::sync_from_clone_source(
     update_request = spider->update_request;
     lock_mode = spider->lock_mode;
     high_priority = spider->high_priority;
-    insert_delayed = spider->insert_delayed;
     low_priority = spider->low_priority;
 
     if ((error_num = spider_check_trx_and_get_conn(spider->trx->thd,
diff --git a/storage/spider/ha_spider.h b/storage/spider/ha_spider.h
@@ -168,7 +168,6 @@ class ha_spider: public handler
   bool               insert_with_update;
   bool               low_priority;
   bool               high_priority;
-  bool               insert_delayed;
   bool               use_pre_call;
   bool               use_pre_records;
   bool               pre_bitmap_checked;
diff --git a/storage/spider/mysql-test/spider/bugfix/r/mdev_28854.result b/storage/spider/mysql-test/spider/bugfix/r/mdev_28854.result
@@ -0,0 +1,32 @@
+#
+# MDEV-28854 Spider: Disallow INSERT DELAYED on Spider table
+#
+for master_1
+for child2
+child2_1
+child2_2
+child2_3
+for child3
+connection child2_1;
+CREATE DATABASE auto_test_remote;
+USE auto_test_remote;
+CREATE TABLE tbl_a (id INT);
+connection master_1;
+CREATE DATABASE auto_test_local;
+USE auto_test_local;
+CREATE TABLE tbl_a (
+id INT
+) ENGINE=Spider DEFAULT CHARSET=utf8 COMMENT='table "tbl_a", srv "s_2_1"';
+connection master_1;
+INSERT DELAYED INTO tbl_a VALUES (1);
+ERROR HY000: DELAYED option not supported for table 'tbl_a'
+connection master_1;
+DROP DATABASE IF EXISTS auto_test_local;
+connection child2_1;
+DROP DATABASE IF EXISTS auto_test_remote;
+for master_1
+for child2
+child2_1
+child2_2
+child2_3
+for child3
diff --git a/storage/spider/mysql-test/spider/bugfix/t/mdev_28854.cnf b/storage/spider/mysql-test/spider/bugfix/t/mdev_28854.cnf
@@ -0,0 +1,3 @@
+!include include/default_mysqld.cnf
+!include ../my_1_1.cnf
+!include ../my_2_1.cnf
diff --git a/storage/spider/mysql-test/spider/bugfix/t/mdev_28854.test b/storage/spider/mysql-test/spider/bugfix/t/mdev_28854.test
@@ -0,0 +1,36 @@
+--echo #
+--echo # MDEV-28854 Spider: Disallow INSERT DELAYED on Spider table
+--echo #
+
+--disable_query_log
+--disable_result_log
+--source ../../t/test_init.inc
+--enable_result_log
+--enable_query_log
+
+--connection child2_1
+CREATE DATABASE auto_test_remote;
+USE auto_test_remote;
+CREATE TABLE tbl_a (id INT);
+
+--connection master_1
+CREATE DATABASE auto_test_local;
+USE auto_test_local;
+eval CREATE TABLE tbl_a (
+    id INT
+) $MASTER_1_ENGINE $MASTER_1_CHARSET COMMENT='table "tbl_a", srv "s_2_1"';
+
+--connection master_1
+--error ER_DELAYED_NOT_SUPPORTED
+INSERT DELAYED INTO tbl_a VALUES (1);
+
+--connection master_1
+DROP DATABASE IF EXISTS auto_test_local;
+--connection child2_1
+DROP DATABASE IF EXISTS auto_test_remote;
+
+--disable_query_log
+--disable_result_log
+--source ../t/test_deinit.inc
+--enable_query_log
+--enable_result_log
diff --git a/storage/spider/spd_db_conn.h b/storage/spider/spd_db_conn.h
@@ -33,7 +33,6 @@
 #define SPIDER_DB_INSERT_IGNORE        (1 << 1)
 #define SPIDER_DB_INSERT_LOW_PRIORITY  (1 << 2)
 #define SPIDER_DB_INSERT_HIGH_PRIORITY (1 << 3)
-#define SPIDER_DB_INSERT_DELAYED       (1 << 4)
 
 #define SPIDER_SQL_OPEN_PAREN_STR "("
 #define SPIDER_SQL_OPEN_PAREN_LEN (sizeof(SPIDER_SQL_OPEN_PAREN_STR) - 1)
@@ -75,8 +74,6 @@
 #define SPIDER_SQL_HIGH_PRIORITY_LEN (sizeof(SPIDER_SQL_HIGH_PRIORITY_STR) - 1)
 #define SPIDER_SQL_LOW_PRIORITY_STR "low_priority "
 #define SPIDER_SQL_LOW_PRIORITY_LEN (sizeof(SPIDER_SQL_LOW_PRIORITY_STR) - 1)
-#define SPIDER_SQL_SQL_DELAYED_STR "delayed "
-#define SPIDER_SQL_SQL_DELAYED_LEN (sizeof(SPIDER_SQL_SQL_DELAYED_STR) - 1)
 #define SPIDER_SQL_SQL_IGNORE_STR "ignore "
 #define SPIDER_SQL_SQL_IGNORE_LEN (sizeof(SPIDER_SQL_SQL_IGNORE_STR) - 1)
 #define SPIDER_SQL_FROM_STR " from "
diff --git a/storage/spider/spd_db_mysql.cc b/storage/spider/spd_db_mysql.cc
@@ -8044,7 +8044,6 @@ int spider_mbase_handler::append_insert(
   spider_string *str,
   int link_idx
 ) {
-  SPIDER_SHARE *share = spider->share;
   DBUG_ENTER("spider_mbase_handler::append_insert");
   if (
     (
@@ -8069,15 +8068,6 @@ int spider_mbase_handler::append_insert(
       DBUG_RETURN(HA_ERR_OUT_OF_MEM);
     str->q_append(SPIDER_SQL_LOW_PRIORITY_STR, SPIDER_SQL_LOW_PRIORITY_LEN);
   }
-  else if (spider->insert_delayed)
-  {
-    if (share->internal_delayed)
-    {
-      if (str->reserve(SPIDER_SQL_SQL_DELAYED_LEN))
-        DBUG_RETURN(HA_ERR_OUT_OF_MEM);
-      str->q_append(SPIDER_SQL_SQL_DELAYED_STR, SPIDER_SQL_SQL_DELAYED_LEN);
-    }
-  }
   else if (
     spider->lock_type >= TL_WRITE &&
     !spider->write_can_replace &&
@@ -14667,12 +14657,6 @@ int spider_mbase_copy_table::append_insert_str(
       DBUG_RETURN(HA_ERR_OUT_OF_MEM);
     sql.q_append(SPIDER_SQL_LOW_PRIORITY_STR, SPIDER_SQL_LOW_PRIORITY_LEN);
   }
-  else if (insert_flg & SPIDER_DB_INSERT_DELAYED)
-  {
-    if (sql.reserve(SPIDER_SQL_SQL_DELAYED_LEN))
-      DBUG_RETURN(HA_ERR_OUT_OF_MEM);
-    sql.q_append(SPIDER_SQL_SQL_DELAYED_STR, SPIDER_SQL_SQL_DELAYED_LEN);
-  }
   else if (insert_flg & SPIDER_DB_INSERT_HIGH_PRIORITY)
   {
     if (sql.reserve(SPIDER_SQL_HIGH_PRIORITY_LEN))
diff --git a/storage/spider/spd_include.h b/storage/spider/spd_include.h
@@ -951,7 +951,6 @@ typedef struct st_spider_share
   int                selupd_lock_mode;
   int                query_cache;
   int                query_cache_sync;
-  int                internal_delayed;
   int                bulk_size;
   int                bulk_update_mode;
   int                bulk_update_size;
diff --git a/storage/spider/spd_table.cc b/storage/spider/spd_table.cc
@@ -2035,7 +2035,6 @@ int spider_parse_connect_info(
   share->selupd_lock_mode = -1;
   share->query_cache = -1;
   share->query_cache_sync = -1;
-  share->internal_delayed = -1;
   share->bulk_size = -1;
   share->bulk_update_mode = -1;
   share->bulk_update_size = -1;
@@ -2259,7 +2258,6 @@ int spider_parse_connect_info(
           SPIDER_PARAM_STR_LIST("hws", hs_write_socks);
 #endif
           SPIDER_PARAM_INT("isa", init_sql_alloc_size, 0);
-          SPIDER_PARAM_INT_WITH_MAX("idl", internal_delayed, 0, 1);
           SPIDER_PARAM_LONGLONG("ilm", internal_limit, 0);
           SPIDER_PARAM_LONGLONG("ios", internal_offset, 0);
           SPIDER_PARAM_INT_WITH_MAX("iom", internal_optimize, 0, 1);
@@ -2484,8 +2482,6 @@ int spider_parse_connect_info(
             "multi_split_read", multi_split_read, 0, 2147483647);
           SPIDER_PARAM_INT_WITH_MAX(
             "selupd_lock_mode", selupd_lock_mode, 0, 2);
-          SPIDER_PARAM_INT_WITH_MAX(
-            "internal_delayed", internal_delayed, 0, 1);
           SPIDER_PARAM_INT_WITH_MAX(
             "table_count_mode", table_count_mode, 0, 3);
           SPIDER_PARAM_INT_WITH_MAX(
@@ -3809,8 +3805,6 @@ int spider_set_connect_info_default(
     share->query_cache = 0;
   if (share->query_cache_sync == -1)
     share->query_cache_sync = 0;
-  if (share->internal_delayed == -1)
-    share->internal_delayed = 0;
   if (share->bulk_size == -1)
     share->bulk_size = 16000;
   if (share->bulk_update_mode == -1)

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+!include include/default_mysqld.cnf`
	`2`	`+!include ../my_1_1.cnf`
	`3`	`+!include ../my_2_1.cnf`