[SPARK-33442][SQL] Change Combine Limit to Eliminate limit using max row by ulysses-you · Pull Request #30368 · apache/spark

ulysses-you · 2020-11-13T09:20:23Z

What changes were proposed in this pull request?

Change CombineLimits name to EliminateLimits and add check if Limit child max row <= limit.

Why are the changes needed?

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

A general negative example is

select count(*) from t limit 100000;

It will be great if we can eliminate limit at Spark side.

Also, we make a benchmark for this case

runBenchmark("Sort and Limit") {
val N = 100000
val benchmark = new Benchmark("benchmark sort and limit", N)
benchmark.addCase("TakeOrderedAndProject", 3) { _ =>
spark.range(N).toDF("c").repartition(200).sort("c").take(200000)
}
benchmark.addCase("Sort And Limit", 3) { _ =>
withSQLConf("spark.sql.execution.topKSortFallbackThreshold" -> "-1") {
spark.range(N).toDF("c").repartition(200).sort("c").take(200000)
}
}
benchmark.addCase("Sort", 3) { _ =>
spark.range(N).toDF("c").repartition(200).sort("c").collect()
}
benchmark.run()
}

and the result is

Java HotSpot(TM) 64-Bit Server VM 1.8.0_191-b12 on Mac OS X 10.15.6
Intel(R) Core(TM) i5-5257U CPU @ 2.70GHz
benchmark sort and limit: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
------------------------------------------------------------------------------------------------------------------------
TakeOrderedAndProject 1833 2259 382 0.1 18327.1 1.0X
Sort And Limit 1417 1658 285 0.1 14167.5 1.3X
Sort 1324 1484 225 0.1 13238.3 1.4X

It shows that it makes sense to replace TakeOrderedAndProjectExec with Sort + Project.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Add test.

SparkQA · 2020-11-13T10:55:18Z

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

SparkQA · 2020-11-13T11:17:51Z

Test build #131054 has finished for PR 30368 at commit 67fd737.

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

SparkQA · 2020-11-13T11:22:16Z

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

SparkQA · 2020-11-13T14:29:29Z

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

SparkQA · 2020-11-13T14:55:46Z

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

SparkQA · 2020-11-13T15:02:06Z

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

SparkQA · 2020-11-13T15:34:32Z

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

SparkQA · 2020-11-13T16:41:34Z

Test build #131066 has finished for PR 30368 at commit 0a89334.

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

SparkQA · 2020-11-13T17:57:34Z

Test build #131068 has finished for PR 30368 at commit 2c6b171.

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

SparkQA · 2020-11-14T14:40:08Z

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

SparkQA · 2020-11-14T15:01:51Z

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

SparkQA · 2020-11-14T17:24:28Z

Test build #131092 has finished for PR 30368 at commit c167a0d.

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

viirya · 2020-11-14T18:20:15Z

@@ -1,5 +1,5 @@
 == Physical Plan ==
-TakeOrderedAndProject (34)
+* Sort (34)


It changes from TakeOrderedAndProject to Sort seems because Limit after Sort is removed?
It might have additional shuffle for global Sort.

This q92 sql:
SELECT sum(ws_ext_discount_amt) AS `Excess Discount Amount ` FROM web_sales, item, date_dim WHERE i_manufact_id = 350 AND i_item_sk = ws_item_sk AND d_date BETWEEN '2000-01-27' AND (cast('2000-01-27' AS DATE) + INTERVAL 90 days) AND d_date_sk = ws_sold_date_sk AND ws_ext_discount_amt > ( SELECT 1.3 * avg(ws_ext_discount_amt) FROM web_sales, date_dim WHERE ws_item_sk = i_item_sk AND d_date BETWEEN '2000-01-27' AND (cast('2000-01-27' AS DATE) + INTERVAL 90 days) AND d_date_sk = ws_sold_date_sk ) ORDER BY sum(ws_ext_discount_amt) LIMIT 100
yes, Limit after Sort is a special case, we will convert to TakeOrderedAndProject, but it seems not necessary to do both sort and limit if child maxRow == 1. Maybe we can do an another check seems like if sort.child.maxRow <= 1 then remove sort ?

Other thought, we can add this pattern
case sort @ Sort(order, true, child) if sort.maxRow < conf.topKSortFallbackThreshold => TakeOrderedAndProjectExec()
In this way, we can infer the TakeOrderedAndProjectExec from Sort which has not Limit after.
What do you think about this? @maropu @viirya @cloud-fan

Ah, @viirya , nice catch! Yea, how about simply excluding the case in the EliminateLimits rule? @ulysses-you

SparkQA · 2020-11-15T03:41:04Z

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

SparkQA · 2020-11-15T04:05:20Z

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

SparkQA · 2020-11-15T07:19:03Z

Test build #131104 has finished for PR 30368 at commit ff1ab01.

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

SparkQA · 2020-11-16T13:47:20Z

Test build #131160 has finished for PR 30368 at commit 4d45c0a.

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

SparkQA · 2020-11-16T14:00:36Z

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

SparkQA · 2020-11-16T14:31:24Z

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

