Skip to content

fix(proto): preserve empty projection when ser/de MemoryScanExec - #24084

Closed
Dandandan wants to merge 1 commit into
apache:mainfrom
Dandandan:preserve-empty-projection-serde
Closed

fix(proto): preserve empty projection when ser/de MemoryScanExec#24084
Dandandan wants to merge 1 commit into
apache:mainfrom
Dandandan:preserve-empty-projection-serde

Conversation

@Dandandan

@Dandandan Dandandan commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Finishes the work started in #23082 (HashJoinExec, NestedLoopJoinExec) and #21885 (FilterExec) — MemoryScanExec was the last physical plan node left with an ambiguous projection.

Rationale for this change

MemorySourceConfig's projection is Option<Vec<usize>> but is encoded as a bare repeated uint32:

// encode
let proto_projection = source_conf.projection().as_ref().map_or_else(Vec::new, |v| ...);
// decode
let projection = if !scan.projection.is_empty() { Some(...) } else { None };

So Some(vec![]) — project away every column — serializes exactly like None and always decodes back as None, and the decoded scan reports every column again.

What changes are included in this PR?

Encode an empty projection with the single-element [u32::MAX] sentinel — the same convention HashJoinExec and NestedLoopJoinExec already use — and decode it back to Some(vec![]). Every other state is unchanged on the wire, so None and non-empty projections round-trip exactly as before.

Are these changes tested?

Yes

Are there any user-facing changes?

No API change. The wire format changes only for the previously-unrepresentable Some(vec![]) case, matching what the joins already do.

`MemorySourceConfig`'s projection is encoded as a bare `repeated uint32`, so
`Some(vec![])` — project away every column — serializes exactly like `None` and
decodes back as `None`. The decoded scan then reports every column again, which
silently changes the plan's output schema and breaks parents that compare
schemas, e.g. a sibling `UnionExec` branch failing with "UnionExec/InterleaveExec
requires all inputs to have the same number of fields".

Encode an empty projection with the same `[u32::MAX]` sentinel `HashJoinExec` and
`NestedLoopJoinExec` already use (apache#23082); `FilterExec` was fixed separately in
apache#21885. This was the last physical plan node with an ambiguous projection.
Dandandan added a commit to coralogix/arrow-datafusion that referenced this pull request Aug 4, 2026
Follow-up to the two cherry-picks: `MemorySourceConfig`'s projection has the same
problem the joins and `FilterExec` had. `Some(vec![])` — project away every
column — serializes exactly like `None` and decodes back as `None`, restoring
every column and silently changing the plan's output schema.

Use the same `[u32::MAX]` sentinel the joins use. Still unfixed upstream; sent as
apache#24084.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@github-actions github-actions Bot added the proto Related to proto crate label Aug 4, 2026
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.89%. Comparing base (db0c31b) to head (d298fba).

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #24084      +/-   ##
==========================================
- Coverage   80.89%   80.89%   -0.01%     
==========================================
  Files        1102     1102              
  Lines      376111   376107       -4     
  Branches   376111   376107       -4     
==========================================
- Hits       304251   304246       -5     
+ Misses      53753    53750       -3     
- Partials    18107    18111       +4     

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

@Dandandan Dandandan closed this Aug 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

proto Related to proto crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants