Conversation
…ge_get_gen() When rebuilding a table into ROW_FORMAT=REDUNDANT, row_merge_buf_add() fetches the full value of an externally stored (off-page) CHAR/VARCHAR column 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. Track whether the field went through the redundant-format conversion, and skip the stale re-flagging step for it: a converted field is already fully local and must not be treated as external again. With the field no longer corrupted, InnoDB's existing row-size check correctly rejects the ALTER with ER_TOO_BIG_ROWSIZE instead of crashing, because REDUNDANT's mandatory local BLOB prefixes make the record too large for a leaf page.
iMineLink
requested review from
Thirunarayanan
and
a lite review from Copilot
September 21, 2026 10:04
|
|
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The change is narrowly scoped to the crash mechanism and includes a targeted regression test to ensure the failure mode is an error rather than an assertion/crash.
Review effort: Lite
Findings: 1
Open (1)
What changed in this PR
Fixes an InnoDB crash during ALTER TABLE ... FORCE when rebuilding into ROW_FORMAT=REDUNDANT and encountering an externally stored CHAR/VARCHAR value that gets materialized during redundant-format conversion.
Changes:
- Track whether a field was materialized from external storage during redundant-format conversion and skip stale re-flagging as “externally stored”.
- Add an InnoDB MTR regression test that verifies the operation fails with
ER_TOO_BIG_ROWSIZEinstead of crashing.
| File | Description |
|---|---|
| storage/innobase/row/row0merge.cc | Prevent re-applying dfield_set_ext() after redundant conversion has already materialized an external value locally. |
| mysql-test/suite/innodb/t/alter_force_redundant_ext_char.test | New regression test reproducing the scenario and asserting ER_TOO_BIG_ROWSIZE. |
| mysql-test/suite/innodb/r/alter_force_redundant_ext_char.result | Expected output for the new regression test. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+769
to
+774
| /* The field was already fetched in | ||
| full and converted to a fixed-width | ||
| ROW_FORMAT=REDUNDANT value above; it | ||
| is no longer externally stored, and | ||
| must not be re-flagged as such below. */ | ||
| converted_from_ext = row_field->ext; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…ge_get_gen()
When rebuilding a table into ROW_FORMAT=REDUNDANT, row_merge_buf_add() fetches the full value of an externally stored (off-page) CHAR/VARCHAR column 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.
Track whether the field went through the redundant-format conversion, and skip the stale re-flagging step for it: a converted field is already fully local and must not be treated as external again.
With the field no longer corrupted, InnoDB's existing row-size check correctly rejects the ALTER with ER_TOO_BIG_ROWSIZE instead of crashing, because REDUNDANT's mandatory local BLOB prefixes make the record too large for a leaf page.