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

mysqld: storage/innobase/btr/btr0cur.cc:507: dberr_t btr_cur_instant_init_low(dict_index_t*, mtr_t*): Assertion `index->n_core_fields + n_add >= index->n_fields' failed.

Details

    Description

      Problem found during RQG testing
      Version: '10.5.0-MariaDB-debug-log'  socket:...
      mysqld: storage/innobase/btr/btr0cur.cc:507: dberr_t btr_cur_instant_init_low(dict_index_t*, mtr_t*): Assertion `index->n_core_fields + n_add >= index->n_fields' failed.
      191126 12:12:18 [ERROR] mysqld got signal 6 ;
      ...
      Query (0x7f8218013980): ALTER TABLE t3 ADD COLUMN IF NOT EXISTS col_text_g_copy TEXT GENERATED ALWAYS AS (SUBSTR(col_text,1,499)) FIRST /* E_R Thread1 QNO 20 CON_ID 12 */
      Connection ID (thread ID): 12
      Status: NOT_KILLED
      ...
      #3  <signal handler called>
      #4  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:58
      #5  0x00007f82685ed37a in __GI_abort () at abort.c:89
      #6  0x00007f82685e3b47 in __assert_fail_base (fmt=<optimized out>, assertion=assertion@entry=0x563907a12358 "index->n_core_fields + n_add >= index->n_fields", file=file@entry=0x563907a11ec0 "storage/innobase/btr/btr0cur.cc", line=line@entry=507, function=function@entry=0x563907a15ec0 <btr_cur_instant_init_low(dict_index_t*, mtr_t*)::__PRETTY_FUNCTION__> "dberr_t btr_cur_instant_init_low(dict_index_t*, mtr_t*)") at assert.c:92
      #7  0x00007f82685e3bf2 in __GI___assert_fail (assertion=0x563907a12358 "index->n_core_fields + n_add >= index->n_fields", file=0x563907a11ec0 "storage/innobase/btr/btr0cur.cc", line=507, function=0x563907a15ec0 <btr_cur_instant_init_low(dict_index_t*, mtr_t*)::__PRETTY_FUNCTION__> "dberr_t btr_cur_instant_init_low(dict_index_t*, mtr_t*)") at assert.c:101
      #8  0x00005639072fa2cf in btr_cur_instant_init_low (index=0x7f82182ff5b8, mtr=0x7f82682a15c0) at storage/innobase/btr/btr0cur.cc:507
      #9  0x00005639072fadb0 in btr_cur_instant_init (table=0x7f82182ff098) at storage/innobase/btr/btr0cur.cc:659
      #10 0x00005639073b801a in dict_load_table_one (name=..., ignore_err=DICT_ERR_IGNORE_NONE, fk_tables=std::deque with 0 elements) at storage/innobase/dict/dict0load.cc:3011
      #11 0x00005639073b6d2e in dict_load_table (name=0x7f82682a2aa0 "test/t3", ignore_err=DICT_ERR_IGNORE_NONE) at storage/innobase/dict/dict0load.cc:2743
      #12 0x000056390739b1f7 in dict_table_open_on_name (table_name=0x7f82682a2aa0 "test/t3", dict_locked=1, try_drop=1, ignore_err=DICT_ERR_IGNORE_NONE) at storage/innobase/dict/dict0dict.cc:886
      #13 0x000056390708fff3 in ha_innobase::commit_inplace_alter_table (this=0x7f82183036f0, altered_table=0x7f82682a4010, ha_alter_info=0x7f82682a3f80, commit=true) at storage/innobase/handler/handler0alter.cc:11151
      #14 0x0000563906c43c95 in handler::ha_commit_inplace_alter_table (this=0x7f82183036f0, altered_table=0x7f82682a4010, ha_alter_info=0x7f82682a3f80, commit=true) at sql/handler.cc:4557
      #15 0x00005639069ed344 in mysql_inplace_alter_table (thd=0x7f8218000a98, table_list=0x7f8218013b58, table=0x7f82183028e8, altered_table=0x7f82682a4010, ha_alter_info=0x7f82682a3f80, inplace_supported=HA_ALTER_INPLACE_INSTANT, target_mdl_request=0x7f82682a4df0, alter_ctx=0x7f82682a5920) at sql/sql_table.cc:7764
      #16 0x00005639069f3e23 in mysql_alter_table (thd=0x7f8218000a98, new_db=0x7f8218005270, new_name=0x7f8218005678, create_info=0x7f82682a6510, table_list=0x7f8218013b58, alter_info=0x7f82682a6450, order_num=0, order=0x0, ignore=false) at sql/sql_table.cc:10079
      #17 0x0000563906a965cf in Sql_cmd_alter_table::execute (this=0x7f82180146e0, thd=0x7f8218000a98) at sql/sql_alter.cc:523
      #18 0x00005639068fd19f in mysql_execute_command (thd=0x7f8218000a98) at sql/sql_parse.cc:5959
      #19 0x00005639069033c6 in mysql_parse (thd=0x7f8218000a98, rawbuf=0x7f8218013980 "ALTER TABLE t3 ADD COLUMN IF NOT EXISTS col_text_g_copy TEXT GENERATED ALWAYS AS (SUBSTR(col_text,1,499)) FIRST /* E_R Thread1 QNO 20 CON_ID 12 */", length=146, parser_state=0x7f82682a75d0, is_com_multi=false, is_next_command=false) at sql/sql_parse.cc:7988
      #20 0x00005639068ee461 in dispatch_command (command=COM_QUERY, thd=0x7f8218000a98, packet=0x7f8218007ec9 "ALTER TABLE t3 ADD COLUMN IF NOT EXISTS col_text_g_copy TEXT GENERATED ALWAYS AS (SUBSTR(col_text,1,499)) FIRST /* E_R Thread1 QNO 20 CON_ID 12 */ ", packet_length=147, is_com_multi=false, is_next_command=false) at sql/sql_parse.cc:1846
      #21 0x00005639068ecbbb in do_command (thd=0x7f8218000a98) at sql/sql_parse.cc:1364
       
      origin/10.5 25e2a556de2e125784d52a0c7ccda4fa6595df50 2019-11-26T10:15:03+02:00
      origin/10.4 f9ceb0a67ffb20631c936a7e8e8776c000d677ac 2019-11-25T15:26:22+02:00
       
      The problem was not reproducible on
      origin/10.3 bf58ec77a1adaa653a0b044b950cf420f8c19de9 2019-11-25
      

      Attachments

        1. MDEV_21148.cfg
          42 kB
        2. MDEV_21148.log
          97 kB
        3. MDEV_21148.yy
          1 kB

        Issue Links

          Activity

            MDEV-21148.yy -- RQG YY grammar
            MDEV-21148.cfg -- config file for rqg_batch.pl
            MDEV-21148.log -- RQG protocol from the run against 10.5
            

            mleich Matthias Leich added a comment - MDEV-21148.yy -- RQG YY grammar MDEV-21148.cfg -- config file for rqg_batch.pl MDEV-21148.log -- RQG protocol from the run against 10.5

            The assertion was added in MDEV-21088. There appears to be another problem in that fix: we seem to be parsing the field descriptors of the metadata record in the reverse order. But should be unrelated to this assertion failure.

            marko Marko Mäkelä added a comment - The assertion was added in MDEV-21088 . There appears to be another problem in that fix: we seem to be parsing the field descriptors of the metadata record in the reverse order. But should be unrelated to this assertion failure.

            Here is a simpler test case:

            --source include/have_innodb.inc
             
            CREATE TABLE t3 (col1 INT, col_text TEXT, PRIMARY KEY(col_text(9)))
            ENGINE = InnoDB ROW_FORMAT = Compact;
            ALTER TABLE t3 ADD COLUMN col_string_copy TEXT FIRST;
            ALTER TABLE t3 ADD COLUMN col_text_g_copy TEXT GENERATED ALWAYS AS (SUBSTR(col_text,1,499)) FIRST;
             
            DROP TABLE t3;
            

            I think that we have a bug in the MDEV-21088 fix, but it will not affect this assertion failure:

            diff --git a/storage/innobase/btr/btr0cur.cc b/storage/innobase/btr/btr0cur.cc
            index 8d4cb6cdb07..ae961943da5 100644
            --- a/storage/innobase/btr/btr0cur.cc
            +++ b/storage/innobase/btr/btr0cur.cc
            @@ -508,7 +508,7 @@ static dberr_t btr_cur_instant_init_low(dict_index_t* index, mtr_t* mtr)
             			ut_ad(index->n_core_fields + n_add >= index->n_fields);
             			lens -= n_add;
             
            -			for (uint i = index->n_uniq; i--; ) {
            +			for (uint i = 0; i < index->n_uniq; i++) {
             				const dict_field_t& f = index->fields[i];
             				unsigned len = f.fixed_len;
             				if (!len) {
            

            10.4 f9ceb0a67ffb20631c936a7e8e8776c000d677ac

            mysqltest: At line 6: query 'ALTER TABLE t3 ADD COLUMN col_text_g_copy TEXT GENERATED ALWAYS AS (SUBSTR(col_text,1,499)) FIRST' failed: 2013: Lost connection to MySQL server during query
            …
            mysqld: /mariadb/10.4/storage/innobase/btr/btr0cur.cc:508: dberr_t btr_cur_instant_init_low(dict_index_t *, mtr_t *): Assertion `index->n_core_fields + n_add >= index->n_fields' failed.
            

            marko Marko Mäkelä added a comment - Here is a simpler test case: --source include/have_innodb.inc   CREATE TABLE t3 (col1 INT , col_text TEXT, PRIMARY KEY (col_text(9))) ENGINE = InnoDB ROW_FORMAT = Compact; ALTER TABLE t3 ADD COLUMN col_string_copy TEXT FIRST ; ALTER TABLE t3 ADD COLUMN col_text_g_copy TEXT GENERATED ALWAYS AS (SUBSTR(col_text,1,499)) FIRST ;   DROP TABLE t3; I think that we have a bug in the MDEV-21088 fix, but it will not affect this assertion failure: diff --git a/storage/innobase/btr/btr0cur.cc b/storage/innobase/btr/btr0cur.cc index 8d4cb6cdb07..ae961943da5 100644 --- a/storage/innobase/btr/btr0cur.cc +++ b/storage/innobase/btr/btr0cur.cc @@ -508,7 +508,7 @@ static dberr_t btr_cur_instant_init_low(dict_index_t* index, mtr_t* mtr) ut_ad(index->n_core_fields + n_add >= index->n_fields); lens -= n_add; - for (uint i = index->n_uniq; i--; ) { + for (uint i = 0; i < index->n_uniq; i++) { const dict_field_t& f = index->fields[i]; unsigned len = f.fixed_len; if (!len) { 10.4 f9ceb0a67ffb20631c936a7e8e8776c000d677ac mysqltest: At line 6: query 'ALTER TABLE t3 ADD COLUMN col_text_g_copy TEXT GENERATED ALWAYS AS (SUBSTR(col_text,1,499)) FIRST' failed: 2013: Lost connection to MySQL server during query … mysqld: /mariadb/10.4/storage/innobase/btr/btr0cur.cc:508: dberr_t btr_cur_instant_init_low(dict_index_t *, mtr_t *): Assertion `index->n_core_fields + n_add >= index->n_fields' failed.

            I am afraid that my attempt at supporting corrupted tables altered before MDEV-21088 was fixed is futile. There is a catch-22 problem in this attempted fix:

            diff --git a/storage/innobase/btr/btr0cur.cc b/storage/innobase/btr/btr0cur.cc
            index 8d4cb6cdb07..cf04567bf1e 100644
            --- a/storage/innobase/btr/btr0cur.cc
            +++ b/storage/innobase/btr/btr0cur.cc
            @@ -502,13 +502,15 @@ static dberr_t btr_cur_instant_init_low(dict_index_t* index, mtr_t* mtr)
             		} else if (index->table->not_redundant()) {
             			/* PRIMARY KEY columns can never be NULL.
             			We can skip the null flag bitmap. */
            -			const byte* lens = rec - (REC_N_NEW_EXTRA_BYTES + 1)
            -				- index->n_core_null_bytes;
            -			unsigned n_add = rec_get_n_add_field(lens);
            +			const byte* nulls = rec - REC_N_NEW_EXTRA_BYTES;
            +			unsigned n_add = rec_get_n_add_field(nulls);
            +			/* FIXME: how many nullable columns are there?
            +			we cannot know before reading the metadata BLOB! */
            +			const byte* lens = nulls - index->n_core_null_bytes
            +				- 1;
             			ut_ad(index->n_core_fields + n_add >= index->n_fields);
            -			lens -= n_add;
             
            -			for (uint i = index->n_uniq; i--; ) {
            +			for (uint i = 0; i < index->n_uniq; i++) {
             				const dict_field_t& f = index->fields[i];
             				unsigned len = f.fixed_len;
             				if (!len) {
            

            This fix is also incorrect in other ways, but the important part is the FIXME comment. As soon as we instantly add at least 8 NULLable columns to a table that has variable-length encoded PRIMARY KEY columns, we would be in trouble.

            I think that we must revert part of the MDEV-21088 fix instead:

            diff --git a/storage/innobase/btr/btr0cur.cc b/storage/innobase/btr/btr0cur.cc
            index 8d4cb6cdb07..6b33c997e75 100644
            --- a/storage/innobase/btr/btr0cur.cc
            +++ b/storage/innobase/btr/btr0cur.cc
            @@ -500,28 +500,32 @@ static dberr_t btr_cur_instant_init_low(dict_index_t* index, mtr_t* mtr)
             		we must duplicate some logic here. */
             		if (trx_id_offset) {
             		} else if (index->table->not_redundant()) {
            -			/* PRIMARY KEY columns can never be NULL.
            -			We can skip the null flag bitmap. */
            -			const byte* lens = rec - (REC_N_NEW_EXTRA_BYTES + 1)
            -				- index->n_core_null_bytes;
            -			unsigned n_add = rec_get_n_add_field(lens);
            -			ut_ad(index->n_core_fields + n_add >= index->n_fields);
            -			lens -= n_add;
            +			/* The PRIMARY KEY contains variable-length columns.
            +			For the metadata record, variable-length columns are
            +			always written with zero length. The DB_TRX_ID will
            +			start right after any fixed-length columns. */
            +
            +			/* OK, before MDEV-21088 was fixed, for
            +			variable-length encoded PRIMARY KEY column of
            +			type CHAR, we wrote more than zero bytes. In
            +			order to allow affected tables to be accessed,
            +			it would be nice to determine the actual
            +			length of each PRIMARY KEY column. However, to
            +			be able to do that, we should determine the
            +			size of the null-bit bitmap in the metadata
            +			record. And we cannot know that before reading
            +			the metadata BLOB, whose starting point we are
            +			trying to find here. (Although the PRIMARY KEY
            +			columns cannot be NULL, we would have to know
            +			where the lengths of variable-length PRIMARY KEY
            +			columns start.)
            +
            +			So, unfortunately we cannot help users who
            +			were affected by MDEV-21088 on a ROW_FORMAT=COMPACT
            +			or ROW_FORMAT=DYNAMIC table. */
             
             			for (uint i = index->n_uniq; i--; ) {
            -				const dict_field_t& f = index->fields[i];
            -				unsigned len = f.fixed_len;
            -				if (!len) {
            -					len = *lens--;
            -					if ((len & 0x80)
            -					    && DATA_BIG_COL(f.col)) {
            -						/* 1exxxxxxx xxxxxxxx */
            -						len &= 0x3f;
            -						len <<= 8;
            -						len |= *lens--;
            -					}
            -				}
            -				trx_id_offset += len;
            +				trx_id_offset += index->fields[i].fixed_len;
             			}
             		} else if (rec_get_1byte_offs_flag(rec)) {
             			trx_id_offset = rec_1_get_field_end_info(
            

            marko Marko Mäkelä added a comment - I am afraid that my attempt at supporting corrupted tables altered before MDEV-21088 was fixed is futile. There is a catch-22 problem in this attempted fix: diff --git a/storage/innobase/btr/btr0cur.cc b/storage/innobase/btr/btr0cur.cc index 8d4cb6cdb07..cf04567bf1e 100644 --- a/storage/innobase/btr/btr0cur.cc +++ b/storage/innobase/btr/btr0cur.cc @@ -502,13 +502,15 @@ static dberr_t btr_cur_instant_init_low(dict_index_t* index, mtr_t* mtr) } else if (index->table->not_redundant()) { /* PRIMARY KEY columns can never be NULL. We can skip the null flag bitmap. */ - const byte* lens = rec - (REC_N_NEW_EXTRA_BYTES + 1) - - index->n_core_null_bytes; - unsigned n_add = rec_get_n_add_field(lens); + const byte* nulls = rec - REC_N_NEW_EXTRA_BYTES; + unsigned n_add = rec_get_n_add_field(nulls); + /* FIXME: how many nullable columns are there? + we cannot know before reading the metadata BLOB! */ + const byte* lens = nulls - index->n_core_null_bytes + - 1; ut_ad(index->n_core_fields + n_add >= index->n_fields); - lens -= n_add; - for (uint i = index->n_uniq; i--; ) { + for (uint i = 0; i < index->n_uniq; i++) { const dict_field_t& f = index->fields[i]; unsigned len = f.fixed_len; if (!len) { This fix is also incorrect in other ways, but the important part is the FIXME comment. As soon as we instantly add at least 8 NULLable columns to a table that has variable-length encoded PRIMARY KEY columns, we would be in trouble. I think that we must revert part of the MDEV-21088 fix instead: diff --git a/storage/innobase/btr/btr0cur.cc b/storage/innobase/btr/btr0cur.cc index 8d4cb6cdb07..6b33c997e75 100644 --- a/storage/innobase/btr/btr0cur.cc +++ b/storage/innobase/btr/btr0cur.cc @@ -500,28 +500,32 @@ static dberr_t btr_cur_instant_init_low(dict_index_t* index, mtr_t* mtr) we must duplicate some logic here. */ if (trx_id_offset) { } else if (index->table->not_redundant()) { - /* PRIMARY KEY columns can never be NULL. - We can skip the null flag bitmap. */ - const byte* lens = rec - (REC_N_NEW_EXTRA_BYTES + 1) - - index->n_core_null_bytes; - unsigned n_add = rec_get_n_add_field(lens); - ut_ad(index->n_core_fields + n_add >= index->n_fields); - lens -= n_add; + /* The PRIMARY KEY contains variable-length columns. + For the metadata record, variable-length columns are + always written with zero length. The DB_TRX_ID will + start right after any fixed-length columns. */ + + /* OK, before MDEV-21088 was fixed, for + variable-length encoded PRIMARY KEY column of + type CHAR, we wrote more than zero bytes. In + order to allow affected tables to be accessed, + it would be nice to determine the actual + length of each PRIMARY KEY column. However, to + be able to do that, we should determine the + size of the null-bit bitmap in the metadata + record. And we cannot know that before reading + the metadata BLOB, whose starting point we are + trying to find here. (Although the PRIMARY KEY + columns cannot be NULL, we would have to know + where the lengths of variable-length PRIMARY KEY + columns start.) + + So, unfortunately we cannot help users who + were affected by MDEV-21088 on a ROW_FORMAT=COMPACT + or ROW_FORMAT=DYNAMIC table. */ for (uint i = index->n_uniq; i--; ) { - const dict_field_t& f = index->fields[i]; - unsigned len = f.fixed_len; - if (!len) { - len = *lens--; - if ((len & 0x80) - && DATA_BIG_COL(f.col)) { - /* 1exxxxxxx xxxxxxxx */ - len &= 0x3f; - len <<= 8; - len |= *lens--; - } - } - trx_id_offset += len; + trx_id_offset += index->fields[i].fixed_len; } } else if (rec_get_1byte_offs_flag(rec)) { trx_id_offset = rec_1_get_field_end_info(

            People

              marko Marko Mäkelä
              mleich Matthias Leich
              Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Git Integration

                  Error rendering 'com.xiplink.jira.git.jira_git_plugin:git-issue-webpanel'. Please contact your Jira administrators.