From 9b8e207ce03b2ab7a766348738055be9520561bd Mon Sep 17 00:00:00 2001 From: Nikita Malyavin Date: Wed, 28 Jul 2021 11:07:49 +0200 Subject: [PATCH 1/6] MDEV-26220 Server crashes with indexed by prefix virtual column Server crashes in Field::register_field_in_read_map upon select from partitioned table with indexed by prefix virtual column. After several read-mark fixes a problem has surfaced: Since KEY (c(10),a) uses only a prefix of c, a new field is created, duplicated from table->field[3], with a new length. However, vcol_inco->expr is not copied. Therefore, (*key_info)->key_part[i].field->vcol_info->expr was left NULL in ha_partition::index_init(). Solution: initialize vcols before key initialization Also key initialization is moved to a function. --- mysql-test/suite/gcol/inc/gcol_partition.inc | 12 ++ .../suite/gcol/r/gcol_partition_innodb.result | 11 ++ .../suite/gcol/r/gcol_partition_myisam.result | 11 ++ sql/table.cc | 113 ++++++++++-------- 4 files changed, 99 insertions(+), 48 deletions(-) diff --git a/mysql-test/suite/gcol/inc/gcol_partition.inc b/mysql-test/suite/gcol/inc/gcol_partition.inc index 4e4af4f0023..50a743c0153 100644 --- a/mysql-test/suite/gcol/inc/gcol_partition.inc +++ b/mysql-test/suite/gcol/inc/gcol_partition.inc @@ -169,3 +169,15 @@ CREATE TABLE t1 ( INSERT INTO t1 () VALUES (),(); UPDATE t1 SET a = 0 WHERE b IS NULL ORDER BY pk; DROP TABLE t1; + +--echo # +--echo # MDEV-26220 Server crashes with indexed by prefix virtual column +--echo # + +CREATE TABLE t1 (pk INT PRIMARY KEY, a INT, b CHAR(20), c CHAR(20) AS (b), + KEY (c(10),a)) PARTITION BY HASH(pk); +INSERT INTO t1 (pk,a,b) VALUES (1,10,'foo'),(2,11,'baz'); +SELECT a FROM t1; + +# Cleanup +DROP TABLE t1; diff --git a/mysql-test/suite/gcol/r/gcol_partition_innodb.result b/mysql-test/suite/gcol/r/gcol_partition_innodb.result index d3f211c9b9a..e61c0a26417 100644 --- a/mysql-test/suite/gcol/r/gcol_partition_innodb.result +++ b/mysql-test/suite/gcol/r/gcol_partition_innodb.result @@ -104,6 +104,17 @@ INSERT INTO t1 () VALUES (),(); UPDATE t1 SET a = 0 WHERE b IS NULL ORDER BY pk; DROP TABLE t1; # +# MDEV-26220 Server crashes with indexed by prefix virtual column +# +CREATE TABLE t1 (pk INT PRIMARY KEY, a INT, b CHAR(20), c CHAR(20) AS (b), +KEY (c(10),a)) PARTITION BY HASH(pk); +INSERT INTO t1 (pk,a,b) VALUES (1,10,'foo'),(2,11,'baz'); +SELECT a FROM t1; +a +11 +10 +DROP TABLE t1; +# # MDEV-16980 Wrongly set tablename len while opening the # table for purge thread # diff --git a/mysql-test/suite/gcol/r/gcol_partition_myisam.result b/mysql-test/suite/gcol/r/gcol_partition_myisam.result index 75e216f903b..e54b0ad83c6 100644 --- a/mysql-test/suite/gcol/r/gcol_partition_myisam.result +++ b/mysql-test/suite/gcol/r/gcol_partition_myisam.result @@ -101,6 +101,17 @@ KEY (b,d) INSERT INTO t1 () VALUES (),(); UPDATE t1 SET a = 0 WHERE b IS NULL ORDER BY pk; DROP TABLE t1; +# +# MDEV-26220 Server crashes with indexed by prefix virtual column +# +CREATE TABLE t1 (pk INT PRIMARY KEY, a INT, b CHAR(20), c CHAR(20) AS (b), +KEY (c(10),a)) PARTITION BY HASH(pk); +INSERT INTO t1 (pk,a,b) VALUES (1,10,'foo'),(2,11,'baz'); +SELECT a FROM t1; +a +11 +10 +DROP TABLE t1; DROP VIEW IF EXISTS v1,v2; DROP TABLE IF EXISTS t1,t2,t3; DROP PROCEDURE IF EXISTS p1; diff --git a/sql/table.cc b/sql/table.cc index b7cf1274400..ab2429f469f 100644 --- a/sql/table.cc +++ b/sql/table.cc @@ -3034,6 +3034,66 @@ static bool check_vcol_forward_refs(Field *field, Virtual_column_info *vcol) return res; } +/** + Copy keys from share to a table, so that fields in key_parts would match + fields in a table. + + Also if key_part uses a field prefix, clone a field and make its length match + that prefix. + + @retval true if success + @retval false if memory allocation fails + */ +static bool initialize_keys(TABLE_SHARE *share, TABLE *outparam) +{ + if (share->key_parts == 0) + return true; + uint n_length= share->keys * sizeof(KEY) + + share->ext_key_parts * sizeof(KEY_PART_INFO); + + KEY *key_info= (KEY*) alloc_root(&outparam->mem_root, n_length); + if (!key_info) + return false; + + outparam->key_info= key_info; + + KEY *key_info_end= key_info + share->keys; + KEY_PART_INFO *key_part= reinterpret_cast(key_info_end); + + memcpy(key_info, share->key_info, sizeof(*key_info) * share->keys); + memcpy(key_part, share->key_info[0].key_part, (sizeof(*key_part) * + share->ext_key_parts)); + + for ( ; key_info < key_info_end; key_info++) + { + key_info->table= outparam; + key_info->key_part= key_part; + + KEY_PART_INFO *key_part_end= key_part + (share->use_ext_keys + ? key_info->ext_key_parts + : key_info->user_defined_key_parts); + for ( ; key_part < key_part_end; key_part++) + { + Field *field= key_part->field= outparam->field[key_part->fieldnr - 1]; + + if (field->key_length() != key_part->length && + !(field->flags & BLOB_FLAG)) + { + /* + We are using only a prefix of the column as a key: + Create a new field for the key part that matches the index + */ + field= key_part->field=field->make_new_field(&outparam->mem_root, + outparam, 0); + field->field_length= key_part->length; + } + } + if (!share->use_ext_keys) + key_part+= key_info->ext_key_parts - key_info->user_defined_key_parts; + } + return true; +} + /* Open a table based on a TABLE_SHARE @@ -3073,6 +3133,7 @@ enum open_frm_error open_table_from_share(THD *thd, TABLE_SHARE *share, Field **field_ptr; uint8 save_context_analysis_only= thd->lex->context_analysis_only; TABLE_SHARE::enum_v_keys check_set_initialized= share->check_set_initialized; + bool success; DBUG_ENTER("open_table_from_share"); DBUG_PRINT("enter",("name: '%s.%s' form: %p", share->db.str, share->table_name.str, outparam)); @@ -3182,54 +3243,6 @@ enum open_frm_error open_table_from_share(THD *thd, TABLE_SHARE *share, outparam->found_next_number_field= outparam->field[(uint) (share->found_next_number_field - share->field)]; - /* Fix key->name and key_part->field */ - if (share->key_parts) - { - KEY *key_info, *key_info_end; - KEY_PART_INFO *key_part; - uint n_length; - n_length= share->keys*sizeof(KEY) + share->ext_key_parts*sizeof(KEY_PART_INFO); - if (!(key_info= (KEY*) alloc_root(&outparam->mem_root, n_length))) - goto err; - outparam->key_info= key_info; - key_part= (reinterpret_cast(key_info+share->keys)); - - memcpy(key_info, share->key_info, sizeof(*key_info)*share->keys); - memcpy(key_part, share->key_info[0].key_part, (sizeof(*key_part) * - share->ext_key_parts)); - - for (key_info_end= key_info + share->keys ; - key_info < key_info_end ; - key_info++) - { - KEY_PART_INFO *key_part_end; - - key_info->table= outparam; - key_info->key_part= key_part; - - key_part_end= key_part + (share->use_ext_keys ? key_info->ext_key_parts : - key_info->user_defined_key_parts) ; - for ( ; key_part < key_part_end; key_part++) - { - Field *field= key_part->field= outparam->field[key_part->fieldnr - 1]; - - if (field->key_length() != key_part->length && - !(field->flags & BLOB_FLAG)) - { - /* - We are using only a prefix of the column as a key: - Create a new field for the key part that matches the index - */ - field= key_part->field=field->make_new_field(&outparam->mem_root, - outparam, 0); - field->field_length= key_part->length; - } - } - if (!share->use_ext_keys) - key_part+= key_info->ext_key_parts - key_info->user_defined_key_parts; - } - } - /* Process virtual and default columns, if any. */ @@ -3286,6 +3299,10 @@ enum open_frm_error open_table_from_share(THD *thd, TABLE_SHARE *share, switch_defaults_to_nullable_trigger_fields(outparam); } + success= initialize_keys(share, outparam); + if (!success) + goto err; + #ifdef WITH_PARTITION_STORAGE_ENGINE if (share->partition_info_str_len && outparam->file) { From 0e8981ef93ff4421e333386072b3a2b4c5014cce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Thu, 29 Jul 2021 09:15:03 +0300 Subject: [PATCH 2/6] Cleanup: Remove redundant conditions ha_innobase::prepare_inplace_alter_table(): Remove always-true conditions. Near the start of the function, we would already have returned if no ALTER TABLE operation flags were set that would require special action from InnoDB. It turns out that the conditions were redundant already when they were introduced in mysql/mysql-server@241387a2b6b61fb8a4f78dc4ad0aaa289400c694 and in commit 068c61978e3a81836d52b8caf11e044290159ad1. Thanks to Nikita Malyavin for noticing this. --- storage/innobase/handler/handler0alter.cc | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/storage/innobase/handler/handler0alter.cc b/storage/innobase/handler/handler0alter.cc index c8888bada02..c78ea76bb00 100644 --- a/storage/innobase/handler/handler0alter.cc +++ b/storage/innobase/handler/handler0alter.cc @@ -5506,11 +5506,7 @@ ha_innobase::prepare_inplace_alter_table( ha_alter_info->key_count)) { err_exit_no_heap: DBUG_ASSERT(m_prebuilt->trx->dict_operation_lock_mode == 0); - if (ha_alter_info->handler_flags & ~INNOBASE_INPLACE_IGNORE) { - - online_retry_drop_indexes( - m_prebuilt->table, m_user_thd); - } + online_retry_drop_indexes(m_prebuilt->table, m_user_thd); DBUG_RETURN(true); } @@ -5969,12 +5965,7 @@ err_exit: } DBUG_ASSERT(m_prebuilt->trx->dict_operation_lock_mode == 0); - if (ha_alter_info->handler_flags & ~INNOBASE_INPLACE_IGNORE) { - - online_retry_drop_indexes( - m_prebuilt->table, m_user_thd); - - } + online_retry_drop_indexes(m_prebuilt->table, m_user_thd); if ((ha_alter_info->handler_flags & Alter_inplace_info::DROP_VIRTUAL_COLUMN) From 22709897b06331fad4660739be37773077d25d62 Mon Sep 17 00:00:00 2001 From: Nikita Malyavin Date: Tue, 6 Jul 2021 10:17:22 +0300 Subject: [PATCH 3/6] MDEV-20154 Assertion `len <= col->len | ...` failed in row_merge_buf_add len was containing garbage, since vctempl->mysql_col_offset was containing old value while calling row_mysql_store_col_in_innobase_format from innobase_get_computed_value(). It was not updated after the first ALTER TABLE call, because it's INPLACE logic considered there's nothing to update, and exited immediately from ha_innobase::inplace_alter_table(). However, vcol metadata needs an update, since vcols structure is changed in mysql record. The regression was introduced by 12614af1fe. There, refcount==1 condition was removed, which turned out to be crucial, though racy. The idea was to update vc_templ after each (sequencing) ALTER TABLE. We should do the same another way, and there may be a plenty of solutions, but the simplest one is to add a following condition: if vcol structure is changed, drop vc_templ; it will be recreated on next ha_innobase::open() call. in prepare_inplace_alter_table. It is safe, since innodb inplace changes require at least HA_ALTER_INPLACE_SHARED_LOCK_AFTER_PREPARE, which guarantee MDL_EXCLUSIVE on this stage. alter_templ_needs_rebuild() also has to track the columns not indexed, to keep vc_templ correct. Note that vc_templ is always kept constructed and available after ha_innobase::open() call, even on INSERT, though no virtual columns are evaluated during that statement inside innodb. In the test case suplied, it will be recreated on the second ALTER TABLE. --- .../suite/gcol/r/innodb_virtual_index.result | 14 ++++ .../suite/gcol/t/innodb_virtual_index.test | 20 ++++++ storage/innobase/handler/handler0alter.cc | 65 +++++++++---------- 3 files changed, 63 insertions(+), 36 deletions(-) diff --git a/mysql-test/suite/gcol/r/innodb_virtual_index.result b/mysql-test/suite/gcol/r/innodb_virtual_index.result index b8dd161c30e..4b7af8784eb 100644 --- a/mysql-test/suite/gcol/r/innodb_virtual_index.result +++ b/mysql-test/suite/gcol/r/innodb_virtual_index.result @@ -300,3 +300,17 @@ Table Op Msg_type Msg_text test.t1 optimize note Table does not support optimize, doing recreate + analyze instead test.t1 optimize status OK DROP TABLE t1; +# +# MDEV-20154 Assertion `len <= col->len || ((col->mtype) == 5 +# || (col->mtype) == 14)' failed in row_merge_buf_add +# +CREATE TABLE t1 ( +a VARCHAR(2500), +b VARCHAR(2499) AS (a) VIRTUAL +) ENGINE=InnoDB; +INSERT INTO t1 (a) VALUES ('foo'); +ALTER TABLE t1 MODIFY a VARCHAR(2600), ALGORITHM=INPLACE; +ALTER TABLE t1 ADD KEY (b), ALGORITHM=INPLACE; +# Cleanup +DROP TABLE t1; +# End of 10.2 tests diff --git a/mysql-test/suite/gcol/t/innodb_virtual_index.test b/mysql-test/suite/gcol/t/innodb_virtual_index.test index 94c3b7f9204..46ffadcdd8c 100644 --- a/mysql-test/suite/gcol/t/innodb_virtual_index.test +++ b/mysql-test/suite/gcol/t/innodb_virtual_index.test @@ -314,3 +314,23 @@ CREATE TABLE t1 (id INT PRIMARY KEY, a CHAR(3), INSERT INTO t1 (id,a) VALUES (1,'foo'); OPTIMIZE TABLE t1; DROP TABLE t1; + +--echo # +--echo # MDEV-20154 Assertion `len <= col->len || ((col->mtype) == 5 +--echo # || (col->mtype) == 14)' failed in row_merge_buf_add +--echo # + +CREATE TABLE t1 ( + a VARCHAR(2500), + b VARCHAR(2499) AS (a) VIRTUAL +) ENGINE=InnoDB; +INSERT INTO t1 (a) VALUES ('foo'); + +ALTER TABLE t1 MODIFY a VARCHAR(2600), ALGORITHM=INPLACE; +ALTER TABLE t1 ADD KEY (b), ALGORITHM=INPLACE; + +--echo # Cleanup +DROP TABLE t1; + +--echo # End of 10.2 tests + diff --git a/storage/innobase/handler/handler0alter.cc b/storage/innobase/handler/handler0alter.cc index c78ea76bb00..2dd88e50c07 100644 --- a/storage/innobase/handler/handler0alter.cc +++ b/storage/innobase/handler/handler0alter.cc @@ -5355,6 +5355,10 @@ alter_fill_stored_column( } } +static bool alter_templ_needs_rebuild(const TABLE* altered_table, + const Alter_inplace_info* ha_alter_info, + const dict_table_t* table); + /** Allows InnoDB to update internal structures with concurrent writes blocked (provided that check_if_supported_inplace_alter() @@ -5951,9 +5955,9 @@ err_exit: == Alter_inplace_info::CHANGE_CREATE_OPTION && !innobase_need_rebuild(ha_alter_info, table))) { + ha_innobase_inplace_ctx *ctx = NULL; if (heap) { - ha_alter_info->handler_ctx - = new ha_innobase_inplace_ctx( + ctx = new ha_innobase_inplace_ctx( m_prebuilt, drop_index, n_drop_index, rename_index, n_rename_index, @@ -5962,6 +5966,7 @@ err_exit: ha_alter_info->online, heap, indexed_table, col_names, ULINT_UNDEFINED, 0, 0, 0); + ha_alter_info->handler_ctx = ctx; } DBUG_ASSERT(m_prebuilt->trx->dict_operation_lock_mode == 0); @@ -5981,6 +5986,24 @@ err_exit: DBUG_RETURN(true); } + if (!(ha_alter_info->handler_flags & INNOBASE_ALTER_DATA) + && alter_templ_needs_rebuild(altered_table, ha_alter_info, + ctx->new_table) + && ctx->new_table->n_v_cols > 0) { + /* Changing maria record structure may end up here only + if virtual columns were altered. In this case, however, + vc_templ should be rebuilt. Since we don't actually + change any stored data, we can just dispose vc_templ; + it will be recreated on next ha_innobase::open(). */ + + DBUG_ASSERT(ctx->new_table == ctx->old_table); + + dict_free_vc_templ(ctx->new_table->vc_templ); + UT_DELETE(ctx->new_table->vc_templ); + + ctx->new_table->vc_templ = NULL; + } + DBUG_RETURN(false); } @@ -6098,35 +6121,6 @@ found_col: add_fts_doc_id_idx)); } -/** Check that the column is part of a virtual index(index contains -virtual column) in the table -@param[in] table Table containing column -@param[in] col column to be checked -@return true if this column is indexed with other virtual columns */ -static -bool -dict_col_in_v_indexes( - dict_table_t* table, - dict_col_t* col) -{ - for (dict_index_t* index = dict_table_get_next_index( - dict_table_get_first_index(table)); index != NULL; - index = dict_table_get_next_index(index)) { - if (!dict_index_has_virtual(index)) { - continue; - } - for (ulint k = 0; k < index->n_fields; k++) { - dict_field_t* field - = dict_index_get_nth_field(index, k); - if (field->col->ind == col->ind) { - return(true); - } - } - } - - return(false); -} - /* Check whether a columnn length change alter operation requires to rebuild the template. @param[in] altered_table TABLE object for new version of table. @@ -6138,9 +6132,9 @@ to rebuild the template. static bool alter_templ_needs_rebuild( - TABLE* altered_table, - Alter_inplace_info* ha_alter_info, - dict_table_t* table) + const TABLE* altered_table, + const Alter_inplace_info* ha_alter_info, + const dict_table_t* table) { ulint i = 0; List_iterator_fast cf_it( @@ -6152,8 +6146,7 @@ alter_templ_needs_rebuild( for (ulint j=0; j < table->n_cols; j++) { dict_col_t* cols = dict_table_get_nth_col(table, j); - if (cf->length > cols->len - && dict_col_in_v_indexes(table, cols)) { + if (cf->length > cols->len) { return(true); } } From 2cdf8a932737f5128004742cafbd3d3bb1df14a2 Mon Sep 17 00:00:00 2001 From: Sergei Golubchik Date: Sat, 31 Jul 2021 08:48:14 +0200 Subject: [PATCH 4/6] MDEV-23752 SHOW EXPLAIN FOR thd waits for sleep fix main.processlist_notembedded test * before EXPLAINing `select sleep` wait for select to start (fixes "Target is not running an EXPLAINable command") * after killing sleep, wait for it to die (fixes test failures on --repeat when old sleep shows on a test rerun) * unify with 10.3, copy minor changes from there (`--echo End of 5.5` vs `--echo # End of 5.5`, etc) --- mysql-test/r/processlist_notembedded.result | 17 ++++++++----- mysql-test/t/processlist_notembedded.test | 27 +++++++++++---------- 2 files changed, 25 insertions(+), 19 deletions(-) diff --git a/mysql-test/r/processlist_notembedded.result b/mysql-test/r/processlist_notembedded.result index ddde45f3031..46b1f0669cb 100644 --- a/mysql-test/r/processlist_notembedded.result +++ b/mysql-test/r/processlist_notembedded.result @@ -1,7 +1,7 @@ # # MDEV-20466: SHOW PROCESSLIST truncates query text on \0 bytes # -connect con1,localhost,root,,; +connect con1,localhost,root; connection con1; SET DEBUG_SYNC= 'before_join_optimize SIGNAL in_sync WAIT_FOR go'; connection default; @@ -13,17 +13,22 @@ user disconnect con1; connection default; SET DEBUG_SYNC = 'RESET'; -End of 5.5 tests +# +# End of 5.5 tests +# # # MDEV-23752: SHOW EXPLAIN FOR thd waits for sleep # -connect con1,localhost,root,,; -select sleep(100000);; +connect con1,localhost,root; +select sleep(100000); connection default; -SHOW EXPLAIN FOR con_id; +SHOW EXPLAIN FOR $con_id; id select_type table type possible_keys key key_len ref rows Extra 1 SIMPLE NULL NULL NULL NULL NULL NULL NULL No tables used Warnings: Note 1003 select sleep(100000) -KILL QUERY con_id; +KILL QUERY $con_id; +disconnect con1; +# # End of 10.2 tests +# diff --git a/mysql-test/t/processlist_notembedded.test b/mysql-test/t/processlist_notembedded.test index 26021040c39..35cac36bb95 100644 --- a/mysql-test/t/processlist_notembedded.test +++ b/mysql-test/t/processlist_notembedded.test @@ -1,4 +1,3 @@ -source include/have_debug.inc; source include/have_debug_sync.inc; source include/not_embedded.inc; source include/count_sessions.inc; @@ -7,7 +6,7 @@ source include/count_sessions.inc; --echo # MDEV-20466: SHOW PROCESSLIST truncates query text on \0 bytes --echo # -connect (con1,localhost,root,,); +connect con1,localhost,root; connection con1; @@ -37,24 +36,26 @@ connection default; SET DEBUG_SYNC = 'RESET'; -source include/wait_until_count_sessions.inc; - ---echo End of 5.5 tests +--echo # +--echo # End of 5.5 tests +--echo # --echo # --echo # MDEV-23752: SHOW EXPLAIN FOR thd waits for sleep --echo # ---connect (con1,localhost,root,,) +--connect con1,localhost,root --let $con_id = `SELECT CONNECTION_ID()` ---send select sleep(100000); +--send select sleep(100000) --connection default +let $wait_condition= SELECT COUNT(*)=1 FROM information_schema.processlist where state='User sleep'; +source include/wait_condition.inc; +evalp SHOW EXPLAIN FOR $con_id; +evalp KILL QUERY $con_id; +disconnect con1; +source include/wait_until_count_sessions.inc; ---replace_result $con_id con_id -eval SHOW EXPLAIN FOR $con_id; - ---replace_result $con_id con_id -eval KILL QUERY $con_id; - +--echo # --echo # End of 10.2 tests +--echo # From 8b6c8a6ce96c2c8ce6fde16049fcd60eb1547aed Mon Sep 17 00:00:00 2001 From: Oleksandr Byelkin Date: Mon, 2 Aug 2021 10:30:18 +0200 Subject: [PATCH 5/6] Revert "MDEV-26220 Server crashes with indexed by prefix virtual column" This reverts commit 9b8e207ce03b2ab7a766348738055be9520561bd. --- mysql-test/suite/gcol/inc/gcol_partition.inc | 12 -- .../suite/gcol/r/gcol_partition_innodb.result | 11 -- .../suite/gcol/r/gcol_partition_myisam.result | 11 -- sql/table.cc | 113 ++++++++---------- 4 files changed, 48 insertions(+), 99 deletions(-) diff --git a/mysql-test/suite/gcol/inc/gcol_partition.inc b/mysql-test/suite/gcol/inc/gcol_partition.inc index 50a743c0153..4e4af4f0023 100644 --- a/mysql-test/suite/gcol/inc/gcol_partition.inc +++ b/mysql-test/suite/gcol/inc/gcol_partition.inc @@ -169,15 +169,3 @@ CREATE TABLE t1 ( INSERT INTO t1 () VALUES (),(); UPDATE t1 SET a = 0 WHERE b IS NULL ORDER BY pk; DROP TABLE t1; - ---echo # ---echo # MDEV-26220 Server crashes with indexed by prefix virtual column ---echo # - -CREATE TABLE t1 (pk INT PRIMARY KEY, a INT, b CHAR(20), c CHAR(20) AS (b), - KEY (c(10),a)) PARTITION BY HASH(pk); -INSERT INTO t1 (pk,a,b) VALUES (1,10,'foo'),(2,11,'baz'); -SELECT a FROM t1; - -# Cleanup -DROP TABLE t1; diff --git a/mysql-test/suite/gcol/r/gcol_partition_innodb.result b/mysql-test/suite/gcol/r/gcol_partition_innodb.result index e61c0a26417..d3f211c9b9a 100644 --- a/mysql-test/suite/gcol/r/gcol_partition_innodb.result +++ b/mysql-test/suite/gcol/r/gcol_partition_innodb.result @@ -104,17 +104,6 @@ INSERT INTO t1 () VALUES (),(); UPDATE t1 SET a = 0 WHERE b IS NULL ORDER BY pk; DROP TABLE t1; # -# MDEV-26220 Server crashes with indexed by prefix virtual column -# -CREATE TABLE t1 (pk INT PRIMARY KEY, a INT, b CHAR(20), c CHAR(20) AS (b), -KEY (c(10),a)) PARTITION BY HASH(pk); -INSERT INTO t1 (pk,a,b) VALUES (1,10,'foo'),(2,11,'baz'); -SELECT a FROM t1; -a -11 -10 -DROP TABLE t1; -# # MDEV-16980 Wrongly set tablename len while opening the # table for purge thread # diff --git a/mysql-test/suite/gcol/r/gcol_partition_myisam.result b/mysql-test/suite/gcol/r/gcol_partition_myisam.result index e54b0ad83c6..75e216f903b 100644 --- a/mysql-test/suite/gcol/r/gcol_partition_myisam.result +++ b/mysql-test/suite/gcol/r/gcol_partition_myisam.result @@ -101,17 +101,6 @@ KEY (b,d) INSERT INTO t1 () VALUES (),(); UPDATE t1 SET a = 0 WHERE b IS NULL ORDER BY pk; DROP TABLE t1; -# -# MDEV-26220 Server crashes with indexed by prefix virtual column -# -CREATE TABLE t1 (pk INT PRIMARY KEY, a INT, b CHAR(20), c CHAR(20) AS (b), -KEY (c(10),a)) PARTITION BY HASH(pk); -INSERT INTO t1 (pk,a,b) VALUES (1,10,'foo'),(2,11,'baz'); -SELECT a FROM t1; -a -11 -10 -DROP TABLE t1; DROP VIEW IF EXISTS v1,v2; DROP TABLE IF EXISTS t1,t2,t3; DROP PROCEDURE IF EXISTS p1; diff --git a/sql/table.cc b/sql/table.cc index ab2429f469f..b7cf1274400 100644 --- a/sql/table.cc +++ b/sql/table.cc @@ -3034,66 +3034,6 @@ static bool check_vcol_forward_refs(Field *field, Virtual_column_info *vcol) return res; } -/** - Copy keys from share to a table, so that fields in key_parts would match - fields in a table. - - Also if key_part uses a field prefix, clone a field and make its length match - that prefix. - - @retval true if success - @retval false if memory allocation fails - */ -static bool initialize_keys(TABLE_SHARE *share, TABLE *outparam) -{ - if (share->key_parts == 0) - return true; - uint n_length= share->keys * sizeof(KEY) - + share->ext_key_parts * sizeof(KEY_PART_INFO); - - KEY *key_info= (KEY*) alloc_root(&outparam->mem_root, n_length); - if (!key_info) - return false; - - outparam->key_info= key_info; - - KEY *key_info_end= key_info + share->keys; - KEY_PART_INFO *key_part= reinterpret_cast(key_info_end); - - memcpy(key_info, share->key_info, sizeof(*key_info) * share->keys); - memcpy(key_part, share->key_info[0].key_part, (sizeof(*key_part) * - share->ext_key_parts)); - - for ( ; key_info < key_info_end; key_info++) - { - key_info->table= outparam; - key_info->key_part= key_part; - - KEY_PART_INFO *key_part_end= key_part + (share->use_ext_keys - ? key_info->ext_key_parts - : key_info->user_defined_key_parts); - for ( ; key_part < key_part_end; key_part++) - { - Field *field= key_part->field= outparam->field[key_part->fieldnr - 1]; - - if (field->key_length() != key_part->length && - !(field->flags & BLOB_FLAG)) - { - /* - We are using only a prefix of the column as a key: - Create a new field for the key part that matches the index - */ - field= key_part->field=field->make_new_field(&outparam->mem_root, - outparam, 0); - field->field_length= key_part->length; - } - } - if (!share->use_ext_keys) - key_part+= key_info->ext_key_parts - key_info->user_defined_key_parts; - } - return true; -} - /* Open a table based on a TABLE_SHARE @@ -3133,7 +3073,6 @@ enum open_frm_error open_table_from_share(THD *thd, TABLE_SHARE *share, Field **field_ptr; uint8 save_context_analysis_only= thd->lex->context_analysis_only; TABLE_SHARE::enum_v_keys check_set_initialized= share->check_set_initialized; - bool success; DBUG_ENTER("open_table_from_share"); DBUG_PRINT("enter",("name: '%s.%s' form: %p", share->db.str, share->table_name.str, outparam)); @@ -3243,6 +3182,54 @@ enum open_frm_error open_table_from_share(THD *thd, TABLE_SHARE *share, outparam->found_next_number_field= outparam->field[(uint) (share->found_next_number_field - share->field)]; + /* Fix key->name and key_part->field */ + if (share->key_parts) + { + KEY *key_info, *key_info_end; + KEY_PART_INFO *key_part; + uint n_length; + n_length= share->keys*sizeof(KEY) + share->ext_key_parts*sizeof(KEY_PART_INFO); + if (!(key_info= (KEY*) alloc_root(&outparam->mem_root, n_length))) + goto err; + outparam->key_info= key_info; + key_part= (reinterpret_cast(key_info+share->keys)); + + memcpy(key_info, share->key_info, sizeof(*key_info)*share->keys); + memcpy(key_part, share->key_info[0].key_part, (sizeof(*key_part) * + share->ext_key_parts)); + + for (key_info_end= key_info + share->keys ; + key_info < key_info_end ; + key_info++) + { + KEY_PART_INFO *key_part_end; + + key_info->table= outparam; + key_info->key_part= key_part; + + key_part_end= key_part + (share->use_ext_keys ? key_info->ext_key_parts : + key_info->user_defined_key_parts) ; + for ( ; key_part < key_part_end; key_part++) + { + Field *field= key_part->field= outparam->field[key_part->fieldnr - 1]; + + if (field->key_length() != key_part->length && + !(field->flags & BLOB_FLAG)) + { + /* + We are using only a prefix of the column as a key: + Create a new field for the key part that matches the index + */ + field= key_part->field=field->make_new_field(&outparam->mem_root, + outparam, 0); + field->field_length= key_part->length; + } + } + if (!share->use_ext_keys) + key_part+= key_info->ext_key_parts - key_info->user_defined_key_parts; + } + } + /* Process virtual and default columns, if any. */ @@ -3299,10 +3286,6 @@ enum open_frm_error open_table_from_share(THD *thd, TABLE_SHARE *share, switch_defaults_to_nullable_trigger_fields(outparam); } - success= initialize_keys(share, outparam); - if (!success) - goto err; - #ifdef WITH_PARTITION_STORAGE_ENGINE if (share->partition_info_str_len && outparam->file) { From b549af69137023ce0f93d312a10d61e467dca07f Mon Sep 17 00:00:00 2001 From: Nikita Malyavin Date: Fri, 23 Jul 2021 14:16:34 +0300 Subject: [PATCH 6/6] MDEV-26220 Server crashes with indexed by prefix virtual column Server crashes in Field::register_field_in_read_map upon select from partitioned table with indexed by prefix virtual column. After several read-mark fixes a problem has surfaced: Since KEY (c(10),a) uses only a prefix of c, a new field is created, duplicated from table->field[3], with a new length. However, vcol_inco->expr is not copied. Therefore, (*key_info)->key_part[i].field->vcol_info->expr was left NULL in ha_partition::index_init(). Solution: copy vcol_info from table field when it's set up. --- mysql-test/suite/gcol/inc/gcol_partition.inc | 12 ++++++++++++ .../suite/gcol/r/gcol_partition_innodb.result | 11 +++++++++++ .../suite/gcol/r/gcol_partition_myisam.result | 11 +++++++++++ sql/table.cc | 15 +++++++++++++++ 4 files changed, 49 insertions(+) diff --git a/mysql-test/suite/gcol/inc/gcol_partition.inc b/mysql-test/suite/gcol/inc/gcol_partition.inc index 4e4af4f0023..50a743c0153 100644 --- a/mysql-test/suite/gcol/inc/gcol_partition.inc +++ b/mysql-test/suite/gcol/inc/gcol_partition.inc @@ -169,3 +169,15 @@ CREATE TABLE t1 ( INSERT INTO t1 () VALUES (),(); UPDATE t1 SET a = 0 WHERE b IS NULL ORDER BY pk; DROP TABLE t1; + +--echo # +--echo # MDEV-26220 Server crashes with indexed by prefix virtual column +--echo # + +CREATE TABLE t1 (pk INT PRIMARY KEY, a INT, b CHAR(20), c CHAR(20) AS (b), + KEY (c(10),a)) PARTITION BY HASH(pk); +INSERT INTO t1 (pk,a,b) VALUES (1,10,'foo'),(2,11,'baz'); +SELECT a FROM t1; + +# Cleanup +DROP TABLE t1; diff --git a/mysql-test/suite/gcol/r/gcol_partition_innodb.result b/mysql-test/suite/gcol/r/gcol_partition_innodb.result index d3f211c9b9a..e61c0a26417 100644 --- a/mysql-test/suite/gcol/r/gcol_partition_innodb.result +++ b/mysql-test/suite/gcol/r/gcol_partition_innodb.result @@ -104,6 +104,17 @@ INSERT INTO t1 () VALUES (),(); UPDATE t1 SET a = 0 WHERE b IS NULL ORDER BY pk; DROP TABLE t1; # +# MDEV-26220 Server crashes with indexed by prefix virtual column +# +CREATE TABLE t1 (pk INT PRIMARY KEY, a INT, b CHAR(20), c CHAR(20) AS (b), +KEY (c(10),a)) PARTITION BY HASH(pk); +INSERT INTO t1 (pk,a,b) VALUES (1,10,'foo'),(2,11,'baz'); +SELECT a FROM t1; +a +11 +10 +DROP TABLE t1; +# # MDEV-16980 Wrongly set tablename len while opening the # table for purge thread # diff --git a/mysql-test/suite/gcol/r/gcol_partition_myisam.result b/mysql-test/suite/gcol/r/gcol_partition_myisam.result index 75e216f903b..e54b0ad83c6 100644 --- a/mysql-test/suite/gcol/r/gcol_partition_myisam.result +++ b/mysql-test/suite/gcol/r/gcol_partition_myisam.result @@ -101,6 +101,17 @@ KEY (b,d) INSERT INTO t1 () VALUES (),(); UPDATE t1 SET a = 0 WHERE b IS NULL ORDER BY pk; DROP TABLE t1; +# +# MDEV-26220 Server crashes with indexed by prefix virtual column +# +CREATE TABLE t1 (pk INT PRIMARY KEY, a INT, b CHAR(20), c CHAR(20) AS (b), +KEY (c(10),a)) PARTITION BY HASH(pk); +INSERT INTO t1 (pk,a,b) VALUES (1,10,'foo'),(2,11,'baz'); +SELECT a FROM t1; +a +11 +10 +DROP TABLE t1; DROP VIEW IF EXISTS v1,v2; DROP TABLE IF EXISTS t1,t2,t3; DROP PROCEDURE IF EXISTS p1; diff --git a/sql/table.cc b/sql/table.cc index b7cf1274400..281b8f82abc 100644 --- a/sql/table.cc +++ b/sql/table.cc @@ -3284,6 +3284,21 @@ enum open_frm_error open_table_from_share(THD *thd, TABLE_SHARE *share, /* Update to use trigger fields */ switch_defaults_to_nullable_trigger_fields(outparam); + + for (uint k= 0; k < share->keys; k++) + { + KEY &key_info= outparam->key_info[k]; + uint parts = (share->use_ext_keys ? key_info.ext_key_parts : + key_info.user_defined_key_parts); + for (uint p= 0; p < parts; p++) + { + KEY_PART_INFO &kp= key_info.key_part[p]; + if (kp.field != outparam->field[kp.fieldnr - 1]) + { + kp.field->vcol_info = outparam->field[kp.fieldnr - 1]->vcol_info; + } + } + } } #ifdef WITH_PARTITION_STORAGE_ENGINE