Skip to content

fix: PushDownLeafProjections yields when the extraction cannot move - #26085

Open
adriangb wants to merge 1 commit into
apache:mainfrom
pydantic:leaf-churn-fix
Open

adriangb wants to merge 1 commit into
apache:mainfrom
pydantic:leaf-churn-fix

Conversation

@adriangb

@adriangb adriangb commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

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.

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:

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 #25455 added found this shape. That test records which rules change the plan in each pass. On main it prints, for the plan above:

pass 0: push_down_leaf_projections, optimize_projections
pass 1: push_down_leaf_projections, optimize_projections

With this PR:

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 early return for a projection without new extractions stays: it also ends the recursion of that push attempt.

  • The decision is recorded in the module documentation of extract_leaf_expressions.rs and optimize_projections, and as a second entry in the "Rule Precedence" section of docs/source/library-user-guide/query-optimizer.md.

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 fix: decide the precedence between PushDownFilter and PushDownLeafProjections #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

@codecov-commenter

codecov-commenter commented Oct 6, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 77.27273% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.72%. Comparing base (6f3f7b7) to head (38a7f6d).

Files with missing lines Patch % Lines
datafusion/optimizer/src/push_down_filter.rs 40.00% 0 Missing and 3 partials ⚠️
...tafusion/optimizer/src/extract_leaf_expressions.rs 88.23% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #26085      +/-   ##
==========================================
- Coverage   82.72%   82.72%   -0.01%     
==========================================
  Files        1147     1147              
  Lines      448179   448163      -16     
  Branches   448179   448163      -16     
==========================================
- Hits       370751   370724      -27     
- Misses      54898    54900       +2     
- Partials    22530    22539       +9     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

A projection of a `MoveTowardsLeafNodes` expression directly over a node
that the extraction cannot go through (for example a `TableScan` or an
`Aggregate`) was split into a recovery projection over an in-place
extraction projection. `OptimizeProjections` then merged the two back into
the original projection. The two rules undid each other in every optimizer
pass, and the loop stopped only because the plan signature repeated.

Now the rule pushes the in-place extraction projection immediately. If it
cannot move below the input, the rule leaves the original projection
unchanged. The final plans do not change, and the source still absorbs the
leaf expressions (for example `DataSourceExec projection=[get_field(...)]`).

`filter_and_extraction_projection_reach_fixed_point` now checks that no
rule changes the plan in the last pass, and covers a projection over a scan.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added the documentation Improvements or additions to documentation label Oct 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation optimizer Optimizer rules sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants