Plan struct colon access as get_field - #25373
osipovartem wants to merge 5 commits into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #25373 +/- ##
==========================================
+ Coverage 82.45% 82.66% +0.20%
==========================================
Files 1140 1147 +7
Lines 436589 446494 +9905
Branches 436589 446494 +9905
==========================================
+ Hits 359996 369089 +9093
- Misses 54838 54997 +159
- Partials 21755 22408 +653 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
2dd5ed9 to
529a724
Compare
529a724 to
87cf279
Compare
kosiew
left a comment
There was a problem hiding this comment.
Thanks for working on this. The Struct colon-path lowering looks good overall. I left one non-blocking suggestion to strengthen the regression coverage.
| +---+ | ||
| "); | ||
| } | ||
|
|
There was a problem hiding this comment.
Could you add a case where the registered colon planner returns PlannerResult::Original for a Struct path? That would verify that a planner can defer colon handling and the built-in Struct-path lowering still produces the expected field access.
There was a problem hiding this comment.
Added test_deferred_struct_colon_operator in 16b92e3. It confirms the registered planner sees : and returns Original, then verifies built-in Struct field access returns 1. The six focused ExprPlanner tests pass locally.
Which issue does this PR close?
Closes no issue.
Rationale for this change
SqlToRel::parse_json_accessrepresents SQL colon access as a colon binary operator. For an Arrow Struct, that expression currently retains the complete Struct type and reaches an unsupported physical binary operator instead of accessing the requested field.What changes are included in this PR?
NestedFunctionPlannernow lowers a static colon access on a Struct to the existing vectorizedget_fieldUDF. Nested paths become nestedget_fieldcalls, which the existing simplifier can flatten and push toward scan leaves. Non-Struct colon expressions remain available to other planners.The tests cover nested Struct access and the non-Struct fallback.
Are there any user-facing changes?
SQL JSON-style access on typed Struct expressions can now be planned and executed through
get_field.How was this change tested?
cargo test -p datafusion-functions-nested planner::tests --lib(2 passed)cargo fmt --all -- --checkA package clippy run reached an unrelated pre-existing
from_iter_instead_of_collectwarning indatafusion/common/src/scalar/mod.rs; the changed crate passed this same focused clippy command on the DataFusion 55 fork.