perf(scan): resolve deletions from row offsets instead of row addresses - #9373
Open
taozhiyi13 wants to merge 1 commit into
Open
taozhiyi13 wants to merge 1 commit into
taozhiyi13 wants to merge 1 commit into
Conversation
A scan over a fragment with deletions probed the vector once per row and materialized a u64 address for every row just to recover the offset. Cost followed rows scanned, not rows deleted: 10 deletes in a 1M-row fragment made a full scan ~3x slower. Build the keep mask from the batch's offset ranges. Stop fetching row addresses only because the fragment has deletions.
Contributor
There was a problem hiding this comment.
✅ Gate recommendation: approve.
The range-driven keep mask addresses the actual hot path: its work follows deletions intersecting a batch instead of all scanned rows, without materializing row addresses unless requested. It preserves batch order for disjoint selections and retains the existing filter/null and system-column semantics. No blocking issues found.
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.
Problem
Filtering a scan batch against a deletion vector does not need row addresses.
A deletion vector stores fragment-local offsets. The batch already knows which offsets it covers. The keep bitmap for
filter_record_batchis just those two sets of offsets. The old path still materialized au64row address per row and probed the vector once per row, only to throw away the fragment id and recover the offset.Where this sits
The first three steps are unchanged. This PR only changes how
apply_row_id_and_deletesbuilds that bitmap.Change
The batch is still filtered with a keep bitmap. This PR only changes how that bitmap is produced.
apply_row_id_and_deletesalready knows the batch's fragment-local offset ranges.DeletionVector::build_keep_maskintersects those ranges with the DV's offsets and writes hits onto a bitmap in batch-row order. That bitmap goes tofilter_record_batch, same as before._rowidand_rowaddrare still attached in this same function when the caller asks for them. They are just no longer a prerequisite for deletion filtering. If this batch hits no deleted offset, skipping the bitmap is the same as filteringwith an all-true mask: every row is kept.Test
build_keep_maskis checked bit-for-bit against the oldbuild_predicate, including a batch whose physical ranges are not one run (0..50, 60..110).test_deletesstill covers_rowidandmake_deletions_null.Effect
Local
to_table()on one 1M-row fragment.00d87ac5vs9f463f63, pylance 13.0.0-beta.4, best of 10 after 3 warmups. Multipliers are vs the unpatched no-deletion scan (3.64 ms):00d87ac59f463f63no_deletionscontiguous_deletionsscattered_deletionsContiguous and scattered cost the same on the old path, so the extra ~16 ms was not IO. After this change both sit on the clean-fragment scan (about 4.5–4.9× faster than the old deletion path). Row counts and
sum(id)matched across builds.