Skip to content

[EPIC] Leaf expression pushdown and filter pushdown: bugs, decisions and tests #25459

Open
Open
[EPIC] Leaf expression pushdown and filter pushdown: bugs, decisions and tests#25459
Labels
EPICA larger project, actively underway, with sub tasksoptimizerOptimizer rules

Description

@adriangb
adriangb
opened on Sep 18, 2026

Summary

This EPIC tracks the open bugs, the design decisions and the test work for the two logical optimizer features that move expressions and filters towards the table scan:

  • ExtractLeafExpressions and PushDownLeafProjections (datafusion.optimizer.enable_leaf_expression_pushdown, default true, in datafusion/optimizer/src/extract_leaf_expressions.rs)
  • PushDownFilter (datafusion/optimizer/src/push_down_filter.rs)

The leaf rules have received ten bug fixes since they landed in February 2026 (#20117). As far as the linked issues show, all ten were reported by users or found by an ad-hoc fuzzer, not by an existing test. Two mechanisms explain most of them:

  1. Columns are resolved by name string (flat_name() or the bare name) when a projection is moved through another node. A computed column with the same name as a table column is then confused with that column. This gave Wrong results: leaf expression pushdown removes a computed column that has the same name as its input column #25414 (wrong results), fix: resolve pass-through columns against the input when merging an extraction projection #25412 (planning error), push_down_leaf_projections fails for self joins in some scenarios #24241 and optimize_projections fails with "No field named ..." when join keys contain get_field (ExtractLeafExpressions) #22895.
  2. A column definition is inlined into a consumer at several sites, and each site has its own guard against duplicating a volatile or expensive expression. Each guard was added after a bug report: fix: do not duplicate volatile expressions when extracting leaf expressions #24720, fix: evaluate struct-returning UDFs once across repeated field accesses #23691, perf: don't re-inline CSE'd expensive expressions in projection pushdown #23459, fix: do not push filters on volatile group keys below Aggregate #25416. CommonSubexprEliminate + leaf-expression pushdown duplicate a UDF call wrapped in get_field #25329 is the shape that none of them covers.

The report behind this EPIC, with SQL reproductions verified on main at 4e90755, is summarized in the sections below.

Strategy

Work in this order. Each step is independent of the ones after it.

  1. Fix the wrong-results bugs in the default configuration first. Add every reproduction to sqllogictest with a result assertion, not only an EXPLAIN.
  2. Replace per-site guards with one inlining policy, so the next duplication bug cannot appear at a site that has no guard.
  3. Lock down the decisions that are ambiguous today, so plans are intentional and deterministic (section "Decisions" below). A decision that is wrong but documented is better than a plan that depends on rule order.
  4. Add tests that find the next bug before a user does: a differential fuzz (leaf pushdown on vs off, Parquet filter pushdown on vs off), an optimizer invariant that forbids new evaluations of volatile and KeepInPlace expressions, and the test gaps that mutation testing found.
  5. Then, and only then, the large refactor: resolve columns by schema index instead of name, and replace the alias-prefix protocol with a typed marker.

Items that depend on data or cost that the planner does not know (which function is cheap for which source, filter-first vs projection-first on a non-Parquet source, pushdown_filters by default) are listed as trade-offs. They are not bugs. They get a decision and a benchmark, not a heuristic.

Decisions

These are the ambiguous behaviours that this EPIC locks down. Each one gets a documented answer in the code, a test that fails if the answer changes, and no configuration option.

Decision Today Decided behaviour Where it is enforced
Order of a Filter and a pure extraction projection that it does not reference (#14540) Depends on rule order in the list. PushDownFilter and PushDownLeafProjections undo each other. One optimizer pass is wasted (analysis by kakiuwang-ui on the issue, who also asked which rule should yield). PushDownFilter yields. A pure extraction projection stays below a filter that does not reference its aliases. Measured on Parquet: the opposite order (leaf rule yields) loses the struct leaf in DataSourceExec at the default pushdown_filters = false, because the physical ProjectionPushdown cannot move the projection through a FilterExec whose predicate needs a column the projection does not produce. The filter loses nothing: PushDownFilter runs before ExtractLeafExpressions, so the predicate is already recorded in TableScan::filters before any extraction projection exists. On a source that cannot absorb the projection the filter node still sits above the projection, so get_field runs before the filter there. That is the order main produces today, so it is not a regression, but it is not the better order for that source. kakiuwang-ui's measurement on the issue (filter first is better for a memory table) is correct; the decision goes the other way because of the Parquet default. #25455 (docs in both rules; max_passes pinned in slt so a change in the order fails a test). Merged.
A projection of a leaf expression directly over a node that the extraction cannot go through (for example a TableScan or an Aggregate) PushDownLeafProjections splits it into a recovery projection over an in-place extraction projection, and OptimizeProjections merges the two back. The two rules undo each other in every pass; the loop stops only because the plan signature repeats. The final plan is correct. Found by the per-rule fixed-point test that #25455 added. PushDownLeafProjections yields. When the in-place extraction projection cannot move below its input, the rule leaves the original projection unchanged. The split is of no use there: a plain Projection(get_field(...)) over a Parquet scan already reaches DataSourceExec projection=[get_field(...)]. If OptimizeProjections yielded instead, the plan would keep a projection node that does no work. #26085 (the fixed-point test now requires that no rule changes the plan in the last pass; conflicts with #25456 in the same block, small resolution)
Inlining a column definition into a consumer Five guards at five sites, each with its own rule. Never inline a volatile definition. Never inline a KeepInPlace definition into more than one evaluation site. Column and MoveTowardsLeafNodes definitions may always be inlined. A literal is classed cheap, so OptimizeProjections keeps its existing rule for it (#8296). #25456 (crate-private ProjectionInliner, four leaf-rule sites plus PushDownFilter::rewrite_projection and the OptimizeProjections merge routed through it; would_duplicate_volatile and merge_would_duplicate_kept_expr deleted)
When PushDownLeafProjections needs a recovery projection Compares the set of unqualified field names, so a computed column with the same name as an input column is dropped. A recovery projection is required whenever a recovery expression is not a pass-through column, or when the set of unqualified output names differs. The check does not compare qualifiers or types. A type change is caught because it implies a computed value. #25445
Pushing a predicate through a projection that computes a KeepInPlace expression referenced once Pushed. The expression is evaluated in the filter and again in the projection. Keep pushing it. The predicate can reach the scan (row-group pruning, partial_filters), which the planner cannot value without a cost model. pepijnve raised this cost himself on his draft #25388: its snapshots show CAST(ts ...) range filters stop reaching the scan. The draft also widens Expr::placement to operators and casts, which is independent of this decision and worth keeping. Documented in the inlining policy; test in push_down_filter.rs.
Which functions are MoveTowardsLeafNodes A global constant per UDF. Unchanged for now. A source-level veto is a follow-up (see design issues). array_length(arr, dim) with a non-literal dim stays KeepInPlace. Not implemented yet. quwin found the array_length gap in the review of #25025; that PR does not carry the change.

Open bugs

Issue Effect Fix
#25414 Wrong results, default config, five SQL shapes #25445
#25412 Planning error, UNION ALL with an empty branch over a CTE PR open
#25329 KeepInPlace UDF evaluated two times in a WHERE #25456 (crate-private ProjectionInliner, four leaf-rule sites plus PushDownFilter::rewrite_projection and the OptimizeProjections merge routed through it; would_duplicate_volatile and merge_would_duplicate_kept_expr deleted) (narrow rule). The issue is assigned to bert-beyondloops, whose #25330 was the first fix. pepijnve's draft #25388 also closes it with a broader rule and has an open design question that needs an answer. The guard that #25456 deletes was written by tohuya6 in #23691 and reviewed by kosiew; it is subsumed, not wrong, and its two regression tests stay.
#14540 Two rules undo each other, one wasted pass #25455 (docs in both rules; max_passes pinned in slt so a change in the order fails a test)
#15046 (helgikrs) Subquery outer references invisible to column_refs, plan fails to execute past an extension node. A PushDownFilter extension-node bug, related but not a leaf-pushdown bug. #25294 by kakiuwang-ui fixes the consumer and deliberately does not widen column_refs, which the issue title asks for; the author asked which of the two a maintainer prefers.
#25268 A conjunct marked pushed at plan time is dropped per file at run time with pushdown_filters = true The reporter, zhuqi-lucas, plans a fix PR. His proposal 1 (make the silent debug! fallback an error or a post-scan filter) is the direction this EPIC prefers.
#25446 Planning error when a sub-query projection renames or swaps columns and a struct field is read above #25478. Not fixed by #25412 (verified on its head). The fix also carries the needs_recovery hunk of #25445, byte-identical, because without it the shape returns swapped columns instead of an error.
#25447 The same get_field is extracted two times into one projection Not started.
#25457 Wrong results: BETWEEN on a volatile operand evaluates it two times, in the logical expansion and again in the physical planner. Found by the invariant in #25458. #25476 (BetweenExpr evaluates the operand one time; non-volatile BETWEEN keeps the two-comparison form so pruning and interval analysis are unchanged)
#25477 Wrong results: COALESCE on a volatile operand evaluates it two times; the query fails with an Arrow non-nullable error when the last argument is a literal. Same mechanism as #25457 through the coalesce to CASE rewrite. Found while fixing #25457. Not started.

Fixed recently, listed so the pattern is visible: #25415 (#25416), #24678 (#24720), #23655 (#23691), #24241, #22955, #22895, #22615, #20430.

Tests

Item Status
Differential fuzz: leaf pushdown on vs off, Parquet pushdown_filters on vs off #25453 (reproduces #25414 in 526 of 3000 seeds, 50 of them silent wrong results, and the #25412 error in 404 of 3000; both shapes gated by a const until the fixes land). Note: fuzz_cases only runs in the merge-queue extended_tests job, not in PR CI.
Optimizer invariant: no rule may add an evaluation site of a volatile or KeepInPlace expression #25458 (evaluation_sites.rs, a per-rule check, +12% to +15% CPU on the sqllogictest run when on, shipped off behind two const switches). An independent review found one reported finding that is not a duplication in the final plan (abs(c1) BETWEEN ...: simplify_expressions duplicates, CSE re-hoists in the same pass) and a key weakness (sites are keyed by printed text, so a rule that duplicates and re-qualifies a column escapes). Decision: the invariant is checked once, at the end of the optimizer run, against the input plan. Rules may cooperate; intermediate plans are not checked. Only the volatile half is an invariant (same results); the KeepInPlace count is a cost and stays a diagnostic. Sites are keyed structurally, not by printed text. Reworked on the PR (2026-09-18): the end-of-run check reports exactly one query on main, random() BETWEEN ... (#25457, fixed by #25476), and its overhead is not measurable (full sqllogictest wall clock inside noise, against +12% CPU for the per-rule version). file_row_index() passes end-to-end on main: CSE hoists it and OptimizeProjections puts it back, so the final plan has the input plan's count. The KeepInPlace count is deleted, not kept as a diagnostic. With the switches on it reports 14 queries in 7 files, including #25329 and #25457.
Test gaps found by mutation testing of the two files #25451 (220 mutants, 139 caught, 25 missed, 9 real gaps); tests for 3 functions in #25452
Result assertions (not only EXPLAIN) for every reproduction in this EPIC Part of each fix PR

Design issues (larger refactors)

Item Issue
Resolve columns by schema index, not by name, in the leaf rules #25448
Replace the __datafusion_extracted / __common_expr alias-prefix protocol with a typed marker #25449 (see also #25330 by bert-beyondloops, a sixth prefix sniff)
Let the data source veto MoveTowardsLeafNodes per function #25450

Trade-offs (not bugs, need a benchmark, not a heuristic)

Findings from building the integration branch

A local branch leaf-pushdown-integration holds all of the PRs above merged on top of main (#25412, #25445, #25456, #25455, #25453, #25452, #25458). Facts that matter for review order:

Integration branch results (2026-09-17)

Branch leaf-pushdown-integration = main at 3a647e4 plus the seven PRs above, with the fuzz switches for #25414 and #25412 turned on and the swap shape (#25446, no fix yet) kept off.

Check Result
cargo fmt --all -- --check clean
cargo clippy --profile ci -p datafusion-optimizer --all-targets -- -D warnings clean
cargo clippy --profile ci -p datafusion --tests --features avro,extended_tests,parquet_encryption -- -D warnings clean
cargo test --profile ci -p datafusion-optimizer 905 + 26 + 5 passed, 0 failed
cargo test --profile ci -p datafusion-sqllogictest --test sqllogictests 520 of 520 files, 0 failures
Differential fuzz, short (250 memory, 125 Parquet, 50 schema-evolution seeds) 0 failures, 0 skips
Differential fuzz, extended (5000 memory + 1000 Parquet seeds) 0 failures, 0 skips
Workspace --lib --tests --bins with the CI feature set (68 suites) 1786 passed, 0 failed after the swap shape was gated
Full sqllogictest with the invariant switches on 7 files flagged, listed below. Not every row is a duplication in the final plan, see the invariant row above.

With every fuzz shape on, the only remaining failures are the swap shape with the ambiguity error (24 of 250 seeds, 555 of 5000), which is #25446.

Round 2 (2026-09-18)

The branch now holds nine PRs: the seven above plus #25476 and #25478, with the reworked #25458 and the updated #25456. The end-of-run volatile invariant is enforced with no exemptions, and every differential shape is enabled, including the swap shape.

Check Result
cargo fmt, clippy (optimizer, core tests) clean
optimizer, physical-expr, proto crate tests (10 suites) 2999 passed, 0 failed
full sqllogictest, invariant enforced 520 of 520 files, 0 failures
differential test, short (all shapes) 0 failures
differential test, extended, 6000 seeds (all shapes) 0 failures
workspace --lib --tests --bins, CI feature set (68 suites) 11951 passed, 0 failed

The table below is from round 1 and is kept for the record; the per-rule findings it lists are no longer checked, by decision.

Duplications the invariant still reports on the integration branch (the arrow_field case of #25329 no longer appears, so #25456 closes it):

Rule File Expression duplicated
simplify_expressions expr.slt, select.slt, array/array_has.slt BETWEEN and COALESCE expansion: random() (#25457), abs(c1), array_has(...)
push_down_filter expr.slt (4 queries) struct(t1.time, t1.load1, t1.load2, t1.host)
extract_leaf_expressions order.slt (5 queries) named_struct(...) referenced from an ORDER BY field access
push_down_leaf_projections struct.slt (2 queries) named_struct(...) referenced from a field access above
replace_distinct_aggregate distinct_on.slt (2 queries) ascii(c1), chr(...), upper(c1)
optimize_projections file_row_index.slt file_row_index() (logical plan only; the physical planner collapses it)

The named_struct and struct rows are the next work item for the inlining policy: the definition is KeepInPlace by the placement API, and the rules still evaluate it at two sites. Whether named_struct should be classed as cheap instead is a placement decision (#25450), and until it is made the rules must not duplicate it.

Independent review (2026-09-18)

An independent review of this EPIC and its PRs found the errors corrected above and left these asks open:

What the review could not break: 15 differential queries over renames, swaps, same-name joins, unions, nested subqueries and correlated subqueries are identical with leaf pushdown on and off; 14 plan shapes probing for a lost TableScan::filters after #25455 all reached DataSourceExec; the three shapes #25456 promises to keep pushing still reach the scan.

Not filed yet

Activity

  1. adriangb
    added
    optimizerOptimizer rules
    EPICA larger project, actively underway, with sub tasks
    on Sep 18, 2026

  2. buraksenn commented on Sep 18, 2026

    @buraksenn
    buraksenn
    on Sep 18, 2026
    Contributor

    I've assigned #25447 and #25457 to myself and plan to work on them soon. But if you want to work on either of them, or if they're not up for grabs, I can drop both.

  3. adriangb commented on Sep 24, 2026

    @adriangb
    adriangb
    on Sep 24, 2026
    ContributorAuthor

    Optimizer convergence

    Two rule fights stop plans from reaching a fixed point before max_passes. PushDownFilter is one side of each fight. I checked both on main at 95bb0a0.

    1. PushDownFilter and PushDownLeafProjections (#14540, #25455)

    On main, push_down_filter, push_down_leaf_projections and optimize_projections change the #14540 plan in every pass. The loop stops after 3 passes only because it sees a plan that it saw before. With max_passes = 10 it also stops after 3 passes. Thus the cost is one extra pass. With #25455, the last pass changes nothing.

    2. PushDownFilter and CommonSubexprEliminate never converge

    This is the fight in the original title of #14540. #25455 does not fix it. I propose a new issue for it and a row in the "Open bugs" table.

    Reproduction (the query is already in floor_preimage.slt):

    CREATE TABLE test_data (id INT, int_val INT) AS VALUES (1, 100);
SET datafusion.optimizer.max_passes = 10;
EXPLAIN VERBOSE SELECT * FROM test_data WHERE floor(int_val) = 100;

    In each pass:

    1. common_sub_expression_eliminate moves CAST(int_val AS Float64) into a projection below the Filter, as __common_expr_N.
    2. push_down_filter pushes the Filter through that projection and puts the cast back into the predicate.
    3. common_sub_expression_eliminate moves it out again, as __common_expr_N+1.

    The alias changes in every pass, so the loop never sees a repeated plan. It runs all 10 passes, and the final plan contains __common_expr_10. The expected plans in floor_preimage.slt and eliminate_outer_join.slt contain __common_expr_3 for the same reason.

    rewrite_projection already refuses to push through a MoveTowardsLeafNodes expression, because ExtractLeafExpressions undoes that push. It has no equivalent guard for a CSE projection.

    I tried the simple fix: rewrite_projection does not push through a __common_expr projection. The plans then converge, but 3 expected plans get worse. A CSE projection stays in the final plan when the simplified predicate uses the expression only once. Thus this needs a design decision. Some options:

    • PushDownFilter does not push through a CSE projection, and a later rule removes a CSE projection whose expression is used only once.
    • CSE does not extract an expression from a Filter predicate if it extracted that expression before in the same optimizer run.
    • CSE runs only after the other rules reach a fixed point.
    • In all cases, a typed marker for a CSE projection (Replace the __datafusion_extracted / __common_expr alias prefix protocol with a typed marker #25449) replaces the alias-prefix check.

    Related

  4. shehabgamin commented on Sep 26, 2026

    @shehabgamin
    shehabgamin
    on Sep 26, 2026
    Contributor

    Thanks, this is super helpful!

  5. lyne7-sc
    added a commit that references this issue on Oct 1, 2026

    test: close five mutation-testing gaps in leaf expression pushdown gu…

    3fa5bc7
  6. zhuqi-lucas
    added 2 commits that reference this issue on Oct 1, 2026

    fix: keep the recovery projection when leaf pushdown would drop a com…

    9ee892b

    test: differential fuzz for leaf expression pushdown and parquet filt…

    7e28141
  7. vim89
    added a commit that references this issue on Oct 6, 2026

    fix: decide the precedence between PushDownFilter and PushDownLeafPro…

    6f3f7b7
  8. Omega359
    added 4 commits that reference this issue on Oct 11, 2026

    test: close five mutation-testing gaps in leaf expression pushdown gu…

    443ef40

    fix: keep the recovery projection when leaf pushdown would drop a com…

    e6f3b56

    test: differential fuzz for leaf expression pushdown and parquet filt…

    1f2064a

    fix: decide the precedence between PushDownFilter and PushDownLeafPro…

    6893e0d
  9. comphead
    added a commit that references this issue on Oct 14, 2026

    fix: resolve pass-through columns against the input when merging an e…

    47135be
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    EPICA larger project, actively underway, with sub tasksoptimizerOptimizer rules

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions