-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
MDEV-38140 : InnoDB index corruption after UPDATE that affects virtual columns #4470
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 10.6
Are you sure you want to change the base?
Conversation
87a16f4 to
3cf7353
Compare
| for (i = 0; i < update->n_fields; i++) { | ||
| const upd_field_t* fld = | ||
| upd_get_nth_field(update, i); | ||
| if (upd_fld_is_virtual_col(fld)) | ||
| continue; | ||
| const ulint field_no | ||
| = upd_get_nth_field(update, i) | ||
| ->field_no; | ||
| = fld->field_no; | ||
| if (field_no >= index->n_fields | ||
| || dict_index_get_nth_field( | ||
| index, field_no)->col |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is changing the way how UPDATE of indexed virtual columns is logged. However, there is no mention how upgrades or downgrades are being affected by this.
I see that you posted some analysis to MDEV-38140. However, you did not comment on my earlier comment regarding the function row_purge_vc_matches_cluster(). Can you please review that logic as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is changing the way how UPDATE of indexed virtual columns is logged.
-
Hmm, actually this code part is about decision of whether to write non-virtual indexed columns to undo record. After we have completed writing the UPDATE changes of both non-virtual and virtual columns.
(Before this code change we are incorrectly skipping writing thenthnon-virtual indexed column into undo record if we already haventhvirtual indexed column written as part of UPDATE changes.) -
After completing the writing of non-virtual indexed columns we then jump to writing the virtual indexed columns.
(Have made no changes to its logic)
However, there is no mention how upgrades or downgrades are being affected by this.
Regarding upgrades and downgrades, I don't see any issues because, even without patch we do write the non-virtual indexed columns into undo record. And while reading from undo record we handle these columns. For example in row_upd_replace_vcol()
Can you please review that logic as well?
Will do..
columns Issue: - Purge thread attempts to purge a secondary index record that is not delete-marked. Root Cause: - When a secondary index includes a virtual column whose v_pos is greater than the number of fields in the clustered index record, the virtual column is incorrectly skipped while reading from the undo record. - This leads the purge logic to incorrectly assume it is safe to purge the secondary index record. - The code also confuses the nth virtual column with the nth stored column when writing ordering columns at the end of the undo record. Fix: - In trx_undo_update_rec_get_update(): Skip a virtual column only when v_pos == FIL_NULL, not when v_pos is greater than the number of fields. - In trx_undo_page_report_modify(): Ensure ordering columns are written based on the correct stored-column positions, without confusing them with virtual-column positions.
3cf7353 to
e253bee
Compare
Description
MDEV-38140: InnoDB index corruption after UPDATE affecting virtual
columns
Issue:
delete-marked.
Root Cause:
greater than the number of fields in the clustered index record, the
virtual column is incorrectly skipped while reading from the undo
record.
the secondary index record.
column when writing ordering columns at the end of the undo record.
Fix:
trx_undo_update_rec_get_update(): Skip a virtual column onlywhen v_pos == FIL_NULL, not when v_pos is greater than the number
of fields.
trx_undo_page_report_modify(): Ensure ordering columns arewritten based on the correct stored-column positions, without
confusing them with virtual-column positions.
Release Notes
Fixed a potential corruption issue for virtual index.
How can this PR be tested?
Modified
mysql-test/suite/innodb/t/innodb-virtual-columns.testandmysql-test/suite/innodb/r/innodb-virtual-columns.resultto cover the changes.Basing the PR against the correct MariaDB version
mainbranch.PR quality check