You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "LuciferYang (via GitHub)" <gi...@apache.org> on 2023/02/04 17:02:52 UTC
[GitHub] [spark] LuciferYang commented on a diff in pull request #39881: [SPARK-42341][SQL][TESTS] Fix JoinSelectionHelperSuite and PlanStabilitySuite to use explicit broadcast threshold
LuciferYang commented on code in PR #39881:
URL: https://github.com/apache/spark/pull/39881#discussion_r1096561150
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/JoinSelectionHelperSuite.scala:
##########
@@ -103,15 +103,17 @@ class JoinSelectionHelperSuite extends PlanTest with JoinSelectionHelper {
}
test("getBroadcastBuildSide (hintOnly = false) return None when right has no broadcast hint") {
- val broadcastSide = getBroadcastBuildSide(
- left,
- right,
- Inner,
- JoinHint(None, hintNotToBroadcast ),
- hintOnly = false,
- SQLConf.get
- )
- assert(broadcastSide === None)
+ withSQLConf(SQLConf.AUTO_BROADCASTJOIN_THRESHOLD.key -> "10MB") {
Review Comment:
Seems `10MB` is the default configuration value of `AUTO_BROADCASTJOIN_THRESHOLD`.
If the default value is change to `11MB`, does it need to be also changed here? Should `SQLConf.AUTO_BROADCASTJOIN_THRESHOLD.defaultValueString` be used?
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/JoinSelectionHelperSuite.scala:
##########
@@ -103,15 +103,17 @@ class JoinSelectionHelperSuite extends PlanTest with JoinSelectionHelper {
}
test("getBroadcastBuildSide (hintOnly = false) return None when right has no broadcast hint") {
- val broadcastSide = getBroadcastBuildSide(
- left,
- right,
- Inner,
- JoinHint(None, hintNotToBroadcast ),
- hintOnly = false,
- SQLConf.get
- )
- assert(broadcastSide === None)
+ withSQLConf(SQLConf.AUTO_BROADCASTJOIN_THRESHOLD.key -> "10MB") {
Review Comment:
Seems `10MB` is the default value of `AUTO_BROADCASTJOIN_THRESHOLD`.
If the default value is change to `11MB`, does it need to be also changed here? Should `SQLConf.AUTO_BROADCASTJOIN_THRESHOLD.defaultValueString` be used?
--
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