Skip to content

Physical optimizer re-runs rules on plans they have already settled #25355

Open

Description

@zhuqi-lucas

Is your feature request related to a problem or challenge?

DataFusion's two optimizers take opposite positions on repeated rule execution, and the physical side is the one without a mechanism.

The logical optimizer iterates (max_passes, default 3) and detects convergence: it records a LogicalPlanSignature (node count + plan hash) per pass and stops once a plan repeats (datafusion/optimizer/src/optimizer.rs).

The physical optimizer runs its rule list exactly once, in a hand-ordered sequence, with no iteration, no convergence detection, and no way to recognize that a rule is being handed a plan it has already settled. That works for the default chain, which is carefully ordered so that everything able to invalidate distribution/ordering requirements runs before the single EnsureRequirements — the per-rule comments in physical-optimizer/src/optimizer.rs say so explicitly.

It stops working as soon as a chain is assembled with rules after that point. DataFusion is a library, with_physical_optimizer_rules is a supported extension point, and a rule inserted late (a custom scan rewrite, a distributed-execution boundary, an MV substitution) invalidates requirements again and needs another enforcement pass behind it.

Measured on a chain that enforces requirements six times, over a 34-node plan:

  • Of the six enforcement passes in that chain, four leave the plan byte-identical and still pay the full tree walk, about 30ms each in a debug build. That is roughly 122ms of a 209ms physical optimization phase spent arriving back at the same plan.
  • Across 31 EnsureRequirements invocations observed in the process, covering several queries, 23 left the plan byte-identical.
  • A mechanism like the one below recovers two of the four, 61ms measured, not all four: after anything changes the plan, the first pass that finds nothing to do still has to run in order to establish that the plan is a fixpoint.

Describe the solution you'd like

Let the chain's owner name the rules whose repeats may be answered from what was already observed, and have the optimizer remember, per planning run, the plans each named rule has been seen to leave untouched.

datafusion.optimizer.skip_unchanged_physical_rules = "EnsureRequirements"

Per pass, for a named rule: if the plan it is about to be given is one this rule has already returned unchanged, return it; otherwise run the rule, and record the plan only if the rule gave that same plan back.

Three properties matter, and two of them were learned the hard way:

  1. Key on the plan the rule was given, not the plan it last returned. The tempting version skips when a rule is handed back its own output, on the assumption that a rule cannot improve on itself. That assumption is false: a rule is not required to reach its fixpoint in one pass, and EnsureRequirements routinely does not. Two consecutive applications both changed the plan in my measurements, the second moving a RepartitionExec below a SortExec and switching that sort to per-partition. Recording only proven fixpoints makes a skip a replay of an observed outcome rather than a prediction.

  2. Compare plans by content, not by pointer. A rule that changes nothing still commonly rebuilds the tree: of the 23 no-op calls above, only 7 also returned the input object. Pointer identity cannot see a fixpoint. This is not a reporting problem that could be fixed in the rule: replace_children_if_necessary already returns the original plan when the child pointers are unchanged, so the passes that really did nothing already preserve identity. The other 16 lost it because the phases inside the rule genuinely rewrote the tree and then rewrote it back, which is EnsureRequirements: distribution and sorting phases undo each other on most calls #25360.

  3. Scope it to one planning run. Rule instances are shared between queries, so this must not live on the rule; and keeping it per-run keeps the config out of the key, since it cannot change midway through a run.

