You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "beliefer (via GitHub)" <gi...@apache.org> on 2025/03/30 09:46:26 UTC
[PR] [SPARK-51663][SQL] Short circuit && operation for JoinSelectionHelper [spark]
beliefer opened a new pull request, #50455:
URL: https://github.com/apache/spark/pull/50455
### What changes were proposed in this pull request?
This PR aims to improve by short circuit && operation for `JoinSelectionHelper`.
### Why are the changes needed?
Short circuit && operation for JoinSelectionHelper and reduce a little of overhead.
### Does this PR introduce _any_ user-facing change?
'No'.
### How was this patch tested?
GA.
### Was this patch authored or co-authored using generative AI tooling?
'No'.
--
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
Re: [PR] [SPARK-51663][SQL] Short circuit && operation for JoinSelectionHelper [spark]
Posted by "beliefer (via GitHub)" <gi...@apache.org>.
beliefer commented on code in PR #50455:
URL: https://github.com/apache/spark/pull/50455#discussion_r2050068719
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/joins.scala:
##########
@@ -304,8 +304,8 @@ trait JoinSelectionHelper extends Logging {
canBroadcastBySize(join.right, conf) && !hintToNotBroadcastRight(join.hint)
}
getBuildSide(
- canBuildBroadcastLeft(join.joinType) && buildLeft,
Review Comment:
Okay, let me try it.
--
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
Re: [PR] [SPARK-51663][SQL] Short circuit && operation for JoinSelectionHelper [spark]
Posted by "beliefer (via GitHub)" <gi...@apache.org>.
beliefer commented on PR #50455:
URL: https://github.com/apache/spark/pull/50455#issuecomment-2765041455
ping @dongjoon-hyun @HyukjinKwon @LuciferYang cc @MaxGekk
--
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
Re: [PR] [SPARK-51663][SQL] Short circuit && operation for JoinSelectionHelper [spark]
Posted by "beliefer (via GitHub)" <gi...@apache.org>.
beliefer commented on PR #50455:
URL: https://github.com/apache/spark/pull/50455#issuecomment-2765663202
@MaxGekk @LuciferYang Thank you!
Merged into 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
Re: [PR] [SPARK-51663][SQL] Short circuit && operation for JoinSelectionHelper [spark]
Posted by "beliefer (via GitHub)" <gi...@apache.org>.
beliefer closed pull request #50455: [SPARK-51663][SQL] Short circuit && operation for JoinSelectionHelper
URL: https://github.com/apache/spark/pull/50455
--
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
Re: [PR] [SPARK-51663][SQL] Short circuit && operation for JoinSelectionHelper [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #50455:
URL: https://github.com/apache/spark/pull/50455#discussion_r2049935287
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/joins.scala:
##########
@@ -304,8 +304,8 @@ trait JoinSelectionHelper extends Logging {
canBroadcastBySize(join.right, conf) && !hintToNotBroadcastRight(join.hint)
}
getBuildSide(
- canBuildBroadcastLeft(join.joinType) && buildLeft,
Review Comment:
This PR reminds me that `buildLeft` is not actually cheap to calculate, likely more expensive than `canBuildBroadcastLeft` invocation.
Shall we turn `buildLeft/buildRight` into lazy val and revert the order 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