From 720e9bd5bed73fdc5b2452a6400431e08ae7eb6f Mon Sep 17 00:00:00 2001 From: Nikita Malyavin Date: Fri, 22 Mar 2019 16:26:42 +1000 Subject: [PATCH 1/3] MDEV-18875 Assertion `thd->transaction.stmt.ha_list == __null || trans == &thd->transaction.stmt' failed or bogus ER_DUP_ENTRY upon ALTER TABLE with versioning Cause: * when autocommit=0 (or transaction is issued by user), `ha_commit_trans` is called twice on ALTER TABLE, causing a duplicated insert into `transaction_registry` (ER_DUP_ENTRY). Solution: * ALTER TABLE makes an implicit commit by a second call. We actually need to make an insert only when it is a real commit. So is_real variable is additionally checked. --- mysql-test/suite/versioning/r/trx_id.result | 13 +++++++++++++ mysql-test/suite/versioning/t/trx_id.test | 15 +++++++++++++++ sql/handler.cc | 3 ++- 3 files changed, 30 insertions(+), 1 deletion(-) diff --git a/mysql-test/suite/versioning/r/trx_id.result b/mysql-test/suite/versioning/r/trx_id.result index 413272f55f9..7c4a540c52a 100644 --- a/mysql-test/suite/versioning/r/trx_id.result +++ b/mysql-test/suite/versioning/r/trx_id.result @@ -476,3 +476,16 @@ SET @@SYSTEM_VERSIONING_ALTER_HISTORY=ERROR; SELECT count(*) from mysql.transaction_registry where begin_timestamp>=commit_timestamp; count(*) 0 +# MDEV-18875 Assertion `thd->transaction.stmt.ha_list == __null || +# trans == &thd->transaction.stmt' failed or bogus ER_DUP_ENTRY upon +# ALTER TABLE with versioning +create or replace table t (x int) engine=innodb; +set autocommit= 0; +alter table t +algorithm=copy, +add column row_start bigint unsigned as row start, +add column row_end bigint unsigned as row end, +add period for system_time(row_start,row_end), +with system versioning; +set autocommit= 1; +create or replace database test; diff --git a/mysql-test/suite/versioning/t/trx_id.test b/mysql-test/suite/versioning/t/trx_id.test index 617c46a9332..97935ffc975 100644 --- a/mysql-test/suite/versioning/t/trx_id.test +++ b/mysql-test/suite/versioning/t/trx_id.test @@ -498,3 +498,18 @@ DROP TABLE t; SET @@SYSTEM_VERSIONING_ALTER_HISTORY=ERROR; SELECT count(*) from mysql.transaction_registry where begin_timestamp>=commit_timestamp; + +--echo # MDEV-18875 Assertion `thd->transaction.stmt.ha_list == __null || +--echo # trans == &thd->transaction.stmt' failed or bogus ER_DUP_ENTRY upon +--echo # ALTER TABLE with versioning +create or replace table t (x int) engine=innodb; +set autocommit= 0; +alter table t + algorithm=copy, + add column row_start bigint unsigned as row start, + add column row_end bigint unsigned as row end, + add period for system_time(row_start,row_end), + with system versioning; +set autocommit= 1; + +create or replace database test; diff --git a/sql/handler.cc b/sql/handler.cc index 1494060e24d..4e2c6afda80 100644 --- a/sql/handler.cc +++ b/sql/handler.cc @@ -1446,7 +1446,8 @@ int ha_commit_trans(THD *thd, bool all) #if 1 // FIXME: This should be done in ha_prepare(). if (rw_trans || (thd->lex->sql_command == SQLCOM_ALTER_TABLE && - thd->lex->alter_info.flags & ALTER_ADD_SYSTEM_VERSIONING)) + thd->lex->alter_info.flags & ALTER_ADD_SYSTEM_VERSIONING && + is_real_trans)) { ulonglong trx_start_id= 0, trx_end_id= 0; for (Ha_trx_info *ha_info= trans->ha_list; ha_info; ha_info= ha_info->next()) From 4923604ee2fba372f28c856a3f41274f7fc6c1c5 Mon Sep 17 00:00:00 2001 From: Nikita Malyavin Date: Sun, 24 Mar 2019 19:01:54 +1000 Subject: [PATCH 2/3] MDEV-18865 Assertion `t->first->versioned_by_id()' failed in innodb_prepare_commit_versioned Cause: * row_start != 0 treated as it exists. Probably, possible row permutations had not been taken in mind. Solution: * Checking both row_start and row_end is correct, so versioned() function is used --- mysql-test/suite/versioning/r/trx_id.result | 9 +++++++++ mysql-test/suite/versioning/t/trx_id.test | 12 ++++++++++++ storage/innobase/handler/ha_innodb.cc | 9 +++++++++ storage/innobase/include/dict0mem.h | 6 +++++- 4 files changed, 35 insertions(+), 1 deletion(-) diff --git a/mysql-test/suite/versioning/r/trx_id.result b/mysql-test/suite/versioning/r/trx_id.result index 7c4a540c52a..a140052439c 100644 --- a/mysql-test/suite/versioning/r/trx_id.result +++ b/mysql-test/suite/versioning/r/trx_id.result @@ -488,4 +488,13 @@ add column row_end bigint unsigned as row end, add period for system_time(row_start,row_end), with system versioning; set autocommit= 1; +# MDEV-18865 Assertion `t->first->versioned_by_id()' +# failed in innodb_prepare_commit_versioned +create or replace table t (x int) engine=innodb; +insert into t values (0); +alter table t add `row_start` bigint unsigned as row start, +add `row_end` bigint unsigned as row end, +add period for system_time(`row_start`,`row_end`), +modify x int after row_start, +with system versioning; create or replace database test; diff --git a/mysql-test/suite/versioning/t/trx_id.test b/mysql-test/suite/versioning/t/trx_id.test index 97935ffc975..38724a47fd1 100644 --- a/mysql-test/suite/versioning/t/trx_id.test +++ b/mysql-test/suite/versioning/t/trx_id.test @@ -512,4 +512,16 @@ alter table t with system versioning; set autocommit= 1; +--echo # MDEV-18865 Assertion `t->first->versioned_by_id()' +--echo # failed in innodb_prepare_commit_versioned + +create or replace table t (x int) engine=innodb; +insert into t values (0); +alter table t add `row_start` bigint unsigned as row start, + add `row_end` bigint unsigned as row end, + add period for system_time(`row_start`,`row_end`), + modify x int after row_start, + with system versioning; + + create or replace database test; diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index e5387cd256e..6b0b49908c8 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -10905,6 +10905,9 @@ create_table_info_t::create_table_def() heap = mem_heap_create(1000); + ut_d(bool have_vers_start = false); + ut_d(bool have_vers_end = false); + for (ulint i = 0, j = 0; j < n_cols; i++) { Field* field = m_form->field[i]; ulint vers_row = 0; @@ -10912,8 +10915,10 @@ create_table_info_t::create_table_def() if (m_form->versioned()) { if (i == m_form->s->row_start_field) { vers_row = DATA_VERS_START; + ut_d(have_vers_start = true); } else if (i == m_form->s->row_end_field) { vers_row = DATA_VERS_END; + ut_d(have_vers_end = true); } else if (!(field->flags & VERS_UPDATE_UNVERSIONED_FLAG)) { vers_row = DATA_VERSIONED; @@ -11034,6 +11039,10 @@ err_col: j++; } + ut_ad(have_vers_start == have_vers_end); + ut_ad(table->versioned() == have_vers_start); + ut_ad(!table->versioned() || table->vers_start != table->vers_end); + if (num_v) { for (ulint i = 0, j = 0; i < n_cols; i++) { dict_v_col_t* v_col; diff --git a/storage/innobase/include/dict0mem.h b/storage/innobase/include/dict0mem.h index 8c5314ed39d..1a22470569e 100644 --- a/storage/innobase/include/dict0mem.h +++ b/storage/innobase/include/dict0mem.h @@ -1603,10 +1603,14 @@ struct dict_table_t { /** Add the table definition to the data dictionary cache */ void add_to_cache(); + /** @return whether the table is versioned. + It is assumed that both vers_start and vers_end set to 0 + iff table is not versioned. In any other case, + these fields correspond to actual positions in cols[]. */ bool versioned() const { return vers_start || vers_end; } bool versioned_by_id() const { - return vers_start && cols[vers_start].mtype == DATA_INT; + return versioned() && cols[vers_start].mtype == DATA_INT; } void inc_fk_checks() From 02e30069573b09f5d26b6c357cd8aee3e876385a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Mon, 30 Dec 2019 10:08:18 +0200 Subject: [PATCH 3/3] MDEV-21405 Assertion failed on instant ADD COLUMN btr_cur_pessimistic_insert(): Relax a too strict debug assertion that would fail when the function is invoked by btr_cur_pessimistic_update() during innobase_add_instant_try(), that is, when updating the hidden metadata record during a subsequent ADD COLUMN operation involves splitting the leftmost clustered index leaf page. This is a partial backport of 301bd62b2536f85a8ce418dcd5e705796d8c5763 from 10.4. --- storage/innobase/btr/btr0cur.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/storage/innobase/btr/btr0cur.cc b/storage/innobase/btr/btr0cur.cc index 5b14a19d9b8..328ed21841c 100644 --- a/storage/innobase/btr/btr0cur.cc +++ b/storage/innobase/btr/btr0cur.cc @@ -3666,8 +3666,8 @@ btr_cur_pessimistic_insert( if (entry->info_bits & REC_INFO_MIN_REC_FLAG) { ut_ad(entry->info_bits == REC_INFO_METADATA); ut_ad(index->is_instant()); - ut_ad((flags & ulint(~BTR_KEEP_IBUF_BITMAP)) - == BTR_NO_LOCKING_FLAG); + ut_ad(flags & BTR_NO_LOCKING_FLAG); + ut_ad(!(flags & BTR_CREATE_FLAG)); } else { btr_search_update_hash_on_insert( cursor, btr_get_search_latch(index));