Repository navigation
PoC: Explicit Invariant Regions in the Physical Optimizer - #26360
2010YOUY01 wants to merge 3 commits into
Conversation
alamb
left a comment
There was a problem hiding this comment.
Thank you so much for working on this @2010YOUY01
The default physical optimizer pipeline works for the optimizations implemented in DataFusion core. However, there are many hidden assumptions between optimizer rules that make the pipeline work as a whole (e.g., rule A must run before rule B to produce a valid plan).
I think I fundamentally disagree with this assesment
what I don't like about this PR is that I think it makes the optimizer more complicated and commits us to non trivial invariants / rules going forward rather than driving towards something simpler.
Rather than encoding the set of existing (and I admit complicated) interconnections, I think the optimizer will be far simpler and easier to understand if there is one rule (in theory) to make the plan valid: EnforceRequirements
Then every other rule would obey "valid plan in --> valid plan out", which is a simple contract that is easy to explain and will make the Optimizer more understandable. I understand that the current code does not follow this ideal, but I feel like adding the API in this PR moves us away from a simpler future
In terms of this PR, I think that roughly boils down to always check these
Step::Begin(Arc::new(InputRequirementsSatisfied)),
Step::End(InputRequirementsSatisfied.name()),
In my opinion, the other examples in this PR such as
Step::Begin(Arc::new(HashJoinModesResolved)),
Step::Begin(Arc::new(NoEmbeddedProjections)),
should be treated like bug / limitation on rules and that we should work to remove these assumptions rather than making them explicit in the pipelines / assumptions
|
Thanks for sharing your perspective, @alamb! I agree we should be more selective about adding invariant regions.
I'm not convinced that more explicit invariants necessarily mean more complexity. However, since I have less experience working with optimizers, I'll trust your judgment here and look for more problem-specific, compelling invariant regions to add in the future. My current intuition is that more explicit constraints can actually reduce uncertainty and make extending the optimizer easier:
When we find some invariants getting in the way, we can remove them relatively cheaply. Making them explicit also encourages reviewers to think more carefully about the design and existing assumptions. Next stepI'll continue the discussion in #25688. Generally, I think it's better to enforce the However, enforcing it seems to require machinery of similar complexity to this PR. If we're going to introduce that machinery, perhaps we should make it extensible to other invariants as well, and see if we can add a few major, less controversial invariant fences in the future. |
To be clear, I don't think the problem is with explicit invariants -- the problem is with some of the invariants themselves (e.g. the assumption that there are no embedded projections). I would rather remove as many of the invariants as possible rather than try and make them explicit
Yes, this is a real problem -- but I think a better solution is to remove the implicit assumptions rather make them explicit
I think it is basically enforced via https://docs.rs/datafusion/latest/datafusion/physical_plan/trait.ExecutionPlan.html#method.check_invariants / https://docs.rs/datafusion-physical-optimizer/55.2.0/datafusion_physical_optimizer/sanity_checker/struct.SanityCheckPlan.html As well as each pass here datafusion/datafusion/core/src/physical_planner.rs Lines 3377 to 3379 in b6f6990 |
Which issue does this PR close?
Rationale for this change
The default physical optimizer pipeline works for the optimizations implemented in DataFusion core. However, there are many hidden assumptions between optimizer rules that make the pipeline work as a whole (e.g., rule A must run before rule B to produce a valid plan).
These implicit constraints make extensions harder, whether adding new rules within DataFusion core or implementing custom pipelines downstream.
This PR introduces explicit invariant regions to make these hidden constraints visible and verifiable, making the optimizer easier to understand, extend, and maintain.
Note that this PR does not introduce new constraints. I think of this primarily as a "removing uncertainty" step. The assumptions already exist; this PR provides a mechanism to surface them so that:
The tradeoff is a certain learning curve for this mechanism. After prototyping the idea, I feel that once we become familiar with the concept, it can make long-term maintenance easier. However, it definitely looks confusing at first glance, and I'm still thinking about how to improve this part.
(Note: The implementation is currently underdocumented; most of the ideas are explained in this writeup.)
Two Types of Constraints
This section explains two types of constraints, with one example each. The following figure illustrates two invariant regions in an example optimizer pipeline, which will be explained further below.
Type I: Optimization-Coverage Constraints (Soft)
Progressive refinement in the optimizer pipeline
The default optimizer pipeline is a progressive refinement process, where the possible plan shapes vary across stages.
For example:
A rule that rewrites joins could be much simpler when placed at an earlier stage.
Requiring every rule to handle every possible representation, regardless of its position, can unnecessarily bloat the implementation of earlier rules.
Making soft constraints explicit
Built-in rules already rely on such canonicalization assumptions, allowing them to match only canonical plan shapes and keep their implementations simple.
Soft constraints provide a mechanism to make these assumptions explicit and verifiable, so custom optimizer pipelines can either:
Example: LimitedDistinctAggregation
See #26069 for a concrete example of how relying on a canonical aggregation shape can simplify an optimizer rule.
The corresponding invariant region would be:
This is illustrated as Region 1 (Aggr) in the figure above.
Type II: Execution-Validity Constraints (Hard)
These constraints are required for correct execution. Extensions must preserve them to ensure that plans remain runnable.
Example: EnsureRequirements
Consider a partitioned hash join.
Before enforcing requirements:
The plan represents the intended computation, but its children do not satisfy the hash join's distribution requirements.
After
EnsureRequirements:Now the join's distribution requirements are satisfied.
We can express this as an invariant region, as shown in the figure above.
Within this region, every optimizer rule must preserve the invariant. If a rule violates it, the checker reports the failure immediately after that rule (when validation is enabled).
For extensions, the responsibility is explicitly one of the following:
What changes are included in this PR?
validate_optimizer_invariants, to control invariant checking. Use a feature flag to enable validation in tests/CI while skipping it in release builds.physical-optimizer/src/optimizer.rs, introducing invariant regions into the existing optimizer rule list.The PoC implements the following regions, which are not exactly the same as the examples above:
One thing that remains implicit is which optimizer rules establish the invariants.
For example, in the figure above, the canonical aggregation region is established by initial physical planning, while the
InputRequirementsSatisfiedregion is established byEnsureRequirements.Therefore, downstream custom optimizer pipelines must follow the same assumptions and manually declare these regions after the corresponding rules have established their invariants.
What is the testing strategy for this PR?
Are there any user-facing changes?