Skip to content

Commit 1aca995

Browse files
committed
MDEV-39792 InnoDB: ALTER TABLE FORCE triggers assertion "s" in buf_page_get_gen()
When rebuilding a table from ROW_FORMAT=COMPACT or DYNAMIC into ROW_FORMAT=REDUNDANT, row_merge_buf_add() fetches the full value of an externally stored (off-page) CHAR column in a multi-byte character set and pads it to REDUNDANT's fixed local width via row_merge_buf_redundant_convert(). That helper already dereferences the BLOB and calls dfield_set_data(), which clears the field's "externally stored" flag, since the value is now held in full locally. The "flag externally stored fields" step further down in row_merge_buf_add() did not know this had happened. It still consulted the row_ext_t cache built from the original (pre-conversion) record and, for a column that is not part of the clustered index's unique key, called dfield_set_ext() again on the very field that had just been converted, without restoring its data pointer to a valid 20-byte external reference. row_merge_copy_blobs() would then read the tail of the padded, space-filled buffer as if it were a BTR_EXTERN_FIELD_REF, deriving a garbage tablespace id and crashing buf_page_get_gen()'s fil_space_get() assertion when the alter tried to build the new clustered index. Skip the re-flagging step for a field whose "externally stored" flag is no longer set. row_build() flags every off-page column, and the row_ext_t cache only holds a subset of those columns, so a field that is not flagged is either a converted one (already fully local) or one that the cache does not hold. With the field no longer re-flagged, the rebuild completes, and the rebuilt table passes CHECK TABLE with the full column value. The MDEV-31025 case in innodb.default_row_format_alter failed on innodb_page_size=4k and 8k: its ROW_FORMAT=REDUNDANT table has eight utf32 CHAR(255) columns, which CREATE TABLE rejects with ER_TOO_BIG_ROWSIZE on those page sizes. Derive the number of columns from the page size, so that the record still exceeds the maximum local record size and the fixed-length column c is stored externally. The whole test now passes on every page size.
1 parent f49e838 commit 1aca995

3 files changed

Lines changed: 65 additions & 9 deletions

File tree

‎mysql-test/suite/innodb/r/default_row_format_alter.result‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -135,10 +135,6 @@ DROP TABLE t1;
135135
#
136136
set @old_sql_mode = @@sql_mode;
137137
SET @@sql_mode='';
138-
CREATE TABLE t1(pk INT,c CHAR(255),c2 CHAR(255),c3 CHAR(255),
139-
c4 char(255), c5 char(255), c6 char(255),
140-
c7 char(255), c8 char(255), primary key(pk)
141-
)Engine=InnoDB character set utf32 ROW_FORMAT=REDUNDANT;
142138
INSERT INTO t1(pk, c) VALUES (1, repeat('a', 255));
143139
ALTER TABLE t1 FORCE;
144140
CHECK TABLE t1;
@@ -150,4 +146,20 @@ LENGTH(c)
150146
DROP TABLE t1;
151147
set @@sql_mode = @old_sql_mode;
152148
# End of 10.4 tests
149+
#
150+
# MDEV-39792 InnoDB: ALTER TABLE FORCE triggers assertion "s"
151+
# in buf_page_get_gen() when converting an externally stored
152+
# CHAR column to ROW_FORMAT=REDUNDANT
153+
#
154+
SET @a=REPEAT(_utf8mb4 0xF09F9880,254);
155+
SET GLOBAL innodb_default_row_format = @row_format;
156+
ALTER TABLE t1 ROW_FORMAT=REDUNDANT;
157+
CHECK TABLE t1;
158+
Table Op Msg_type Msg_text
159+
test.t1 check status OK
160+
SELECT c1=@a FROM t1;
161+
c1=@a
162+
1
163+
DROP TABLE t1;
164+
# End of 11.4 tests
153165
SET GLOBAL innodb_default_row_format = @row_format;

‎mysql-test/suite/innodb/t/default_row_format_alter.test‎

Lines changed: 44 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -156,10 +156,20 @@ DROP TABLE t1;
156156
--echo #
157157
set @old_sql_mode = @@sql_mode;
158158
SET @@sql_mode='';
159-
CREATE TABLE t1(pk INT,c CHAR(255),c2 CHAR(255),c3 CHAR(255),
160-
c4 char(255), c5 char(255), c6 char(255),
161-
c7 char(255), c8 char(255), primary key(pk)
162-
)Engine=InnoDB character set utf32 ROW_FORMAT=REDUNDANT;
159+
# The record must exceed the maximum local record size of the page,
160+
# so that the fixed-length column c is stored externally.
161+
let $n_cols = `SELECT LEAST(8, @@innodb_page_size DIV 2048)`;
162+
let $cols = c CHAR(255);
163+
let $i = 2;
164+
while ($i <= $n_cols)
165+
{
166+
let $cols = $cols, c$i CHAR(255);
167+
inc $i;
168+
}
169+
--disable_query_log
170+
eval CREATE TABLE t1(pk INT, $cols, PRIMARY KEY(pk))
171+
ENGINE=InnoDB CHARACTER SET utf32 ROW_FORMAT=REDUNDANT;
172+
--enable_query_log
163173
INSERT INTO t1(pk, c) VALUES (1, repeat('a', 255));
164174
ALTER TABLE t1 FORCE;
165175
CHECK TABLE t1;
@@ -169,4 +179,34 @@ set @@sql_mode = @old_sql_mode;
169179

170180
--echo # End of 10.4 tests
171181

182+
--echo #
183+
--echo # MDEV-39792 InnoDB: ALTER TABLE FORCE triggers assertion "s"
184+
--echo # in buf_page_get_gen() when converting an externally stored
185+
--echo # CHAR column to ROW_FORMAT=REDUNDANT
186+
--echo #
187+
# The record must exceed the maximum local record size of the page, so
188+
# that c1 is stored externally, and still fit in ROW_FORMAT=REDUNDANT.
189+
let $n_cols = `SELECT IF(@@innodb_page_size <= 16384, @@innodb_page_size DIV 2048, 18)`;
190+
let $cols = c1 CHAR(255);
191+
let $vals = @a;
192+
let $i = 2;
193+
while ($i <= $n_cols)
194+
{
195+
let $cols = $cols, c$i CHAR(255);
196+
let $vals = $vals, @a;
197+
inc $i;
198+
}
199+
SET @a=REPEAT(_utf8mb4 0xF09F9880,254);
200+
SET GLOBAL innodb_default_row_format = @row_format;
201+
--disable_query_log
202+
eval CREATE TABLE t1 ($cols, KEY(c1(1))) ENGINE=InnoDB CHARSET=utf8mb4;
203+
eval INSERT INTO t1 VALUES ($vals);
204+
--enable_query_log
205+
ALTER TABLE t1 ROW_FORMAT=REDUNDANT;
206+
CHECK TABLE t1;
207+
SELECT c1=@a FROM t1;
208+
DROP TABLE t1;
209+
210+
--echo # End of 11.4 tests
211+
172212
SET GLOBAL innodb_default_row_format = @row_format;

‎storage/innobase/row/row0merge.cc‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -774,7 +774,11 @@ row_merge_buf_add(
774774
if (dfield_is_null(field)) {
775775
ut_ad(!(col->prtype & DATA_NOT_NULL));
776776
continue;
777-
} else if (!ext) {
777+
} else if (!ext || !dfield_is_ext(field)) {
778+
/* A field that
779+
row_merge_buf_redundant_convert() fetched
780+
in full is no longer externally stored,
781+
and must not be re-flagged as such. */
778782
} else if (dict_index_is_clust(index)) {
779783
/* Flag externally stored fields. */
780784
const byte* buf = row_ext_lookup(ext, col->ind,

0 commit comments

Comments
 (0)