fix: preserve Arrow field metadata on scalar subquery expressions - #24939
Open
M-Tesla wants to merge 1 commit into
Open
fix: preserve Arrow field metadata on scalar subquery expressions#24939M-Tesla wants to merge 1 commit into
M-Tesla wants to merge 1 commit into
Conversation
Uncorrelated scalar subqueries copied only data type and nullability into the physical expression, so UDFs that rely on Arrow extension metadata saw a bare storage type. Keep the subquery output metadata through physical planning and protobuf round-trips.
github-actions
Bot
added
physical-expr
paleolimbot
reviewed
Sep 4, 2026
paleolimbot
left a comment
paleolimbot
left a comment
Member
There was a problem hiding this comment.
Thank you for fixing! This seems reasonable but I have also not spent much time in this part of the code. I wonder if @neilconway has any opinions / is aware of implications of this I haven't thought of.
This needs CI to run (I can't help with that since I'm not a committer here) which may highlight some issues, and I wonder if the metadata.slt file can help add another layer to prevent a regression.
Comment on lines
+2341
to
+2346
| SELECT id | ||
| FROM l | ||
| WHERE metadata_required( | ||
| l.geometry, | ||
| (SELECT r.geometry FROM r WHERE r.id = 1) | ||
| ) |
Member
There was a problem hiding this comment.
Is there a good way to get a query that exercises this into a .slt file as well? Maybe datafusion/sqllogictest/test_files/metadata.slt?
|
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 |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #24939 +/- ##
==========================================
- Coverage 81.64% 81.63% -0.01%
==========================================
Files 1123 1123
Lines 410248 410383 +135
Branches 410248 410383 +135
==========================================
+ Hits 334940 335026 +86
- Misses 55617 55660 +43
- Partials 19691 19697 +6 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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.
Which issue does this PR close?
Related: apache/sedona-db#1231, apache/sedona-db#1226. This is the uncorrelated physical-plan counterpart of the earlier outer-reference metadata fix in #17524 / #17422.
Rationale for this change
UDFs that distinguish Arrow extension types from their storage types (for example spatial predicates such as
ST_Intersects) needARROW:extension:nameon every argument. Uncorrelated scalar subqueries kept that metadata in the logical plan, but physical planning built aScalarSubqueryExprfrom only the data type and nullability. The synthesized physical field had empty metadata, so queries likeWHERE udf(col, (SELECT geometry FROM t WHERE id = 1))failed even though the equivalent join form worked.What changes are included in this PR?
ScalarSubqueryExprnow stores the outputFieldRef(namescalar_subquery, original type/nullability, and metadata) vianew_with_metadata. The existingnewconstructor is unchanged and still produces a field with empty metadata.Expr::nullableso zero-row subqueries remain nullable.metadatamap onPhysicalScalarSubqueryExprNodeso plan round-trips keep extension metadata.What is the testing strategy for this PR?
scalar_subquery_preserves_output_field_metadatainplanner.rsreproduces the drop during physical lowering.return_field_preserves_extension_metadataand an updated proto round-trip inscalar_subquery.rs.test_extension_metadata_preserve_in_uncorrelated_scalar_subqueryinuser_defined_scalar_functions.rs, based on the issue reproducer. The existing EXISTS-subquery metadata test still passes.Are there any user-facing changes?
Additive only:
ScalarSubqueryExpr::new_with_metadataand an optional protobufmetadatamap (older payloads decode as empty metadata). Existingnew(data_type, nullable, ...)keeps working. Queries whose UDFs inspect argument field metadata now see the subquery's original extension metadata in the physical plan.