Skip to content

fix: preserve cast target metadata - #24725

Draft
gene-bordegaray wants to merge 1 commit into
apache:mainfrom
gene-bordegaray:gene.bordegaray/2026/08/preserve-cast-target-metadata
Draft

fix: preserve cast target metadata#24725
gene-bordegaray wants to merge 1 commit into
apache:mainfrom
gene-bordegaray:gene.bordegaray/2026/08/preserve-cast-target-metadata

Conversation

@gene-bordegaray

@gene-bordegaraygene-bordegaray commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

A cast to a data type and a cast to an explicit Arrow field have different metadata semantics:

  • A type-only cast should inherit metadata from the source field.
  • An explicit target field should use its own metadata, including an empty map that intentionally clears source metadata.

DataFusion currently stores both forms as a target FieldRef. Once that field has empty metadata, planning cannot determine whether it was synthesized from a DataType or explicitly supplied. This can lose extension metadata, incorrectly retain metadata that should be cleared, or retain unnecessary same-type casts.

What changes are included in this PR?

  • Introduce CastTarget::{DataType, Field} to represent cast intent
  • Preserve the distinction through logical schema inference, physical planning, expression rewrites, and protobuf serialization
  • Keep built-in SQL casts and standard Substrait casts type-only
  • Decode legacy protobuf casts that do not contain target_field

This is the prerequisite fix for #24670

Are these changes tested?

Yes.

Are there any user-facing changes?

This changes the public Cast::field and TryCast::field fields from FieldRef to CastTarget so callers that use these structs must handle new enum.

@github-actionsgithub-actionsBot added sql SQL Planner logical-expr Logical plan and expressions physical-expr Changes to the physical-expr crates substrait Changes to the substrait crate proto Related to proto crate functions Changes to functions implementation labels Aug 27, 2026
@github-actions

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-expr v55.0.0 (current)
Built [ 32.497s] (current)
Parsing datafusion-expr v55.0.0 (current)
Parsed [ 0.073s] (current)
Building datafusion-expr v55.0.0 (baseline)
Built [ 28.035s] (baseline)
Parsing datafusion-expr v55.0.0 (baseline)
Parsed [ 0.071s] (baseline)
Checking datafusion-expr v55.0.0 -> v55.0.0 (no change; assume patch)
Checked [ 1.523s] 223 checks: 223 pass, 31 skip
Summary no semver update required
Finished [ 63.876s] datafusion-expr
Building datafusion-functions v55.0.0 (current)
Built [ 30.138s] (current)
Parsing datafusion-functions v55.0.0 (current)
Parsed [ 0.082s] (current)
Building datafusion-functions v55.0.0 (baseline)
Built [ 30.283s] (baseline)
Parsing datafusion-functions v55.0.0 (baseline)
Parsed [ 0.082s] (baseline)
Checking datafusion-functions v55.0.0 -> v55.0.0 (no change; assume patch)
Checked [ 0.448s] 223 checks: 223 pass, 31 skip
Summary no semver update required
Finished [ 62.666s] datafusion-functions
Building datafusion-physical-expr v55.0.0 (current)
Built [ 28.416s] (current)
Parsing datafusion-physical-expr v55.0.0 (current)
Parsed [ 0.051s] (current)
Building datafusion-physical-expr v55.0.0 (baseline)
Built [ 28.081s] (baseline)
Parsing datafusion-physical-expr v55.0.0 (baseline)
Parsed [ 0.053s] (baseline)
Checking datafusion-physical-expr v55.0.0 -> v55.0.0 (no change; assume patch)
Checked [ 0.395s] 223 checks: 223 pass, 31 skip
Summary no semver update required
Finished [ 58.226s] datafusion-physical-expr
Building datafusion-proto v55.0.0 (current)
Built [ 53.277s] (current)
Parsing datafusion-proto v55.0.0 (current)
Parsed [ 0.020s] (current)
Building datafusion-proto v55.0.0 (baseline)
Built [ 52.503s] (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.132s] 223 checks: 223 pass, 31 skip
Summary no semver update required
Finished [ 107.453s] datafusion-proto
Building datafusion-proto-models v55.0.0 (current)
Built [ 24.589s] (current)
Parsing datafusion-proto-models v55.0.0 (current)
Parsed [ 0.124s] (current)
Building datafusion-proto-models v55.0.0 (baseline)
Built [ 24.398s] (baseline)
Parsing datafusion-proto-models v55.0.0 (baseline)
Parsed [ 0.125s] (baseline)
Checking datafusion-proto-models v55.0.0 -> v55.0.0 (no change; assume patch)
Checked [ 2.017s] 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 PhysicalCastNode.target_field in /home/runner/work/datafusion/datafusion/datafusion/proto-models/src/generated/prost.rs:1859
field PhysicalCastNode.target_field in /home/runner/work/datafusion/datafusion/datafusion/proto-models/src/generated/prost.rs:1859
field CastNode.target_field in /home/runner/work/datafusion/datafusion/datafusion/proto-models/src/generated/prost.rs:1182
field CastNode.target_field in /home/runner/work/datafusion/datafusion/datafusion/proto-models/src/generated/prost.rs:1182
field PhysicalTryCastNode.target_field in /home/runner/work/datafusion/datafusion/datafusion/proto-models/src/generated/prost.rs:1850
field PhysicalTryCastNode.target_field in /home/runner/work/datafusion/datafusion/datafusion/proto-models/src/generated/prost.rs:1850
field TryCastNode.target_field in /home/runner/work/datafusion/datafusion/datafusion/proto-models/src/generated/prost.rs:1198
field TryCastNode.target_field in /home/runner/work/datafusion/datafusion/datafusion/proto-models/src/generated/prost.rs:1198
Summary semver requires new major version: 1 major and 0 minor checks failed
Finished [ 52.747s] datafusion-proto-models
Building datafusion-pruning v55.0.0 (current)
Built [ 39.494s] (current)
Parsing datafusion-pruning v55.0.0 (current)
Parsed [ 0.014s] (current)
Building datafusion-pruning v55.0.0 (baseline)
Built [ 39.260s] (baseline)
Parsing datafusion-pruning v55.0.0 (baseline)
Parsed [ 0.013s] (baseline)
Checking datafusion-pruning v55.0.0 -> v55.0.0 (no change; assume patch)
Checked [ 0.088s] 223 checks: 223 pass, 31 skip
Summary no semver update required
Finished [ 80.126s] datafusion-pruning
Building datafusion-sql v55.0.0 (current)
Built [ 41.294s] (current)
Parsing datafusion-sql v55.0.0 (current)
Parsed [ 0.030s] (current)
Building datafusion-sql v55.0.0 (baseline)
Built [ 41.405s] (baseline)
Parsing datafusion-sql v55.0.0 (baseline)
Parsed [ 0.031s] (baseline)
Checking datafusion-sql v55.0.0 -> v55.0.0 (no change; assume patch)
Checked [ 0.282s] 223 checks: 223 pass, 31 skip
Summary no semver update required
Finished [ 84.391s] datafusion-sql
Building datafusion-substrait v55.0.0 (current)
Built [ 295.774s] (current)
Parsing datafusion-substrait v55.0.0 (current)
Parsed [ 0.019s] (current)
Building datafusion-substrait v55.0.0 (baseline)
Built [ 296.631s] (baseline)
Parsing datafusion-substrait v55.0.0 (baseline)
Parsed [ 0.017s] (baseline)
Checking datafusion-substrait v55.0.0 -> v55.0.0 (no change; assume patch)
Checked [ 0.263s] 223 checks: 223 pass, 31 skip
Summary no semver update required
Finished [ 594.905s] datafusion-substrait

