fix: normalize list offsets in merge_with_schema for sliced arrays#6594
Draft
LuciferYang wants to merge 1 commit intolance-format:mainfrom
Draft
fix: normalize list offsets in merge_with_schema for sliced arrays#6594LuciferYang wants to merge 1 commit intolance-format:mainfrom
LuciferYang wants to merge 1 commit intolance-format:mainfrom
Conversation
…ays (lance-format#6580) When a ListArray is sliced (e.g. from a filtered scan's trailing batch), its offsets don't start at 0. `trimmed_values()` returns values starting from index 0, but `offsets().clone()` retained the original absolute offsets, causing an "offset past values" panic. Normalize offsets by subtracting offsets[0] so they are consistent with the zero-based trimmed values buffer. Applies to both List and LargeList branches.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Contributor
Author
|
Let me fix the CI issues first. |
LuciferYang
force-pushed
the
fix-6580-list-merge-offsets
branch
from
a2bbcc2 to
27f63bd
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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.
Summary
Fixes an "offset past values" panic in
merge_with_schemawhen the left input is a slicedListArray(orLargeListArray) whose first offset is non-zero.Root cause
In
rust/lance-arrow/src/lib.rs, the list-merge branches pairtrimmed_values()(which returns values starting at index 0) withoffsets().clone()(which retains the original absolute offsets from before slicing). When the sliced array's first offset is > 0, the re-assembledListArrayhas offsets that point past the end of the trimmed values buffer, and Arrow's invariant check panics.Sliced list inputs are common in practice — filtered scans and
takeboth produce them, e.g. the trailing batch of amerge_insertagainst a filtered dataset.Fix
Added
normalize_offsets(), which subtracts the first offset from each offset so the buffer is zero-based and consistent with the trimmed values. Zero-first-offset inputs short-circuit and clone without allocation. Applied in both theListandLargeListmerge branches.Test plan
test_merge_with_schema_sliced_list_with_nonzero_offsetinlance-arrow— builds alist<struct{id}>, slices rows 1..3 so offsets start at 2, merges against a schema that adds a new struct field, and verifies the merged batch opens without panic and carries the correct values.cargo test -p lance-arrowcargo clippy --all --tests -- -D warningscargo fmt --all -- --check