[SPARK-36424][SQL] Support eliminate limits in AQE Optimizer #33651

ulysses-you · 2021-08-05T09:15:38Z

What changes were proposed in this pull request?

override the maxRows method in LogicalQueryStage
add rule EliminateLimits in AQEOptimizer

Why are the changes needed?

In Ad-hoc scenario, we always add limit for the query if user have no special limit value, but not all limit is nesessary.

With the power of AQE, we can eliminate limits using running statistics.

Does this PR introduce any user-facing change?

no

How was this patch tested?

add test

ulysses-you · 2021-08-05T09:51:25Z

cc @maropu @cloud-fan @viirya @HyukjinKwon

SparkQA · 2021-08-05T10:21:07Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46592/

SparkQA · 2021-08-05T11:02:44Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46595/

SparkQA · 2021-08-05T11:16:18Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46592/

cloud-fan · 2021-08-05T11:32:40Z

sql/core/src/main/scala/org/apache/spark/sql/execution/adaptive/AQEEliminateLimits.scala

+ * if we can eliminate limits. And we check if [[LogicalQueryStage]] is materialized at stats,
+ * if it is not materialized the maxRows is none.
+ */
+object AQEEliminateLimits extends EliminateLimitsBase {


what's the difference between this and EliminateLimits?

They are same. Just consider we don't need transformDownWithPruning in AQE Optimizer since it's batch only run once.

that's too minor to justify creating a new class

yea, seems can reuse EliminateLimits?

OK, I'm fine to reuse the EliminateLimits

SparkQA · 2021-08-05T11:46:25Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46595/

SparkQA · 2021-08-05T11:48:12Z

Test build #142082 has finished for PR 33651 at commit 4299af5.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
trait EliminateLimitsBase extends Rule[LogicalPlan]

SparkQA · 2021-08-05T13:21:20Z

Test build #142084 has finished for PR 33651 at commit c076d76.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
trait EliminateLimitsBase extends Rule[LogicalPlan]

ulysses-you · 2021-08-05T15:13:46Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala

    case Limit(l, child) if canEliminate(l, child) =>
      child
    case GlobalLimit(l, child) if canEliminate(l, child) =>
      child

    case GlobalLimit(le, GlobalLimit(ne, grandChild)) =>
-      GlobalLimit(Least(Seq(ne, le)), grandChild)
+      GlobalLimit(Literal(Least(Seq(ne, le)).eval().asInstanceOf[Int]), grandChild)


It's flaky that this expression highly depend on ConstantFolding.

SparkQA · 2021-08-05T15:22:14Z

Kubernetes integration test unable to build dist.

exiting with code: 1
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46613/

SparkQA · 2021-08-05T16:01:04Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46614/

SparkQA · 2021-08-05T16:40:25Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46614/

SparkQA · 2021-08-05T19:46:44Z

Test build #142101 has finished for PR 33651 at commit 8aae958.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2021-08-05T20:05:17Z

Test build #142102 has finished for PR 33651 at commit 4510816.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2021-08-06T00:47:52Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46629/

SparkQA · 2021-08-06T01:43:11Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46629/

ulysses-you · 2021-08-06T02:59:09Z

sql/core/src/main/scala/org/apache/spark/sql/execution/adaptive/LogicalQueryStage.scala

@@ -54,4 +54,6 @@ case class LogicalQueryStage(
    }
    physicalStats.getOrElse(logicalPlan.stats)
  }
+
+  override def maxRows: Option[Long] = stats.rowCount.map(_.min(Long.MaxValue).toLong)


If there is other physical nodes above QueryStageExec the stats is not accurate. Seems the only node can exist here is aggregate, so the maxRows is still accurate.

We should trust the existing framework. If the maxRows stats can be wrong, then EliminateLimits is also wrong even without AQE.

I don't think we need to highlight LogicalQueryStage in the doc of EliminateLimits. We just need to follow the existing framework and make sure the maxRows is not under-estimated.

OK, cleaned the comment of EliminateLimits

SparkQA · 2021-08-06T02:59:14Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46632/

sql/core/src/test/scala/org/apache/spark/sql/execution/adaptive/AdaptiveQueryExecSuite.scala

SparkQA · 2021-08-06T03:22:51Z

Test build #142130 has started for PR 33651 at commit e8f17a4.

SparkQA · 2021-08-06T03:42:05Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46632/

SparkQA · 2021-08-06T04:52:05Z

Test build #142117 has finished for PR 33651 at commit ba7c42a.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2021-08-06T04:52:49Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46642/

sql/core/src/test/scala/org/apache/spark/sql/execution/adaptive/AdaptiveQueryExecSuite.scala

