Skip to content

fix: Substrait serialization mishandles sliced map literals - #25543

Open
neilconway wants to merge 2 commits into
apache:mainfrom
neilconway:neilc/fix-substrait-sliced-map-literal
Open

neilconway wants to merge 2 commits into
apache:mainfrom
neilconway:neilc/fix-substrait-sliced-map-literal

Conversation

@neilconway

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

  • N/A

Rationale for this change

ScalarValue::Map has an unfortunate property: it exposes the keys() and values() methods from the underlying Arrow MapArray, but those methods return all of the keys/values in the backing array, not just keys/values in the visible span. As a result, calling keys() on a ScalarValue::Map can return values outside the logical content of the scalar map.

Fix an instance of this bug in the substrait serialization code.

What changes are included in this PR?

  • Fix substrait map literal serialization bug
  • Add unit test

What is the testing strategy for this PR?

Existing tests pass, new test added.

Are there any user-facing changes?

No, aside from fixing the bug.

@github-actions github-actions Bot added the substrait Changes to the substrait crate label Sep 20, 2026
@neilconway

Copy link
Copy Markdown
Contributor Author

Not sure if there's anything to be done about the confusing nature of the APIs here...

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 69.23077% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.39%. Comparing base (24a7f13) to head (0433f76).
⚠️ Report is 18 commits behind head on main.

Files with missing lines Patch % Lines
...ubstrait/src/logical_plan/producer/expr/literal.rs 69.23% 0 Missing and 8 partials ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##             main   #25543     +/-   ##
=========================================
  Coverage   82.38%   82.39%             
=========================================
  Files        1138     1138             
  Lines      433739   434744   +1005     
  Branches   433739   434744   +1005     
=========================================
+ Hits       357338   358193    +855     
- Misses      54847    54861     +14     
- Partials    21554    21690    +136     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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

Labels

substrait Changes to the substrait crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants