[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. Created: 2019-11-26  Updated: 2020-03-16  Resolved: 2019-11-26

Status: Closed
Project: MariaDB Server
Component/s: Storage Engine - InnoDB
Affects Version/s: 10.4.11, 10.5.0
Fix Version/s: 10.4.11

Type: Bug Priority: Blocker
Reporter: Matthias Leich Assignee: Marko Mäkelä
Resolution: Fixed Votes: 0
Labels: None

Attachments: File MDEV_21148.cfg     File MDEV_21148.log     File MDEV_21148.yy    
Issue Links:
Relates
relates to MDEV-21088 Table cannot be loaded after instant ... Closed
relates to MDEV-21230 Corrupt database with MariaDB 10.4.10... Closed

 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



 Comments   
Comment by Matthias Leich [ 2019-11-26 ] 

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

Comment by Marko Mäkelä [ 2019-11-26 ]

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.

Comment by Marko Mäkelä [ 2019-11-26 ]

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.

Comment by Marko Mäkelä [ 2019-11-26 ]

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(

Generated at Thu Feb 08 09:04:57 UTC 2024 using Jira 8.20.16#820016-sha1:9d11dbea5f4be3d4cc21f03a88dd11d8c8687422.