Skip to content

fix(substrait): preserve nested nullability - #24158

Open
huymq1710 wants to merge 3 commits into
apache:mainfrom
huymq1710:substrait-preserve-nested-nullability
Open

fix(substrait): preserve nested nullability#24158
huymq1710 wants to merge 3 commits into
apache:mainfrom
huymq1710:substrait-preserve-nested-nullability

Conversation

@huymq1710

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

Nested literal types could mismatch the declared VirtualTable schema, causing valid Values plans to fail

What changes are included in this PR?

  • Convert Values literals using expected schema fields
  • Preserve nested Struct, List, and Map nullability
  • Keep schema-less literal conversion unchanged

Are these changes tested?

Yes,
Added unit, round-trip, deprecated Values, and integration tests

Are there any user-facing changes?

Substrait Values plans now preserve nested nullability and avoid schema/literal type mismatches

@codecov-commenter

codecov-commenter commented Aug 7, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.34615% with 45 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.05%. Comparing base (e64e3f7) to head (9418567).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...ubstrait/src/logical_plan/consumer/expr/literal.rs 94.47% 8 Missing and 10 partials ⚠️
...ubstrait/src/logical_plan/producer/expr/literal.rs 82.43% 2 Missing and 11 partials ⚠️
...ubstrait/src/logical_plan/consumer/rel/read_rel.rs 90.72% 4 Missing and 5 partials ⚠️
...usion/substrait/src/logical_plan/producer/types.rs 84.21% 0 Missing and 3 partials ⚠️
...usion/substrait/src/logical_plan/consumer/types.rs 50.00% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #24158      +/-   ##
==========================================
+ Coverage   81.02%   81.05%   +0.02%     
==========================================
  Files        1106     1106              
  Lines      380718   381147     +429     
  Branches   380718   381147     +429     
==========================================
+ Hits       308489   308923     +434     
+ Misses      54001    53967      -34     
- Partials    18228    18257      +29     

☔ 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.

@huymq1710

huymq1710 commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

I see, Codecov was newly added
I added more test in 22cf4bc commit

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.

Substrait literal conversion should preserve nested nullability

2 participants