[core] Merge deletion vectors across a bitmap64 flip - #10282
Open
LuciferYang wants to merge 3 commits into
Open
LuciferYang wants to merge 3 commits into
LuciferYang wants to merge 3 commits into
Conversation
Flipping deletion-vectors.bitmap64 on a table with existing deletion vectors leaves bitmap32 vectors in the index files — a state the Iceberg callback already acknowledges. The append maintainer merged the stored vector into the fresh bitmap64 one unconditionally, which threw for the mixed types and crashed every subsequent delete on those files. Convert a stored bitmap32 vector to bitmap64 before merging. Assisted-by: GLM-5.3
Extract DeletionVector.mergeVectors, promoting the bitmap32 side to bitmap64 whenever either side is bitmap64, so notifyNewDeletionVector no longer throws when a fresh bitmap32 vector meets a stored bitmap64 one after the option is flipped off (the reverse of the case the original fix covered).
BucketedDvMaintainer.mergeNewDeletion (the bucketed-append path via BucketedAppendDeleteFileMaintainer) had the same mixed-type merge crash as the unaware-append path: a fresh option-driven vector was merged into a stored vector of the other type after the option was flipped. Route it through DeletionVector.mergeVectors so the bitmap32 side is promoted first.
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
1 participant
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.
Purpose
deletion-vectors.bitmap64is not an immutable option, so it can be flipped withALTER TABLE ... SETon a table that already has deletion vectors. After a flip, a freshly created deletion vector (in the new format) has to merge with a stored one (in the old format), butBitmap64DeletionVector.mergeandBitmapDeletionVector.mergereject a different class withOnly instance with the same class type can be merged., crashing the delete. Both flip directions are affected, and the crash occurs on both the unaware-append and bucketed-append merge paths.This adds
DeletionVector.mergeVectors(fresh, stored), which promotes the bitmap32 side to bitmap64 (via the existingfromBitmapDeletionVector) whenever either side is bitmap64 and then merges at the wider format, and routes bothAppendDeleteFileMaintainer.notifyNewDeletionVectorandBucketedDvMaintainer.mergeNewDeletionthrough it. A bitmap64 result reads back correctly regardless of the current option value, because deletion vectors are dispatched by magic number on read and bitmap32/bitmap64 vectors already coexist within one index file.This closes #10281.
Tests
AppendDeletionFileMaintainerTestpins both flip directions on the unaware-append path (stored bitmap32 + fresh bitmap64, and the reverse).BucketedDvMaintainerTestpins both flip directions on the bucketed-append path.Each fails on the pre-fix code with the class-type
RuntimeException.API and Format
No. The on-disk deletion-vector format is unchanged; the fix only merges the two existing formats in memory.
Documentation
No.