From cb437417d2214474102a11f0a207bb3bc86252ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Tue, 19 May 2020 12:33:47 +0300 Subject: MDEV-19114 gcol.innodb_virtual_debug: Assertion n_fields>0 failed This is a regression due to MDEV-16376 commit 8dc70c862b8ec115fd9a3c2b37c746ffc4f0d3cc. To make dict_index_t::detach_columns() idempotent, we cleared dict_index_t::n_fields. But, this could cause trouble with purge after a secondary index creation failed (not even involving virtual columns). A better way is to clear the dict_field_t::col pointers that point to virtual columns that are being freed due to aborting index creation on an index that depends on a virtual column. Note: the v_cols[] of an existing dict_table_t object will never be modified. If any virtual columns are added or removed, ha_innobase::commit_inplace_alter_table() would invoke dict_table_remove_from_cache() and reload the table to dict_sys. Index creation is a special case where the dict_index_t points to virtual columns that do not yet exist in dict_table_t. --- storage/innobase/dict/dict0dict.cc | 2 +- storage/innobase/handler/handler0alter.cc | 16 +++---- storage/innobase/include/dict0mem.h | 69 +++++++++++++++++-------------- 3 files changed, 45 insertions(+), 42 deletions(-) diff --git a/storage/innobase/dict/dict0dict.cc b/storage/innobase/dict/dict0dict.cc index cff213dea3d..3dfbe92e34c 100644 --- a/storage/innobase/dict/dict0dict.cc +++ b/storage/innobase/dict/dict0dict.cc @@ -2200,7 +2200,7 @@ dict_index_remove_from_v_col_list(dict_index_t* index) { for (ulint i = 0; i < dict_index_get_n_fields(index); i++) { col = dict_index_get_nth_col(index, i); - if (dict_col_is_virtual(col)) { + if (col && col->is_virtual()) { vcol = reinterpret_cast( col); /* This could be NULL, when we do add diff --git a/storage/innobase/handler/handler0alter.cc b/storage/innobase/handler/handler0alter.cc index 9b1d99d3730..9afbcf4d1f4 100644 --- a/storage/innobase/handler/handler0alter.cc +++ b/storage/innobase/handler/handler0alter.cc @@ -248,16 +248,12 @@ struct ha_innobase_inplace_ctx : public inplace_alter_handler_ctx @return whether the table will be rebuilt */ bool need_rebuild () const { return(old_table != new_table); } - /** Clear uncommmitted added indexes after a failed operation. */ - void clear_added_indexes() - { - for (ulint i = 0; i < num_to_add_index; i++) { - if (!add_index[i]->is_committed()) { - add_index[i]->detach_columns(); - add_index[i]->n_fields = 0; - } - } - } + /** Clear uncommmitted added indexes after a failed operation. */ + void clear_added_indexes() + { + for (ulint i= 0; i < num_to_add_index; i++) + add_index[i]->detach_columns(true); + } /** Share context between partitions. @param[in] ctx context from another partition of the table */ diff --git a/storage/innobase/include/dict0mem.h b/storage/innobase/include/dict0mem.h index 80d18dcbb2a..0d4ee9d23ec 100644 --- a/storage/innobase/include/dict0mem.h +++ b/storage/innobase/include/dict0mem.h @@ -634,12 +634,12 @@ struct dict_col_t{ this column. Our current max limit is 3072 for Barracuda table */ - /** @return whether this is a virtual column */ - bool is_virtual() const { return prtype & DATA_VIRTUAL; } + /** @return whether this is a virtual column */ + bool is_virtual() const { return prtype & DATA_VIRTUAL; } - /** Detach the column from an index. - @param[in] index index to be detached from */ - inline void detach(const dict_index_t& index); + /** Detach a virtual column from an index. + @param index being-freed index */ + inline void detach(const dict_index_t &index); }; /** Index information put in a list of virtual column structure. Index @@ -1017,15 +1017,22 @@ struct dict_index_t{ /** @return whether the index is corrupted */ inline bool is_corrupted() const; - /** Detach the columns from the index that is to be freed. */ - void detach_columns() - { - if (has_virtual()) { - for (unsigned i = 0; i < n_fields; i++) { - fields[i].col->detach(*this); - } - } - } + /** Detach the virtual columns from the index that is to be removed. + @param whether to reset fields[].col */ + void detach_columns(bool clear= false) + { + if (!has_virtual()) + return; + for (unsigned i= 0; i < n_fields; i++) + { + dict_col_t* col= fields[i].col; + if (!col || !col->is_virtual()) + continue; + col->detach(*this); + if (clear) + fields[i].col= NULL; + } + } #ifdef BTR_CUR_HASH_ADAPT /** @return a clone of this */ @@ -1102,24 +1109,24 @@ struct dict_index_t{ inline record_size_info_t record_size_info() const; }; -/** Detach a column from an index. -@param[in] index index to be detached from */ -inline void dict_col_t::detach(const dict_index_t& index) +/** Detach a virtual column from an index. +@param index being-freed index */ +inline void dict_col_t::detach(const dict_index_t &index) { - if (!is_virtual()) { - return; - } - - if (dict_v_idx_list* v_indexes = reinterpret_cast - (this)->v_indexes) { - for (dict_v_idx_list::iterator i = v_indexes->begin(); - i != v_indexes->end(); i++) { - if (i->index == &index) { - v_indexes->erase(i); - return; - } - } - } + ut_ad(is_virtual()); + + if (dict_v_idx_list *v_indexes= reinterpret_cast(this) + ->v_indexes) + { + for (dict_v_idx_list::iterator i= v_indexes->begin(); + i != v_indexes->end(); i++) + { + if (i->index == &index) { + v_indexes->erase(i); + return; + } + } + } } /** The status of online index creation */ -- cgit v1.2.1