diff options
author | Marko Mäkelä <marko.makela@mariadb.com> | 2019-11-20 14:12:53 +0800 |
---|---|---|
committer | Marko Mäkelä <marko.makela@mariadb.com> | 2019-11-20 14:12:53 +0800 |
commit | 6cedb671e99038f1a10e0d8504f835aaabed9780 (patch) | |
tree | 62234b9b0a1380114e584c4e2c2c3ca3f863e52b | |
parent | 1c9240633792ecce562358d86cd8d2c18b94ae32 (diff) | |
download | mariadb-git-6cedb671e99038f1a10e0d8504f835aaabed9780.tar.gz |
MDEV-21088 Table cannot be loaded after instant ADD/DROP COLUMN
btr_cur_instant_init_low(): Accurately parse the metadata record
header for ROW_FORMAT=DYNAMIC and ROW_FORMAT=COMPACT. CHAR columns
used to be unnecessarily written as nonempty strings of bytes.
-rw-r--r-- | mysql-test/suite/innodb/r/instant_alter,4k.rdiff | 4 | ||||
-rw-r--r-- | mysql-test/suite/innodb/r/instant_alter.result | 26 | ||||
-rw-r--r-- | mysql-test/suite/innodb/t/instant_alter.test | 9 | ||||
-rw-r--r-- | storage/innobase/btr/btr0cur.cc | 53 | ||||
-rw-r--r-- | storage/innobase/include/rem0rec.h | 18 | ||||
-rw-r--r-- | storage/innobase/rem/rem0rec.cc | 18 |
6 files changed, 101 insertions, 27 deletions
diff --git a/mysql-test/suite/innodb/r/instant_alter,4k.rdiff b/mysql-test/suite/innodb/r/instant_alter,4k.rdiff index 200868cbf4e..d3a1cee8a85 100644 --- a/mysql-test/suite/innodb/r/instant_alter,4k.rdiff +++ b/mysql-test/suite/innodb/r/instant_alter,4k.rdiff @@ -318,8 +318,8 @@ FROM information_schema.global_status WHERE variable_name = 'innodb_instant_alter_column'; instants --184 -+186 +-187 ++189 SET GLOBAL innodb_purge_rseg_truncate_frequency= @saved_frequency; # # MDEV-18266: Changing an index comment unnecessarily rebuilds index diff --git a/mysql-test/suite/innodb/r/instant_alter.result b/mysql-test/suite/innodb/r/instant_alter.result index b7bc5abb81f..e03ddfb9956 100644 --- a/mysql-test/suite/innodb/r/instant_alter.result +++ b/mysql-test/suite/innodb/r/instant_alter.result @@ -904,6 +904,14 @@ a b c 1 a NULL 2 bah 3 DROP TABLE t1; +CREATE TABLE t1(a CHAR(5) CHARACTER SET utf8 PRIMARY KEY) ENGINE=InnoDB ROW_FORMAT=REDUNDANT; +INSERT INTO t1 VALUES('barf'); +ALTER TABLE t1 ADD b INT FIRST, ALGORITHM=INSTANT; +ALTER TABLE t1 ADD vb INT AS (b); +SELECT * FROM t1; +b a vb +NULL barf NULL +DROP TABLE t1; CREATE TABLE t1 (id INT PRIMARY KEY, c2 INT UNIQUE, c3 POINT NOT NULL DEFAULT ST_GeomFromText('POINT(3 4)'), @@ -1753,6 +1761,14 @@ a b c 1 a NULL 2 bah 3 DROP TABLE t1; +CREATE TABLE t1(a CHAR(5) CHARACTER SET utf8 PRIMARY KEY) ENGINE=InnoDB ROW_FORMAT=COMPACT; +INSERT INTO t1 VALUES('barf'); +ALTER TABLE t1 ADD b INT FIRST, ALGORITHM=INSTANT; +ALTER TABLE t1 ADD vb INT AS (b); +SELECT * FROM t1; +b a vb +NULL barf NULL +DROP TABLE t1; CREATE TABLE t1 (id INT PRIMARY KEY, c2 INT UNIQUE, c3 POINT NOT NULL DEFAULT ST_GeomFromText('POINT(3 4)'), @@ -2602,12 +2618,20 @@ a b c 1 a NULL 2 bah 3 DROP TABLE t1; +CREATE TABLE t1(a CHAR(5) CHARACTER SET utf8 PRIMARY KEY) ENGINE=InnoDB ROW_FORMAT=DYNAMIC; +INSERT INTO t1 VALUES('barf'); +ALTER TABLE t1 ADD b INT FIRST, ALGORITHM=INSTANT; +ALTER TABLE t1 ADD vb INT AS (b); +SELECT * FROM t1; +b a vb +NULL barf NULL +DROP TABLE t1; disconnect analyze; SELECT variable_value-@old_instant instants FROM information_schema.global_status WHERE variable_name = 'innodb_instant_alter_column'; instants -184 +187 SET GLOBAL innodb_purge_rseg_truncate_frequency= @saved_frequency; # # MDEV-18266: Changing an index comment unnecessarily rebuilds index diff --git a/mysql-test/suite/innodb/t/instant_alter.test b/mysql-test/suite/innodb/t/instant_alter.test index 8fc8fe6cd6d..c48a9e45fdc 100644 --- a/mysql-test/suite/innodb/t/instant_alter.test +++ b/mysql-test/suite/innodb/t/instant_alter.test @@ -790,6 +790,15 @@ INSERT INTO t1 VALUES(2,'bah',3); SELECT * FROM t1; DROP TABLE t1; +# MDEV-21088 Table cannot be loaded after instant ADD/DROP COLUMN +eval CREATE TABLE t1(a CHAR(5) CHARACTER SET utf8 PRIMARY KEY) $engine; +INSERT INTO t1 VALUES('barf'); +ALTER TABLE t1 ADD b INT FIRST, ALGORITHM=INSTANT; +# this evicts and reloads the table definition until MDEV-17468 is fixed +ALTER TABLE t1 ADD vb INT AS (b); +SELECT * FROM t1; +DROP TABLE t1; + dec $format; let $redundant_4k= 0; } diff --git a/storage/innobase/btr/btr0cur.cc b/storage/innobase/btr/btr0cur.cc index 90dce5a030a..8d4cb6cdb07 100644 --- a/storage/innobase/btr/btr0cur.cc +++ b/storage/innobase/btr/btr0cur.cc @@ -484,14 +484,55 @@ incompatible: /* This metadata record includes a BLOB that identifies any dropped or reordered columns. */ ulint trx_id_offset = index->trx_id_offset; - if (!trx_id_offset) { - /* 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. */ + /* If !index->trx_id_offset, the PRIMARY KEY contains + variable-length columns. For the metadata record, + variable-length columns should be written with zero + length. However, before MDEV-21088 was fixed, for + variable-length encoded PRIMARY KEY column of type + CHAR, we wrote more than zero bytes. That is why we + must determine the actual length of each PRIMARY KEY + column. The DB_TRX_ID will start right after any + PRIMARY KEY columns. */ + ut_ad(index->n_uniq); + + /* We cannot invoke rec_get_offsets() before + index->table->deserialise_columns(). Therefore, + 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; + for (uint i = index->n_uniq; i--; ) { - trx_id_offset += index->fields[i].fixed_len; + 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; } + } else if (rec_get_1byte_offs_flag(rec)) { + trx_id_offset = rec_1_get_field_end_info( + rec, index->n_uniq - 1); + ut_ad(!(trx_id_offset & REC_1BYTE_SQL_NULL_MASK)); + trx_id_offset &= ~REC_1BYTE_SQL_NULL_MASK; + } else { + trx_id_offset = rec_2_get_field_end_info( + rec, index->n_uniq - 1); + ut_ad(!(trx_id_offset & REC_2BYTE_SQL_NULL_MASK)); + trx_id_offset &= ~REC_2BYTE_SQL_NULL_MASK; } const byte* ptr = rec + trx_id_offset diff --git a/storage/innobase/include/rem0rec.h b/storage/innobase/include/rem0rec.h index 0aadf59bfb2..29923839ce1 100644 --- a/storage/innobase/include/rem0rec.h +++ b/storage/innobase/include/rem0rec.h @@ -291,6 +291,24 @@ inline unsigned rec_get_n_add_field_len(ulint n_add_field) return n_add_field < 0x80 ? 1 : 2; } +/** Get the added field count in a REC_STATUS_INSTANT record. +@param[in,out] header variable header of a REC_STATUS_INSTANT record +@return number of added fields */ +inline unsigned rec_get_n_add_field(const byte*& header) +{ + unsigned n_fields_add = *--header; + if (n_fields_add < 0x80) { + ut_ad(rec_get_n_add_field_len(n_fields_add) == 1); + return n_fields_add; + } + + n_fields_add &= 0x7f; + n_fields_add |= unsigned(*--header) << 7; + ut_ad(n_fields_add < REC_MAX_N_FIELDS); + ut_ad(rec_get_n_add_field_len(n_fields_add) == 2); + return n_fields_add; +} + /** Set the added field count in a REC_STATUS_INSTANT record. @param[in,out] header variable header of a REC_STATUS_INSTANT record @param[in] n_add number of added fields, minus 1 diff --git a/storage/innobase/rem/rem0rec.cc b/storage/innobase/rem/rem0rec.cc index 1f0f661448c..91fdddeaf07 100644 --- a/storage/innobase/rem/rem0rec.cc +++ b/storage/innobase/rem/rem0rec.cc @@ -231,24 +231,6 @@ rec_get_n_extern_new( return(n_extern); } -/** Get the added field count in a REC_STATUS_INSTANT record. -@param[in,out] header variable header of a REC_STATUS_INSTANT record -@return number of added fields */ -static inline unsigned rec_get_n_add_field(const byte*& header) -{ - unsigned n_fields_add = *--header; - if (n_fields_add < 0x80) { - ut_ad(rec_get_n_add_field_len(n_fields_add) == 1); - return n_fields_add; - } - - n_fields_add &= 0x7f; - n_fields_add |= unsigned(*--header) << 7; - ut_ad(n_fields_add < REC_MAX_N_FIELDS); - ut_ad(rec_get_n_add_field_len(n_fields_add) == 2); - return n_fields_add; -} - /** Format of a leaf-page ROW_FORMAT!=REDUNDANT record */ enum rec_leaf_format { /** Temporary file record */ |