jayzhan211 commented on code in PR #26461:
URL: https://github.com/apache/datafusion/pull/26461#discussion_r4314641544
##########
datafusion/physical-optimizer/src/optimizer.rs:
##########
@@ -196,15 +195,10 @@ impl PhysicalOptimizer {
// Therefore, it should be run at the end of the optimization
process since any changes to the plan may break the dynamic filter's references.
// See `FilterPushdownPhase` for more details.
Arc::new(FilterPushdown::new_post_optimization()),
- // The SanityCheckPlan rule checks whether the order and
- // distribution requirements of each node in the plan
- // is satisfied. It will also reject non-runnable query
- // plans that use pipeline-breaking operators on infinite
- // input(s). The rule generates a diagnostic error
- // message for invalid plans. It makes no changes to the
- // given query plan; i.e. it only acts as a final
- // gatekeeping rule.
- Arc::new(SanityCheckPlan::new()),
+ // There is no `SanityCheckPlan` rule at the end of this list. The
Review Comment:
Done. `SanityCheckPlan` is now deprecated since 56.0.0, and the note points
to `check_plan_tree_sanity` for code that runs optimizer rules without the
planner. Tests that only used the rule to validate a plan now call
`check_plan_tree_sanity` directly, and the upgrade guide says the rule is
deprecated.
I deprecated it rather than removing it because several downstream projects
construct it in their own rule lists, for example greptimedb, cube, sail and
openobserve. They should see a deprecation warning before they hit a compile
error.
##########
datafusion/core/src/optimizer_rule_reference.md:
##########
@@ -69,26 +69,30 @@ Rule order matters. The default pipeline may change between
releases.
The same rule name may appear more than once when the default pipeline runs it
in multiple phases.
-| order | rule | phase | summary
|
-| ----- | ------------------------------ | ----------------------- |
------------------------------------------------------------------------------------------------------------
|
-| 1 | `OutputRequirements` | add phase | Adds
helper nodes so output requirements survive later physical rewrites.
|
-| 2 | `aggregate_statistics` | - | Uses
exact source statistics to answer some aggregates without scanning data.
|
-| 3 | `join_selection` | - | Chooses
join implementation, build side, and partition mode from statistics and stream
properties. |
-| 4 | `LimitedDistinctAggregation` | - | Pushes
limit hints into grouped distinct-style aggregations when only a small result
is needed. |
-| 5 | `FilterPushdown` | pre-optimization phase | Pushes
supported physical filters down toward data sources before distribution and
sorting are enforced. |
-| 6 | `WindowTopN` | - | Replaces
eligible row-number window and filter patterns with per-partition TopK
execution. |
-| 7 | `EnsureRequirements` | - | Enforces
both distribution and sorting requirements in a single idempotent rule.
|
-| 8 | `CombinePartialFinalAggregate` | - | Collapses
adjacent partial and final aggregates when the distributed shape makes them
redundant. |
-| 9 | `OptimizeAggregateOrder` | - | Updates
aggregate expressions to use the best ordering once sort requirements are
known. |
-| 10 | `ProjectionPushdown` | early pass | Pushes
projections toward inputs before later physical rewrites add more limit and
TopK structure. |
-| 11 | `OutputRequirements` | remove phase | Removes
the temporary output-requirement helper nodes after requirement-sensitive
planning is done. |
-| 12 | `LimitAggregation` | - | Passes a
limit hint into eligible aggregations so they can keep fewer accumulator
buckets. |
-| 13 | `LimitPushPastWindows` | - | Pushes
fetch limits through bounded window operators when doing so keeps the result
correct. |
-| 14 | `HashJoinBuffering` | - | Adds
buffering on the probe side of hash joins so probing can start before build
completion. |
-| 15 | `LimitPushdown` | - | Moves
physical limits into child operators or fetch-enabled variants to cut data
early. |
-| 16 | `TopKRepartition` | - | Pushes
TopK below hash repartition when the partition key is a prefix of the sort key.
|
-| 17 | `ProjectionPushdown` | late pass | Runs
projection pushdown again after limit and TopK rewrites expose new pruning
opportunities. |
-| 18 | `PushdownSort` | - | Pushes
sort requirements into data sources that can already return sorted output.
|
-| 19 | `EnsureCooperative` | - | Wraps
non-cooperative plan parts so long-running tasks yield fairly.
|
-| 20 | `FilterPushdown(Post)` | post-optimization phase | Pushes
dynamic filters at the end of optimization, after plan references stop moving.
|
-| 21 | `SanityCheckPlan` | - | Validates
that the final physical plan meets ordering, distribution, and infinite-input
safety requirements. |
+After the last rule, including any rule added by the user, the physical planner
Review Comment:
Agreed. #26351 has merged, so a follow-up PR can move this list next to the
contract docs.
This PR is now rebased on it. I also removed `SanityCheckPlan` from the
contract's list of rules that must keep a fixed order, and added a sentence
saying the planner checks the plan after the last rule.
##########
datafusion/core/src/optimizer_rule_reference.md:
##########
@@ -69,26 +69,30 @@ Rule order matters. The default pipeline may change between
releases.
The same rule name may appear more than once when the default pipeline runs it
in multiple phases.
-| order | rule | phase | summary
|
-| ----- | ------------------------------ | ----------------------- |
------------------------------------------------------------------------------------------------------------
|
-| 1 | `OutputRequirements` | add phase | Adds
helper nodes so output requirements survive later physical rewrites.
|
-| 2 | `aggregate_statistics` | - | Uses
exact source statistics to answer some aggregates without scanning data.
|
-| 3 | `join_selection` | - | Chooses
join implementation, build side, and partition mode from statistics and stream
properties. |
-| 4 | `LimitedDistinctAggregation` | - | Pushes
limit hints into grouped distinct-style aggregations when only a small result
is needed. |
-| 5 | `FilterPushdown` | pre-optimization phase | Pushes
supported physical filters down toward data sources before distribution and
sorting are enforced. |
-| 6 | `WindowTopN` | - | Replaces
eligible row-number window and filter patterns with per-partition TopK
execution. |
-| 7 | `EnsureRequirements` | - | Enforces
both distribution and sorting requirements in a single idempotent rule.
|
-| 8 | `CombinePartialFinalAggregate` | - | Collapses
adjacent partial and final aggregates when the distributed shape makes them
redundant. |
-| 9 | `OptimizeAggregateOrder` | - | Updates
aggregate expressions to use the best ordering once sort requirements are
known. |
-| 10 | `ProjectionPushdown` | early pass | Pushes
projections toward inputs before later physical rewrites add more limit and
TopK structure. |
-| 11 | `OutputRequirements` | remove phase | Removes
the temporary output-requirement helper nodes after requirement-sensitive
planning is done. |
-| 12 | `LimitAggregation` | - | Passes a
limit hint into eligible aggregations so they can keep fewer accumulator
buckets. |
-| 13 | `LimitPushPastWindows` | - | Pushes
fetch limits through bounded window operators when doing so keeps the result
correct. |
-| 14 | `HashJoinBuffering` | - | Adds
buffering on the probe side of hash joins so probing can start before build
completion. |
-| 15 | `LimitPushdown` | - | Moves
physical limits into child operators or fetch-enabled variants to cut data
early. |
-| 16 | `TopKRepartition` | - | Pushes
TopK below hash repartition when the partition key is a prefix of the sort key.
|
-| 17 | `ProjectionPushdown` | late pass | Runs
projection pushdown again after limit and TopK rewrites expose new pruning
opportunities. |
-| 18 | `PushdownSort` | - | Pushes
sort requirements into data sources that can already return sorted output.
|
-| 19 | `EnsureCooperative` | - | Wraps
non-cooperative plan parts so long-running tasks yield fairly.
|
-| 20 | `FilterPushdown(Post)` | post-optimization phase | Pushes
dynamic filters at the end of optimization, after plan references stop moving.
|
-| 21 | `SanityCheckPlan` | - | Validates
that the final physical plan meets ordering, distribution, and infinite-input
safety requirements. |
+After the last rule, including any rule added by the user, the physical planner
Review Comment:
> TIL about this document 🤯
+1 😆
--
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]
