From 9053047f3db37a174f6c1333acf189b6558c50c4 Mon Sep 17 00:00:00 2001 From: Monty Date: Thu, 27 Jun 2019 18:51:34 +0300 Subject: [PATCH 1/4] MDEV-17551 assert or crashed table when using blobs The bug was that when long item-strings was converted to VARCHAR, type_handler::string_type_handler() didn't take into account max VARCHAR length. The resulting Aria temporary table was created with a VARCHAR field of length 1 when it should have been 65537. This caused MariaDB to send impossible records to ma_write() and Aria reported eventually the table as crashed. Fixed by updating Type_handler::string_type_handler() to not create too long VARCHAR fields. To make things extra safe, I also added checks in when writing dynamic Aria records to ensure we find the wrong record during write instead of during read. --- mysql-test/main/type_varchar.result | 19 +++++++++++++++ mysql-test/main/type_varchar.test | 19 +++++++++++++++ sql/sql_const.h | 2 +- sql/sql_type.cc | 5 ++++ storage/maria/ma_check.c | 7 +++++- storage/maria/ma_close.c | 6 +++-- storage/maria/ma_dynrec.c | 38 ++++++++++++++++++++++++++--- 7 files changed, 88 insertions(+), 8 deletions(-) diff --git a/mysql-test/main/type_varchar.result b/mysql-test/main/type_varchar.result index 0b2a5b54d08..daf9ab8a1f0 100644 --- a/mysql-test/main/type_varchar.result +++ b/mysql-test/main/type_varchar.result @@ -686,3 +686,22 @@ DROP TABLE t1,t2; # # End of 10.0 tests # +# +# MDEV-17551 +# Assertion `(&(&share->intern_lock)->m_mutex)->count > 0 && +# pthread_equal(pthread_self(), (&(&share->intern_lock)->m_mutex)-> +# thread)' failed in _ma_state_info_write or ER_CRASHED_ON_USAGE +# upon SELECT with UNION +# +CREATE TABLE t1 (b BLOB, vb BLOB AS (b) VIRTUAL); +INSERT INTO t1 (b) VALUES ('foobar'); +SELECT 'foo' AS f1, CONVERT( 'bar' USING latin1 ) AS f2 FROM t1 +UNION +SELECT b AS f1, CONVERT( vb USING latin1 ) AS f2 FROM t1; +f1 f2 +foo bar +foobar foobar +DROP TABLE t1; +# +# End of 10.3 tests +# diff --git a/mysql-test/main/type_varchar.test b/mysql-test/main/type_varchar.test index d70cb86fa7b..8060b383981 100644 --- a/mysql-test/main/type_varchar.test +++ b/mysql-test/main/type_varchar.test @@ -328,3 +328,22 @@ DROP TABLE t1,t2; --echo # --echo # End of 10.0 tests --echo # + +--echo # +--echo # MDEV-17551 +--echo # Assertion `(&(&share->intern_lock)->m_mutex)->count > 0 && +--echo # pthread_equal(pthread_self(), (&(&share->intern_lock)->m_mutex)-> +--echo # thread)' failed in _ma_state_info_write or ER_CRASHED_ON_USAGE +--echo # upon SELECT with UNION +--echo # + +CREATE TABLE t1 (b BLOB, vb BLOB AS (b) VIRTUAL); +INSERT INTO t1 (b) VALUES ('foobar'); +SELECT 'foo' AS f1, CONVERT( 'bar' USING latin1 ) AS f2 FROM t1 + UNION +SELECT b AS f1, CONVERT( vb USING latin1 ) AS f2 FROM t1; +DROP TABLE t1; + +--echo # +--echo # End of 10.3 tests +--echo # diff --git a/sql/sql_const.h b/sql/sql_const.h index 06f9a6042ad..e33d8b73626 100644 --- a/sql/sql_const.h +++ b/sql/sql_const.h @@ -64,7 +64,7 @@ CREATE TABLE t1 (c VARBINARY(65534)); CREATE TABLE t1 (c VARBINARY(65535)); Like VARCHAR(65536), they will be converted to BLOB automatically - in non-sctict mode. + in non-strict mode. */ #define MAX_FIELD_VARCHARLENGTH (65535-2-1) #define MAX_FIELD_BLOBLENGTH UINT_MAX32 /* cf field_blob::get_length() */ diff --git a/sql/sql_type.cc b/sql/sql_type.cc index d07296aad7e..665ccd28595 100644 --- a/sql/sql_type.cc +++ b/sql/sql_type.cc @@ -327,6 +327,8 @@ Type_handler::string_type_handler(uint max_octet_length) return &type_handler_long_blob; else if (max_octet_length >= 65536) return &type_handler_medium_blob; + else if (max_octet_length >= MAX_FIELD_VARCHARLENGTH) + return &type_handler_blob; return &type_handler_varchar; } @@ -1372,6 +1374,7 @@ Field *Type_handler_varchar::make_conversion_table_field(TABLE *table, const Field *target) const { + DBUG_ASSERT(HA_VARCHAR_PACKLENGTH(metadata) <= MAX_FIELD_VARCHARLENGTH); return new(table->in_use->mem_root) Field_varstring(NULL, metadata, HA_VARCHAR_PACKLENGTH(metadata), (uchar *) "", 1, Field::NONE, &empty_clex_str, @@ -2364,6 +2367,8 @@ Field *Type_handler_varchar::make_table_field(const LEX_CSTRING *name, TABLE *table) const { + DBUG_ASSERT(HA_VARCHAR_PACKLENGTH(attr.max_length) <= + MAX_FIELD_VARCHARLENGTH); return new (table->in_use->mem_root) Field_varstring(addr.ptr, attr.max_length, HA_VARCHAR_PACKLENGTH(attr.max_length), diff --git a/storage/maria/ma_check.c b/storage/maria/ma_check.c index f5dade5997d..bfed6698428 100644 --- a/storage/maria/ma_check.c +++ b/storage/maria/ma_check.c @@ -5426,7 +5426,12 @@ int _ma_sort_write_record(MARIA_SORT_PARAM *sort_param) info->cur_row.checksum= (*share->calc_check_checksum)(info, sort_param-> record); - reclength= _ma_rec_pack(info,from,sort_param->record); + if (!(reclength= _ma_rec_pack(info,from,sort_param->record))) + { + _ma_check_print_error(param,"Got error %d when packing record", + my_errno); + DBUG_RETURN(1); + } flag=0; do diff --git a/storage/maria/ma_close.c b/storage/maria/ma_close.c index 03501dc49cf..44f65230c5d 100644 --- a/storage/maria/ma_close.c +++ b/storage/maria/ma_close.c @@ -114,8 +114,10 @@ int maria_close(register MARIA_HA *info) share->deleting ? FLUSH_IGNORE_CHANGED : FLUSH_RELEASE)) error= my_errno; unmap_file(info); - if (((share->changed && share->base.born_transactional) || - maria_is_crashed(info) || (share->temporary && !share->deleting))) + if (!internal_table && + (((share->changed && share->base.born_transactional) || + maria_is_crashed(info) || + (share->temporary && !share->deleting)))) { if (save_global_changed) { diff --git a/storage/maria/ma_dynrec.c b/storage/maria/ma_dynrec.c index 5273c1bddb7..ae6fc57c114 100644 --- a/storage/maria/ma_dynrec.c +++ b/storage/maria/ma_dynrec.c @@ -224,6 +224,8 @@ my_bool _ma_write_dynamic_record(MARIA_HA *info, const uchar *record) { ulong reclength= _ma_rec_pack(info,info->rec_buff + MARIA_REC_BUFF_OFFSET, record); + if (!reclength) + return 1; return (write_dynamic_record(info,info->rec_buff + MARIA_REC_BUFF_OFFSET, reclength)); } @@ -234,6 +236,8 @@ my_bool _ma_update_dynamic_record(MARIA_HA *info, MARIA_RECORD_POS pos, { uint length= _ma_rec_pack(info, info->rec_buff + MARIA_REC_BUFF_OFFSET, record); + if (!length) + return 1; return (update_dynamic_record(info, pos, info->rec_buff + MARIA_REC_BUFF_OFFSET, length)); @@ -258,12 +262,19 @@ my_bool _ma_write_blob_record(MARIA_HA *info, const uchar *record) reclength2= _ma_rec_pack(info, rec_buff+ALIGN_SIZE(MARIA_MAX_DYN_BLOCK_HEADER), record); + if (!reclength2) + { + error= 1; + goto err; + } + DBUG_PRINT("info",("reclength: %lu reclength2: %lu", reclength, reclength2)); DBUG_ASSERT(reclength2 <= reclength); error= write_dynamic_record(info, rec_buff+ALIGN_SIZE(MARIA_MAX_DYN_BLOCK_HEADER), reclength2); +err: my_safe_afree(rec_buff, reclength); return(error != 0); } @@ -293,12 +304,19 @@ my_bool _ma_update_blob_record(MARIA_HA *info, MARIA_RECORD_POS pos, my_errno= HA_ERR_OUT_OF_MEM; /* purecov: inspected */ return(1); } - reclength2= _ma_rec_pack(info,rec_buff+ALIGN_SIZE(MARIA_MAX_DYN_BLOCK_HEADER), - record); + reclength2= _ma_rec_pack(info, rec_buff+ + ALIGN_SIZE(MARIA_MAX_DYN_BLOCK_HEADER), + record); + if (!reclength2) + { + error= 1; + goto err; + } DBUG_ASSERT(reclength2 <= reclength); error=update_dynamic_record(info,pos, rec_buff+ALIGN_SIZE(MARIA_MAX_DYN_BLOCK_HEADER), reclength2); +err: my_safe_afree(rec_buff, reclength); return(error != 0); } @@ -938,7 +956,12 @@ err: } - /* Pack a record. Return new reclength */ +/** + Pack a record. + + @return new reclength + @return 0 in case of wrong data in record +*/ uint _ma_rec_pack(MARIA_HA *info, register uchar *to, register const uchar *from) @@ -1042,6 +1065,11 @@ uint _ma_rec_pack(MARIA_HA *info, register uchar *to, tmp_length= uint2korr(from); store_key_length_inc(to,tmp_length); } + if (tmp_length > column->length) + { + my_errno= HA_ERR_WRONG_IN_RECORD; + DBUG_RETURN(0); + } memcpy(to, from+pack_length,tmp_length); to+= tmp_length; continue; @@ -1613,7 +1641,9 @@ my_bool _ma_cmp_dynamic_record(register MARIA_HA *info, if (!(buffer=(uchar*) my_safe_alloca(buffer_length))) DBUG_RETURN(1); } - reclength= _ma_rec_pack(info,buffer,record); + if (!(reclength= _ma_rec_pack(info,buffer,record))) + goto err; + record= buffer; filepos= info->cur_row.lastpos; From 685b527f0ca39ac176f1a5923c448f47a865f207 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Mon, 1 Jul 2019 16:56:16 +0300 Subject: [PATCH 2/4] MDEV-16060: Speed up the test by 1 second --- mysql-test/suite/innodb/r/foreign-keys.result | 2 +- mysql-test/suite/innodb/t/foreign-keys.test | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/mysql-test/suite/innodb/r/foreign-keys.result b/mysql-test/suite/innodb/r/foreign-keys.result index c4cf3a6a72d..930e6f1d819 100644 --- a/mysql-test/suite/innodb/r/foreign-keys.result +++ b/mysql-test/suite/innodb/r/foreign-keys.result @@ -110,7 +110,7 @@ SET debug_sync='alter_table_intermediate_table_created SIGNAL ready WAIT_FOR go' ALTER TABLE t1 ADD FOREIGN KEY(pk) REFERENCES t2(pk) ON UPDATE CASCADE; connect con1, localhost, root; SET debug_sync='now WAIT_FOR ready'; -SET lock_wait_timeout=1; +SET lock_wait_timeout=0; UPDATE t2 SET pk=10 WHERE pk=1; ERROR HY000: Lock wait timeout exceeded; try restarting transaction PREPARE stmt FROM 'UPDATE t2 SET pk=10 WHERE pk=1'; diff --git a/mysql-test/suite/innodb/t/foreign-keys.test b/mysql-test/suite/innodb/t/foreign-keys.test index be2c891771b..c20798aa824 100644 --- a/mysql-test/suite/innodb/t/foreign-keys.test +++ b/mysql-test/suite/innodb/t/foreign-keys.test @@ -140,7 +140,7 @@ send ALTER TABLE t1 ADD FOREIGN KEY(pk) REFERENCES t2(pk) ON UPDATE CASCADE; connect con1, localhost, root; SET debug_sync='now WAIT_FOR ready'; -SET lock_wait_timeout=1; # change to 0 in 10.3 +SET lock_wait_timeout=0; --error ER_LOCK_WAIT_TIMEOUT UPDATE t2 SET pk=10 WHERE pk=1; PREPARE stmt FROM 'UPDATE t2 SET pk=10 WHERE pk=1'; From 92bbf4f53defdff6ffb3d3ec79469496f64626b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Mon, 1 Jul 2019 18:24:35 +0300 Subject: [PATCH 3/4] MDEV-19916: Improve page_validate() page_validate(): Validate also the page type, and try to list all errors that were encountered for the page, with a little more detail. --- storage/innobase/include/dict0mem.h | 3 + storage/innobase/page/page0page.cc | 182 ++++++++++++++-------------- 2 files changed, 97 insertions(+), 88 deletions(-) diff --git a/storage/innobase/include/dict0mem.h b/storage/innobase/include/dict0mem.h index 1a3c1c0f9a5..7536c4eb4e3 100644 --- a/storage/innobase/include/dict0mem.h +++ b/storage/innobase/include/dict0mem.h @@ -1037,6 +1037,9 @@ struct dict_index_t{ return DICT_CLUSTERED == (type & (DICT_CLUSTERED | DICT_IBUF)); } + /** @return whether this is a spatial index */ + bool is_spatial() const { return UNIV_UNLIKELY(type & DICT_SPATIAL); } + /** @return whether the index includes virtual columns */ bool has_virtual() const { return type & DICT_VIRTUAL; } diff --git a/storage/innobase/page/page0page.cc b/storage/innobase/page/page0page.cc index 9218689ea38..2323bfbea1a 100644 --- a/storage/innobase/page/page0page.cc +++ b/storage/innobase/page/page0page.cc @@ -2385,18 +2385,11 @@ page_validate( the page record type definition */ { const page_dir_slot_t* slot; - mem_heap_t* heap; - byte* buf; - ulint count; - ulint own_count; - ulint rec_own_count; - ulint slot_no; - ulint data_size; const rec_t* rec; const rec_t* old_rec = NULL; ulint offs; ulint n_slots; - ibool ret = FALSE; + ibool ret = TRUE; ulint i; ulint* offsets = NULL; ulint* old_offsets = NULL; @@ -2410,7 +2403,13 @@ page_validate( if (UNIV_UNLIKELY((ibool) !!page_is_comp(page) != dict_table_is_comp(index->table))) { ib::error() << "'compact format' flag mismatch"; - goto func_exit2; +func_exit2: + ib::error() << "Apparent corruption in space " + << page_get_space_id(page) << " page " + << page_get_page_no(page) + << " of index " << index->name + << " of table " << index->table->name; + return FALSE; } if (page_is_comp(page)) { if (UNIV_UNLIKELY(!page_simple_validate_new(page))) { @@ -2435,19 +2434,12 @@ page_validate( if (max_trx_id == 0 || max_trx_id > sys_max_trx_id) { ib::error() << "PAGE_MAX_TRX_ID out of bounds: " << max_trx_id << ", " << sys_max_trx_id; - goto func_exit2; + ret = FALSE; } } else { ut_ad(srv_force_recovery >= SRV_FORCE_NO_UNDO_LOG_SCAN); } - heap = mem_heap_create(srv_page_size + 200); - - /* The following buffer is used to check that the - records in the page record heap do not overlap */ - - buf = static_cast(mem_heap_zalloc(heap, srv_page_size)); - /* Check first that the record heap and the directory do not overlap. */ @@ -2456,20 +2448,45 @@ page_validate( if (UNIV_UNLIKELY(!(page_header_get_ptr(page, PAGE_HEAP_TOP) <= page_dir_get_nth_slot(page, n_slots - 1)))) { - ib::warn() << "Record heap and dir overlap on space " - << page_get_space_id(page) << " page " - << page_get_page_no(page) << " index " << index->name - << ", " << page_header_get_ptr(page, PAGE_HEAP_TOP) - << ", " << page_dir_get_nth_slot(page, n_slots - 1); - - goto func_exit; + ib::warn() << "Record heap and directory overlap"; + goto func_exit2; } + switch (uint16_t type = fil_page_get_type(page)) { + case FIL_PAGE_RTREE: + if (!index->is_spatial()) { +wrong_page_type: + ib::warn() << "Wrong page type " << type; + ret = FALSE; + } + break; + case FIL_PAGE_TYPE_INSTANT: + if (index->is_instant() + && page_get_page_no(page) == index->page) { + break; + } + goto wrong_page_type; + case FIL_PAGE_INDEX: + if (index->is_spatial()) { + goto wrong_page_type; + } + if (index->is_instant() + && page_get_page_no(page) == index->page) { + goto wrong_page_type; + } + break; + default: + goto wrong_page_type; + } + + /* The following buffer is used to check that the + records in the page record heap do not overlap */ + mem_heap_t* heap = mem_heap_create(srv_page_size + 200);; + byte* buf = static_cast(mem_heap_zalloc(heap, srv_page_size)); + /* Validate the record list in a loop checking also that it is consistent with the directory. */ - count = 0; - data_size = 0; - own_count = 1; + ulint count = 0, data_size = 0, own_count = 1, slot_no = 0; slot_no = 0; slot = page_dir_get_nth_slot(page, slot_no); @@ -2484,11 +2501,13 @@ page_validate( && UNIV_UNLIKELY(rec_get_node_ptr_flag(rec) == page_is_leaf(page))) { ib::error() << "'node_ptr' flag mismatch"; - goto func_exit; + ret = FALSE; + goto next_rec; } if (UNIV_UNLIKELY(!page_rec_validate(rec, offsets))) { - goto func_exit; + ret = FALSE; + goto next_rec; } /* Check that the records are in the ascending order */ @@ -2500,16 +2519,10 @@ page_validate( /* For spatial index, on nonleaf leavel, we allow recs to be equal. */ - bool rtr_equal_nodeptrs = - (ret == 0 && dict_index_is_spatial(index) - && !page_is_leaf(page)); + if (ret <= 0 && !(ret == 0 && index->is_spatial() + && !page_is_leaf(page))) { - if (ret <= 0 && !rtr_equal_nodeptrs) { - - ib::error() << "Records in wrong order on" - " space " << page_get_space_id(page) - << " page " << page_get_page_no(page) - << " index " << index->name; + ib::error() << "Records in wrong order"; fputs("\nInnoDB: previous record ", stderr); /* For spatial index, print the mbr info.*/ @@ -2530,7 +2543,7 @@ page_validate( putc('\n', stderr); } - goto func_exit; + ret = FALSE; } } @@ -2550,41 +2563,41 @@ page_validate( offs = page_offset(rec_get_start(rec, offsets)); i = rec_offs_size(offsets); if (UNIV_UNLIKELY(offs + i >= srv_page_size)) { - ib::error() << "Record offset out of bounds"; - goto func_exit; + ib::error() << "Record offset out of bounds: " + << offs << '+' << i; + ret = FALSE; + goto next_rec; } - while (i--) { if (UNIV_UNLIKELY(buf[offs + i])) { - /* No other record may overlap this */ - ib::error() << "Record overlaps another"; - goto func_exit; + ib::error() << "Record overlaps another: " + << offs << '+' << i; + ret = FALSE; + break; } - buf[offs + i] = 1; } - if (page_is_comp(page)) { - rec_own_count = rec_get_n_owned_new(rec); - } else { - rec_own_count = rec_get_n_owned_old(rec); - } - - if (UNIV_UNLIKELY(rec_own_count != 0)) { + if (ulint rec_own_count = page_is_comp(page) + ? rec_get_n_owned_new(rec) + : rec_get_n_owned_old(rec)) { /* This is a record pointed to by a dir slot */ if (UNIV_UNLIKELY(rec_own_count != own_count)) { - ib::error() << "Wrong owned count " - << rec_own_count << ", " << own_count; - goto func_exit; + ib::error() << "Wrong owned count at " << offs + << ": " << rec_own_count + << ", " << own_count; + ret = FALSE; } if (page_dir_slot_get_rec(slot) != rec) { ib::error() << "Dir slot does not" - " point to right rec"; - goto func_exit; + " point to right rec at " << offs; + ret = FALSE; } - page_dir_slot_check(slot); + if (ret) { + page_dir_slot_check(slot); + } own_count = 0; if (!page_rec_is_supremum(rec)) { @@ -2593,6 +2606,7 @@ page_validate( } } +next_rec: if (page_rec_is_supremum(rec)) { break; } @@ -2617,14 +2631,14 @@ page_validate( } } else if (UNIV_UNLIKELY(rec_get_n_owned_old(rec) == 0)) { n_owned_zero: - ib::error() << "n owned is zero"; - goto func_exit; + ib::error() << "n owned is zero at " << offs; + ret = FALSE; } if (UNIV_UNLIKELY(slot_no != n_slots - 1)) { ib::error() << "n slots wrong " << slot_no << " " << (n_slots - 1); - goto func_exit; + ret = FALSE; } if (UNIV_UNLIKELY(ulint(page_header_get_field(page, PAGE_N_RECS)) @@ -2633,65 +2647,57 @@ n_owned_zero: ib::error() << "n recs wrong " << page_header_get_field(page, PAGE_N_RECS) + PAGE_HEAP_NO_USER_LOW << " " << (count + 1); - goto func_exit; + ret = FALSE; } if (UNIV_UNLIKELY(data_size != page_get_data_size(page))) { ib::error() << "Summed data size " << data_size << ", returned by func " << page_get_data_size(page); - goto func_exit; + ret = FALSE; } /* Check then the free list */ - rec = page_header_get_ptr(page, PAGE_FREE); - - while (rec != NULL) { + for (rec = page_header_get_ptr(page, PAGE_FREE); + rec; + rec = page_rec_get_next_const(rec)) { offsets = rec_get_offsets(rec, index, offsets, page_is_leaf(page), ULINT_UNDEFINED, &heap); if (UNIV_UNLIKELY(!page_rec_validate(rec, offsets))) { - - goto func_exit; + ret = FALSE; + continue; } count++; offs = page_offset(rec_get_start(rec, offsets)); i = rec_offs_size(offsets); if (UNIV_UNLIKELY(offs + i >= srv_page_size)) { - ib::error() << "Record offset out of bounds"; - goto func_exit; + ib::error() << "Free record offset out of bounds: " + << offs << '+' << i; + ret = FALSE; + continue; } - while (i--) { - if (UNIV_UNLIKELY(buf[offs + i])) { - ib::error() << "Record overlaps another" - " in free list"; - goto func_exit; + ib::error() << "Free record overlaps another: " + << offs << '+' << i; + ret = FALSE; + break; } - buf[offs + i] = 1; } - - rec = page_rec_get_next_const(rec); } if (UNIV_UNLIKELY(page_dir_get_n_heap(page) != count + 1)) { ib::error() << "N heap is wrong " << page_dir_get_n_heap(page) << " " << count + 1; - goto func_exit; + ret = FALSE; } - ret = TRUE; - -func_exit: mem_heap_free(heap); - if (UNIV_UNLIKELY(ret == FALSE)) { -func_exit2: - ib::error() << "Apparent corruption in space " - << page_get_space_id(page) << " page " - << page_get_page_no(page) << " index " << index->name; + if (UNIV_UNLIKELY(!ret)) { + goto func_exit2; } return(ret); From 0e1ba364a12b6569c75a7dadc38b7ef2b4910d34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Mon, 1 Jul 2019 18:24:54 +0300 Subject: [PATCH 4/4] MDEV-19916 Corruption after instant ADD/DROP and shrinking the tree btr_lift_page_up(): Correct the incorrect condition. page_validate(): Validate the page type. --- .../suite/innodb/r/instant_alter_debug.result | 27 +++++++++++++++++++ .../suite/innodb/t/instant_alter_debug.test | 25 +++++++++++++++++ storage/innobase/btr/btr0btr.cc | 3 ++- 3 files changed, 54 insertions(+), 1 deletion(-) diff --git a/mysql-test/suite/innodb/r/instant_alter_debug.result b/mysql-test/suite/innodb/r/instant_alter_debug.result index ebb24d2a5e9..62df17e066f 100644 --- a/mysql-test/suite/innodb/r/instant_alter_debug.result +++ b/mysql-test/suite/innodb/r/instant_alter_debug.result @@ -236,4 +236,31 @@ a b c d 1 2 NULL 1 2 3 4 1 DROP TABLE t1; +# +# MDEV-19916 Corruption after instant ADD/DROP and shrinking the tree +# +CREATE TABLE t1 (a INT PRIMARY KEY) ENGINE=InnoDB; +SET @old_limit = @@innodb_limit_optimistic_insert_debug; +SET GLOBAL innodb_limit_optimistic_insert_debug = 2; +INSERT INTO t1 VALUES (1),(5),(4),(3),(2); +SET GLOBAL innodb_limit_optimistic_insert_debug = @old_limit; +ALTER TABLE t1 ADD COLUMN b INT, ALGORITHM=INSTANT; +SET @old_defragment = @@innodb_defragment; +SET GLOBAL innodb_defragment = 1; +OPTIMIZE TABLE t1; +Table Op Msg_type Msg_text +test.t1 optimize status OK +SET GLOBAL innodb_defragment = @old_defragment; +ALTER TABLE t1 ADD vb INT AS (b) VIRTUAL; +CHECK TABLE t1; +Table Op Msg_type Msg_text +test.t1 check status OK +SELECT * FROM t1; +a b vb +1 NULL NULL +2 NULL NULL +3 NULL NULL +4 NULL NULL +5 NULL NULL +DROP TABLE t1; SET GLOBAL innodb_purge_rseg_truncate_frequency = @save_frequency; diff --git a/mysql-test/suite/innodb/t/instant_alter_debug.test b/mysql-test/suite/innodb/t/instant_alter_debug.test index e54623b9cbd..5b8624a3186 100644 --- a/mysql-test/suite/innodb/t/instant_alter_debug.test +++ b/mysql-test/suite/innodb/t/instant_alter_debug.test @@ -267,4 +267,29 @@ SET DEBUG_SYNC = RESET; SELECT * FROM t1; DROP TABLE t1; +--echo # +--echo # MDEV-19916 Corruption after instant ADD/DROP and shrinking the tree +--echo # +CREATE TABLE t1 (a INT PRIMARY KEY) ENGINE=InnoDB; + +# Create an index tree with 2 levels of node pointer pages. + +SET @old_limit = @@innodb_limit_optimistic_insert_debug; +SET GLOBAL innodb_limit_optimistic_insert_debug = 2; +INSERT INTO t1 VALUES (1),(5),(4),(3),(2); +SET GLOBAL innodb_limit_optimistic_insert_debug = @old_limit; + +ALTER TABLE t1 ADD COLUMN b INT, ALGORITHM=INSTANT; + +SET @old_defragment = @@innodb_defragment; +SET GLOBAL innodb_defragment = 1; +OPTIMIZE TABLE t1; +SET GLOBAL innodb_defragment = @old_defragment; + +# Exploit MDEV-17468 to force the table definition to be reloaded +ALTER TABLE t1 ADD vb INT AS (b) VIRTUAL; +CHECK TABLE t1; +SELECT * FROM t1; +DROP TABLE t1; + SET GLOBAL innodb_purge_rseg_truncate_frequency = @save_frequency; diff --git a/storage/innobase/btr/btr0btr.cc b/storage/innobase/btr/btr0btr.cc index abbf29af7bb..e0fce3fc132 100644 --- a/storage/innobase/btr/btr0btr.cc +++ b/storage/innobase/btr/btr0btr.cc @@ -3453,7 +3453,8 @@ btr_lift_page_up( /* btr_page_empty() is supposed to zero-initialize the field. */ ut_ad(!page_get_instant(father_block->frame)); - if (page_level == 0 && index->is_instant()) { + if (index->is_instant() + && father_block->page.id.page_no() == root_page_no) { ut_ad(!father_page_zip); byte* page_type = father_block->frame + FIL_PAGE_TYPE; ut_ad(mach_read_from_2(page_type) == FIL_PAGE_INDEX);