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/10/01 14:04:29 UTC
[GitHub] [spark] wangyum opened a new pull request, #38069: [SPARK-40628][SQL] Do not push complex left semi/anti join condition through project
wangyum opened a new pull request, #38069:
URL: https://github.com/apache/spark/pull/38069
### What changes were proposed in this pull request?
This PR makes `PushDownLeftSemiAntiJoin` do not push complex left semi/anti join condition through project.
### Why are the changes needed?
It will impact performance because the complex expression will evaluate three times if it is SortMergeJoin. For example:
```sql
CREATE TABLE t1 (item_id BIGINT, event_type STRING, dt STRING) USING parquet PARTITIONED BY (dt);
CREATE TABLE t2 (item_id BIGINT, cal_dt DATE) using parquet;
set spark.sql.autoBroadcastJoinThreshold=-1;
SELECT t1.item_id, t1.event_type FROM t1 LEFT SEMI JOIN t2 ON t1.item_id = t2.item_id AND To_date(t1.dt, 'yyyyMMdd') = t2.cal_dt;
```
Before this PR, `cast(gettimestamp(dt#30, yyyyMMdd, TimestampType, Some(America/Los_Angeles), false) as date)` will evaluate three times:
```
AdaptiveSparkPlan isFinalPlan=false
+- Project [item_id#28L, event_type#29]
+- SortMergeJoin [item_id#28L, cast(gettimestamp(dt#30, yyyyMMdd, TimestampType, Some(America/Los_Angeles), false) as date)], [item_id#31L, cal_dt#32], LeftSemi
:- Sort [item_id#28L ASC NULLS FIRST, cast(gettimestamp(dt#30, yyyyMMdd, TimestampType, Some(America/Los_Angeles), false) as date) ASC NULLS FIRST], false, 0
: +- Exchange hashpartitioning(item_id#28L, cast(gettimestamp(dt#30, yyyyMMdd, TimestampType, Some(America/Los_Angeles), false) as date), 5), ENSURE_REQUIREMENTS, [plan_id=110]
: +- Filter isnotnull(item_id#28L)
: +- FileScan parquet spark_catalog.default.t1[item_id#28L,event_type#29,dt#30]
+- Sort [item_id#31L ASC NULLS FIRST, cal_dt#32 ASC NULLS FIRST], false, 0
+- Exchange hashpartitioning(item_id#31L, cal_dt#32, 5), ENSURE_REQUIREMENTS, [plan_id=111]
+- Filter (isnotnull(item_id#31L) AND isnotnull(cal_dt#32))
+- FileScan parquet spark_catalog.default.t2[item_id#31L,cal_dt#32]
```
The task stack trace:
```
java.base@17.0.4.1/java.text.DecimalFormat.parse(DecimalFormat.java:2149)
java.base@17.0.4.1/java.text.SimpleDateFormat.subParse(SimpleDateFormat.java:1935)
java.base@17.0.4.1/java.text.SimpleDateFormat.parse(SimpleDateFormat.java:1545)
java.base@17.0.4.1/java.text.DateFormat.parse(DateFormat.java:397)
app//org.apache.spark.sql.catalyst.util.LegacySimpleTimestampFormatter.parse(TimestampFormatter.scala:237)
org.apache.spark.sql.catalyst.expressions.GeneratedClass$SpecificOrdering.Cast_0$(Unknown Source)
org.apache.spark.sql.catalyst.expressions.GeneratedClass$SpecificOrdering.compare(Unknown Source)
app//org.apache.spark.sql.catalyst.expressions.BaseOrdering.compare(ordering.scala:29)
...
```
After this PR:
```
== Physical Plan ==
AdaptiveSparkPlan isFinalPlan=false
+- Project [item_id#28L, event_type#29]
+- SortMergeJoin [item_id#28L, new_dt#37], [item_id#31L, cal_dt#32], LeftSemi
:- Sort [item_id#28L ASC NULLS FIRST, new_dt#37 ASC NULLS FIRST], false, 0
: +- Exchange hashpartitioning(item_id#28L, new_dt#37, 5), ENSURE_REQUIREMENTS, [plan_id=110]
: +- Project [item_id#28L, event_type#29, cast(gettimestamp(dt#30, yyyyMMdd, TimestampType, Some(America/Los_Angeles), false) as date) AS new_dt#37]
: +- Filter isnotnull(item_id#28L)
: +- FileScan parquet spark_catalog.default.t1[item_id#28L,event_type#29,dt#30]
+- Sort [item_id#31L ASC NULLS FIRST, cal_dt#32 ASC NULLS FIRST], false, 0
+- Exchange hashpartitioning(item_id#31L, cal_dt#32, 5), ENSURE_REQUIREMENTS, [plan_id=111]
+- Filter (isnotnull(item_id#31L) AND isnotnull(cal_dt#32))
+- FileScan parquet spark_catalog.default.t2[item_id#31L,cal_dt#32]
```
### 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] dongjoon-hyun commented on a diff in pull request #38069: [SPARK-40628][SQL] Do not push complex left semi/anti join condition through project
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on code in PR #38069:
URL: https://github.com/apache/spark/pull/38069#discussion_r988629288
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/PushDownLeftSemiAntiJoin.scala:
##########
@@ -47,12 +47,17 @@ object PushDownLeftSemiAntiJoin extends Rule[LogicalPlan]
p.copy(child = Join(gChild, rightOp, joinType, joinCond, hint))
} else {
val aliasMap = getAliasMap(p)
- val newJoinCond = if (aliasMap.nonEmpty) {
- Option(replaceAlias(joinCond.get, aliasMap))
+ // Do not push complex join condition
Review Comment:
Thanks.
--
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] dongjoon-hyun closed pull request #38069: [SPARK-40628][SQL] Do not push complex left semi/anti join condition through project
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun closed pull request #38069: [SPARK-40628][SQL] Do not push complex left semi/anti join condition through project
URL: https://github.com/apache/spark/pull/38069
--
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] dongjoon-hyun commented on a diff in pull request #38069: [SPARK-40628][SQL] Do not push complex left semi/anti join condition through project
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on code in PR #38069:
URL: https://github.com/apache/spark/pull/38069#discussion_r985997893
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/PushDownLeftSemiAntiJoin.scala:
##########
@@ -47,12 +47,17 @@ object PushDownLeftSemiAntiJoin extends Rule[LogicalPlan]
p.copy(child = Join(gChild, rightOp, joinType, joinCond, hint))
} else {
val aliasMap = getAliasMap(p)
- val newJoinCond = if (aliasMap.nonEmpty) {
- Option(replaceAlias(joinCond.get, aliasMap))
+ // Do not push complex join condition
Review Comment:
Could you elaborate a little more explicitly about the condition instead of too general term, `complex`, @wangyum ?
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/PushDownLeftSemiAntiJoin.scala:
##########
@@ -47,12 +47,17 @@ object PushDownLeftSemiAntiJoin extends Rule[LogicalPlan]
p.copy(child = Join(gChild, rightOp, joinType, joinCond, hint))
} else {
val aliasMap = getAliasMap(p)
- val newJoinCond = if (aliasMap.nonEmpty) {
- Option(replaceAlias(joinCond.get, aliasMap))
+ // Do not push complex join condition
Review Comment:
Could you please elaborate a little more explicitly about the condition instead of too general term, `complex`, @wangyum ?
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/PushDownLeftSemiAntiJoin.scala:
##########
@@ -47,12 +47,17 @@ object PushDownLeftSemiAntiJoin extends Rule[LogicalPlan]
p.copy(child = Join(gChild, rightOp, joinType, joinCond, hint))
} else {
val aliasMap = getAliasMap(p)
- val newJoinCond = if (aliasMap.nonEmpty) {
- Option(replaceAlias(joinCond.get, aliasMap))
+ // Do not push complex join condition
Review Comment:
This PR simply bans all aliases and seems to aim to allow a plain column?
--
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] dongjoon-hyun commented on pull request #38069: [SPARK-40628][SQL] Do not push complex left semi/anti join condition through project
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on PR #38069:
URL: https://github.com/apache/spark/pull/38069#issuecomment-1271141861
Merged to master for Apache Spark 3.4.0.
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] dongjoon-hyun commented on pull request #38069: [SPARK-40628][SQL] Do not push complex left semi/anti join condition through project
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on PR #38069:
URL: https://github.com/apache/spark/pull/38069#issuecomment-1269489612
cc @cloud-fan
--
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