-
Notifications
You must be signed in to change notification settings - Fork 29.3k
[SPARK-58233][SQL] Push down local sort to reduce redundant sorts within a stage #57396
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
ulysses-you
wants to merge
5
commits into
apache:master
from
ulysses-you:spark-pushdown-merge-local-sort
Closed
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
0b2f14f
[SPARK-58233][SQL] Push down local sort to reduce redundant sorts wit…
ulysses-you 2b05aa9
[SPARK-58233][SQL][FOLLOWUP] Address review: drop unreachable SortAgg…
ulysses-you dd42f73
[SPARK-58233][SQL][FOLLOWUP] Guard determinism and gate cardinality-r…
ulysses-you 4b5520c
[SPARK-58233][SQL][FOLLOWUP] Thread throughCardinalityReducer as a pa…
ulysses-you b6503d8
[SPARK-58233][SQL][FOLLOWUP] Check the final adaptive plan in sort-co…
ulysses-you File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
156 changes: 156 additions & 0 deletions
156
sql/core/src/main/scala/org/apache/spark/sql/execution/PushDownLocalSort.scala
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,156 @@ | ||
| /* | ||
| * Licensed to the Apache Software Foundation (ASF) under one or more | ||
| * contributor license agreements. See the NOTICE file distributed with | ||
| * this work for additional information regarding copyright ownership. | ||
| * The ASF licenses this file to You under the Apache License, Version 2.0 | ||
| * (the "License"); you may not use this file except in compliance with | ||
| * the License. You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| package org.apache.spark.sql.execution | ||
|
|
||
| import org.apache.spark.sql.catalyst.expressions.{Alias, Attribute, AttributeMap, AttributeReference, AttributeSet, SortOrder} | ||
| import org.apache.spark.sql.catalyst.rules.Rule | ||
| import org.apache.spark.sql.execution.window.{WindowExecBase, WindowGroupLimitExec} | ||
| import org.apache.spark.sql.internal.SQLConf | ||
|
|
||
| /** | ||
| * Pushes a wider local sort down through order-preserving operators onto a narrower local sort | ||
| * below, widening it so that a single sort satisfies several operators' ordering requirements | ||
| * instead of re-sorting once per operator. | ||
| * | ||
| * `EnsureRequirements` adds one local `SortExec` (`global = false`) above every operator whose | ||
| * `requiredChildOrdering` is not already satisfied. When such requirements are in a prefix-cover | ||
| * relationship, this produces multiple local sorts that only differ in width. A canonical case is a | ||
| * sort aggregate stacked on a window over the same clustering keys, where the aggregate needs a | ||
| * wider ordering than the window: | ||
| * | ||
| * {{{ | ||
| * SortAggregate(key = [a, b, c]) | ||
| * Sort([a, b, c], global = false) <- upper, wider | ||
| * Window([a], [b]) | ||
| * Sort([a, b], global = false) <- lower, narrower | ||
| * Exchange(hashpartitioning([a])) | ||
| * }}} | ||
| * | ||
| * Because every operator between the two sorts is order-preserving and the upper ordering | ||
| * prefix-covers everything required along the way, the wider ordering can be pushed down to widen | ||
| * the lower sort, and the upper sort then dropped entirely: | ||
| * | ||
| * {{{ | ||
| * SortAggregate(key = [a, b, c]) | ||
| * Window([a], [b]) requiredChildOrdering [a, b] is satisfied by [a, b, c] | ||
| * Sort([a, b, c], global = false) <- single sort now serves both operators | ||
| * Exchange(hashpartitioning([a])) | ||
| * }}} | ||
| * | ||
| * When a `ProjectExec` on the path renames an ordering column in its output (`b AS x`), the | ||
| * ordering is rewritten from the project's output space back to its child's space (`x` -> `b`) as | ||
| * it is pushed through, so a sort over the renamed column is still matched below. Only plain | ||
| * renames are followed, and the rule never crosses a shuffle or a non-order-preserving operator. | ||
| */ | ||
| object PushDownLocalSort extends Rule[SparkPlan] { | ||
|
|
||
| def apply(plan: SparkPlan): SparkPlan = { | ||
| if (!conf.getConf(SQLConf.PUSH_DOWN_LOCAL_SORT_ENABLED)) { | ||
| return plan | ||
| } | ||
| val throughCardinalityReducer = | ||
| conf.getConf(SQLConf.PUSH_DOWN_LOCAL_SORT_THROUGH_CARDINALITY_REDUCER_ENABLED) | ||
|
|
||
| plan.transform { | ||
| case upper @ SortExec(upperOrder, false, child, _) => | ||
| pushDown(child, upperOrder, throughCardinalityReducer).getOrElse(upper) | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Walks down from `plan` through a chain of order-preserving unary operators, looking for a | ||
| * lower local `SortExec` that `upperOrder` strictly covers. When found, widens that lower sort | ||
| * to `upperOrder` and returns the rebuilt subtree (which re-exposes `upperOrder` at its top); | ||
| * returns `None` if no safe widening applies, leaving the plan untouched. As it crosses an | ||
| * operator that renames ordering columns, `upperOrder` is rewritten into that operator's child | ||
| * space so the search continues against the child's own attributes. `throughCardinalityReducer` | ||
| * controls whether cardinality-reducing operators (`FilterExec`, `WindowGroupLimitExec`) may be | ||
| * crossed. | ||
| */ | ||
| private def pushDown( | ||
| plan: SparkPlan, | ||
| upperOrder: Seq[SortOrder], | ||
| throughCardinalityReducer: Boolean): Option[SparkPlan] = plan match { | ||
| case lower @ SortExec(lowerOrder, false, _, _) | ||
| // Only widen when the upper ordering strictly covers the lower one. When they are | ||
| // equivalent the upper sort is plainly redundant and is left to `RemoveRedundantSorts`; a | ||
| // non-covering ordering cannot serve the lower requirement. The column check keeps the | ||
| // widened sort well-formed (every key of `upperOrder` is available below the lower sort). | ||
| if SortOrder.orderingSatisfies(upperOrder, lowerOrder) && | ||
| !SortOrder.orderingSatisfies(lowerOrder, upperOrder) && | ||
| AttributeSet(upperOrder.flatMap(_.references)).subsetOf(lower.child.outputSet) => | ||
| Some(SortExec(upperOrder, global = false, child = lower.child)) | ||
|
|
||
| case op: UnaryExecNode if isOrderPreserving(op, throughCardinalityReducer) => | ||
| // A `ProjectExec` may rename ordering columns in its output (`b AS x`). Rewrite `upperOrder` | ||
| // from the operator's output space back to its child's space before pushing further down, so | ||
| // a sort over the renamed column is still matched below. Only plain renames are followed; an | ||
| // expression alias leaves the sort key referencing an output attribute the child does not | ||
| // produce, so the `subsetOf(op.child.outputSet)` check below rejects it. | ||
| val outputExprs = plan match { | ||
| case p: ProjectExec => p.projectList | ||
| case _ => Nil | ||
| } | ||
| val rewrittenUpperOrder = if (outputExprs.isEmpty) { | ||
| upperOrder | ||
| } else { | ||
| val aliasToAttributeMap = AttributeMap(outputExprs.collect { | ||
| case a @ Alias(child: AttributeReference, _) => (a.toAttribute, child: Attribute) | ||
| }) | ||
| upperOrder.map { _.transformUp { | ||
| case a: Attribute => aliasToAttributeMap.getOrElse(a, a) | ||
| }.asInstanceOf[SortOrder] | ||
| } | ||
| } | ||
| if (SortOrder.orderingSatisfies(rewrittenUpperOrder, op.requiredChildOrdering.head) && | ||
| AttributeSet(rewrittenUpperOrder.flatMap(_.references)).subsetOf(op.child.outputSet)) { | ||
| pushDown(op.child, rewrittenUpperOrder, throughCardinalityReducer) | ||
| .map(newChild => op.withNewChildren(Seq(newChild))) | ||
| } else { | ||
| None | ||
| } | ||
|
|
||
| case _ => None | ||
| } | ||
|
|
||
| private def isOrderPreserving( | ||
| plan: UnaryExecNode, | ||
| throughCardinalityReducer: Boolean): Boolean = plan match { | ||
| // A non-deterministic project/filter must not be crossed: moving the sort below it changes | ||
| // which rows a seeded non-deterministic expression is evaluated over (a different row-value | ||
| // association for a project, a different surviving set for a filter). This mirrors the | ||
| // determinism guard in the logical `EliminateSorts.canEliminateSort`. | ||
| case p: ProjectExec => p.projectList.forall(_.deterministic) | ||
| // `FilterExec` and `WindowGroupLimitExec` are cardinality reducers: pushing the wider sort | ||
| // below them sorts the full input instead of only the surviving rows, which can outweigh the | ||
| // saved sort. Only cross them when explicitly enabled. `FilterExec` additionally needs the | ||
| // determinism guard, for the same reason as the project above. | ||
| case f: FilterExec => throughCardinalityReducer && f.condition.deterministic | ||
| case _: WindowGroupLimitExec => throughCardinalityReducer | ||
| // Pushing a wider sort under a window refines the input order the window sees within ties of | ||
| // its own `ORDER BY`. For order-sensitive window functions (`first_value`/`last_value`/ | ||
| // `collect_list`, `lead`/`lag`, `ROWS`-frame aggregates) that can change the result -- but only | ||
| // for a non-unique window `ORDER BY`, where the result is already non-deterministic by Spark's | ||
| // contract, so this does not change any deterministic result. This differs from | ||
| // `CollectMetricsExec`, which is deliberately excluded: its observed metric is a documented, | ||
| // expected-stable side output, not a non-deterministic query result, so refining the order it | ||
| // sees would be a real surprise. | ||
| case _: WindowExecBase => true | ||
| case _ => false | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Widening the sort that feeds a window changes the row order the window sees within ties of its own
ORDER BY. For order-sensitive window functions that's observable:collect_list/collect_set/first_value/last_value/nth_valuecollect/pick in input order;lead/lagand aggregates with aROWSframe depend on physical adjacency;row_number()-basedWindowGroupLimitExeckeeps a different set of rows when ties reorder.Example:
collect_list(c) OVER (PARTITION BY a ORDER BY b)stacked under a wider window that needs[a, b, c]. Today the collect_list window sorts by[a, b]; after this rule it sorts by[a, b, c], so within equalbthe list followsc.These are all non-deterministic over a non-unique
ORDER BY, so it isn't wrong per Spark's contract -- but it's exactly the order-sensitivity you use to keepCollectMetricsExecout ofisOrderPreserving(and the reason cited in the PR description). So the description's "query results are unchanged" holds for deterministic queries but not for these, and the line between "exclude CollectMetrics" and "push through a window carryingfirst/last/collect_list" is hard to defend.Two asks: (1) soften that claim in the description; (2) decide whether a window carrying an order-sensitive function should be treated like
CollectMetricsExec. If you deliberately keep them in (they're non-deterministic anyway), a one-line comment here explaining why the window case is acceptable while CollectMetrics isn't would settle it for the next reader.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, thanks. I kept pushing through windows on purpose: it only refines the tie order for a non-unique window
ORDER BY, which is already non-deterministic per Spark's contract, so no deterministic result changes. That is genuinely different fromCollectMetricsExec, whose observed metric is an expected-stable side output rather than a non-deterministic query result — so I keep that one excluded.I added a comment at the
WindowExecBase/WindowGroupLimitExecentries inisOrderPreservingspelling out exactly this distinction, and softened the PR description: "query results are unchanged" -> "the result of a deterministic query is unchanged", plus an explicit note that order-sensitive window functions over a non-uniqueORDER BYmay return a different (still valid) non-deterministic result.