[branch-55] fix: preserve projection metadata during optimization (#24670) - #24992
Open
gene-bordegaray wants to merge 1 commit into
Open
[branch-55] fix: preserve projection metadata during optimization (#24670)#24992gene-bordegaray wants to merge 1 commit into
gene-bordegaray wants to merge 1 commit into
Conversation
- Closes apache#24721 There were four ways metadata could disappear. Consider: ```text ProjectionExec: i@0 AS i output field metadata = {"event_field": "true"} DataSourceExec: i field metadata = {} ``` The check to remove the projection asked: - Is every expression a column? - Does column 0 remain column 0? - Does the alias match the column name? - Does the number of columns match? All answers yes so optimizer removed projection: ```text DataSourceExec: i metadata = {} ``` Metadata lost. Consider: ```text ProjectionExec: arrow_metadata(i, 'event_field') AS metadata ProjectionExec: i@0 AS i output metadata = {"event_field": "true"} DataSourceExec: i metadata = {} ``` The correct result is `true`. The previous projection colapse logic would substitute the outer expression through the inner projection: ```text ProjectionExec: arrow_metadata(i, 'event_field') AS metadata DataSourceExec: i metadata = {} ``` Now the func sees the scan field instead of the inner projection field giving use result as `NULL` now. ### 3 Rebuilding a projection with a new child Some optimizer paths replace the child of a projection: ```text Old: ProjectionExec(metadata={"key": "value"}) OldChild --- New: ProjectionExec(...?) NewChild ``` The previous `make_with_child` implementation did this: ```rust ProjectionExec::try_new(projection.expr().to_vec(), new_child) ``` where try_new derives the output schema from the expressions and new child so it woudlnt retain metadata from the original projection. ### 4 Cast target metadata lost before optimization This one was a little confusing because main passed the UUID metadata test, but the first version of this PR did not (@gabotechs this is what you called out) Basically a cast can have an explicit target field with metadata. For example, the UUID type planner produces: ```text FixedSizeBinary(16) metadata = {"ARROW:extension:name": "arrow.uuid"} ``` But logical cast schema only used the target data type when deriving and kept th source metadata: ```text Source field: raw: FixedSizeBinary(16) metadata: {} Cast target: FixedSizeBinary(16) metadata: {"ARROW:extension:name": "arrow.uuid"} Derived cast output: FixedSizeBinary(16) metadata: {} ``` This appeared in CI when common sub-expr elimination extracts a repeated cast into its own projection: ```text ProjectionExec: arrow_metadata(__common_expr_1, 'ARROW:extension:name') ProjectionExec: CAST(raw AS UUID) AS __common_expr_1 ``` The inner projection was initially created with incorrect empty metadata, so the physical optimizer rebuilt that projection and rederived its schema so isthe was accidentally repairing the logical schema bug. Once this PR started preserving projection metadata correctly had this pop up this other bug. --- So then I solve the optimizer bugs in this PR --------- Co-authored-by: Tim Saucer <timsaucer@gmail.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> (cherry picked from commit 50cab14)
This was referenced Sep 6, 2026
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## branch-55 #24992 +/- ##
===========================================
Coverage 81.20% 81.21%
===========================================
Files 1110 1110
Lines 388267 388499 +232
Branches 388267 388499 +232
===========================================
+ Hits 315311 315514 +203
- Misses 54419 54433 +14
- Partials 18537 18552 +15 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
Author
|
cc @timsaucer |
Member
|
Thanks. I am only on my phone but will try to merge and get the release running on Tuesday |
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
3 participants
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.
Backports #24670 to branch-55.