Skip to content

fix: shift offsets after trimming values in merge_with_schema#6581

Open
kaan-simbe wants to merge 3 commits into
lance-format:mainfrom
SimbeRobotics:fix/merge-with-schema-sliced-list
Open

fix: shift offsets after trimming values in merge_with_schema#6581
kaan-simbe wants to merge 3 commits into
lance-format:mainfrom
SimbeRobotics:fix/merge-with-schema-sliced-list

Conversation

@kaan-simbe

Copy link
Copy Markdown

Summary

  • Fix panic InvalidArgumentError("Max offset of X exceeds length of values Y") in ListArray::new during to_table(filter=..., columns=[list_struct_col, ...]) on v2.0 datasets.
  • Root cause: merge_with_schema (called from TakeStream::map_batch) passed left_list.trimmed_values() alongside left_list.offsets().clone(). When the left list is a sliced view (e.g. a filtered batch), offsets do not start at zero and reference positions past the end of the trimmed child, panicking in ListArray::new.
  • Add ListArrayExt::trimmed_offsets() that returns offsets shifted to start at zero, and use it in the List/LargeList branches of merge_with_schema.

Test plan

  • New regression test test_merge_with_schema_sliced_list_struct in lance-arrow: fails on main with the exact panic, passes with the fix.
  • All existing lance-arrow merge tests still pass (9/9).
  • Python repro from the issue (1M rows, 200k + 800k + 14650 sparse-tail pattern) no longer panics and returns the expected 214,650 rows with correct data (verified against a manually-filtered reference batch).

Fixes #6580

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@github-actions github-actions Bot added the bug Something isn't working label Apr 21, 2026
@Xuanwo

Xuanwo commented May 19, 2026

Copy link
Copy Markdown
Collaborator

Thanks, I agree with the root cause and the fix direction.

Could you also add a higher-level regression through the scan/take path? The new unit test covers the local invariant, but the user-facing failure is to_table(filter=..., columns=[list_struct_col]), which goes through TakeExec -> merge_with_schema. A LargeList case would also be useful since this PR changes that branch too.

@kaan-simbe

Copy link
Copy Markdown
Author

Thanks for the review @Xuanwo!

Added in aecf0bc:

  • Refactored the lance-arrow test into a generic helper + thin i32/i64 wrappers (matching the existing test_merge_struct_lists_generic pattern), so the LargeList branch is covered too.
  • Added a higher-level regression in scanner.rs that goes through the full scan().filter().project([list_struct_col]) path. To get the planner to actually emit TakeExec with items split across input and take (which is what triggers the List<Struct> merge branch), I had to nudge it with MaterializationStyle::AllEarlyExcept([items.b]). Parametrized over V2_0 / Stable / V2_2 since the panic is v2-specific.

One note: I couldn't shrink the dataset below ~500k rows and still reproduce the panic — at ~250k it stops triggering, so the regression test runs ~0.7s for all 3 cases.

@kaan-simbe kaan-simbe force-pushed the fix/merge-with-schema-sliced-list branch from aecf0bc to dadd3cd Compare May 20, 2026 18:13
- Refactor the sliced-list merge_with_schema test into a generic helper
  and add a LargeList case.
- Add a scan().filter().project([list_struct_col]) regression in scanner.rs
  that forces the panicking TakeExec -> merge_with_schema path via
  MaterializationStyle::AllEarlyExcept.
@kaan-simbe kaan-simbe force-pushed the fix/merge-with-schema-sliced-list branch from dadd3cd to 6e5fbe0 Compare May 20, 2026 18:15
@kaan-simbe

Copy link
Copy Markdown
Author

@Xuanwo anything else I can do here?

@xloya

xloya commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

We encountered a similar problem in our production environment, and the issue was resolved after trying to cherry pick this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Panic in take.rs when projecting a list<struct> column under a pushdown filter with sparse selectivity

3 participants