-
Notifications
You must be signed in to change notification settings - Fork 29.3k
[SPARK-29800][SQL] Rewrite non-correlated EXISTS subquery use ScalaSubquery to optimize perf #26437
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 17 commits
2d762b4
1c577bc
5fa971b
1401349
7b943aa
95e446d
20cda42
8e3ce4f
c290411
866ddc7
3de0ecc
32f85c3
e47a757
4c86605
626e41f
ce76e0c
4a4ca9b
88f804d
7668bd6
a6b8485
34046be
4c6c04d
ac6a4d2
59162c6
89a1721
fb98b54
67b4281
821ed40
e319fee
2c387f2
2aff8eb
2b7b417
88fcdbf
9f084ee
8c6060a
9a9d9d1
173942d
26258b0
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 |
|---|---|---|
|
|
@@ -96,7 +96,8 @@ object RewritePredicateSubquery extends Rule[LogicalPlan] with PredicateHelper { | |
| def apply(plan: LogicalPlan): LogicalPlan = plan transform { | ||
| case Filter(condition, child) => | ||
| val (withSubquery, withoutSubquery) = | ||
| splitConjunctivePredicates(condition).partition(SubqueryExpression.hasInOrExistsSubquery) | ||
| splitConjunctivePredicates(condition) | ||
| .partition(SubqueryExpression.hasInOrCorrelatedExistsSubquery) | ||
|
Comment on lines
+99
to
+100
Member
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. Unrelated change?
Member
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. Oh, nvm, I saw it. |
||
|
|
||
| // Construct the pruned filter condition. | ||
| val newFilter: LogicalPlan = withoutSubquery match { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,8 +23,9 @@ import scala.collection.mutable.ArrayBuffer | |
| import org.apache.spark.broadcast.Broadcast | ||
| import org.apache.spark.sql.SparkSession | ||
| import org.apache.spark.sql.catalyst.{expressions, InternalRow} | ||
| import org.apache.spark.sql.catalyst.expressions.{AttributeSeq, CreateNamedStruct, Expression, ExprId, InSet, ListQuery, Literal, PlanExpression} | ||
| import org.apache.spark.sql.catalyst.expressions.{CreateNamedStruct, Expression, ExprId, InSet, ListQuery, Literal, PlanExpression} | ||
| import org.apache.spark.sql.catalyst.expressions.codegen.{CodegenContext, ExprCode} | ||
| import org.apache.spark.sql.catalyst.plans.logical.Project | ||
| import org.apache.spark.sql.catalyst.rules.Rule | ||
| import org.apache.spark.sql.internal.SQLConf | ||
| import org.apache.spark.sql.types.{BooleanType, DataType, StructType} | ||
|
|
@@ -171,6 +172,44 @@ case class InSubqueryExec( | |
| } | ||
| } | ||
|
|
||
| /** | ||
| * The physical node of non-correlated EXISTS subquery. | ||
| */ | ||
| case class ExistsExec( | ||
|
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. Can we name it
Contributor
Author
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.
Sure, update to |
||
| plan: BaseSubqueryExec, | ||
| exprId: ExprId) | ||
| extends ExecSubqueryExpression { | ||
|
|
||
| @volatile private var result: Option[Boolean] = None | ||
|
|
||
| override def dataType: DataType = BooleanType | ||
| override def children: Seq[Expression] = Nil | ||
| override def nullable: Boolean = false | ||
| override def toString: String = s"EXISTS (${plan.simpleString(SQLConf.get.maxToStringFields)})" | ||
| override def withNewPlan(plan: BaseSubqueryExec): ExistsExec = copy(plan = plan) | ||
|
|
||
| override def semanticEquals(other: Expression): Boolean = other match { | ||
| case in: ExistsExec => plan.sameResult(in.plan) | ||
| case _ => false | ||
| } | ||
|
|
||
| def updateResult(): Unit = { | ||
| result = Some(plan.executeTake(1).length == 1) | ||
| } | ||
|
|
||
| def values(): Option[Boolean] = result | ||
|
|
||
| override def eval(input: InternalRow): Any = { | ||
| require(result.isDefined, s"$this has not finished") | ||
| result.get | ||
| } | ||
|
|
||
| override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = { | ||
|
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. again, can we follow
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. as well as
Contributor
Author
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.
Done |
||
| require(result.isDefined, s"$this has not finished") | ||
| Literal.create(result.get, dataType).doGenCode(ctx, ev) | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Plans subqueries that are present in the given [[SparkPlan]]. | ||
| */ | ||
|
|
@@ -194,6 +233,9 @@ case class PlanSubqueries(sparkSession: SparkSession) extends Rule[SparkPlan] { | |
| } | ||
| val executedPlan = new QueryExecution(sparkSession, query).executedPlan | ||
| InSubqueryExec(expr, SubqueryExec(s"subquery#${exprId.id}", executedPlan), exprId) | ||
| case expressions.Exists(sub, children, exprId) => | ||
|
cloud-fan marked this conversation as resolved.
Outdated
|
||
| val executedPlan = new QueryExecution(sparkSession, Project(Nil, sub)).executedPlan | ||
|
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. Can we add the Project a bit earlier? This seems weird to add Project and do column pruning at planning time.
Contributor
Author
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.
add this 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. how about the rule
Contributor
Author
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.
correlated and non-correlated subqueries all match here, and we only handle non-correlated exists. If we add
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. Isn't it useful for correlated EXISTS as well? logically it's always valid to prune all columns from EXISTS subquery, no matter it's correlated or not.
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. how about WHERE on a single table? it's a known problem that correlated subqueries may not work well in join condition.
Contributor
Author
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 ok with WHERE on a single table.
Contributor
Author
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. For correlated exists, convert to join, seems don't need to do this?
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. OK let's keep it here now. But I do think it's better to put logical optimization in logical rules.
Contributor
Author
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.
Understand, but current code about subqueries disperse in different places and interacting with each other |
||
| ExistsExec(SubqueryExec(s"subquery#${exprId.id}", executedPlan), exprId) | ||
| } | ||
| } | ||
| } | ||
|
|
||
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.
We should update the doc too.