adriangb opened a new pull request, #26085: URL: https://github.com/apache/datafusion/pull/26085
## Which issue does this PR close? - Part of https://github.com/apache/datafusion/issues/25459. This is a follow-up to https://github.com/apache/datafusion/pull/25455 (https://github.com/apache/datafusion/issues/14540). ## Rationale for this change `PushDownLeafProjections` and `OptimizeProjections` undo each other in every optimizer pass for a common plan shape: a projection of a struct field (or another `MoveTowardsLeafNodes` expression) directly over a node that the extraction cannot go through, such as a `TableScan` or an `Aggregate`. ```text Projection: leaf_udf(test.a) TableScan: test ``` In each pass, `push_down_leaf_projections` splits this into a recovery projection over an in-place extraction projection: ```text Projection: __datafusion_extracted_1 AS leaf_udf(test.a) Projection: leaf_udf(test.a) AS __datafusion_extracted_1, test.a TableScan: test ``` Then `optimize_projections` merges the two back into the original plan. The loop stops only because the plan at the end of a pass is the same as at the end of the previous pass. The final plan is correct, but each pass does the work of both rules again. The test that https://github.com/apache/datafusion/pull/25455 added found this shape. That test records which rules change the plan in each pass. On `main` it prints, for the plan above: ```text pass 0: push_down_leaf_projections, optimize_projections pass 1: push_down_leaf_projections, optimize_projections ``` With this PR: ```text pass 0: optimize_projections ``` ### Which rule yields `PushDownLeafProjections` yields. The split has no use when the extraction projection cannot move: a plain `Projection(get_field(...))` directly over a Parquet scan already reaches `DataSourceExec projection=[..., get_field(s@1, value) ...]`. The existing `EXPLAIN` checks in `projection_pushdown.slt` show this and do not change. If `OptimizeProjections` yielded instead, the plan would keep a projection node that does no work. ## What changes are included in this PR? In `split_and_push_projection` (`extract_leaf_expressions.rs`), the rule still builds the in-place extraction projection, because a mixed projection over a `Filter` must become a pure extraction projection before it can go through the filter. The rule now immediately tries to push that extraction projection into its input. If it cannot move, the rule returns the original projection unchanged. The unused `has_new_extractions` flag is removed. ## What is the testing strategy for this PR? - `filter_and_extraction_projection_reach_fixed_point` in `push_down_filter.rs` now checks that no rule at all changes the plan in the last optimizer pass (before, it checked only `PushDownFilter` and `PushDownLeafProjections`). It also covers a projection over a scan with no filter. Without the fix it fails with `[["push_down_leaf_projections","optimize_projections"],["push_down_leaf_projections","optimize_projections"]]`. - Six stage snapshots in `extract_leaf_expressions.rs` change. In each one, only the "After Pushdown" stage changes, to "(same as after extraction)". The "Optimized" plan is the same in all six. - One line in `unnest.slt` changes (logical plan line 03). A nested alias is now kept: `get_field(__unnest_placeholder(...,depth=1) AS UNNEST(recursive_unnest_table.column3), Utf8("c1"))`. The split and merge used to remove that alias as a side effect. The physical plan is the same. This is the same display-only nested alias that https://github.com/apache/datafusion/pull/25455 showed for `length(...)`. Commands run: - `cargo test --profile ci -p datafusion-optimizer --lib`: 932 passed. - `cargo test --profile ci -p datafusion-sqllogictest --test sqllogictests`: 526 of 526 files pass. - `cargo fmt --all` and `cargo clippy --profile ci -p datafusion-optimizer --all-targets -- -D warnings`: clean. ## Are there any user-facing changes? No. Final plans and results do not change, except for the alias display in the `unnest` plan above. Planning does less work for queries that read struct fields. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