SparkQA · 2020-11-17T01:07:54Z

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

SparkQA · 2020-11-17T01:37:23Z

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

SparkQA · 2020-11-17T03:33:52Z

Test build #131182 has finished for PR 30368 at commit 2f8f139.

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

…3442

SparkQA · 2020-11-17T06:36:41Z

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

SparkQA · 2020-11-17T07:06:06Z

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

SparkQA · 2020-11-17T07:12:21Z

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

SparkQA · 2020-11-17T07:36:43Z

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

SparkQA · 2020-11-17T08:05:02Z

Test build #131199 has finished for PR 30368 at commit 45ab8b5.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2020-11-17T08:05:02Z

Test build #131197 has finished for PR 30368 at commit 9fb4039.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

cloud-fan

LGTM

SparkQA · 2020-11-17T15:30:18Z

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

SparkQA · 2020-11-17T15:53:41Z

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

SparkQA · 2020-11-17T19:15:01Z

Test build #131225 has finished for PR 30368 at commit 2f55ec0.

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

cloud-fan · 2020-11-18T14:05:57Z

Hi @ulysses-you , can you put the conclusion of #30368 (comment) in PR description, to mention that we may end up replacing TakeOrderedAndProjectExec with Sort + Project, which is faster?

ulysses-you · 2020-11-19T00:08:05Z

@cloud-fan updated the description.

cloud-fan · 2020-11-19T13:31:02Z

thanks, merging to master!

ulysses-you · 2020-11-20T00:06:06Z

thanks for merging!

init
67fd737

ulysses-you commented Nov 13, 2020
View reviewed changes

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

github-actionsBot added the SQL label Nov 13, 2020

maropu reviewed Nov 13, 2020
View reviewed changes

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

ulysses-you added 3 commits November 13, 2020 20:29

fix LimitPushdownSuite
46aea2e

keep global limit
0a89334

use long
2c6b171

fix StreamSuite
e570b8e

viirya reviewed Nov 14, 2020
View reviewed changes

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

ulysses-you added 2 commits November 14, 2020 21:48

golden file
314cb1e

remove eliminate local limit
c167a0d

github-actionsBot added the STRUCTURED STREAMING label Nov 14, 2020

viirya reviewed Nov 14, 2020
View reviewed changes

golden file
ff1ab01

skip sort after limit
6576929

cloud-fan reviewed Nov 16, 2020
View reviewed changes

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

fix
2f8f139

ulysses-you added 3 commits November 17, 2020 13:47

Merge branch 'master' ofhttps://github.com/apache/sparkinto SPARK-3…
2882a36
…3442

fix test
9fb4039

golden file
45ab8b5

cloud-fan reviewed Nov 17, 2020
View reviewed changes

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

cloud-fan approved these changes Nov 17, 2020
View reviewed changes

add comment
2f55ec0

cloud-fan closed this in 21b1350Nov 19, 2020

ulysses-you deleted the SPARK-33442 branch March 3, 2021 04:30

Uh oh!

Conversation

ulysses-you commented Nov 13, 2020• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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?

Uh oh!

Uh oh!

SparkQA commented Nov 13, 2020

Uh oh!

SparkQA commented Nov 13, 2020

Uh oh!

SparkQA commented Nov 13, 2020

Uh oh!

Uh oh!

Uh oh!

SparkQA commented Nov 13, 2020

Uh oh!

SparkQA commented Nov 13, 2020

Uh oh!

SparkQA commented Nov 13, 2020

Uh oh!

SparkQA commented Nov 13, 2020

Uh oh!

SparkQA commented Nov 13, 2020

Uh oh!

SparkQA commented Nov 13, 2020

Uh oh!

Uh oh!

SparkQA commented Nov 14, 2020

Uh oh!

SparkQA commented Nov 14, 2020

Uh oh!

SparkQA commented Nov 14, 2020

Uh oh!

viiryaNov 14, 2020

Choose a reason for hiding this comment

Uh oh!

ulysses-youNov 15, 2020

Choose a reason for hiding this comment

Uh oh!

ulysses-youNov 15, 2020• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maropuNov 15, 2020• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ulysses-youNov 16, 2020

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Nov 15, 2020

Uh oh!

SparkQA commented Nov 15, 2020

Uh oh!

SparkQA commented Nov 15, 2020

Uh oh!

SparkQA commented Nov 16, 2020

Uh oh!

Uh oh!

SparkQA commented Nov 16, 2020

Uh oh!

SparkQA commented Nov 16, 2020

Uh oh!

SparkQA commented Nov 17, 2020

Uh oh!

SparkQA commented Nov 17, 2020

Uh oh!

SparkQA commented Nov 17, 2020

Uh oh!

SparkQA commented Nov 17, 2020

Uh oh!

SparkQA commented Nov 17, 2020

Uh oh!

SparkQA commented Nov 17, 2020

Uh oh!

SparkQA commented Nov 17, 2020

ulysses-you commented Nov 13, 2020•
edited
Loading

ulysses-youNov 15, 2020•
edited
Loading

maropuNov 15, 2020•
edited
Loading