SparkQA · 2021-08-06T05:32:14Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46642/

ulysses-you · 2021-08-06T08:39:18Z

retest this please

SparkQA · 2021-08-06T09:08:18Z

Test build #142152 has finished for PR 33651 at commit e8f17a4.

This patch fails to build.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2021-08-06T10:21:28Z

Test build #142119 has finished for PR 33651 at commit 180ef97.

This patch fails from timeout after a configured wait of 500m.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2021-08-06T13:00:46Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46664/

SparkQA · 2021-08-06T13:37:58Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46664/

SparkQA · 2021-08-06T15:11:05Z

Test build #142162 has finished for PR 33651 at commit 6f28a37.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2021-08-06T16:57:38Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46675/

SparkQA · 2021-08-06T17:53:46Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46675/

viirya · 2021-08-06T20:46:55Z

retest this please

ulysses-you · 2021-08-07T05:54:21Z

retest this please

SparkQA · 2021-08-07T06:46:06Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46699/

SparkQA · 2021-08-07T07:40:23Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46699/

SparkQA · 2021-08-07T10:57:26Z

Test build #142187 has finished for PR 33651 at commit 6f28a37.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

cloud-fan · 2021-08-09T08:51:50Z

thanks, merging to master!

github-actions bot added the SQL label Aug 5, 2021

ulysses-you force-pushed the SPARK-36424 branch from 4299af5 to c076d76 Compare August 5, 2021 09:23

cloud-fan reviewed Aug 5, 2021

View reviewed changes

ulysses-you force-pushed the SPARK-36424 branch 2 times, most recently from 8aae958 to 4510816 Compare August 5, 2021 15:09

ulysses-you commented Aug 5, 2021

View reviewed changes

ulysses-you commented Aug 6, 2021

View reviewed changes

cloud-fan reviewed Aug 6, 2021

View reviewed changes

sql/core/src/test/scala/org/apache/spark/sql/execution/adaptive/AdaptiveQueryExecSuite.scala Show resolved Hide resolved

cloud-fan approved these changes Aug 6, 2021

View reviewed changes

viirya reviewed Aug 6, 2021

View reviewed changes

sql/core/src/test/scala/org/apache/spark/sql/execution/adaptive/AdaptiveQueryExecSuite.scala Show resolved Hide resolved

viirya approved these changes Aug 6, 2021

View reviewed changes

ulysses-you added 5 commits August 6, 2021 21:23

Support eliminate limits in AQE Optimizer

44b7fe7

fix

5dcd577

reuse

509f5e7

Limit

e65182d

simplify comment

6f28a37

ulysses-you force-pushed the SPARK-36424 branch from e8f17a4 to 6f28a37 Compare August 6, 2021 13:24

cloud-fan closed this in bb6f65a Aug 9, 2021

ulysses-you deleted the SPARK-36424 branch August 9, 2021 11:17

[SPARK-36424][SQL] Support eliminate limits in AQE Optimizer #33651

[SPARK-36424][SQL] Support eliminate limits in AQE Optimizer #33651

Conversation

ulysses-you commented Aug 5, 2021 • edited

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

ulysses-you commented Aug 5, 2021

SparkQA commented Aug 5, 2021

SparkQA commented Aug 5, 2021

SparkQA commented Aug 5, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SparkQA commented Aug 5, 2021

SparkQA commented Aug 5, 2021

SparkQA commented Aug 5, 2021

Choose a reason for hiding this comment

SparkQA commented Aug 5, 2021

SparkQA commented Aug 5, 2021

SparkQA commented Aug 5, 2021

SparkQA commented Aug 5, 2021

SparkQA commented Aug 5, 2021

SparkQA commented Aug 6, 2021

SparkQA commented Aug 6, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SparkQA commented Aug 6, 2021

SparkQA commented Aug 6, 2021

SparkQA commented Aug 6, 2021

SparkQA commented Aug 6, 2021

SparkQA commented Aug 6, 2021

SparkQA commented Aug 6, 2021

ulysses-you commented Aug 6, 2021

SparkQA commented Aug 6, 2021

SparkQA commented Aug 6, 2021

SparkQA commented Aug 6, 2021

SparkQA commented Aug 6, 2021

SparkQA commented Aug 6, 2021

SparkQA commented Aug 6, 2021

SparkQA commented Aug 6, 2021

viirya commented Aug 6, 2021

ulysses-you commented Aug 7, 2021

SparkQA commented Aug 7, 2021

SparkQA commented Aug 7, 2021

SparkQA commented Aug 7, 2021

cloud-fan commented Aug 9, 2021

ulysses-you commented Aug 5, 2021 •

edited