Keep cast metadata changes non-breaking - #2
Merged
paleolimbot merged 1 commit intoAug 31, 2026
Merged
Conversation
Restore the two public signatures on `CastExpr` that this branch changed,
so the metadata fixes can be backported to a patch release:
* `CastExpr::new_with_target_field` takes `FieldRef` by value again
* `CastExpr::target_field()` returns `&FieldRef` again
Both are achieved by storing the target `FieldRef` as before and adding a
private `explicit_target` flag that records whether the field was supplied
by the caller or synthesized from a `DataType`. That flag carries exactly
the information the `Option<HashMap<..>>` / `Option<bool>` pair carried, so
`target_metadata()`, `target_nullable()`, `has_explicit_metadata()` and
`has_explicit_nullability()` keep their meaning and are derived from it.
`Hash`/`PartialEq` compare the same components as before, so the field name
still does not participate.
`TryCastExpr` is given the same shape for symmetry. Its metadata-aware API
is new on this branch, so nothing there is a compatibility constraint.
Cast semantics are unchanged: explicit target fields still supply their
metadata and nullability verbatim, type-only casts still pass source
metadata through with the extension type keys stripped, and the output
field name still comes from the source expression.
Comparing the public signatures against the merge base with main, the only
remaining deltas are additions:
+ CastExpr::target_metadata / target_nullable
+ CastExpr::has_explicit_metadata / has_explicit_nullability
+ TryCastExpr::new_with_target_field / target_metadata / target_field
+ expressions::try_cast_with_target_field
`cast_with_target_field` keeps its `&FieldRef` parameter: `mod cast` is
private and it is only re-exported as `pub(crate)`, so it is not part of
the public API.
Verified: `cargo clippy --workspace --all-targets -- -D warnings` clean,
504/504 sqllogictest files pass, and the unit tests for physical-expr,
physical-expr-adapter, physical-plan, pruning, expr, functions and the
datafusion core lib all pass.
Author
|
To see the diff against main: apache#24828 |
paleolimbot
merged commit Aug 31, 2026
007b8ef
into
paleolimbot:cast-metadata-cleanup
1 check passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
cast-metadata-cleanupbranch (Align metadata propagation through Physical and Logical casts apache/datafusion#23169), notmain.55.1.0(minor/patch) Release (Sep 2026) apache/datafusion#24462).Rationale for this change
We would like the logical/physical cast metadata alignment from apache#23169 in the
55.1.0patch release, because it fixes the cast half of apache#24721. Onbranch-55today, a
CASTto an extension type produced by aTypePlannergives thelogical plan a field with no metadata while the physical
CastExprproduces onewith
ARROW:extension:name, andProjectionPushdownthen fails with:The blocker for backporting is that apache#23169 changes two public signatures on
CastExpr. This PR removes that blocker without changing any of the semanticsyou implemented.
What changes are included in this PR?
One commit on top of your branch, restoring the two signatures to what they are
on
main:CastExpr::new_with_target_fieldtakesFieldRefby value againCastExpr::target_field()returns&FieldRefagainBoth fall out of storing the target
FieldRefas before and adding a privateexplicit_target: boolrecording whether the field came from the caller or wassynthesized from a
DataType. That flag carries exactly the information theOption<HashMap<..>>/Option<bool>pair carried, sotarget_metadata(),target_nullable(),has_explicit_metadata()andhas_explicit_nullability()keep their meaning and are derived from it.
Hash/PartialEqstill compare thesame components, so the field name still does not participate.
TryCastExpris given the same shape for symmetry. Its metadata-aware API isnew on this branch, so nothing there is a compatibility constraint.
cast_with_target_fieldkeeps its&FieldRefparameter —mod castis privateand it is only re-exported as
pub(crate), so it is not public API.Cast semantics are untouched: explicit target fields still supply their metadata
and nullability verbatim, type-only casts still pass source metadata through with
the extension type keys stripped, and the output field name still comes from the
source expression. One test assertion comes back, since
target_field()againreturns the field it was constructed with:
Comparing public signatures against the merge base with
main, the onlyremaining deltas are additions:
Are these changes tested?
Yes — existing tests, plus one new unit test
(
target_field_accessor_returns_the_constructed_field) covering the restoredaccessor and the derived explicit/type-only reporting.
cargo clippy --all-targets --workspace --features avro,integration-tests,extended_tests -- -D warnings— cleandatafusion-physical-expr(1634),datafusion-physical-expr-adapter(42),datafusion-physical-plan(1844),datafusion-pruning(95),datafusion-expr(256),datafusion-functions(336) anddatafusioncore lib (444)Are there any user-facing changes?
No API changes relative to
mainbeyond the additions listed above. Thebehavioral changes described in apache#23169 are unaffected by this commit — worth
noting for a patch release that they still include the cast output field name
coming from the source rather than the target, extension keys being stripped on
type-only casts,
arrow_cast/arrow_try_castno longer eliding same-type castsover extension-typed sources, and
TRY_CASTto an extension type no longererroring.