|
This was first reported by elenst in MDEV-17721:
diff --git a/mysql-test/suite/innodb/t/instant_alter.test b/mysql-test/suite/innodb/t/instant_alter.test
|
index fd7fbe94450..4dd00a227f9 100644
|
--- a/mysql-test/suite/innodb/t/instant_alter.test
|
+++ b/mysql-test/suite/innodb/t/instant_alter.test
|
@@ -397,6 +397,14 @@ ALTER TABLE t1 ADD COLUMN t TEXT;
|
SELECT * FROM t1;
|
DROP TABLE t1;
|
|
+eval CREATE TABLE t1 (a INT AUTO_INCREMENT, b INT, KEY(a)) $engine;
|
+INSERT INTO t1 SET a=NULL;
|
+ALTER TABLE t1 DROP COLUMN b;
|
+INSERT INTO t1 SET a=NULL;
|
+UPDATE t1 SET a=a+2;
|
+SELECT * FROM t1;
|
+DROP TABLE t1;
|
+
|
dec $format;
|
}
|
disconnect analyze;
|
With this change to the test, InnoDB would crash as follows:
|
10.4 89337d510e9780d6533c3db4d42969b2338a7610
|
CURRENT_TEST: innodb.instant_alter
|
mysqltest: At line 402: query 'ALTER TABLE t1 DROP COLUMN b' failed: 2013: Lost connection to MySQL server during query
|
…
|
2018-11-16 12:36:36 0x7ff5ac4c1700 InnoDB: Assertion failure in file /mariadb/10.4/storage/innobase/include/row0row.ic line 215
|
…
|
#6 0x000056295ac4c0e5 in ut_dbg_assertion_failed (expr=0x0, file=0x56295b2b6780 "/mariadb/10.4/storage/innobase/include/row0row.ic", line=215) at /mariadb/10.4/storage/innobase/ut/ut0dbg.cc:61
|
#7 0x000056295ab702a2 in row_parse_int (data=0x7ff5541cfbf0 "", len=8, mtype=5, unsigned_type=false) at /mariadb/10.4/storage/innobase/include/row0row.ic:215
|
#8 0x000056295ab6c432 in row_ins_clust_index_entry_low (flags=2, mode=33, index=0x7ff554106a28, n_uniq=1, entry=0x7ff5541cfb18, n_ext=0, thr=0x7ff5541cfcb8, dup_chk_only=false) at /mariadb/10.4/storage/innobase/row/row0ins.cc:2613
|
#9 0x000056295aa6726c in innobase_instant_try (ha_alter_info=0x7ff5ac4bdb60, ctx=0x7ff5540153e8, altered_table=0x7ff5541cd528, table=0x7ff55419c618, trx=0x7ff5ac75b300) at /mariadb/10.4/storage/innobase/handler/handler0alter.cc:5522
|
This should only occur if a table contains an AUTO_INCREMENT column that is not part of the PRIMARY KEY, and if the first instant ALTER TABLE is one of the MDEV-15562 operations: permuting columns, adding a column somewhere else than the last column, or dropping a column.
If there was a prior MDEV-11369 instant ADD COLUMN operation on the table, then this code would not be executed.
This code is part of the MDEV-6076 logic, to ensure that the persistent AUTO_INCREMENT value of the table will be updated on every INSERT. At this point of the execution, index->table->persistent_autoinc is pointing to the wrong index field.
So, we are trying to read an AUTO_INCREMENT value from the wrong position, from a hidden metadata record where the AUTO_INCREMENT value surely does not matter. The position is off by one, because in the MDEV-15562 metadata record, there is a metadata BLOB after the index->first_user_record().
In 10.3, there is no issue, because the dummy value for the AUTO_INCREMENT column in the MDEV-11369 metadata record would be 0 or NULL, and because the field will be (unnecessarily) accessed at the correct position.
We can avoid the unnecessary and harmful access during ALTER TABLE:
diff --git a/storage/innobase/row/row0ins.cc b/storage/innobase/row/row0ins.cc
|
index 777d69cf127..1014168d389 100644
|
--- a/storage/innobase/row/row0ins.cc
|
+++ b/storage/innobase/row/row0ins.cc
|
@@ -2597,25 +2597,32 @@ row_ins_clust_index_entry_low(
|
} else {
|
index->set_modified(mtr);
|
|
- if (mode == BTR_MODIFY_LEAF
|
- && dict_index_is_online_ddl(index)) {
|
- mode = BTR_MODIFY_LEAF_ALREADY_S_LATCHED;
|
- mtr_s_lock(dict_index_get_lock(index), &mtr);
|
- }
|
+ if (UNIV_UNLIKELY(entry->is_metadata())) {
|
+ ut_ad(index->is_instant());
|
+ ut_ad(!dict_index_is_online_ddl(index));
|
+ ut_ad(mode == BTR_MODIFY_TREE);
|
+ } else {
|
+ if (mode == BTR_MODIFY_LEAF
|
+ && dict_index_is_online_ddl(index)) {
|
+ mode = BTR_MODIFY_LEAF_ALREADY_S_LATCHED;
|
+ mtr_s_lock(dict_index_get_lock(index), &mtr);
|
+ }
|
|
- if (unsigned ai = index->table->persistent_autoinc) {
|
- /* Prepare to persist the AUTO_INCREMENT value
|
- from the index entry to PAGE_ROOT_AUTO_INC. */
|
- const dfield_t* dfield = dtuple_get_nth_field(
|
- entry, ai - 1);
|
- auto_inc = dfield_is_null(dfield)
|
- ? 0
|
- : row_parse_int(static_cast<const byte*>(
|
+ if (unsigned ai = index->table->persistent_autoinc) {
|
+ /* Prepare to persist the AUTO_INCREMENT value
|
+ from the index entry to PAGE_ROOT_AUTO_INC. */
|
+ const dfield_t* dfield = dtuple_get_nth_field(
|
+ entry, ai - 1);
|
+ if (!dfield_is_null(dfield)) {
|
+ auto_inc = row_parse_int(
|
+ static_cast<const byte*>(
|
dfield->data),
|
dfield->len,
|
dfield->type.mtype,
|
dfield->type.prtype
|
& DATA_UNSIGNED);
|
+ }
|
+ }
|
}
|
}
|
|
Apart from this piece of code, index->table->persistent_autoinc plays a role in ha_innobase::update_row(), and in ALTER TABLE operations. In those cases, the field would have been initialized by ha_innobase::open() invoking initialize_auto_increment(), or by create_table_info_t::create_table_update_dict().
|