You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2022/06/19 02:50:31 UTC
[GitHub] [spark] wangyum opened a new pull request, #36909: [SPARK-39511][SQL] Push down limit 1 for right side of left semi/anti join if join condition is empty
wangyum opened a new pull request, #36909:
URL: https://github.com/apache/spark/pull/36909
### What changes were proposed in this pull request?
This PR enhances https://github.com/apache/spark/pull/35216 to support more cases. For example:
```sql
CREATE TABLE t1(id int) using parquet;
CREATE TABLE t2(id int, type string) using parquet;
CREATE TEMP VIEW v1 AS SELECT id, 't' AS type FROM t1;
EXPLAIN EXTENDED SELECT * FROM v1 WHERE type IN (SELECT type FROM t2);
```
Before this PR:
```
=== Result of Batch RewriteSubquery ===
Project [id#241, t AS type#246] Project [id#241, t AS type#246]
!+- Filter t IN (list#243 []) +- Join LeftSemi, (t = type#248)
! : +- Project [type#248] :- Relation default.t1[id#241] parquet
! : +- Relation default.t2[id#247,type#248] parquet +- Project [type#248]
! +- Relation default.t1[id#241] parquet +- Relation default.t2[id#247,type#248] parquet
```
After this PR:
```
=== Result of Batch RewriteSubquery ===
Project [id#241, t AS type#246] Project [id#241, t AS type#246]
!+- Filter t IN (list#243 []) +- Join LeftSemi
! : +- Project [type#248] :- Relation default.t1[id#241] parquet
! : +- Relation default.t2[id#247,type#248] parquet +- GlobalLimit 1
! +- Relation default.t1[id#241] parquet +- LocalLimit 1
! +- Project
! +- Filter (t = type#248)
! +- Relation default.t2[id#247,type#248] parquet
```
### Why are the changes needed?
Improve query performance.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Unit test.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum commented on pull request #36909: [SPARK-39511][SQL] Enhance push down local limit 1 for right side of left semi/anti join if join condition is empty
Posted by GitBox <gi...@apache.org>.
wangyum commented on PR #36909:
URL: https://github.com/apache/spark/pull/36909#issuecomment-1165150355
Merged to master.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum commented on a diff in pull request #36909: [SPARK-39511][SQL] Push down limit 1 for right side of left semi/anti join if join condition is empty
Posted by GitBox <gi...@apache.org>.
wangyum commented on code in PR #36909:
URL: https://github.com/apache/spark/pull/36909#discussion_r903698220
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala:
##########
@@ -753,6 +753,9 @@ object LimitPushDown extends Rule[LogicalPlan] {
// Merge offset value and limit value into LocalLimit and pushes down LocalLimit through Offset.
case LocalLimit(le, Offset(oe, grandChild)) =>
Offset(oe, LocalLimit(Add(le, oe), grandChild))
+ // Push down local limit 1 if join type is LeftSemiOrAnti and join condition is empty.
+ case j @ Join(_, right, LeftSemiOrAnti(_), None, _) if !right.maxRows.exists(_ <= 1) =>
Review Comment:
when right.maxRows > 0. It can not guarantee the result must not be empty.
when right.maxRows = 0. It already supported through `OptimizeLimitZero` and `PropagateEmptyRelationBase`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum commented on a diff in pull request #36909: [SPARK-39511][SQL] Push down limit 1 for right side of left semi/anti join if join condition is empty
Posted by GitBox <gi...@apache.org>.
wangyum commented on code in PR #36909:
URL: https://github.com/apache/spark/pull/36909#discussion_r904407888
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala:
##########
@@ -753,6 +753,9 @@ object LimitPushDown extends Rule[LogicalPlan] {
// Merge offset value and limit value into LocalLimit and pushes down LocalLimit through Offset.
case LocalLimit(le, Offset(oe, grandChild)) =>
Offset(oe, LocalLimit(Add(le, oe), grandChild))
+ // Push down local limit 1 if join type is LeftSemiOrAnti and join condition is empty.
+ case j @ Join(_, right, LeftSemiOrAnti(_), None, _) if !right.maxRows.exists(_ <= 1) =>
Review Comment:
```scala
import org.apache.spark.sql.catalyst.plans.logical.Join
val df = spark.sql("select * from range(5) t1 left semi join (select * from range(10) where id < 0) t2")
println(df.queryExecution.optimizedPlan.asInstanceOf[Join].right.maxRows)
```
The `right.maxRows` is `Some(10)` but actually rows is 0.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sigmod commented on a diff in pull request #36909: [SPARK-39511][SQL] Push down limit 1 for right side of left semi/anti join if join condition is empty
Posted by GitBox <gi...@apache.org>.
sigmod commented on code in PR #36909:
URL: https://github.com/apache/spark/pull/36909#discussion_r903350188
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala:
##########
@@ -753,6 +753,9 @@ object LimitPushDown extends Rule[LogicalPlan] {
// Merge offset value and limit value into LocalLimit and pushes down LocalLimit through Offset.
case LocalLimit(le, Offset(oe, grandChild)) =>
Offset(oe, LocalLimit(Add(le, oe), grandChild))
+ // Push down local limit 1 if join type is LeftSemiOrAnti and join condition is empty.
+ case j @ Join(_, right, LeftSemiOrAnti(_), None, _) if !right.maxRows.exists(_ <= 1) =>
Review Comment:
In this case,
when right.maxRows > 0
- for SEMI join, can we eliminate the right side and the join entirely?
- for ANTI, can we rewrite it to an empty Relation?
when when right.maxRows = 0
- for SEMI, can we rewrite it to an empty Relation?
- for ANTI join, can we eliminate the right side and the join entirely?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a diff in pull request #36909: [SPARK-39511][SQL] Push down limit 1 for right side of left semi/anti join if join condition is empty
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on code in PR #36909:
URL: https://github.com/apache/spark/pull/36909#discussion_r902664764
##########
sql/core/src/test/scala/org/apache/spark/sql/JoinSuite.scala:
##########
@@ -1324,7 +1325,17 @@ class JoinSuite extends QueryTest with SharedSparkSession with AdaptiveSparkPlan
val plan = sql(getAggQuery(selectExpr, joinType)).queryExecution.executedPlan
assert(collect(plan) { case _: BroadcastNestedLoopJoinExec => true }.size === 1)
// No extra shuffle before aggregation
- assert(collect(plan) { case _: ShuffleExchangeExec => true }.size === 0)
+ if (joinType.equals("LEFT SEMI") || joinType.equals("LEFT ANTI")) {
+ assert(collect(plan) {
+ case s: ShuffleExchangeExec if s.outputPartitioning != SinglePartition => true
+ }.size === 0)
+ // Shuffle Exchange between LocalLimit and GlobalLimit
Review Comment:
does it mean it's safer to only add an extra local limit. not global limit?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a diff in pull request #36909: [SPARK-39511][SQL] Push down limit 1 for right side of left semi/anti join if join condition is empty
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on code in PR #36909:
URL: https://github.com/apache/spark/pull/36909#discussion_r902227110
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala:
##########
@@ -753,6 +753,9 @@ object LimitPushDown extends Rule[LogicalPlan] {
// Merge offset value and limit value into LocalLimit and pushes down LocalLimit through Offset.
case LocalLimit(le, Offset(oe, grandChild)) =>
Offset(oe, LocalLimit(Add(le, oe), grandChild))
+ // Push down limit 1 if join type is LeftSemiOrAnti and join condition is empty
+ case j @ Join(_, right, LeftSemiOrAnti(_), None, _) if !right.maxRows.exists(_ <= 1) =>
Review Comment:
what's the reason to move it from `pushLocalLimitThroughJoin` to here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a diff in pull request #36909: [SPARK-39511][SQL] Push down limit 1 for right side of left semi/anti join if join condition is empty
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on code in PR #36909:
URL: https://github.com/apache/spark/pull/36909#discussion_r902242101
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala:
##########
@@ -753,6 +753,9 @@ object LimitPushDown extends Rule[LogicalPlan] {
// Merge offset value and limit value into LocalLimit and pushes down LocalLimit through Offset.
case LocalLimit(le, Offset(oe, grandChild)) =>
Offset(oe, LocalLimit(Add(le, oe), grandChild))
+ // Push down limit 1 if join type is LeftSemiOrAnti and join condition is empty
+ case j @ Join(_, right, LeftSemiOrAnti(_), None, _) if !right.maxRows.exists(_ <= 1) =>
Review Comment:
we should write this down explicitly in the PR description...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum closed pull request #36909: [SPARK-39511][SQL] Enhance push down local limit 1 for right side of left semi/anti join if join condition is empty
Posted by GitBox <gi...@apache.org>.
wangyum closed pull request #36909: [SPARK-39511][SQL] Enhance push down local limit 1 for right side of left semi/anti join if join condition is empty
URL: https://github.com/apache/spark/pull/36909
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum commented on a diff in pull request #36909: [SPARK-39511][SQL] Push down limit 1 for right side of left semi/anti join if join condition is empty
Posted by GitBox <gi...@apache.org>.
wangyum commented on code in PR #36909:
URL: https://github.com/apache/spark/pull/36909#discussion_r902231084
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala:
##########
@@ -753,6 +753,9 @@ object LimitPushDown extends Rule[LogicalPlan] {
// Merge offset value and limit value into LocalLimit and pushes down LocalLimit through Offset.
case LocalLimit(le, Offset(oe, grandChild)) =>
Offset(oe, LocalLimit(Add(le, oe), grandChild))
+ // Push down limit 1 if join type is LeftSemiOrAnti and join condition is empty
+ case j @ Join(_, right, LeftSemiOrAnti(_), None, _) if !right.maxRows.exists(_ <= 1) =>
Review Comment:
To support queries do not contain limit. For example:
```sql
SELECT * FROM v1 WHERE type IN (SELECT type FROM t2)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sigmod commented on a diff in pull request #36909: [SPARK-39511][SQL] Push down limit 1 for right side of left semi/anti join if join condition is empty
Posted by GitBox <gi...@apache.org>.
sigmod commented on code in PR #36909:
URL: https://github.com/apache/spark/pull/36909#discussion_r904429256
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala:
##########
@@ -753,6 +753,9 @@ object LimitPushDown extends Rule[LogicalPlan] {
// Merge offset value and limit value into LocalLimit and pushes down LocalLimit through Offset.
case LocalLimit(le, Offset(oe, grandChild)) =>
Offset(oe, LocalLimit(Add(le, oe), grandChild))
+ // Push down local limit 1 if join type is LeftSemiOrAnti and join condition is empty.
+ case j @ Join(_, right, LeftSemiOrAnti(_), None, _) if !right.maxRows.exists(_ <= 1) =>
Review Comment:
Gotcha, make sense. Thank you, @wangyum !
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on pull request #36909: [SPARK-39511][SQL] Push down limit 1 for right side of left semi/anti join if join condition is empty
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on PR #36909:
URL: https://github.com/apache/spark/pull/36909#issuecomment-1161354675
Please clarify what enhancements this PR brings, instead of just throwing out an example.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum commented on pull request #36909: [SPARK-39511][SQL] Push down limit 1 for right side of left semi/anti join if join condition is empty
Posted by GitBox <gi...@apache.org>.
wangyum commented on PR #36909:
URL: https://github.com/apache/spark/pull/36909#issuecomment-1160143206
cc @cloud-fan @c21 @sigmod
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum commented on a diff in pull request #36909: [SPARK-39511][SQL] Push down limit 1 for right side of left semi/anti join if join condition is empty
Posted by GitBox <gi...@apache.org>.
wangyum commented on code in PR #36909:
URL: https://github.com/apache/spark/pull/36909#discussion_r902442252
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala:
##########
@@ -753,6 +753,9 @@ object LimitPushDown extends Rule[LogicalPlan] {
// Merge offset value and limit value into LocalLimit and pushes down LocalLimit through Offset.
case LocalLimit(le, Offset(oe, grandChild)) =>
Offset(oe, LocalLimit(Add(le, oe), grandChild))
+ // Push down limit 1 if join type is LeftSemiOrAnti and join condition is empty
+ case j @ Join(_, right, LeftSemiOrAnti(_), None, _) if !right.maxRows.exists(_ <= 1) =>
Review Comment:
OK. updated the pr description.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a diff in pull request #36909: [SPARK-39511][SQL] Push down limit 1 for right side of left semi/anti join if join condition is empty
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on code in PR #36909:
URL: https://github.com/apache/spark/pull/36909#discussion_r902666585
##########
sql/core/src/test/scala/org/apache/spark/sql/SubquerySuite.scala:
##########
@@ -2204,4 +2204,20 @@ class SubquerySuite extends QueryTest with SharedSparkSession with AdaptiveSpark
|""".stripMargin),
Row("2022-06-01"))
}
+
+ test("SPARK-39511: Push limit 1 to right side if join type is Left Semi/Anti") {
Review Comment:
this enhancement should also be mentioned in the PR description. Now we apply this optimization to left/semi join generated from subquereis as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sigmod commented on a diff in pull request #36909: [SPARK-39511][SQL] Push down limit 1 for right side of left semi/anti join if join condition is empty
Posted by GitBox <gi...@apache.org>.
sigmod commented on code in PR #36909:
URL: https://github.com/apache/spark/pull/36909#discussion_r904002034
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala:
##########
@@ -753,6 +753,9 @@ object LimitPushDown extends Rule[LogicalPlan] {
// Merge offset value and limit value into LocalLimit and pushes down LocalLimit through Offset.
case LocalLimit(le, Offset(oe, grandChild)) =>
Offset(oe, LocalLimit(Add(le, oe), grandChild))
+ // Push down local limit 1 if join type is LeftSemiOrAnti and join condition is empty.
+ case j @ Join(_, right, LeftSemiOrAnti(_), None, _) if !right.maxRows.exists(_ <= 1) =>
Review Comment:
> when right.maxRows > 0. It can not guarantee the result must not be empty.
- For left semi with empty condition, it doesn't matter? Non empty right means all left rows are join output rows?
- For left anti with empty condition, it doesn't matter neither? Non empty right means all left rows are killed and join doesn't output any row?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sigmod commented on a diff in pull request #36909: [SPARK-39511][SQL] Push down limit 1 for right side of left semi/anti join if join condition is empty
Posted by GitBox <gi...@apache.org>.
sigmod commented on code in PR #36909:
URL: https://github.com/apache/spark/pull/36909#discussion_r903350188
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala:
##########
@@ -753,6 +753,9 @@ object LimitPushDown extends Rule[LogicalPlan] {
// Merge offset value and limit value into LocalLimit and pushes down LocalLimit through Offset.
case LocalLimit(le, Offset(oe, grandChild)) =>
Offset(oe, LocalLimit(Add(le, oe), grandChild))
+ // Push down local limit 1 if join type is LeftSemiOrAnti and join condition is empty.
+ case j @ Join(_, right, LeftSemiOrAnti(_), None, _) if !right.maxRows.exists(_ <= 1) =>
Review Comment:
In this case,
when right.maxRows > 0
- for SEMI join, can we eliminate the right side and the join entirely?
- for ANTI, can we rewrite it to an empty Relation?
when right.maxRows = 0
- for SEMI, can we rewrite it to an empty Relation?
- for ANTI join, can we eliminate the right side and the join entirely?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum commented on a diff in pull request #36909: [SPARK-39511][SQL] Push down limit 1 for right side of left semi/anti join if join condition is empty
Posted by GitBox <gi...@apache.org>.
wangyum commented on code in PR #36909:
URL: https://github.com/apache/spark/pull/36909#discussion_r903698220
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala:
##########
@@ -753,6 +753,9 @@ object LimitPushDown extends Rule[LogicalPlan] {
// Merge offset value and limit value into LocalLimit and pushes down LocalLimit through Offset.
case LocalLimit(le, Offset(oe, grandChild)) =>
Offset(oe, LocalLimit(Add(le, oe), grandChild))
+ // Push down local limit 1 if join type is LeftSemiOrAnti and join condition is empty.
+ case j @ Join(_, right, LeftSemiOrAnti(_), None, _) if !right.maxRows.exists(_ <= 1) =>
Review Comment:
when right.maxRows > 0. It can not guarantee the result must not be empty.
when right.maxRows = 0. We can enhance `PropagateEmptyRelation` to support this case.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum commented on a diff in pull request #36909: [SPARK-39511][SQL] Push down limit 1 for right side of left semi/anti join if join condition is empty
Posted by GitBox <gi...@apache.org>.
wangyum commented on code in PR #36909:
URL: https://github.com/apache/spark/pull/36909#discussion_r903272275
##########
sql/core/src/test/scala/org/apache/spark/sql/JoinSuite.scala:
##########
@@ -1324,7 +1325,17 @@ class JoinSuite extends QueryTest with SharedSparkSession with AdaptiveSparkPlan
val plan = sql(getAggQuery(selectExpr, joinType)).queryExecution.executedPlan
assert(collect(plan) { case _: BroadcastNestedLoopJoinExec => true }.size === 1)
// No extra shuffle before aggregation
- assert(collect(plan) { case _: ShuffleExchangeExec => true }.size === 0)
+ if (joinType.equals("LEFT SEMI") || joinType.equals("LEFT ANTI")) {
+ assert(collect(plan) {
+ case s: ShuffleExchangeExec if s.outputPartitioning != SinglePartition => true
+ }.size === 0)
+ // Shuffle Exchange between LocalLimit and GlobalLimit
Review Comment:
OK. local limit also benefit query.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sigmod commented on a diff in pull request #36909: [SPARK-39511][SQL] Push down limit 1 for right side of left semi/anti join if join condition is empty
Posted by GitBox <gi...@apache.org>.
sigmod commented on code in PR #36909:
URL: https://github.com/apache/spark/pull/36909#discussion_r904002034
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala:
##########
@@ -753,6 +753,9 @@ object LimitPushDown extends Rule[LogicalPlan] {
// Merge offset value and limit value into LocalLimit and pushes down LocalLimit through Offset.
case LocalLimit(le, Offset(oe, grandChild)) =>
Offset(oe, LocalLimit(Add(le, oe), grandChild))
+ // Push down local limit 1 if join type is LeftSemiOrAnti and join condition is empty.
+ case j @ Join(_, right, LeftSemiOrAnti(_), None, _) if !right.maxRows.exists(_ <= 1) =>
Review Comment:
> when right.maxRows > 0. It can not guarantee the result must not be empty.
For left semi/anti with empty condition, it doesn't matter? Non empty right means all left rows are join output rows?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sigmod commented on a diff in pull request #36909: [SPARK-39511][SQL] Push down limit 1 for right side of left semi/anti join if join condition is empty
Posted by GitBox <gi...@apache.org>.
sigmod commented on code in PR #36909:
URL: https://github.com/apache/spark/pull/36909#discussion_r904002034
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala:
##########
@@ -753,6 +753,9 @@ object LimitPushDown extends Rule[LogicalPlan] {
// Merge offset value and limit value into LocalLimit and pushes down LocalLimit through Offset.
case LocalLimit(le, Offset(oe, grandChild)) =>
Offset(oe, LocalLimit(Add(le, oe), grandChild))
+ // Push down local limit 1 if join type is LeftSemiOrAnti and join condition is empty.
+ case j @ Join(_, right, LeftSemiOrAnti(_), None, _) if !right.maxRows.exists(_ <= 1) =>
Review Comment:
> when right.maxRows > 0. It can not guarantee the result must not be empty.
- For left semi with empty condition, it doesn't matter? Non empty `right` means all `left` rows are join output rows?
- For left anti with empty condition, it doesn't matter neither? Non empty `right` means all `left` rows are killed and join doesn't output any row?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum commented on a diff in pull request #36909: [SPARK-39511][SQL] Push down limit 1 for right side of left semi/anti join if join condition is empty
Posted by GitBox <gi...@apache.org>.
wangyum commented on code in PR #36909:
URL: https://github.com/apache/spark/pull/36909#discussion_r902232283
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala:
##########
@@ -753,6 +753,9 @@ object LimitPushDown extends Rule[LogicalPlan] {
// Merge offset value and limit value into LocalLimit and pushes down LocalLimit through Offset.
case LocalLimit(le, Offset(oe, grandChild)) =>
Offset(oe, LocalLimit(Add(le, oe), grandChild))
+ // Push down limit 1 if join type is LeftSemiOrAnti and join condition is empty
+ case j @ Join(_, right, LeftSemiOrAnti(_), None, _) if !right.maxRows.exists(_ <= 1) =>
Review Comment:
`pushLocalLimitThroughJoin` only supports the following case:
```sql
SELECT * FROM v1 WHERE type IN (SELECT type FROM t2) LIMIT 10;
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum commented on a diff in pull request #36909: [SPARK-39511][SQL] Push down limit 1 for right side of left semi/anti join if join condition is empty
Posted by GitBox <gi...@apache.org>.
wangyum commented on code in PR #36909:
URL: https://github.com/apache/spark/pull/36909#discussion_r903272352
##########
sql/core/src/test/scala/org/apache/spark/sql/SubquerySuite.scala:
##########
@@ -2204,4 +2204,20 @@ class SubquerySuite extends QueryTest with SharedSparkSession with AdaptiveSpark
|""".stripMargin),
Row("2022-06-01"))
}
+
+ test("SPARK-39511: Push limit 1 to right side if join type is Left Semi/Anti") {
Review Comment:
Updated PR description.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org