diff options
| -rw-r--r-- | mysql-test/suite/gcol/r/virtual_index_drop.result | 28 | ||||
| -rw-r--r-- | mysql-test/suite/gcol/t/virtual_index_drop.test | 28 | ||||
| -rw-r--r-- | storage/innobase/dict/dict0mem.cc | 2 | ||||
| -rw-r--r-- | storage/innobase/handler/handler0alter.cc | 92 | ||||
| -rw-r--r-- | storage/innobase/include/dict0mem.h | 80 | ||||
| -rw-r--r-- | storage/innobase/include/row0merge.h | 21 | ||||
| -rw-r--r-- | storage/innobase/row/row0merge.cc | 29 | ||||
| -rw-r--r-- | storage/innobase/row/row0purge.cc | 2 |
8 files changed, 233 insertions, 49 deletions
diff --git a/mysql-test/suite/gcol/r/virtual_index_drop.result b/mysql-test/suite/gcol/r/virtual_index_drop.result new file mode 100644 index 00000000000..29caa04e464 --- /dev/null +++ b/mysql-test/suite/gcol/r/virtual_index_drop.result @@ -0,0 +1,28 @@ +CREATE TABLE t1(f1 INT, f2 INT AS (f1 + 2) VIRTUAL)ENGINE=InnoDB; +INSERT INTO t1(f1) VALUES(1), (1); +ALTER TABLE t1 ADD UNIQUE INDEX(f2), ALGORITHM=INPLACE, LOCK=EXCLUSIVE; +ERROR 23000: Duplicate entry '3' for key 'f2' +SHOW CREATE TABLE t1; +Table Create Table +t1 CREATE TABLE `t1` ( + `f1` int(11) DEFAULT NULL, + `f2` int(11) GENERATED ALWAYS AS (`f1` + 2) VIRTUAL +) ENGINE=InnoDB DEFAULT CHARSET=latin1 +DROP TABLE t1; +CREATE TABLE t1(f1 INT, f2 INT AS (f1) VIRTUAL)ENGINE=InnoDB; +SET DEBUG_DBUG="+d,create_index_fail"; +SET DEBUG_SYNC="innodb_inplace_alter_table_enter SIGNAL con1_go WAIT_FOR alter_signal"; +ALTER TABLE t1 ADD INDEX(f2); +connect con1,localhost,root,,,; +SET DEBUG_SYNC="now WAIT_FOR con1_go"; +BEGIN; +INSERT INTO t1(f1) VALUES(1); +SET DEBUG_SYNC="now SIGNAL alter_signal"; +connection default; +ERROR 23000: Duplicate entry '' for key '*UNKNOWN*' +connection con1; +rollback; +connection default; +disconnect con1; +DROP TABLE t1; +SET DEBUG_SYNC=RESET; diff --git a/mysql-test/suite/gcol/t/virtual_index_drop.test b/mysql-test/suite/gcol/t/virtual_index_drop.test new file mode 100644 index 00000000000..de3644bc34b --- /dev/null +++ b/mysql-test/suite/gcol/t/virtual_index_drop.test @@ -0,0 +1,28 @@ +--source include/have_innodb.inc +--source include/have_debug.inc + +CREATE TABLE t1(f1 INT, f2 INT AS (f1 + 2) VIRTUAL)ENGINE=InnoDB; +INSERT INTO t1(f1) VALUES(1), (1); +--error ER_DUP_ENTRY +ALTER TABLE t1 ADD UNIQUE INDEX(f2), ALGORITHM=INPLACE, LOCK=EXCLUSIVE; +SHOW CREATE TABLE t1; +DROP TABLE t1; + +CREATE TABLE t1(f1 INT, f2 INT AS (f1) VIRTUAL)ENGINE=InnoDB; +SET DEBUG_DBUG="+d,create_index_fail"; +SET DEBUG_SYNC="innodb_inplace_alter_table_enter SIGNAL con1_go WAIT_FOR alter_signal"; +send ALTER TABLE t1 ADD INDEX(f2); +connect(con1,localhost,root,,,); +SET DEBUG_SYNC="now WAIT_FOR con1_go"; +BEGIN; +INSERT INTO t1(f1) VALUES(1); +SET DEBUG_SYNC="now SIGNAL alter_signal"; +connection default; +--error ER_DUP_ENTRY +reap; +connection con1; +rollback; +connection default; +disconnect con1; +DROP TABLE t1; +SET DEBUG_SYNC=RESET; diff --git a/storage/innobase/dict/dict0mem.cc b/storage/innobase/dict/dict0mem.cc index f4a56faaf28..7bf72dd3b3d 100644 --- a/storage/innobase/dict/dict0mem.cc +++ b/storage/innobase/dict/dict0mem.cc @@ -937,7 +937,7 @@ dict_mem_fill_vcol_from_v_indexes( Later virtual column set will be refreshed during loading of table. */ if (!dict_index_has_virtual(index) - || index->has_new_v_col) { + || index->has_new_v_col()) { continue; } diff --git a/storage/innobase/handler/handler0alter.cc b/storage/innobase/handler/handler0alter.cc index 58e17cd3ead..873e9065515 100644 --- a/storage/innobase/handler/handler0alter.cc +++ b/storage/innobase/handler/handler0alter.cc @@ -1023,13 +1023,6 @@ 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++) - add_index[i]->detach_columns(true); - } - /** Convert table-rebuilding ALTER to instant ALTER. */ void prepare_instant() { @@ -1076,6 +1069,19 @@ struct ha_innobase_inplace_ctx : public inplace_alter_handler_ctx return instant_table; } + /** @return whether the given column is added as a part of alter */ + bool is_new_vcol(const dict_v_col_t &v_col) const + { + if (num_to_add_vcol == 0) + return false; + for (ulint i= 0; i < num_to_add_vcol; i++) + { + if (&add_vcol[i] == &v_col) + return true; + } + return false; + } + /** Create an index table where indexes are ordered as follows: IF a new primary key is defined for the table THEN @@ -1127,6 +1133,41 @@ struct ha_innobase_inplace_ctx : public inplace_alter_handler_ctx } } + /** During rollback, InnoDB should duplicate the newly added + virtual column in newly added virtual index. It should be + present in new_vcol_info of index. */ + void clean_new_vcol_index() + { + ut_ad(old_table == new_table); + const dict_index_t *index= dict_table_get_first_index(old_table); + while ((index = dict_table_get_next_index(index)) != NULL) + { + if (!index->has_virtual() || !index->has_new_v_col() + || index->is_committed()) + continue; + ulint n_drop_new_vcol= 0; + ulint n_new_vcol= index->get_new_n_vcol(); + index->assign_drop_v_col(); + for (ulint i= 0; i < index->n_fields; i++) + { + dict_col_t *col= index->fields[i].col; + /* Skip the non-virtual and old virtual columns */ + if (!col->is_virtual() + || !is_new_vcol(reinterpret_cast<dict_v_col_t&>(*col))) + continue; + + dict_v_col_t* drop_vcol= index->add_drop_v_col( + reinterpret_cast<dict_v_col_t*>(col), n_drop_new_vcol); + drop_vcol->detach(*index); + /* Re-assign the index field with newly stored virtual column */ + index->fields[i].col = reinterpret_cast<dict_col_t*>(drop_vcol); + n_drop_new_vcol++; + if (n_drop_new_vcol == n_new_vcol) + break; + } + } + } + private: // Disable copying ha_innobase_inplace_ctx(const ha_innobase_inplace_ctx&); @@ -6812,7 +6853,7 @@ new_table_failed: for (ulint a = 0; a < ctx->num_to_add_index; a++) { dict_index_t* index = ctx->add_index[a]; - const bool has_new_v_col = index->has_new_v_col; + const ulint n_v_col = index->get_new_n_vcol(); index = create_index_dict(ctx->trx, index, add_v); error = ctx->trx->error_state; if (error != DB_SUCCESS) { @@ -6842,7 +6883,10 @@ error_handling_drop_uncached_1: goto error_handling_drop_uncached_1; } index->parser = index_defs[a].parser; - index->has_new_v_col = has_new_v_col; + if (n_v_col) { + index->assign_new_v_col(n_v_col); + } + /* Note the id of the transaction that created this index, we use it to restrict readers from accessing this index, to ensure read consistency. */ @@ -6913,7 +6957,7 @@ error_handling_drop_uncached_1: for (ulint a = 0; a < ctx->num_to_add_index; a++) { dict_index_t* index = ctx->add_index[a]; - const bool has_new_v_col = index->has_new_v_col; + const ulint n_v_col = index->get_new_n_vcol(); DBUG_EXECUTE_IF( "create_index_metadata_fail", if (a + 1 == ctx->num_to_add_index) { @@ -6945,7 +6989,9 @@ error_handling_drop_uncached: } index->parser = index_defs[a].parser; - index->has_new_v_col = has_new_v_col; + if (n_v_col) { + index->assign_new_v_col(n_v_col); + } /* Note the id of the transaction that created this index, we use it to restrict readers from accessing this index, to ensure read consistency. */ @@ -8532,7 +8578,6 @@ oom: that we hold at most a shared lock on the table. */ m_prebuilt->trx->error_info = NULL; ctx->trx->error_state = DB_SUCCESS; - ctx->clear_added_indexes(); DBUG_RETURN(true); } @@ -8624,17 +8669,18 @@ temparary index prefix @param table the TABLE @param locked TRUE=table locked, FALSE=may need to do a lazy drop @param trx the transaction -*/ -static MY_ATTRIBUTE((nonnull)) +@param alter_trx transaction which takes S-lock on the table + while creating the index */ +static void innobase_rollback_sec_index( -/*========================*/ - dict_table_t* user_table, - const TABLE* table, - ibool locked, - trx_t* trx) + dict_table_t* user_table, + const TABLE* table, + ibool locked, + trx_t* trx, + const trx_t* alter_trx=NULL) { - row_merge_drop_indexes(trx, user_table, locked); + row_merge_drop_indexes(trx, user_table, locked, alter_trx); /* Free the table->fts only if there is no FTS_DOC_ID in the table */ @@ -8762,7 +8808,11 @@ rollback_inplace_alter_table( } innobase_rollback_sec_index( - prebuilt->table, table, FALSE, ctx->trx); + prebuilt->table, table, + (ha_alter_info->alter_info->requested_lock + == Alter_info::ALTER_TABLE_LOCK_EXCLUSIVE), + ctx->trx, prebuilt->trx); + ctx->clean_new_vcol_index(); } trx_commit_for_mysql(ctx->trx); diff --git a/storage/innobase/include/dict0mem.h b/storage/innobase/include/dict0mem.h index 2d87d7f689f..a5acb15bbc7 100644 --- a/storage/innobase/include/dict0mem.h +++ b/storage/innobase/include/dict0mem.h @@ -790,6 +790,15 @@ struct dict_v_col_t{ } }; +/* Data structure for newly added virtual column in a index. +It is used only during rollback of addition of virtual index +and uses index heap. Should be freed when index is being +removed from cache. */ +struct dict_add_v_col_info { + ulint n_v_col; + dict_v_col_t *v_col; +}; + /** Data structure for newly added virtual column in a table */ struct dict_add_v_col_t{ /** number of new virtual column */ @@ -1037,9 +1046,10 @@ struct dict_index_t { dict_field_t* fields; /*!< array of field descriptions */ st_mysql_ftparser* parser; /*!< fulltext parser plugin */ - bool has_new_v_col; - /*!< whether it has a newly added virtual - column in ALTER */ + /* It just indicates whether newly added virtual column + during alter. It stores column in case of alter failure */ + dict_add_v_col_info* new_vcol_info; + UT_LIST_NODE_T(dict_index_t) indexes;/*!< list of indexes of the table */ #ifdef BTR_CUR_ADAPT @@ -1201,7 +1211,7 @@ public: /** Detach the virtual columns from the index that is to be removed. @param whether to reset fields[].col */ - void detach_columns(bool clear= false) + void detach_columns() { if (!has_virtual() || !cached) return; @@ -1211,8 +1221,6 @@ public: if (!col || !col->is_virtual()) continue; col->detach(*this); - if (clear) - fields[i].col= nullptr; } } @@ -1285,6 +1293,55 @@ public: bool contains_col_or_prefix(ulint n, bool is_virtual) const MY_ATTRIBUTE((warn_unused_result)); + /** Assign the number of new column to be added as a part + of the index + @param n_vcol number of virtual columns to be added */ + void assign_new_v_col(ulint n_vcol) + { + new_vcol_info= static_cast<dict_add_v_col_info*>( + mem_heap_alloc(heap, sizeof *new_vcol_info)); + new_vcol_info->n_v_col = n_vcol; + } + + /* @return whether index has new virtual column */ + bool has_new_v_col() const + { + return new_vcol_info != nullptr; + } + + /* @return number of newly added virtual column */ + ulint get_new_n_vcol() const + { + if (new_vcol_info) + return new_vcol_info->n_v_col; + return 0; + } + + /** Duplicate the newly added virtual column into index + while rollbacking the alter table */ + void assign_drop_v_col() const + { + ut_ad(new_vcol_info); + ut_ad(new_vcol_info->n_v_col); + new_vcol_info->v_col= static_cast<dict_v_col_t*>( + mem_heap_alloc(heap, + new_vcol_info->n_v_col * sizeof *new_vcol_info->v_col)); + } + + /** Add the newly added virtual column while rollbacking + the virtual index which contains those virtual columns + @param col virtual column to be duplicated + @param offset offset where to duplicate virtual column */ + dict_v_col_t* add_drop_v_col(dict_v_col_t *col, ulint offset) const + { + ut_ad(new_vcol_info); + ut_ad(new_vcol_info->n_v_col); + ut_ad(offset < new_vcol_info->n_v_col); + new (&new_vcol_info->v_col[offset]) dict_v_col_t(); + new_vcol_info->v_col[offset].m_col= col->m_col; + new_vcol_info->v_col[offset].v_pos= col->v_pos; + return &new_vcol_info->v_col[offset]; + } #ifdef BTR_CUR_HASH_ADAPT /** @return a clone of this */ dict_index_t* clone() const; @@ -2385,6 +2442,17 @@ public: /** mysql_row_templ_t for base columns used for compute the virtual columns */ dict_vcol_templ_t* vc_templ; + + /* @return whether the table has any other transcation lock + other than the given transaction */ + bool has_lock_for_other_trx(const trx_t *trx) + { + for (lock_t *lock= UT_LIST_GET_FIRST(locks); lock; + lock= UT_LIST_GET_NEXT(un_member.tab_lock.locks, lock)) + if (lock->trx != trx) + return true; + return false; + } }; inline void dict_index_t::set_modified(mtr_t& mtr) const diff --git a/storage/innobase/include/row0merge.h b/storage/innobase/include/row0merge.h index d9410298c01..f5a9cd59d98 100644 --- a/storage/innobase/include/row0merge.h +++ b/storage/innobase/include/row0merge.h @@ -167,18 +167,21 @@ row_merge_drop_indexes_dict( table_id_t table_id)/*!< in: table identifier */ MY_ATTRIBUTE((nonnull)); -/*********************************************************************//** -Drop those indexes which were created before an error occurred. + +/** Drop indexes that were created before an error occurred. The data dictionary must have been locked exclusively by the caller, -because the transaction will not be committed. */ +because the transaction will not be committed. +@param trx dictionary transaction +@param table table containing the indexes +@param locked True if table is locked, + false - may need to do lazy drop +@param alter_trx Alter table transaction */ void row_merge_drop_indexes( -/*===================*/ - trx_t* trx, /*!< in/out: transaction */ - dict_table_t* table, /*!< in/out: table containing the indexes */ - ibool locked) /*!< in: TRUE=table locked, - FALSE=may need to do a lazy drop */ - MY_ATTRIBUTE((nonnull)); + trx_t* trx, + dict_table_t* table, + ibool locked, + const trx_t* alter_trx=NULL); /*********************************************************************//** Drop all partially created indexes during crash recovery. */ diff --git a/storage/innobase/row/row0merge.cc b/storage/innobase/row/row0merge.cc index b21b17be15f..27a0bcbdc70 100644 --- a/storage/innobase/row/row0merge.cc +++ b/storage/innobase/row/row0merge.cc @@ -3790,17 +3790,20 @@ row_merge_drop_indexes_dict( trx->op_info = ""; } -/*********************************************************************//** -Drop indexes that were created before an error occurred. +/** Drop indexes that were created before an error occurred. The data dictionary must have been locked exclusively by the caller, -because the transaction will not be committed. */ +because the transaction will not be committed. +@param trx dictionary transaction +@param table table containing the indexes +@param locked True if table is locked, + false - may need to do lazy drop +@param alter_trx Alter table transaction */ void row_merge_drop_indexes( -/*===================*/ - trx_t* trx, /*!< in/out: dictionary transaction */ - dict_table_t* table, /*!< in/out: table containing the indexes */ - ibool locked) /*!< in: TRUE=table locked, - FALSE=may need to do a lazy drop */ + trx_t* trx, + dict_table_t* table, + ibool locked, + const trx_t* alter_trx) { dict_index_t* index; dict_index_t* next_index; @@ -3823,9 +3826,8 @@ row_merge_drop_indexes( or waiting for a meta-data lock. A concurrent purge will be prevented by dict_sys.latch. */ - if (!locked && (table->get_ref_count() > 1 - || UT_LIST_GET_FIRST(table->locks))) { + || table->has_lock_for_other_trx(alter_trx))) { /* We will have to drop the indexes later, when the table is guaranteed to be no longer in use. Mark the indexes as incomplete and corrupted, so that other @@ -4251,6 +4253,7 @@ row_merge_create_index( dict_index_t* index; ulint n_fields = index_def->n_fields; ulint i; + ulint n_add_vcol = 0; DBUG_ENTER("row_merge_create_index"); @@ -4275,7 +4278,7 @@ row_merge_create_index( ut_ad(ifield->col_no >= table->n_v_def); name = add_v->v_col_name[ ifield->col_no - table->n_v_def]; - index->has_new_v_col = true; + n_add_vcol++; } else { name = dict_table_get_v_col_name( table, ifield->col_no); @@ -4287,6 +4290,10 @@ row_merge_create_index( dict_mem_index_add_field(index, name, ifield->prefix_len); } + if (n_add_vcol) { + index->assign_new_v_col(n_add_vcol); + } + DBUG_RETURN(index); } diff --git a/storage/innobase/row/row0purge.cc b/storage/innobase/row/row0purge.cc index 0fdc1763eb4..a883d37c3fc 100644 --- a/storage/innobase/row/row0purge.cc +++ b/storage/innobase/row/row0purge.cc @@ -615,7 +615,7 @@ row_purge_skip_uncommitted_virtual_index( not support LOCK=NONE when adding an index on newly added virtual column.*/ while (index != NULL && dict_index_has_virtual(index) - && !index->is_committed() && index->has_new_v_col) { + && !index->is_committed() && index->has_new_v_col()) { index = dict_table_get_next_index(index); } } |