Skip to content

Preserve field metadata through physical CASE expressions - #26158

Draft
osipovartem wants to merge 8 commits into
apache:mainfrom
Embucket:upstream-case-physical-metadata
Draft

osipovartem wants to merge 8 commits into
apache:mainfrom
Embucket:upstream-case-physical-metadata

Conversation

@osipovartem

@osipovartem osipovartem commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Part of #24860. The SQL logical field change is in #26145. Keep this PR draft until the logical and physical contracts can land together.

Rationale for this change

Physical CaseExpr::return_field drops Arrow field metadata even when all result branches agree. This breaks metadata-aware consumers and Arrow extension types. Physical lowering can also erase logical metadata context (for example aliases, scalar variables, and NOT), so physical branch inspection alone cannot always reproduce the logical field.

What changes are included in this PR?

  • Resolve physical CASE metadata bottom-up, including nested CASE and cast wrappers; preserve the unmarked Column/Literal fast path.
  • Pin the logical result type and metadata on nontrivial planned CASE expressions. Nested physical CASE traversal honors that pin, including typed-NULL behavior.
  • Preserve the pin through physical-expression protobuf serialization using additive fields on PhysicalCaseNode. Previously encoded plans still decode without a pin.

What is the testing strategy for this PR?

  • cargo +stable test -p datafusion-physical-expr --features proto --lib --quiet: 1,803 passed, 2 ignored on this branch.
  • Regression tests cover agreeing/conflicting metadata, NULL branches, nested CASE, CAST/TRY_CAST, alias-lowered marked UDFs, typed-NULL scalar variables, marked NOT, child rewrites, and protobuf encode/decode.
  • cargo +stable fmt --all --check and focused Clippy passed locally. The local Clippy invocation used allowances for two unrelated pre-existing lints in this toolchain.
  • A focused debug planning measurement for nested CASE depths 16/32/64/128 was approximately 0.4/1.3/4.5/17.1 ms. The repeated logical inference on nontrivial CASE is measurable at pathological depth but does not add per-row execution work; the ordinary unmarked fast path is unchanged. A more general field-summary cache is separate work.
  • Independent read-only review approved this branch's Arrow 60 adaptation, correctness, API/protobuf compatibility, tests, and planning-cost assessment. Merge fix: preserve field metadata for simple CASE branches #26145 first and run the combined SQL-level regression suite.

Are there any user-facing changes?

Physical CASE result fields retain agreed Arrow metadata. The protobuf schema change is additive; older readers will not retain the new optional logical-field pin. End-to-end SQL metadata requires #26145.

@osipovartem
osipovartem marked this pull request as draft October 9, 2026 14:04
@github-actions

github-actions Bot commented Oct 9, 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-expr v55.1.0 (current)
       Built [  35.255s] (current)
     Parsing datafusion-physical-expr v55.1.0 (current)
      Parsed [   0.047s] (current)
    Building datafusion-physical-expr v55.1.0 (baseline)
       Built [  27.656s] (baseline)
     Parsing datafusion-physical-expr v55.1.0 (baseline)
      Parsed [   0.048s] (baseline)
    Checking datafusion-physical-expr v55.1.0 -> v55.1.0 (no change; assume patch)
     Checked [   0.338s] 223 checks: 223 pass, 31 skip
     Summary no semver update required
    Finished [  64.864s] datafusion-physical-expr
    Building datafusion-proto-models v55.1.0 (current)
       Built [  24.239s] (current)
     Parsing datafusion-proto-models v55.1.0 (current)
      Parsed [   0.135s] (current)
    Building datafusion-proto-models v55.1.0 (baseline)
       Built [  24.222s] (baseline)
     Parsing datafusion-proto-models v55.1.0 (baseline)
      Parsed [   0.136s] (baseline)
    Checking datafusion-proto-models v55.1.0 -> v55.1.0 (no change; assume patch)
     Checked [   1.792s] 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 PhysicalCaseNode.logical_result_type in /home/runner/work/datafusion/datafusion/datafusion/proto-models/src/generated/prost.rs:1932
  field PhysicalCaseNode.logical_result_metadata in /home/runner/work/datafusion/datafusion/datafusion/proto-models/src/generated/prost.rs:1934
  field PhysicalCaseNode.logical_result_type in /home/runner/work/datafusion/datafusion/datafusion/proto-models/src/generated/prost.rs:1932
  field PhysicalCaseNode.logical_result_metadata in /home/runner/work/datafusion/datafusion/datafusion/proto-models/src/generated/prost.rs:1934

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

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

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 78.44575% with 147 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.76%. Comparing base (06aa131) to head (57507b6).

Files with missing lines Patch % Lines
datafusion/physical-expr/src/expressions/case.rs 82.68% 38 Missing and 47 partials ⚠️
datafusion/physical-expr/src/planner.rs 78.65% 9 Missing and 26 partials ⚠️
datafusion/proto-models/src/generated/pbjson.rs 0.00% 27 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #26158      +/-   ##
==========================================
- Coverage   82.77%   82.76%   -0.01%     
==========================================
  Files        1147     1147              
  Lines      450909   451585     +676     
  Branches   450909   451585     +676     
==========================================
+ Hits       373252   373776     +524     
- Misses      54934    55007      +73     
- Partials    22723    22802      +79     

☔ 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

auto detected api change Auto detected API change physical-expr Changes to the physical-expr crates

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants