-
Notifications
You must be signed in to change notification settings - Fork 29.1k
[SPARK-30188][SQL] Resolve the failed unit tests when enable AQE #26813
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
Changes from all commits
cff0ff5
5d44f3e
626a448
82973df
3b354ac
6a3e12d
ef2e571
afbc4c1
f8a9cc0
b222471
55c3db5
07e7fb3
08828b4
da7bd9f
19748db
b3ebac8
c21d0db
a91d719
7c50070
115f940
ceb72c9
920de79
1915f82
d6bb22a
6e24639
98d19b4
987395c
9ad3a79
9478a89
3554cae
fa0d1be
ffe8e3e
253e6fa
c559b80
5d4152f
5d1af66
82a8906
18e00de
8b5e744
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,12 +20,14 @@ package org.apache.spark.sql.execution.adaptive | |
| import scala.collection.mutable | ||
|
|
||
| import org.apache.spark.sql.catalyst.expressions | ||
| import org.apache.spark.sql.catalyst.expressions.{CreateNamedStruct, DynamicPruningSubquery, ListQuery, Literal} | ||
| import org.apache.spark.sql.catalyst.expressions.{CreateNamedStruct, DynamicPruningSubquery, ListQuery, Literal, SubqueryExpression} | ||
| import org.apache.spark.sql.catalyst.plans.logical.LogicalPlan | ||
| import org.apache.spark.sql.catalyst.plans.physical.UnspecifiedDistribution | ||
| import org.apache.spark.sql.catalyst.rules.Rule | ||
| import org.apache.spark.sql.execution | ||
| import org.apache.spark.sql.execution._ | ||
| import org.apache.spark.sql.execution.command.ExecutedCommandExec | ||
| import org.apache.spark.sql.execution.exchange.Exchange | ||
| import org.apache.spark.sql.internal.SQLConf | ||
|
|
||
| /** | ||
|
|
@@ -39,11 +41,26 @@ case class InsertAdaptiveSparkPlan( | |
|
|
||
| private val conf = adaptiveExecutionContext.session.sessionState.conf | ||
|
|
||
| def containShuffle(plan: SparkPlan): Boolean = { | ||
| plan.find { | ||
| case _: Exchange => true | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you remember which case would fail because of this? We should not see an Exchange without a logical link in
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it's not about "no logical plan link", it's just: we should do AQE if there are already exchanges in the physical plan before
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If it's test failure related, we need to look into it, otherwise can we pull these changes out into a separate PR, and control the bypass with a flag, so at least we can still cover this code path in testing? |
||
| case s: SparkPlan => !s.requiredChildDistribution.forall(_ == UnspecifiedDistribution) | ||
| }.isDefined | ||
| } | ||
|
|
||
| def containSubQuery(plan: SparkPlan): Boolean = { | ||
| plan.find(_.expressions.exists(_.find { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is absolutely ridiculous to do so here. We deliberately added
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's still all-in or all-out. The complete check is |
||
| case _: SubqueryExpression => true | ||
| case _ => false | ||
| }.isDefined)).isDefined | ||
| } | ||
|
|
||
| override def apply(plan: SparkPlan): SparkPlan = applyInternal(plan, false) | ||
|
|
||
| private def applyInternal(plan: SparkPlan, isSubquery: Boolean): SparkPlan = plan match { | ||
| case _: ExecutedCommandExec => plan | ||
| case _ if conf.adaptiveExecutionEnabled && supportAdaptive(plan) => | ||
| case _ if conf.adaptiveExecutionEnabled && supportAdaptive(plan) | ||
| && (isSubquery || containShuffle(plan) || containSubQuery(plan)) => | ||
| try { | ||
| // Plan sub-queries recursively and pass in the shared stage cache for exchange reuse. Fall | ||
| // back to non-adaptive mode if adaptive execution is supported in any of the sub-queries. | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.