Skip to content

fix: preserve ProjectionExec metadata during serialization - #25009

Open
goutamadwant wants to merge 1 commit into
apache:mainfrom
goutamadwant:fix-projection-proto-metadata-24695
Open

fix: preserve ProjectionExec metadata during serialization#25009
goutamadwant wants to merge 1 commit into
apache:mainfrom
goutamadwant:fix-projection-proto-metadata-24695

Conversation

@goutamadwant

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

Physical-plan serialization loses projection field and schema metadata when it differs from metadata derived from the input and expressions.

What changes are included in this PR?

  • Add an optional schema to ProjectionExecNode, emitted only when the projection overrides metadata.
  • Restore metadata through try_new_with_schema_metadata, keeping field names, types, and nullability derived from the expressions.
  • Preserve decoding of older payloads without the optional field.

What is the testing strategy for this PR?

  • roundtrip_projection_metadata reproduces the loss before the fix and passes afterward.
  • Added binary and JSON coverage for overrides, explicit clearing, extension metadata, and older payloads. decode_projection_schema_only_replaces_metadata checks expression-derived field properties and invalid field counts.
  • The required extended workspace test command passes, including 2,014 physical-plan tests, 268 protobuf integration tests, and 511 SQL logic-test files.
  • All-target, all-feature Clippy passes for the three affected crates; the contributor guide's documented lint steps also pass.
  • Full-workspace Clippy with all features enabled hits the existing PostgreSQL decimal-formatting lint in Full workspace clippy fails in PostgreSQL decimal formatting #24974; the affected source is unchanged here.

Are there any user-facing changes?

Projection metadata survives physical-plan serialization. The generated ProjectionExecNode Rust struct gains an optional schema field. New readers continue to accept older payloads. This targets main, not a 55.1 backport.

@github-actions github-actions Bot added proto Related to proto crate physical-plan Changes to the physical-plan crate labels Sep 7, 2026
@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown

Thank you for opening this pull request!

Reviewer note: cargo-semver-checks reported the current version number is not SemVer-compatible with the changes in this pull request (compared against the base branch).

Details
     Cloning apache/main
    Building datafusion-physical-plan v55.0.0 (current)
       Built [  40.801s] (current)
     Parsing datafusion-physical-plan v55.0.0 (current)
      Parsed [   0.157s] (current)
    Building datafusion-physical-plan v55.0.0 (baseline)
       Built [  39.944s] (baseline)
     Parsing datafusion-physical-plan v55.0.0 (baseline)
      Parsed [   0.157s] (baseline)
    Checking datafusion-physical-plan v55.0.0 -> v55.0.0 (no change; assume patch)
     Checked [   0.693s] 223 checks: 223 pass, 31 skip
     Summary no semver update required
    Finished [  83.677s] datafusion-physical-plan
    Building datafusion-proto v55.0.0 (current)
       Built [  55.621s] (current)
     Parsing datafusion-proto v55.0.0 (current)
      Parsed [   0.018s] (current)
    Building datafusion-proto v55.0.0 (baseline)
       Built [  54.730s] (baseline)
     Parsing datafusion-proto v55.0.0 (baseline)
      Parsed [   0.019s] (baseline)
    Checking datafusion-proto v55.0.0 -> v55.0.0 (no change; assume patch)
     Checked [   0.115s] 223 checks: 223 pass, 31 skip
     Summary no semver update required
    Finished [ 111.737s] datafusion-proto
    Building datafusion-proto-models v55.0.0 (current)
       Built [  25.712s] (current)
     Parsing datafusion-proto-models v55.0.0 (current)
      Parsed [   0.131s] (current)
    Building datafusion-proto-models v55.0.0 (baseline)
       Built [  25.619s] (baseline)
     Parsing datafusion-proto-models v55.0.0 (baseline)
      Parsed [   0.134s] (baseline)
    Checking datafusion-proto-models v55.0.0 -> v55.0.0 (no change; assume patch)
     Checked [   1.711s] 223 checks: 222 pass, 1 fail, 0 warn, 31 skip

--- failure constructible_struct_adds_field: struct exhaustively constructible through public API adds field ---

Description:
A pub struct that could be exhaustively constructed with a literal using only public API has a new pub field, breaking existing exhaustive literals.
        ref: https://doc.rust-lang.org/reference/expressions/struct-expr.html
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.50.0/src/lints/constructible_struct_adds_field.ron

Failed in:
  field ProjectionExecNode.schema in /home/runner/work/datafusion/datafusion/datafusion/proto-models/src/generated/prost.rs:2207
  field ProjectionExecNode.schema in /home/runner/work/datafusion/datafusion/datafusion/proto-models/src/generated/prost.rs:2207

     Summary semver requires new major version: 1 major and 0 minor checks failed
    Finished [  54.657s] datafusion-proto-models

@github-actions github-actions Bot added the auto detected api change Auto detected API change label Sep 7, 2026
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 77.77778% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.75%. Comparing base (e1ca94f) to head (d58dcb7).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
datafusion/physical-plan/src/projection.rs 78.57% 0 Missing and 3 partials ⚠️
datafusion/proto-models/src/generated/pbjson.rs 76.92% 1 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #25009      +/-   ##
==========================================
+ Coverage   81.69%   81.75%   +0.05%     
==========================================
  Files        1127     1127              
  Lines      415471   415680     +209     
  Branches   415471   415680     +209     
==========================================
+ Hits       339424   339844     +420     
+ Misses      56108    55850     -258     
- Partials    19939    19986      +47     

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

@jayzhan211 jayzhan211 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @goutamadwant !

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

Labels

auto detected api change Auto detected API change physical-plan Changes to the physical-plan crate proto Related to proto crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Physical-plan serialization drops ProjectionExec metadata

3 participants