From 73a10cbcc5178ae5378abb821428d35d3276b4da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Wed, 2 May 2018 15:44:52 +0300 Subject: [PATCH] MDEV-16065 Assertion failed in btr_pcur_restore_position_func on UPDATE btr_pcur_store_position(): Assert that the 'default row' record never is the only record in a page. (If that would happen, an empty root page would be re-created in the non-instant format, not containing the special record.) When the cursor is positioned on the page infimum, never use the 'default row' as the BTR_PCUR_BEFORE reference. (This is additional cleanup, not fixing the bug.) rec_copy_prefix_to_buf(): When converting a record prefix to the non-instant-add format, copy the original number of null flags. Rename the variable instant_len to instant_omit, and introduce a few more variables to make the code easiser to read. Note: In purge, rec_copy_prefix_to_buf() is also used for storing the persistent cursor position on a 'default row' record. The stored record reference will be garbage, but row_search_on_row_ref() will do special handling to reposition the cursor on the 'default row', based on ref->info_bits. innodb.dml_purge: Also cover the 'default row'. --- mysql-test/suite/innodb/r/dml_purge.result | 18 +++-- .../suite/innodb/r/instant_alter.result | 20 +++++- .../suite/innodb/r/instant_alter_debug.result | 2 +- mysql-test/suite/innodb/t/dml_purge.test | 3 +- mysql-test/suite/innodb/t/instant_alter.test | 8 +++ storage/innobase/btr/btr0pcur.cc | 17 +++-- storage/innobase/rem/rem0rec.cc | 72 +++++++++++-------- 7 files changed, 94 insertions(+), 46 deletions(-) diff --git a/mysql-test/suite/innodb/r/dml_purge.result b/mysql-test/suite/innodb/r/dml_purge.result index eb542484724..95330b80d33 100644 --- a/mysql-test/suite/innodb/r/dml_purge.result +++ b/mysql-test/suite/innodb/r/dml_purge.result @@ -11,6 +11,7 @@ connect prevent_purge,localhost,root; START TRANSACTION WITH CONSISTENT SNAPSHOT; connection default; INSERT INTO t1 VALUES(1,2),(3,4); +ALTER TABLE t1 ADD COLUMN c INT; UPDATE t1 SET b=-3 WHERE a=3; connect con1,localhost,root; BEGIN; @@ -21,8 +22,13 @@ InnoDB 0 transactions not purged disconnect con1; FLUSH TABLE t1 FOR EXPORT; Clustered index root page contents: -N_RECS=2; LEVEL=0 -header=0x010000030087 (a=0x696e66696d756d00) +N_RECS=3; LEVEL=0 +header=0x0100000300c6 (a=0x696e66696d756d00) +header=0x1000200b0087 (a=0x80000000, + DB_TRX_ID=0x000000000000, + DB_ROLL_PTR=0x80000000000000, + b=0x80000000, + c=NULL(4 bytes)) header=0x0000100900a6 (a=0x80000001, DB_TRX_ID=0x000000000000, DB_ROLL_PTR=0x80000000000000, @@ -31,11 +37,11 @@ header=0x000018090074 (a=0x80000003, DB_TRX_ID=0x000000000000, DB_ROLL_PTR=0x80000000000000, b=0x7ffffffd) -header=0x030008030000 (a=0x73757072656d756d00) +header=0x040008030000 (a=0x73757072656d756d00) UNLOCK TABLES; SELECT * FROM t1; -a b -1 2 -3 -3 +a b c +1 2 NULL +3 -3 NULL DROP TABLE t1; SET GLOBAL innodb_purge_rseg_truncate_frequency = @saved_frequency; diff --git a/mysql-test/suite/innodb/r/instant_alter.result b/mysql-test/suite/innodb/r/instant_alter.result index 1580ba29717..a70a3d077e0 100644 --- a/mysql-test/suite/innodb/r/instant_alter.result +++ b/mysql-test/suite/innodb/r/instant_alter.result @@ -440,6 +440,12 @@ SELECT * FROM t1; a b a 1 DROP TABLE t1; +CREATE TABLE t1 (a INT, b VARCHAR(8), PRIMARY KEY(b,a)) ENGINE=InnoDB ROW_FORMAT=REDUNDANT; +INSERT INTO t1 VALUES (1,'foo'); +ALTER TABLE t1 ADD COLUMN c INT; +UPDATE t1 SET c = 1; +UPDATE t1 SET c = 2; +DROP TABLE t1; CREATE TABLE t1 (id INT PRIMARY KEY, c2 INT UNIQUE, c3 POINT NOT NULL DEFAULT ST_GeomFromText('POINT(3 4)'), @@ -826,6 +832,12 @@ SELECT * FROM t1; a b a 1 DROP TABLE t1; +CREATE TABLE t1 (a INT, b VARCHAR(8), PRIMARY KEY(b,a)) ENGINE=InnoDB ROW_FORMAT=COMPACT; +INSERT INTO t1 VALUES (1,'foo'); +ALTER TABLE t1 ADD COLUMN c INT; +UPDATE t1 SET c = 1; +UPDATE t1 SET c = 2; +DROP TABLE t1; CREATE TABLE t1 (id INT PRIMARY KEY, c2 INT UNIQUE, c3 POINT NOT NULL DEFAULT ST_GeomFromText('POINT(3 4)'), @@ -1212,10 +1224,16 @@ SELECT * FROM t1; a b a 1 DROP TABLE t1; +CREATE TABLE t1 (a INT, b VARCHAR(8), PRIMARY KEY(b,a)) ENGINE=InnoDB ROW_FORMAT=DYNAMIC; +INSERT INTO t1 VALUES (1,'foo'); +ALTER TABLE t1 ADD COLUMN c INT; +UPDATE t1 SET c = 1; +UPDATE t1 SET c = 2; +DROP TABLE t1; disconnect analyze; SELECT variable_value-@old_instant instants FROM information_schema.global_status WHERE variable_name = 'innodb_instant_alter_column'; instants -36 +39 SET GLOBAL innodb_purge_rseg_truncate_frequency= @saved_frequency; diff --git a/mysql-test/suite/innodb/r/instant_alter_debug.result b/mysql-test/suite/innodb/r/instant_alter_debug.result index 3aec7553ff0..bd7d538678a 100644 --- a/mysql-test/suite/innodb/r/instant_alter_debug.result +++ b/mysql-test/suite/innodb/r/instant_alter_debug.result @@ -35,7 +35,7 @@ ALTER TABLE t4 ADD COLUMN b INT; SELECT COUNT(*) FROM INFORMATION_SCHEMA.COLUMNS LEFT JOIN t4 ON (NUMERIC_SCALE = pk); COUNT(*) -1748 +953 SET DEBUG_SYNC='innodb_inplace_alter_table_enter SIGNAL enter WAIT_FOR delete'; ALTER TABLE t4 ADD COLUMN c INT; connect dml,localhost,root,,; diff --git a/mysql-test/suite/innodb/t/dml_purge.test b/mysql-test/suite/innodb/t/dml_purge.test index 4f93e9eb917..37178982c8d 100644 --- a/mysql-test/suite/innodb/t/dml_purge.test +++ b/mysql-test/suite/innodb/t/dml_purge.test @@ -20,6 +20,7 @@ START TRANSACTION WITH CONSISTENT SNAPSHOT; --connection default INSERT INTO t1 VALUES(1,2),(3,4); +ALTER TABLE t1 ADD COLUMN c INT; UPDATE t1 SET b=-3 WHERE a=3; --connect (con1,localhost,root) @@ -47,7 +48,7 @@ sysseek(FILE, 3*$ps, 0) || die "Unable to seek $file"; die "Unable to read $file" unless sysread(FILE, $page, $ps) == $ps; print "N_RECS=", unpack("n", substr($page,38+16,2)); print "; LEVEL=", unpack("n", substr($page,38+26,2)), "\n"; -my @fields=("a","DB_TRX_ID","DB_ROLL_PTR", "b"); +my @fields=qw(a DB_TRX_ID DB_ROLL_PTR b c); for (my $offset= 0x65; $offset; $offset= unpack("n", substr($page,$offset-2,2))) { diff --git a/mysql-test/suite/innodb/t/instant_alter.test b/mysql-test/suite/innodb/t/instant_alter.test index d95f412fb37..c414b92f713 100644 --- a/mysql-test/suite/innodb/t/instant_alter.test +++ b/mysql-test/suite/innodb/t/instant_alter.test @@ -311,6 +311,14 @@ INSERT INTO t1 SET a='a'; SELECT * FROM t1; DROP TABLE t1; +# MDEV-16065 Assertion failed in btr_pcur_restore_position_func on UPDATE +eval CREATE TABLE t1 (a INT, b VARCHAR(8), PRIMARY KEY(b,a)) $engine; +INSERT INTO t1 VALUES (1,'foo'); +ALTER TABLE t1 ADD COLUMN c INT; +UPDATE t1 SET c = 1; +UPDATE t1 SET c = 2; +DROP TABLE t1; + dec $format; } disconnect analyze; diff --git a/storage/innobase/btr/btr0pcur.cc b/storage/innobase/btr/btr0pcur.cc index e8a74551f67..46f563a7379 100644 --- a/storage/innobase/btr/btr0pcur.cc +++ b/storage/innobase/btr/btr0pcur.cc @@ -127,6 +127,8 @@ btr_pcur_store_position( mtr, dict_index_get_lock(index), MTR_MEMO_X_LOCK | MTR_MEMO_SX_LOCK))); + cursor->old_stored = true; + if (page_is_empty(page)) { /* It must be an empty index tree; NOTE that in this case we do not store the modify_clock, but always do a search @@ -136,10 +138,7 @@ btr_pcur_store_position( ut_ad(page_is_leaf(page)); ut_ad(page_get_page_no(page) == index->page); - cursor->old_stored = true; - if (page_rec_is_supremum_low(offs)) { - cursor->rel_pos = BTR_PCUR_AFTER_LAST_IN_TREE; } else { cursor->rel_pos = BTR_PCUR_BEFORE_FIRST_IN_TREE; @@ -149,21 +148,25 @@ btr_pcur_store_position( } if (page_rec_is_supremum_low(offs)) { - rec = page_rec_get_prev(rec); + ut_ad(!page_rec_is_infimum(rec)); + ut_ad(!rec_is_default_row(rec, index)); + cursor->rel_pos = BTR_PCUR_AFTER; - } else if (page_rec_is_infimum_low(offs)) { - rec = page_rec_get_next(rec); + if (rec_is_default_row(rec, index)) { + rec = page_rec_get_next(rec); + ut_ad(!page_rec_is_supremum(rec)); + } + cursor->rel_pos = BTR_PCUR_BEFORE; } else { cursor->rel_pos = BTR_PCUR_ON; } - cursor->old_stored = true; cursor->old_rec = dict_index_copy_rec_order_prefix( index, rec, &cursor->old_n_fields, &cursor->old_rec_buf, &cursor->buf_size); diff --git a/storage/innobase/rem/rem0rec.cc b/storage/innobase/rem/rem0rec.cc index 0180d10ed44..6da2866b0c1 100644 --- a/storage/innobase/rem/rem0rec.cc +++ b/storage/innobase/rem/rem0rec.cc @@ -1835,11 +1835,6 @@ rec_copy_prefix_to_buf( or NULL */ ulint* buf_size) /*!< in/out: buffer size */ { - const byte* nulls; - const byte* lens; - ulint prefix_len = 0; - ulint instant_len = 0; - ut_ad(n_fields <= index->n_fields || dict_index_is_ibuf(index)); ut_ad(index->n_core_null_bytes <= UT_BITS_IN_BYTES(index->n_nullable)); UNIV_PREFETCH_RW(*buf); @@ -1852,6 +1847,12 @@ rec_copy_prefix_to_buf( buf, buf_size)); } + ulint prefix_len = 0; + ulint instant_omit = 0; + const byte* nulls = rec - (REC_N_NEW_EXTRA_BYTES + 1); + const byte* nullf = nulls; + const byte* lens = nulls - index->n_core_null_bytes; + switch (rec_get_status(rec)) { default: /* infimum or supremum record: no sense to copy anything */ @@ -1859,12 +1860,8 @@ rec_copy_prefix_to_buf( return(NULL); case REC_STATUS_ORDINARY: ut_ad(n_fields <= index->n_core_fields); - nulls = rec - (REC_N_NEW_EXTRA_BYTES + 1); - lens = nulls - index->n_core_null_bytes; break; case REC_STATUS_NODE_PTR: - nulls = rec - (REC_N_NEW_EXTRA_BYTES + 1); - lens = nulls - index->n_core_null_bytes; /* For R-tree, we need to copy the child page number field. */ compile_time_assert(DICT_INDEX_SPATIAL_NODEPTR_SIZE == 1); if (dict_index_is_spatial(index)) { @@ -1890,15 +1887,18 @@ rec_copy_prefix_to_buf( /* We would have !index->is_instant() when rolling back an instant ADD COLUMN operation. */ ut_ad(index->is_instant() || page_rec_is_default_row(rec)); - nulls = &rec[-REC_N_NEW_EXTRA_BYTES]; + nulls++; const ulint n_rec = ulint(index->n_core_fields) + 1 + rec_get_n_add_field(nulls); - instant_len = ulint(&rec[-REC_N_NEW_EXTRA_BYTES] - nulls); - ut_ad(instant_len == 1 || instant_len == 2); - const uint n_nullable = index->get_n_nullable(n_rec); - lens = --nulls - UT_BITS_IN_BYTES(n_nullable); + instant_omit = ulint(&rec[-REC_N_NEW_EXTRA_BYTES] - nulls); + ut_ad(instant_omit == 1 || instant_omit == 2); + nullf = nulls; + const uint nb = UT_BITS_IN_BYTES(index->get_n_nullable(n_rec)); + instant_omit += nb - index->n_core_null_bytes; + lens = --nulls - nb; } + const byte* const lenf = lens; UNIV_PREFETCH_R(lens); /* read the lengths of fields 0..n */ @@ -1950,27 +1950,39 @@ rec_copy_prefix_to_buf( UNIV_PREFETCH_R(rec + prefix_len); - prefix_len += ulint(rec - (lens + 1)) - instant_len; + ulint size = prefix_len + ulint(rec - (lens + 1)) - instant_omit; - if ((*buf == NULL) || (*buf_size < prefix_len)) { + if (*buf == NULL || *buf_size < size) { ut_free(*buf); - *buf_size = prefix_len; - *buf = static_cast(ut_malloc_nokey(prefix_len)); + *buf_size = size; + *buf = static_cast(ut_malloc_nokey(size)); } - if (instant_len) { - ulint hdr = ulint(&rec[-REC_N_NEW_EXTRA_BYTES] - (lens + 1)) - - instant_len; - memcpy(*buf, lens + 1, hdr); - memcpy(*buf + hdr, &rec[-REC_N_NEW_EXTRA_BYTES], - prefix_len - hdr); - ut_ad(rec_get_status(*buf + hdr + REC_N_NEW_EXTRA_BYTES) - == REC_STATUS_COLUMNS_ADDED); - rec_set_status(*buf + hdr + REC_N_NEW_EXTRA_BYTES, - REC_STATUS_ORDINARY); - return *buf + hdr + REC_N_NEW_EXTRA_BYTES; + if (instant_omit) { + /* Copy and convert the record header to a format where + instant ADD COLUMN has not been used: + + lengths of variable-length fields in the prefix + - omit any null flag bytes for any instantly added columns + + index->n_core_null_bytes of null flags + - omit the n_add_fields header (1 or 2 bytes) + + REC_N_NEW_EXTRA_BYTES of fixed header */ + byte* b = *buf; + /* copy the lengths of the variable-length fields */ + memcpy(b, lens + 1, ulint(lenf - lens)); + b += ulint(lenf - lens); + /* copy the null flags */ + memcpy(b, nullf - index->n_core_null_bytes, + index->n_core_null_bytes); + b += index->n_core_null_bytes + REC_N_NEW_EXTRA_BYTES; + ut_ad(ulint(b - *buf) + prefix_len == size); + /* copy the fixed-size header and the record prefix */ + memcpy(b - REC_N_NEW_EXTRA_BYTES, rec - REC_N_NEW_EXTRA_BYTES, + prefix_len + REC_N_NEW_EXTRA_BYTES); + ut_ad(rec_get_status(b) == REC_STATUS_COLUMNS_ADDED); + rec_set_status(b, REC_STATUS_ORDINARY); + return b; } else { - memcpy(*buf, lens + 1, prefix_len); + memcpy(*buf, lens + 1, size); return *buf + (rec - (lens + 1)); } }