Debug builds re-run a skipped rule and assert it still leaves the plan alone, so a rule that does not depend only on the plan and the config fails a test rather than a query. Spark is the only surveyed engine that checks this (RuleExecutor.checkBatchIdempotence under Utils.isTesting); engines relying on counters or an Optional instead have public incidents from non-idempotent rules looping (trinodb/trino#11559, prestodb/presto#9362).

Default behaviour is unchanged, since the option is empty.

This is not a DataFusion-specific shape. DuckDB's pipeline runs CTE_INLINING, UNUSED_COLUMNS and COLUMN_LIFETIME twice with no dedup at all. Spark Catalyst runs the same ~30-rule set twice around "Infer Filters". The difference is that the others grew a mechanism for it:

Engine Mechanism Granularity
Spark Catalyst per-TreeNode _ineffectiveRules BitSet, plus batch fixpoint via fastEquals rule × subtree, opt-in at the call site
ClickHouse (QueryPlan) a pass returns size_t update_depth, re-descent bounded by it pass × node
Calcite HepPlanner firedRulesCache keyed on matched node ids, opt-in via setEnableFiredRulesCache (new in 1.42, CALCITE-7416/7422) rule × match
StarRocks / Doris Cascades ruleMasks BitSet rule × GroupExpression
Trino / Presto explicit Optional per invocation, no structural comparison rule invocation

Describe alternatives you've considered

  • A defaulted trait method, fn skip_if_unchanged(&self) -> bool { false }, letting each rule declare for itself. Wrong object: whether a chain repeats a rule is a property of the chain, not of the rule, and upstream cannot know what a downstream chain and node types will do to a rule's behaviour. It also carries a trap, since the optimizer asks only the outermost rule, so a rule wrapped for timing or tracing must forward the answer and a wrapper that leaves the default in place disables the feature silently. That is the same shape as Wrapper PhysicalOptimizerRules silently drop the schema_check() of the rules they wrap #25316, where a wrapper drops schema_check(). Matching on names needs no cooperation from a wrapper, which already reports the name it wraps.
  • Iterating the whole chain to a fixpoint, like the logical optimizer. Closer to the logical side's shape, but it changes when every existing rule runs, which is a much larger behavioural change than making redundant repeats cheap.
  • ClickHouse's update_depth. Strictly more information and it bounds re-traversal precisely, but it changes the signature of every rule.

Additional context

Implemented in #25356.

Three problems found in EnsureRequirements while measuring this are filed separately, since each stands on its own:

Fixing #25360 would remove much of the need for this feature, and would also let the comparison here fall back to pointers for the cases it covers. It is not a prerequisite for this PR.

Activity

  1. self-assigned this
    on Sep 16, 2026
  2. changed the title [-]Physical optimizer has no way to skip a rule whose input is the plan it just produced[/-] [+]Physical optimizer re-runs rules on plans they have already settled[/+] on Sep 16, 2026
  3. alamb commented on Sep 21, 2026

    @alamb
    Contributor

    The physical optimizer runs its rule list exactly once, in a hand-ordered sequence, with no iteration, no convergence detection, and no way to recognize that a rule is being handed a plan it has already settled.

    I think the core problem is that some PhysicalOptimizer passes such as EnforceRequirements as are not actually "optimizers" (make the query faster) they are "enforcers" using you terminology -- that ensure certain invariants are upheld.

    The LogicalPlan has a split for the two types of passes

    • OptimizerRule: passes to make the plan correct (e.g. type coercion)
    • AnalyzerRule : passes to make the plan faster (e.g. push filters down)

    AnalyzerRules are different than an OptimizerRules which must preserve the semantics of the LogicalPlan, while computing results in a more optimal way.

    So thus, I would propose an alternate strategy here:

    1. Pull the "enforcement" rules into a new trait (perhaps PhysicalAnalyzerRule to mirror the Logical plan terminology)

    Then we would be in a position to rerun the PhysicalOptimizer (not analyzer) rules multiple times, checking for convergence, etc.

    And our optimizers would be consistent and easy to explain

  4. alamb commented on Sep 21, 2026

    @alamb
    Contributor

    I can also file a independent ticket to describe the need for a split in the PhysicalOptimizerRules

  5. zhuqi-lucas commented on Sep 21, 2026

    @zhuqi-lucas
    ContributorAuthor

    Thank you @alamb for the great idea!

    Strongly agree, and the chain behind #25356 is direct evidence for the split: it hand-repeats enforcement after every rewrite that can invalidate requirements, and 28 of its 35 rule passes leave the plan untouched in production. Filed the independent ticket as #25572 with the design questions we hit while running that chain, three of which seem worth deciding early:

    1. Some downstream optimizer rules consume the operators enforcement materializes rather than the abstract requirement, and are deliberately scheduled after an enforcement pass today, so the split needs either a sanctioned enforce/optimize alternation or a "rules read requirements, not materialized operators" convention.
    2. The loop needs revisit detection, not just "unchanged since last pass": we have a measured case where an enforcement rule oscillates between two forms, which the logical loop's HashSet of prior signatures handles and a last-pass comparison would not.
    3. ExecutionPlan has no Hash/Eq, so a PhysicalPlanSignature has to be derived; the fingerprint from feat(core): skip a named rule handed a plan it has been seen to leave alone #25356 can be repurposed wholesale as that identity, and in the loop model the per-rule memoization and config key there become unnecessary, which I am happy to drop.

    Glad to drive #25572 and bring this chain as the case study.

  6. alamb commented on Sep 21, 2026

    @alamb
    Contributor

    Thanks -- I will try and help review #25572 to make this happen

  7. alamb commented on Oct 1, 2026

    @alamb
    Contributor

    Could you describe your usecase clearly? My interpretation is you are trying to speed up physical planning by knowing when you can stop running optimizer passes

    Related to @2010YOUY01 's question here #25688 (comment)

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

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions