From ea1e7203910e673ae7ddeea25bdc64e13a12d8c7 Mon Sep 17 00:00:00 2001 From: Sergei Golubchik Date: Sun, 6 Oct 2024 11:55:54 +0200 Subject: [PATCH] MDEV-35078 Server crash or ASAN errors in mhnsw_insert when adding a column or index that uses plugin-defined sysvar-based options with ALTER TABLE ... ADD, the server was using the default value of the sysvar, not the current one. CREATE TABLE was correctly using the current sysvar value. Fix it so that new columns/indexes added in ALTER TABLE ... ADD would use a current sysvar value. Existing columns/indexes in ALTER TABLE would keep using the default sysvar value (unless they had an explicit value in frm). --- mysql-test/main/vector2.result | 25 +++++++++++++++++++++++++ mysql-test/main/vector2.test | 15 +++++++++++++++ sql/create_options.cc | 19 ++++++++++--------- sql/create_options.h | 7 ++++--- sql/sql_table.cc | 5 +++-- sql/sql_table.h | 6 +++--- sql/table.cc | 2 +- 7 files changed, 61 insertions(+), 18 deletions(-) diff --git a/mysql-test/main/vector2.result b/mysql-test/main/vector2.result index fadd5683193..a6af88526c2 100644 --- a/mysql-test/main/vector2.result +++ b/mysql-test/main/vector2.result @@ -51,3 +51,28 @@ create table t (a int, v blob not null, vector index(v)) engine=myisam; create table tm (a int, v blob not null, vector index(v)) engine=merge union=(t); ERROR HY000: Table storage engine 'MERGE' does not support the create option 'VECTOR' drop table t; +# +# MDEV-35078 Server crash or ASAN errors in mhnsw_insert +# +set session mhnsw_max_edges_per_node = 4; +create table t (a int, v blob not null); +insert into t select seq, x'00000000' from seq_1_to_10; +alter table t add vector(v); +show create table t; +Table Create Table +t CREATE TABLE `t` ( + `a` int(11) DEFAULT NULL, + `v` blob NOT NULL, + VECTOR KEY `v` (`v`) `max_edges_per_node`=4 +) ENGINE=MyISAM DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_uca1400_ai_ci +create table x like t; +show create table x; +Table Create Table +x CREATE TABLE `x` ( + `a` int(11) DEFAULT NULL, + `v` blob NOT NULL, + VECTOR KEY `v` (`v`) `max_edges_per_node`=4 +) ENGINE=MyISAM DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_uca1400_ai_ci +insert into t values (11,x'00000000'); +drop table t, x; +set session mhnsw_max_edges_per_node = default; diff --git a/mysql-test/main/vector2.test b/mysql-test/main/vector2.test index bcdd635bdb4..6358994ce88 100644 --- a/mysql-test/main/vector2.test +++ b/mysql-test/main/vector2.test @@ -1,6 +1,7 @@ # # tests that don't need to be run on many engines # +--source include/have_sequence.inc --echo # Aria doesn't support VECTOR yet --error ER_ILLEGAL_HA_CREATE_OPTION @@ -55,3 +56,17 @@ create table t (a int, v blob not null, vector index(v)) engine=myisam; --error ER_ILLEGAL_HA_CREATE_OPTION create table tm (a int, v blob not null, vector index(v)) engine=merge union=(t); drop table t; + +--echo # +--echo # MDEV-35078 Server crash or ASAN errors in mhnsw_insert +--echo # +set session mhnsw_max_edges_per_node = 4; +create table t (a int, v blob not null); +insert into t select seq, x'00000000' from seq_1_to_10; +alter table t add vector(v); +show create table t; +create table x like t; +show create table x; +insert into t values (11,x'00000000'); +drop table t, x; +set session mhnsw_max_edges_per_node = default; diff --git a/sql/create_options.cc b/sql/create_options.cc index df71631706f..ac68c951d11 100644 --- a/sql/create_options.cc +++ b/sql/create_options.cc @@ -220,6 +220,7 @@ static const size_t ha_option_type_sizeof[]= @param option_list list of options given by user @param rules list of option description by engine @param suppress_warning second parse so we do not need warnings + @param create a new object is created, use current sysvar value @param root MEM_ROOT where allocate memory @retval TRUE Error @@ -229,7 +230,7 @@ static const size_t ha_option_type_sizeof[]= bool parse_option_list(THD* thd, st_plugin_int *plugin, void *option_struct_arg, engine_option_value **option_list, ha_create_table_option *rules, - bool suppress_warning, MEM_ROOT *root) + bool suppress_warning, bool create, MEM_ROOT *root) { ha_create_table_option *opt; size_t option_struct_size= 0; @@ -276,7 +277,7 @@ bool parse_option_list(THD* thd, st_plugin_int *plugin, void *option_struct_arg, /* Okay, here's the logic for sysvar options: - 1. When we parse CREATE TABLE and sysvar option was not explicitly + 1. When an object is created and sysvar option was not explicitly mentioned we add it to the list as if it was specified with the *current* value of the underlying sysvar. 2. But only if the underlying sysvar value is different from the @@ -298,12 +299,12 @@ bool parse_option_list(THD* thd, st_plugin_int *plugin, void *option_struct_arg, Note that if the option was set explicitly (not =DEFAULT) it wouldn't have passes the if() condition above. */ - if (!suppress_warning && opt->var && - (thd->lex->sql_command == SQLCOM_CREATE_TABLE || seen)) + if (opt->var && (create || seen)) { // take a value from the variable and add it to the list sys_var *sysvar= find_plugin_sysvar(plugin, opt->var); DBUG_ASSERT(sysvar); + DBUG_ASSERT(!suppress_warning); // always warn when creating if (!sysvar->session_is_default(thd)) { @@ -438,14 +439,14 @@ bool parse_engine_table_options(THD *thd, handlerton *ht, TABLE_SHARE *share) DBUG_ENTER("parse_engine_table_options"); if (parse_option_list(thd, ht, &share->option_struct, & share->option_list, - ht->table_options, TRUE, root)) + ht->table_options, TRUE, FALSE, root)) DBUG_RETURN(TRUE); for (Field **field= share->field; *field; field++) { if (parse_option_list(thd, ht, &(*field)->option_struct, & (*field)->option_list, - ht->field_options, TRUE, root)) + ht->field_options, TRUE, FALSE, root)) DBUG_RETURN(TRUE); } @@ -453,7 +454,7 @@ bool parse_engine_table_options(THD *thd, handlerton *ht, TABLE_SHARE *share) { if (parse_option_list(thd, ht, &share->key_info[index].option_struct, & share->key_info[index].option_list, - ht->index_options, TRUE, root)) + ht->index_options, TRUE, FALSE, root)) DBUG_RETURN(TRUE); } @@ -497,7 +498,7 @@ bool parse_engine_part_options(THD *thd, TABLE *table) { ht= part_elem->engine_type; if (parse_option_list(thd, ht, &part_elem->option_struct, - &tmp_option_list, ht->table_options, TRUE, root)) + &tmp_option_list, ht->table_options, TRUE, FALSE, root)) DBUG_RETURN(TRUE); } else @@ -507,7 +508,7 @@ bool parse_engine_part_options(THD *thd, TABLE *table) { ht= sub_part_elem->engine_type; if (parse_option_list(thd, ht, &sub_part_elem->option_struct, - &tmp_option_list, ht->table_options, TRUE, root)) + &tmp_option_list, ht->table_options, TRUE, FALSE, root)) DBUG_RETURN(TRUE); } } diff --git a/sql/create_options.h b/sql/create_options.h index f7bb77c5772..511d5d54a9e 100644 --- a/sql/create_options.h +++ b/sql/create_options.h @@ -114,14 +114,15 @@ bool parse_engine_part_options(THD *thd, TABLE *table); bool parse_option_list(THD* thd, st_plugin_int *plugin, void *option_struct, engine_option_value **option_list, ha_create_table_option *rules, - bool suppress_warning, MEM_ROOT *root); + bool suppress_warning, bool create, MEM_ROOT *root); static inline bool parse_option_list(THD* thd, handlerton *hton, void *option_struct, engine_option_value **option_list, - ha_create_table_option *rules, bool suppress_warning, MEM_ROOT *root) + ha_create_table_option *rules, bool suppress_warning, bool create, + MEM_ROOT *root) { return parse_option_list(thd, hton2plugin[hton->slot], option_struct, - option_list, rules, suppress_warning, root); + option_list, rules, suppress_warning, create, root); } bool engine_table_options_frm_read(const uchar *buff, size_t length, diff --git a/sql/sql_table.cc b/sql/sql_table.cc index 44e9a1f1d4f..c48c351f4ce 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -3035,7 +3035,7 @@ mysql_prepare_create_table_finalize(THD *thd, HA_CREATE_INFO *create_info, if (parse_option_list(thd, create_info->db_type, &sql_field->option_struct, &sql_field->option_list, create_info->db_type->field_options, FALSE, - thd->mem_root)) + sql_field->field == NULL, thd->mem_root)) DBUG_RETURN(TRUE); /* For now skip fields that are not physically stored in the database @@ -3320,7 +3320,7 @@ mysql_prepare_create_table_finalize(THD *thd, HA_CREATE_INFO *create_info, key_info->option_list= key->option_list; if (parse_option_list(thd, index_plugin, &key_info->option_struct, &key_info->option_list, index_options, - FALSE, thd->mem_root)) + FALSE, !key->old, thd->mem_root)) DBUG_RETURN(TRUE); if (key->type == Key::FULLTEXT) @@ -3916,6 +3916,7 @@ without_overlaps_err: if (parse_option_list(thd, file->partition_ht(), &create_info->option_struct, &create_info->option_list, file->partition_ht()->table_options, FALSE, + create_table_mode > C_ALTER_TABLE, thd->mem_root)) DBUG_RETURN(TRUE); diff --git a/sql/sql_table.h b/sql/sql_table.h index 6a9806c449a..78b1dce29c3 100644 --- a/sql/sql_table.h +++ b/sql/sql_table.h @@ -129,9 +129,9 @@ bool add_keyword_to_query(THD *thd, String *result, const LEX_CSTRING *keyword, */ #define C_CREATE_SELECT(X) ((X) > 0 ? (X) : 0) #define C_ORDINARY_CREATE 0 -#define C_ALTER_TABLE -1 -#define C_ALTER_TABLE_FRM_ONLY -2 -#define C_ASSISTED_DISCOVERY -3 +#define C_ASSISTED_DISCOVERY -1 +#define C_ALTER_TABLE -2 +#define C_ALTER_TABLE_FRM_ONLY -3 int mysql_create_table_no_lock(THD *thd, DDL_LOG_STATE *ddl_log_state, diff --git a/sql/table.cc b/sql/table.cc index b4c7edd6149..d5ca87545de 100644 --- a/sql/table.cc +++ b/sql/table.cc @@ -3432,7 +3432,7 @@ int TABLE_SHARE::init_from_binary_frm_image(THD *thd, bool write, keyinfo= share->key_info + share->keys; if (parse_option_list(thd, mhnsw_plugin, &keyinfo->option_struct, &keyinfo->option_list, mhnsw_index_options, - TRUE, thd->mem_root)) + TRUE, FALSE, thd->mem_root)) goto err; }