@github-actionsgithub-actionsBot added the auto detected api change Auto detected API change label Aug 27, 2026
@codecov-commenter

codecov-commenter commented Aug 27, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 78.36257% with 111 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.43%. Comparing base (9617fdf) to head (f6adc82).
⚠️ Report is 48 commits behind head on main.

Files with missing linesPatch %Lines
datafusion/proto-models/src/generated/pbjson.rs11.53%40 Missing and 6 partials ⚠️
...tafusion/physical-expr/src/expressions/try_cast.rs75.72%20 Missing and 5 partials ⚠️
datafusion/proto/src/logical_plan/from_proto.rs75.67%6 Missing and 3 partials ⚠️
datafusion/physical-expr/src/expressions/cast.rs92.85%4 Missing and 4 partials ⚠️
datafusion/physical-expr/src/planner.rs64.28%0 Missing and 5 partials ⚠️
datafusion/expr/src/expr_schema.rs91.30%0 Missing and 4 partials ⚠️
...n/substrait/src/logical_plan/consumer/expr/cast.rs86.66%0 Missing and 4 partials ⚠️
...n/substrait/src/logical_plan/producer/expr/cast.rs50.00%0 Missing and 4 partials ⚠️
datafusion/proto/src/logical_plan/to_proto.rs88.88%0 Missing and 2 partials ⚠️
datafusion/sql/src/planner.rs91.30%0 Missing and 2 partials ⚠️
... and 1 more
Additional details and impacted files
@@ Coverage Diff @@## main #24725 +/- ##
==========================================
- Coverage 81.45% 81.43% -0.02% 
==========================================
Files 1119 1120 +1 Lines 400411 401980 +1569 Branches 400411 401980 +1569 ==========================================
+ Hits 326138 327368 +1230 - Misses 55198 55423 +225 - Partials 19075 19189 +114 

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

@alamb

Copy link
Copy Markdown
Contributor

@gene-bordegaray

gene-bordegaray commented Aug 27, 2026

Copy link
Copy Markdown
ContributorAuthor

Is this the same / similar to

?

yes it is the same. this popped up in my other PR and got too big for one PR. I am willing to review / discuss which approach is preferred. cc: @paleolimbot

either way since this is blocking correctness I think should be in 55.1 release

@gene-bordegaray

Copy link
Copy Markdown
ContributorAuthor

drafting since we are taking #23169 for backpirt then discussing casting semantics further in community meething

@gene-bordegaray
gene-bordegaray marked this pull request as draft September 1, 2026 20:11
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto detected api changeAuto detected API changefunctionsChanges to functions implementationlogical-exprLogical plan and expressionsphysical-exprChanges to the physical-expr cratesprotoRelated to proto cratesqlSQL PlannersubstraitChanges to the substrait crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cast expressions cannot distinguish inherited metadata from an explicitly empty target

3 participants

@gene-bordegaray@codecov-commenter@alamb