Uploaded image for project: 'MariaDB Server'
  1. MariaDB Server
  2. MDEV-17735

Assertion failure in row_parse_int() on first ADD/DROP COLUMN when an AUTO_INCREMENT column is not in PRIMARY KEY

    Details

      Description

      This was first reported by Elena Stepanova 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().

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                marko Marko Mäkelä
                Reporter:
                marko Marko Mäkelä
              • Votes:
                0 Vote for this issue
                Watchers:
                1 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: