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 2021/04/27 02:59:56 UTC
[GitHub] [spark] ulysses-you opened a new pull request #32355: [SPARK-35221][SQL] Add join hint build side check
ulysses-you opened a new pull request #32355:
URL: https://github.com/apache/spark/pull/32355
<!--
Thanks for sending a pull request! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://spark.apache.org/contributing.html
2. Ensure you have added or run the appropriate tests for your PR: https://spark.apache.org/developer-tools.html
3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][SPARK-XXXX] Your PR title ...'.
4. Be sure to keep the PR description updated to reflect all changes.
5. Please write your PR title to summarize what this PR proposes.
6. If possible, provide a concise example to reproduce the issue for a faster review.
7. If you want to add a new configuration, please read the guideline first for naming configurations in
'core/src/main/scala/org/apache/spark/internal/config/ConfigEntry.scala'.
8. If you want to add or modify an error message, please read the guideline first:
https://spark.apache.org/error-message-guidelines.html
-->
### What changes were proposed in this pull request?
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
Print warning msg if join hint is not supported for the specified build side.
### Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
Currently we support specify the join implementation with hint, but Spark did not promised it.
For example broadcast outer join and hash outer join we need to check if its build side was supported. And at least we should print some warning log instead of changing to other join implementation silently.
### Does this PR introduce _any_ user-facing change?
<!--
Note that it means *any* user-facing change including all aspects such as the documentation fix.
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change compared to the released Spark versions or within the unreleased branches such as master.
If no, write 'No'.
-->
Yes, warn logging might be printed.
### How was this patch tested?
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
-->
Add new 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.
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] ulysses-you commented on a change in pull request #32355: [SPARK-35221][SQL] Add the check of supported join hints
Posted by GitBox <gi...@apache.org>.
ulysses-you commented on a change in pull request #32355:
URL: https://github.com/apache/spark/pull/32355#discussion_r683503861
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/JoinHintSuite.scala
##########
@@ -597,4 +597,82 @@ class JoinHintSuite extends PlanTest with SharedSparkSession with AdaptiveSparkP
assert(df9.collect().size == df10.collect().size)
}
}
+
+ test("SPARK-35221: Add join hint build side check") {
+ withSQLConf(SQLConf.AUTO_BROADCASTJOIN_THRESHOLD.key -> "-1",
+ SQLConf.PREFER_SORTMERGEJOIN.key -> "true") {
+ Seq("left_outer", "left_semi", "left_anti").foreach { joinType =>
+ val hintAppender = new LogAppender(s"join hint build side check for $joinType")
+ withLogAppender(hintAppender, level = Some(Level.WARN)) {
+ assertShuffleMergeJoin(
+ df1.hint("BROADCAST").join(df2, $"a1" === $"b1", joinType))
+ assertBroadcastHashJoin(
+ df1.join(df2.hint("BROADCAST"), $"a1" === $"b1", joinType), BuildRight)
+
+ assertShuffleMergeJoin(
+ df1.hint("SHUFFLE_HASH").join(df2, $"a1" === $"b1", joinType))
+ assertShuffleHashJoin(
+ df1.join(df2.hint("SHUFFLE_HASH"), $"a1" === $"b1", joinType), BuildRight)
+ }
+
+ val logs = hintAppender.loggingEvents.map(_.getRenderedMessage)
+ .filter(_.contains("is not supported in the query:"))
+ assert(logs.nonEmpty)
Review comment:
It should be. According to the https://github.com/apache/spark/pull/32355#discussion_r680435570 and to make it clear. Now I split the related test into parties: positive and negative. I think it would be less flaky FYI @HyukjinKwon .
--
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 change in pull request #32355: [SPARK-35221][SQL] Add the check of supported join hints
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32355:
URL: https://github.com/apache/spark/pull/32355#discussion_r683388652
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/JoinHintSuite.scala
##########
@@ -597,4 +597,82 @@ class JoinHintSuite extends PlanTest with SharedSparkSession with AdaptiveSparkP
assert(df9.collect().size == df10.collect().size)
}
}
+
+ test("SPARK-35221: Add join hint build side check") {
+ withSQLConf(SQLConf.AUTO_BROADCASTJOIN_THRESHOLD.key -> "-1",
+ SQLConf.PREFER_SORTMERGEJOIN.key -> "true") {
+ Seq("left_outer", "left_semi", "left_anti").foreach { joinType =>
+ val hintAppender = new LogAppender(s"join hint build side check for $joinType")
+ withLogAppender(hintAppender, level = Some(Level.WARN)) {
+ assertShuffleMergeJoin(
+ df1.hint("BROADCAST").join(df2, $"a1" === $"b1", joinType))
+ assertBroadcastHashJoin(
+ df1.join(df2.hint("BROADCAST"), $"a1" === $"b1", joinType), BuildRight)
+
+ assertShuffleMergeJoin(
+ df1.hint("SHUFFLE_HASH").join(df2, $"a1" === $"b1", joinType))
+ assertShuffleHashJoin(
+ df1.join(df2.hint("SHUFFLE_HASH"), $"a1" === $"b1", joinType), BuildRight)
+ }
+
+ val logs = hintAppender.loggingEvents.map(_.getRenderedMessage)
+ .filter(_.contains("is not supported in the query:"))
+ assert(logs.nonEmpty)
Review comment:
shall we assert `logs.length == 2`?
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/JoinHintSuite.scala
##########
@@ -597,4 +597,82 @@ class JoinHintSuite extends PlanTest with SharedSparkSession with AdaptiveSparkP
assert(df9.collect().size == df10.collect().size)
}
}
+
+ test("SPARK-35221: Add join hint build side check") {
+ withSQLConf(SQLConf.AUTO_BROADCASTJOIN_THRESHOLD.key -> "-1",
+ SQLConf.PREFER_SORTMERGEJOIN.key -> "true") {
+ Seq("left_outer", "left_semi", "left_anti").foreach { joinType =>
+ val hintAppender = new LogAppender(s"join hint build side check for $joinType")
+ withLogAppender(hintAppender, level = Some(Level.WARN)) {
+ assertShuffleMergeJoin(
+ df1.hint("BROADCAST").join(df2, $"a1" === $"b1", joinType))
+ assertBroadcastHashJoin(
+ df1.join(df2.hint("BROADCAST"), $"a1" === $"b1", joinType), BuildRight)
+
+ assertShuffleMergeJoin(
+ df1.hint("SHUFFLE_HASH").join(df2, $"a1" === $"b1", joinType))
+ assertShuffleHashJoin(
+ df1.join(df2.hint("SHUFFLE_HASH"), $"a1" === $"b1", joinType), BuildRight)
+ }
+
+ val logs = hintAppender.loggingEvents.map(_.getRenderedMessage)
+ .filter(_.contains("is not supported in the query:"))
+ assert(logs.nonEmpty)
+ logs.forall(_.contains(s"build left for ${joinType.split("_").mkString(" ")} join."))
+ }
+
+ Seq("right_outer").foreach { joinType =>
+ val hintAppender = new LogAppender(s"join hint build side check for $joinType")
+ withLogAppender(hintAppender, level = Some(Level.WARN)) {
+ assertBroadcastHashJoin(
+ df1.hint("BROADCAST").join(df2, $"a1" === $"b1", joinType), BuildLeft)
+ assertShuffleMergeJoin(
+ df1.join(df2.hint("BROADCAST"), $"a1" === $"b1", joinType))
+
+ assertShuffleHashJoin(
+ df1.hint("SHUFFLE_HASH").join(df2, $"a1" === $"b1", joinType), BuildLeft)
+ assertShuffleMergeJoin(
+ df1.join(df2.hint("SHUFFLE_HASH"), $"a1" === $"b1", joinType))
+ }
+ val logs = hintAppender.loggingEvents.map(_.getRenderedMessage)
+ .filter(_.contains("is not supported in the query:"))
+ assert(logs.nonEmpty)
Review comment:
ditto
--
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 change in pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32355:
URL: https://github.com/apache/spark/pull/32355#discussion_r620895895
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/HintErrorLogger.scala
##########
@@ -42,6 +45,17 @@ object HintErrorLogger extends HintErrorHandler with Logging {
logWarning(s"A join hint $hint is specified but it is not part of a join relation.")
}
+ override def joinBuildSideNotSupported(joinType: JoinType, joinHint: JoinHint): Unit = {
Review comment:
cc @maryannxue
--
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.
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] AmplabJenkins removed a comment on pull request #32355: [SPARK-35221][SQL] Add the check of supported join hints
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-893612710
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/46612/
--
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] SparkQA commented on pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-891811386
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46506/
--
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] HyukjinKwon commented on a change in pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #32355:
URL: https://github.com/apache/spark/pull/32355#discussion_r680435570
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/JoinHintSuite.scala
##########
@@ -597,4 +597,68 @@ class JoinHintSuite extends PlanTest with SharedSparkSession with AdaptiveSparkP
assert(df9.collect().size == df10.collect().size)
}
}
+
+ test("SPARK-35221: Add join hint build side check") {
+ withSQLConf(SQLConf.AUTO_BROADCASTJOIN_THRESHOLD.key -> "-1",
+ SQLConf.PREFER_SORTMERGEJOIN.key -> "true") {
+ Seq("left_outer", "left_semi", "left_anti").foreach { joinType =>
+ val hintAppender = new LogAppender(s"join hint build side check for $joinType")
+ withLogAppender(hintAppender, level = Some(Level.WARN)) {
+ assertShuffleMergeJoin(
+ df1.hint("BROADCAST").join(df2, $"a1" === $"b1", joinType))
+ assertBroadcastHashJoin(
+ df1.join(df2.hint("BROADCAST"), $"a1" === $"b1", joinType), BuildRight)
+
+ assertShuffleMergeJoin(
+ df1.hint("SHUFFLE_HASH").join(df2, $"a1" === $"b1", joinType))
+ assertShuffleHashJoin(
+ df1.join(df2.hint("SHUFFLE_HASH"), $"a1" === $"b1", joinType), BuildRight)
+ }
+
+ val logs = hintAppender.loggingEvents.map(_.getRenderedMessage)
+ .filter(_.startsWith("A join hint"))
Review comment:
nit but I think the test here can be flaky (e.g., the test will be broken if there's another log with "A join hint" added). What about just checking if there's a log that contains the message instead of checking the length of these logs?
--
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] ulysses-you commented on a change in pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
ulysses-you commented on a change in pull request #32355:
URL: https://github.com/apache/spark/pull/32355#discussion_r620942003
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/SparkStrategies.scala
##########
@@ -139,6 +139,7 @@ abstract class SparkStrategies extends QueryPlanner[SparkPlan] {
object JoinSelection extends Strategy
with PredicateHelper
with JoinSelectionHelper {
+ private val hintErrorHandler = conf.hintErrorHandler
Review comment:
It's a point. How about make a new rule at Optimizer `Finish Analysis` batch to check if join is illegal. Move to Optimizer is to avoid effect like `EliminateOuterJoin`.
--
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.
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] SparkQA commented on pull request #32355: [SPARK-35221][SQL] Add the check of supported join hints
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-893564352
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46610/
--
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] SparkQA commented on pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-827409169
**[Test build #137978 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137978/testReport)** for PR 32355 at commit [`97029f2`](https://github.com/apache/spark/commit/97029f2765df9694c0e698c5addf3059c2c926d7).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
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.
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] ulysses-you commented on a change in pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
ulysses-you commented on a change in pull request #32355:
URL: https://github.com/apache/spark/pull/32355#discussion_r680923147
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/JoinHintSuite.scala
##########
@@ -597,4 +597,68 @@ class JoinHintSuite extends PlanTest with SharedSparkSession with AdaptiveSparkP
assert(df9.collect().size == df10.collect().size)
}
}
+
+ test("SPARK-35221: Add join hint build side check") {
+ withSQLConf(SQLConf.AUTO_BROADCASTJOIN_THRESHOLD.key -> "-1",
+ SQLConf.PREFER_SORTMERGEJOIN.key -> "true") {
+ Seq("left_outer", "left_semi", "left_anti").foreach { joinType =>
+ val hintAppender = new LogAppender(s"join hint build side check for $joinType")
+ withLogAppender(hintAppender, level = Some(Level.WARN)) {
+ assertShuffleMergeJoin(
+ df1.hint("BROADCAST").join(df2, $"a1" === $"b1", joinType))
+ assertBroadcastHashJoin(
+ df1.join(df2.hint("BROADCAST"), $"a1" === $"b1", joinType), BuildRight)
+
+ assertShuffleMergeJoin(
+ df1.hint("SHUFFLE_HASH").join(df2, $"a1" === $"b1", joinType))
+ assertShuffleHashJoin(
+ df1.join(df2.hint("SHUFFLE_HASH"), $"a1" === $"b1", joinType), BuildRight)
+ }
+
+ val logs = hintAppender.loggingEvents.map(_.getRenderedMessage)
+ .filter(_.startsWith("A join hint"))
Review comment:
thank you @HyukjinKwon , agree. Change to `assert(logs.nonEmpty)` that ensure `logs.forall` works.
--
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] SparkQA commented on pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-891720052
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46497/
--
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] AmplabJenkins removed a comment on pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-891085931
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/46466/
--
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] AmplabJenkins removed a comment on pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-892041670
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/141986/
--
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] SparkQA commented on pull request #32355: [SPARK-35221][SQL] Add the check of supported join hints
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-893494332
**[Test build #142098 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/142098/testReport)** for PR 32355 at commit [`d90dd66`](https://github.com/apache/spark/commit/d90dd667baf228012bcd02260fb5c4b538c5c3ca).
--
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] SparkQA commented on pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-891663929
**[Test build #141986 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/141986/testReport)** for PR 32355 at commit [`307c8ac`](https://github.com/apache/spark/commit/307c8acd2193b30baabea8137669e61eb78a38c1).
--
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 change in pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32355:
URL: https://github.com/apache/spark/pull/32355#discussion_r681122743
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/HintErrorLogger.scala
##########
@@ -42,6 +45,17 @@ object HintErrorLogger extends HintErrorHandler with Logging {
logWarning(s"A join hint $hint is specified but it is not part of a join relation.")
}
+ override def joinBuildSideNotSupported(joinType: JoinType, joinHint: JoinHint): Unit = {
Review comment:
can we make the name more general like `hintNotSupported`?
--
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] ulysses-you commented on a change in pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
ulysses-you commented on a change in pull request #32355:
URL: https://github.com/apache/spark/pull/32355#discussion_r620915076
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/SparkStrategies.scala
##########
@@ -139,6 +139,7 @@ abstract class SparkStrategies extends QueryPlanner[SparkPlan] {
object JoinSelection extends Strategy
with PredicateHelper
with JoinSelectionHelper {
+ private val hintErrorHandler = conf.hintErrorHandler
Review comment:
I thought about it but seems we need to do this check in physical side.
We don't konw the join strategy at analysis or optimizer (`EliminateResolvedHint`). For example, if user specify `boradcast`, we need to check if it's a equijoin first then check the build side, since this join could be bhj or bnlj.
--
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.
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] ulysses-you commented on pull request #32355: [SPARK-35221][SQL] Add the check of supported join hints
Posted by GitBox <gi...@apache.org>.
ulysses-you commented on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-894103820
thank you all for the reivew !
--
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 change in pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32355:
URL: https://github.com/apache/spark/pull/32355#discussion_r681124146
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/HintErrorLogger.scala
##########
@@ -42,6 +45,17 @@ object HintErrorLogger extends HintErrorHandler with Logging {
logWarning(s"A join hint $hint is specified but it is not part of a join relation.")
}
+ override def joinBuildSideNotSupported(joinType: JoinType, joinHint: JoinHint): Unit = {
Review comment:
e.g.
```
def hintNotSupported(hint: HintInfo, reason: String): Unit = {
logWarning("Hint $hint is not supported in the query: " + reason)
}
```
--
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] SparkQA commented on pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-891523414
**[Test build #141981 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/141981/testReport)** for PR 32355 at commit [`11c5700`](https://github.com/apache/spark/commit/11c57008d2b61ac3ea258ca4edc0777e9a8fb7cc).
--
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 change in pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32355:
URL: https://github.com/apache/spark/pull/32355#discussion_r620896691
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/SparkStrategies.scala
##########
@@ -139,6 +139,7 @@ abstract class SparkStrategies extends QueryPlanner[SparkPlan] {
object JoinSelection extends Strategy
with PredicateHelper
with JoinSelectionHelper {
+ private val hintErrorHandler = conf.hintErrorHandler
Review comment:
can we check this in `ResolveJoinStrategyHints`?
--
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.
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] ulysses-you commented on a change in pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
ulysses-you commented on a change in pull request #32355:
URL: https://github.com/apache/spark/pull/32355#discussion_r681429032
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/HintErrorLogger.scala
##########
@@ -42,6 +45,17 @@ object HintErrorLogger extends HintErrorHandler with Logging {
logWarning(s"A join hint $hint is specified but it is not part of a join relation.")
}
+ override def joinBuildSideNotSupported(joinType: JoinType, joinHint: JoinHint): Unit = {
Review comment:
updated
--
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] AmplabJenkins removed a comment on pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-891549708
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/46492/
--
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] SparkQA removed a comment on pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-827287492
**[Test build #137978 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137978/testReport)** for PR 32355 at commit [`97029f2`](https://github.com/apache/spark/commit/97029f2765df9694c0e698c5addf3059c2c926d7).
--
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.
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] ulysses-you commented on a change in pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
ulysses-you commented on a change in pull request #32355:
URL: https://github.com/apache/spark/pull/32355#discussion_r629786382
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/SparkStrategies.scala
##########
@@ -139,6 +139,7 @@ abstract class SparkStrategies extends QueryPlanner[SparkPlan] {
object JoinSelection extends Strategy
with PredicateHelper
with JoinSelectionHelper {
+ private val hintErrorHandler = conf.hintErrorHandler
Review comment:
How about move `EliminateResolvedHint` from optimizer to physical side ? then we can add the check in one rule.
--
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.
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] ulysses-you commented on pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
ulysses-you commented on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-891519254
thank you @cloud-fan for review, added two methods in `JoinSelection`:
* `checkHintBuildSide` is to check hint build side
* `checkHintNonEquiJoin` is to check hint equi join
And also added test for the equi join check.
--
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] SparkQA removed a comment on pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-891783126
**[Test build #141994 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/141994/testReport)** for PR 32355 at commit [`2bb4200`](https://github.com/apache/spark/commit/2bb420046fd411b80876bfed32f0a7d0060a2a0a).
--
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] SparkQA commented on pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-892297358
**[Test build #142012 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/142012/testReport)** for PR 32355 at commit [`2bb4200`](https://github.com/apache/spark/commit/2bb420046fd411b80876bfed32f0a7d0060a2a0a).
--
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] SparkQA commented on pull request #32355: [SPARK-35221][SQL] Add the check of supported join hints
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-893993124
**[Test build #142116 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/142116/testReport)** for PR 32355 at commit [`f1e7ba0`](https://github.com/apache/spark/commit/f1e7ba082bfb557c67adad8f6c5c4ec7af622167).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
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] ulysses-you commented on a change in pull request #32355: [SPARK-35221][SQL] Add the check of supported join hints
Posted by GitBox <gi...@apache.org>.
ulysses-you commented on a change in pull request #32355:
URL: https://github.com/apache/spark/pull/32355#discussion_r683504415
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/SparkStrategies.scala
##########
@@ -139,6 +141,36 @@ abstract class SparkStrategies extends QueryPlanner[SparkPlan] {
object JoinSelection extends Strategy
with PredicateHelper
with JoinSelectionHelper {
+ private val hintErrorHandler = conf.hintErrorHandler
+
+ private def checkHintBuildSide(
+ onlyLookingAtHint: Boolean,
+ buildSide: Option[BuildSide],
+ joinType: JoinType,
+ hint: JoinHint,
+ isBroadcast: Boolean): Unit = {
+ if (onlyLookingAtHint && buildSide.isEmpty) {
+ if ((isBroadcast && hintToBroadcastLeft(hint)) ||
Review comment:
looks better
--
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] SparkQA commented on pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-827303225
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42498/
--
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.
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] AmplabJenkins removed a comment on pull request #32355: [SPARK-35221][SQL] Add the check of supported join hints
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-893923084
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/46628/
--
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] SparkQA commented on pull request #32355: [SPARK-35221][SQL] Add the check of supported join hints
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-893535633
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46610/
--
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] SparkQA commented on pull request #32355: [SPARK-35221][SQL] Add the check of supported join hints
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-893705653
**[Test build #142098 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/142098/testReport)** for PR 32355 at commit [`d90dd66`](https://github.com/apache/spark/commit/d90dd667baf228012bcd02260fb5c4b538c5c3ca).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
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] AmplabJenkins commented on pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-891862631
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/46506/
--
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] SparkQA commented on pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-891225713
**[Test build #141955 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/141955/testReport)** for PR 32355 at commit [`f231e31`](https://github.com/apache/spark/commit/f231e31bcb8e119d8288f806df5afb6e0f9c2f6b).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
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] SparkQA commented on pull request #32355: [SPARK-35221][SQL] Add the check of supported join hints
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-893529896
**[Test build #142100 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/142100/testReport)** for PR 32355 at commit [`5ac7952`](https://github.com/apache/spark/commit/5ac79523c67f167894b7be859e51742d4f1b23a8).
--
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] SparkQA removed a comment on pull request #32355: [SPARK-35221][SQL] Add the check of supported join hints
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-893494332
**[Test build #142098 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/142098/testReport)** for PR 32355 at commit [`d90dd66`](https://github.com/apache/spark/commit/d90dd667baf228012bcd02260fb5c4b538c5c3ca).
--
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] SparkQA commented on pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-891059282
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46466/
--
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] AmplabJenkins removed a comment on pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-891862631
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/46506/
--
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] SparkQA commented on pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-891989509
**[Test build #141994 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/141994/testReport)** for PR 32355 at commit [`2bb4200`](https://github.com/apache/spark/commit/2bb420046fd411b80876bfed32f0a7d0060a2a0a).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
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] SparkQA commented on pull request #32355: [SPARK-35221][SQL] Add join hint is supported check
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-892316663
Kubernetes integration test unable to build dist.
exiting with code: 1
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46524/
--
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 change in pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32355:
URL: https://github.com/apache/spark/pull/32355#discussion_r681628771
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/JoinHintSuite.scala
##########
@@ -597,4 +597,82 @@ class JoinHintSuite extends PlanTest with SharedSparkSession with AdaptiveSparkP
assert(df9.collect().size == df10.collect().size)
}
}
+
+ test("SPARK-35221: Add join hint build side check") {
+ withSQLConf(SQLConf.AUTO_BROADCASTJOIN_THRESHOLD.key -> "-1",
+ SQLConf.PREFER_SORTMERGEJOIN.key -> "true") {
+ Seq("left_outer", "left_semi", "left_anti").foreach { joinType =>
+ val hintAppender = new LogAppender(s"join hint build side check for $joinType")
+ withLogAppender(hintAppender, level = Some(Level.WARN)) {
+ assertShuffleMergeJoin(
+ df1.hint("BROADCAST").join(df2, $"a1" === $"b1", joinType))
+ assertBroadcastHashJoin(
+ df1.join(df2.hint("BROADCAST"), $"a1" === $"b1", joinType), BuildRight)
+
+ assertShuffleMergeJoin(
+ df1.hint("SHUFFLE_HASH").join(df2, $"a1" === $"b1", joinType))
+ assertShuffleHashJoin(
+ df1.join(df2.hint("SHUFFLE_HASH"), $"a1" === $"b1", joinType), BuildRight)
+ }
+
+ val logs = hintAppender.loggingEvents.map(_.getRenderedMessage)
+ .filter(_.contains("is not supported in the query:"))
Review comment:
what does the full message look like?
--
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] AmplabJenkins removed a comment on pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-891249986
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/141955/
--
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] SparkQA removed a comment on pull request #32355: [SPARK-35221][SQL] Add join hint is supported check
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-892297358
**[Test build #142012 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/142012/testReport)** for PR 32355 at commit [`2bb4200`](https://github.com/apache/spark/commit/2bb420046fd411b80876bfed32f0a7d0060a2a0a).
--
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] AmplabJenkins removed a comment on pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-891743197
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/46497/
--
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] SparkQA commented on pull request #32355: [SPARK-35221][SQL] Add the check of supported join hints
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-893887130
**[Test build #142116 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/142116/testReport)** for PR 32355 at commit [`f1e7ba0`](https://github.com/apache/spark/commit/f1e7ba082bfb557c67adad8f6c5c4ec7af622167).
--
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] SparkQA commented on pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-891032976
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46466/
--
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 change in pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32355:
URL: https://github.com/apache/spark/pull/32355#discussion_r620918996
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/SparkStrategies.scala
##########
@@ -139,6 +139,7 @@ abstract class SparkStrategies extends QueryPlanner[SparkPlan] {
object JoinSelection extends Strategy
with PredicateHelper
with JoinSelectionHelper {
+ private val hintErrorHandler = conf.hintErrorHandler
Review comment:
maybe we can move `ExtractEquiJoinKeys` to catalyst?
--
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.
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] SparkQA commented on pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-827287492
**[Test build #137978 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137978/testReport)** for PR 32355 at commit [`97029f2`](https://github.com/apache/spark/commit/97029f2765df9694c0e698c5addf3059c2c926d7).
--
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.
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] SparkQA commented on pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-891692100
**[Test build #141981 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/141981/testReport)** for PR 32355 at commit [`11c5700`](https://github.com/apache/spark/commit/11c57008d2b61ac3ea258ca4edc0777e9a8fb7cc).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
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] SparkQA commented on pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-891694088
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46497/
--
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] AmplabJenkins commented on pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-892041670
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/141986/
--
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 change in pull request #32355: [SPARK-35221][SQL] Add the check of supported join hints
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32355:
URL: https://github.com/apache/spark/pull/32355#discussion_r683636480
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/JoinHintSuite.scala
##########
@@ -597,4 +597,99 @@ class JoinHintSuite extends PlanTest with SharedSparkSession with AdaptiveSparkP
assert(df9.collect().size == df10.collect().size)
}
}
+
+ test("SPARK-35221: Add join hint build side check") {
+ withSQLConf(SQLConf.AUTO_BROADCASTJOIN_THRESHOLD.key -> "-1",
+ SQLConf.PREFER_SORTMERGEJOIN.key -> "true") {
+ Seq("left_outer", "left_semi", "left_anti").foreach { joinType =>
+ val hintAppender = new LogAppender(s"join hint build side check for $joinType")
+ withLogAppender(hintAppender, level = Some(Level.WARN)) {
+ assertShuffleMergeJoin(
+ df1.hint("BROADCAST").join(df2, $"a1" === $"b1", joinType))
+ assertShuffleMergeJoin(
+ df1.hint("SHUFFLE_HASH").join(df2, $"a1" === $"b1", joinType))
+ }
+
+ val logs = hintAppender.loggingEvents.map(_.getRenderedMessage)
+ .filter(_.contains("is not supported in the query:"))
+ assert(logs.size == 2)
+ logs.forall(_.contains(s"build left for ${joinType.split("_").mkString(" ")} join."))
+ }
+
+ Seq("left_outer", "left_semi", "left_anti").foreach { joinType =>
+ val hintAppender = new LogAppender(s"join hint build side check for $joinType")
+ withLogAppender(hintAppender, level = Some(Level.WARN)) {
+ assertBroadcastHashJoin(
+ df1.join(df2.hint("BROADCAST"), $"a1" === $"b1", joinType), BuildRight)
+ assertShuffleHashJoin(
+ df1.join(df2.hint("SHUFFLE_HASH"), $"a1" === $"b1", joinType), BuildRight)
+ }
+
+ val logs = hintAppender.loggingEvents.map(_.getRenderedMessage)
+ .filter(_.contains("is not supported in the query:"))
+ assert(logs.isEmpty)
+ }
+
+ Seq("right_outer").foreach { joinType =>
+ val hintAppender = new LogAppender(s"join hint build side check for $joinType")
+ withLogAppender(hintAppender, level = Some(Level.WARN)) {
+ assertShuffleMergeJoin(
+ df1.join(df2.hint("BROADCAST"), $"a1" === $"b1", joinType))
+ assertShuffleMergeJoin(
+ df1.join(df2.hint("SHUFFLE_HASH"), $"a1" === $"b1", joinType))
+ }
+ val logs = hintAppender.loggingEvents.map(_.getRenderedMessage)
+ .filter(_.contains("is not supported in the query:"))
+ assert(logs.size == 2)
+ logs.forall(_.contains(s"build right for ${joinType.split("_").mkString(" ")} join."))
+ }
+
+ Seq("right_outer").foreach { joinType =>
+ val hintAppender = new LogAppender(s"join hint build side check for $joinType")
+ withLogAppender(hintAppender, level = Some(Level.WARN)) {
+ assertBroadcastHashJoin(
+ df1.hint("BROADCAST").join(df2, $"a1" === $"b1", joinType), BuildLeft)
+ assertShuffleHashJoin(
+ df1.hint("SHUFFLE_HASH").join(df2, $"a1" === $"b1", joinType), BuildLeft)
+ }
+ val logs = hintAppender.loggingEvents.map(_.getRenderedMessage)
+ .filter(_.contains("is not supported in the query:"))
+ assert(logs.isEmpty)
+ }
+
+ Seq("inner", "cross").foreach { joinType =>
+ val hintAppender = new LogAppender(s"join hint build side check for $joinType")
+ withLogAppender(hintAppender, level = Some(Level.WARN)) {
+ assertBroadcastHashJoin(
+ df1.hint("BROADCAST").join(df2, $"a1" === $"b1", joinType), BuildLeft)
+ assertBroadcastHashJoin(
+ df1.join(df2.hint("BROADCAST"), $"a1" === $"b1", joinType), BuildRight)
+
+ assertShuffleHashJoin(
+ df1.hint("SHUFFLE_HASH").join(df2, $"a1" === $"b1", joinType), BuildLeft)
+ assertShuffleHashJoin(
+ df1.join(df2.hint("SHUFFLE_HASH"), $"a1" === $"b1", joinType), BuildRight)
+ }
+ val logs = hintAppender.loggingEvents.map(_.getRenderedMessage)
+ .filter(_.contains("is not supported in the query:"))
+ assert(logs.isEmpty)
+ }
+ }
+ }
+
+ test("SPARK-35221: Add join hint non equi-join check") {
+ val hintAppender = new LogAppender(s"join hint check for equi-join")
+ withLogAppender(hintAppender, level = Some(Level.WARN)) {
+ assertBroadcastNLJoin(
+ df1.hint("SHUFFLE_HASH").join(df2, $"a1" !== $"b1"), BuildRight)
+ }
+ withLogAppender(hintAppender, level = Some(Level.WARN)) {
+ assertBroadcastNLJoin(
+ df1.join(df2.hint("MERGE"), $"a1" !== $"b1"), BuildRight)
+ }
+ val logs = hintAppender.loggingEvents.map(_.getRenderedMessage)
+ .filter(_.contains("is not supported in the query:"))
+ assert(logs.nonEmpty)
Review comment:
should this be size 2 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] ulysses-you commented on a change in pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
ulysses-you commented on a change in pull request #32355:
URL: https://github.com/apache/spark/pull/32355#discussion_r681557185
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/hints.scala
##########
@@ -213,6 +213,11 @@ trait HintErrorHandler {
*/
def joinNotFoundForJoinHint(hint: HintInfo): Unit
+ /**
+ * Callback for a join hint specified on a join that doesn't support this build side.
Review comment:
done
--
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 change in pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32355:
URL: https://github.com/apache/spark/pull/32355#discussion_r681628239
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/SparkStrategies.scala
##########
@@ -139,6 +141,36 @@ abstract class SparkStrategies extends QueryPlanner[SparkPlan] {
object JoinSelection extends Strategy
with PredicateHelper
with JoinSelectionHelper {
+ private val hintErrorHandler = conf.hintErrorHandler
+
+ private def checkHintBuildSide(
+ onlyLookingAtHint: Boolean,
+ buildSide: Option[BuildSide],
+ joinType: JoinType,
+ hint: JoinHint,
+ isBroadcast: Boolean): Unit = {
+ if (onlyLookingAtHint && buildSide.isEmpty) {
+ if ((isBroadcast && hintToBroadcastLeft(hint)) ||
+ (!isBroadcast && hintToShuffleHashJoinLeft(hint))) {
+ assert(hint.leftHint.isDefined)
+ hintErrorHandler.joinHintNotSupported(hint.leftHint.get,
+ s"build left for ${joinType.sql.toLowerCase(Locale.ROOT)} join")
+ } else if ((isBroadcast && hintToBroadcastRight(hint)) ||
+ (!isBroadcast && hintToShuffleHashJoinRight(hint))) {
+ assert(hint.rightHint.isDefined)
+ hintErrorHandler.joinHintNotSupported(hint.rightHint.get,
+ s"build right for ${joinType.sql.toLowerCase(Locale.ROOT)} join")
+ }
+ }
+ }
+
+ private def checkHintNonEquiJoin(hint: JoinHint): Unit = {
+ if (hintToShuffleHashJoin(hint) || hintToSortMergeJoin(hint)) {
+ assert(hint.leftHint.orElse(hint.rightHint).isDefined)
+ hintErrorHandler.joinHintNotSupported(hint.leftHint.orElse(hint.rightHint).get,
+ "equi join keys is not existed")
Review comment:
`no equi-join keys`
--
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] AmplabJenkins removed a comment on pull request #32355: [SPARK-35221][SQL] Add the check of supported join hints
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-893755677
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/142100/
--
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] ulysses-you commented on a change in pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
ulysses-you commented on a change in pull request #32355:
URL: https://github.com/apache/spark/pull/32355#discussion_r681664615
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/JoinHintSuite.scala
##########
@@ -597,4 +597,82 @@ class JoinHintSuite extends PlanTest with SharedSparkSession with AdaptiveSparkP
assert(df9.collect().size == df10.collect().size)
}
}
+
+ test("SPARK-35221: Add join hint build side check") {
+ withSQLConf(SQLConf.AUTO_BROADCASTJOIN_THRESHOLD.key -> "-1",
+ SQLConf.PREFER_SORTMERGEJOIN.key -> "true") {
+ Seq("left_outer", "left_semi", "left_anti").foreach { joinType =>
+ val hintAppender = new LogAppender(s"join hint build side check for $joinType")
+ withLogAppender(hintAppender, level = Some(Level.WARN)) {
+ assertShuffleMergeJoin(
+ df1.hint("BROADCAST").join(df2, $"a1" === $"b1", joinType))
+ assertBroadcastHashJoin(
+ df1.join(df2.hint("BROADCAST"), $"a1" === $"b1", joinType), BuildRight)
+
+ assertShuffleMergeJoin(
+ df1.hint("SHUFFLE_HASH").join(df2, $"a1" === $"b1", joinType))
+ assertShuffleHashJoin(
+ df1.join(df2.hint("SHUFFLE_HASH"), $"a1" === $"b1", joinType), BuildRight)
+ }
+
+ val logs = hintAppender.loggingEvents.map(_.getRenderedMessage)
+ .filter(_.contains("is not supported in the query:"))
+ assert(logs.nonEmpty)
+ logs.forall(_.contains(s"build left for ${joinType.split("_").mkString(" ")} join."))
+ }
+
+ Seq("right_outer").foreach { joinType =>
+ val hintAppender = new LogAppender(s"join hint build side check for $joinType")
+ withLogAppender(hintAppender, level = Some(Level.WARN)) {
+ assertBroadcastHashJoin(
+ df1.hint("BROADCAST").join(df2, $"a1" === $"b1", joinType), BuildLeft)
+ assertShuffleMergeJoin(
+ df1.join(df2.hint("BROADCAST"), $"a1" === $"b1", joinType))
+
+ assertShuffleHashJoin(
+ df1.hint("SHUFFLE_HASH").join(df2, $"a1" === $"b1", joinType), BuildLeft)
+ assertShuffleMergeJoin(
+ df1.join(df2.hint("SHUFFLE_HASH"), $"a1" === $"b1", joinType))
+ }
+ val logs = hintAppender.loggingEvents.map(_.getRenderedMessage)
+ .filter(_.contains("is not supported in the query:"))
+ assert(logs.nonEmpty)
+ logs.forall(_.contains(s"build right for ${joinType.split("_").mkString(" ")} join."))
+ }
+
+ Seq("inner", "cross").foreach { joinType =>
+ val hintAppender = new LogAppender(s"join hint build side check for $joinType")
+ withLogAppender(hintAppender, level = Some(Level.WARN)) {
+ assertBroadcastHashJoin(
+ df1.hint("BROADCAST").join(df2, $"a1" === $"b1", joinType), BuildLeft)
+ assertBroadcastHashJoin(
+ df1.join(df2.hint("BROADCAST"), $"a1" === $"b1", joinType), BuildRight)
+
+ assertShuffleHashJoin(
+ df1.hint("SHUFFLE_HASH").join(df2, $"a1" === $"b1", joinType), BuildLeft)
+ assertShuffleHashJoin(
+ df1.join(df2.hint("SHUFFLE_HASH"), $"a1" === $"b1", joinType), BuildRight)
+ }
+ val logs = hintAppender.loggingEvents.map(_.getRenderedMessage)
+ .filter(_.contains("is not supported in the query:"))
+ assert(logs.isEmpty)
+ }
+ }
+ }
+
+ test("SPARK-35221: Add join hint non equi-join check") {
+ val hintAppender = new LogAppender(s"join hint check for equi-join")
+ withLogAppender(hintAppender, level = Some(Level.WARN)) {
+ assertBroadcastNLJoin(
+ df1.hint("SHUFFLE_HASH").join(df2, $"a1" !== $"b1"), BuildRight)
+ }
+ withLogAppender(hintAppender, level = Some(Level.WARN)) {
+ assertBroadcastNLJoin(
+ df1.join(df2.hint("MERGE"), $"a1" !== $"b1"), BuildRight)
+ }
+ val logs = hintAppender.loggingEvents.map(_.getRenderedMessage)
+ .filter(_.contains("is not supported in the query:"))
Review comment:
`Hint (strategy=shuffle_hash) is not supported in the query: no equi-join keys.`
--
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] AmplabJenkins removed a comment on pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-891702986
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/141981/
--
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] SparkQA removed a comment on pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-891663929
**[Test build #141986 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/141986/testReport)** for PR 32355 at commit [`307c8ac`](https://github.com/apache/spark/commit/307c8acd2193b30baabea8137669e61eb78a38c1).
--
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] AmplabJenkins commented on pull request #32355: [SPARK-35221][SQL] Add the check of supported join hints
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-893709398
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/142098/
--
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] AmplabJenkins commented on pull request #32355: [SPARK-35221][SQL] Add the check of supported join hints
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-893570887
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/46610/
--
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] maropu commented on a change in pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #32355:
URL: https://github.com/apache/spark/pull/32355#discussion_r621937979
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/hints.scala
##########
@@ -200,6 +201,11 @@ trait HintErrorHandler {
*/
def joinNotFoundForJoinHint(hint: HintInfo): Unit
+ /**
+ * Callback for a join hint specified on a join that doesn't support this build side.
+ */
+ def joinBuildSideNotSupported(joinType: JoinType, joinHint: JoinHint): Unit
Review comment:
nit: how about `joinBuildSideNotSupported` -> `hintBuildSideNotSupported ` ?
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/HintErrorLogger.scala
##########
@@ -42,6 +45,17 @@ object HintErrorLogger extends HintErrorHandler with Logging {
logWarning(s"A join hint $hint is specified but it is not part of a join relation.")
}
+ override def joinBuildSideNotSupported(joinType: JoinType, joinHint: JoinHint): Unit = {
+ assert(joinHint.leftHint.isDefined || joinHint.rightHint.isDefined)
+ val (hint, buildSide) = if (joinHint.leftHint.isDefined) {
+ (joinHint.leftHint.get, "left")
+ } else {
+ (joinHint.rightHint.get, "right")
+ }
+ logWarning(s"A join hint $hint is specified but it is not supported with build $buildSide " +
Review comment:
nit: `A join hint $hint ` => `Hint $hint` to follow the existing msg: https://github.com/apache/spark/pull/32355/files#diff-afe669f38210f85c87450ace74311b4ce2301e6aeab820aea444fcd802c2d5c2R60
--
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.
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] SparkQA commented on pull request #32355: [SPARK-35221][SQL] Add the check of supported join hints
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-893606617
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46612/
--
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 change in pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32355:
URL: https://github.com/apache/spark/pull/32355#discussion_r681527086
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/hints.scala
##########
@@ -213,6 +213,11 @@ trait HintErrorHandler {
*/
def joinNotFoundForJoinHint(hint: HintInfo): Unit
+ /**
+ * Callback for a join hint specified on a join that doesn't support this build side.
Review comment:
if we only target for join hint, let's name it `joinHintNotSupported`
--
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 change in pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32355:
URL: https://github.com/apache/spark/pull/32355#discussion_r681125659
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/SparkStrategies.scala
##########
@@ -163,7 +164,13 @@ abstract class SparkStrategies extends QueryPlanner[SparkPlan] {
// other choice.
case j @ ExtractEquiJoinKeys(joinType, leftKeys, rightKeys, nonEquiCond, left, right, hint) =>
def createBroadcastHashJoin(onlyLookingAtHint: Boolean) = {
- getBroadcastBuildSide(left, right, joinType, hint, onlyLookingAtHint, conf).map {
+ val buildSide = getBroadcastBuildSide(
Review comment:
can we make it more general? I think we can also give a warning if SMJ/SHJ hint is specified but the join has no join keys.
--
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] AmplabJenkins removed a comment on pull request #32355: [SPARK-35221][SQL] Add the check of supported join hints
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-894000251
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/142116/
--
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] ulysses-you commented on pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
ulysses-you commented on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-892287951
retest this please
--
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 change in pull request #32355: [SPARK-35221][SQL] Add the check of supported join hints
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32355:
URL: https://github.com/apache/spark/pull/32355#discussion_r683387845
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/JoinHintSuite.scala
##########
@@ -597,4 +597,82 @@ class JoinHintSuite extends PlanTest with SharedSparkSession with AdaptiveSparkP
assert(df9.collect().size == df10.collect().size)
}
}
+
+ test("SPARK-35221: Add join hint build side check") {
+ withSQLConf(SQLConf.AUTO_BROADCASTJOIN_THRESHOLD.key -> "-1",
+ SQLConf.PREFER_SORTMERGEJOIN.key -> "true") {
+ Seq("left_outer", "left_semi", "left_anti").foreach { joinType =>
+ val hintAppender = new LogAppender(s"join hint build side check for $joinType")
+ withLogAppender(hintAppender, level = Some(Level.WARN)) {
+ assertShuffleMergeJoin(
+ df1.hint("BROADCAST").join(df2, $"a1" === $"b1", joinType))
Review comment:
does it provide extra test coverage? I thought we already have many tests for join hints.
--
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] SparkQA commented on pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-827301506
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42498/
--
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.
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] SparkQA commented on pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-891543486
Kubernetes integration test unable to build dist.
exiting with code: 1
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46492/
--
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 change in pull request #32355: [SPARK-35221][SQL] Add the check of supported join hints
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32355:
URL: https://github.com/apache/spark/pull/32355#discussion_r683386765
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/SparkStrategies.scala
##########
@@ -139,6 +141,36 @@ abstract class SparkStrategies extends QueryPlanner[SparkPlan] {
object JoinSelection extends Strategy
with PredicateHelper
with JoinSelectionHelper {
+ private val hintErrorHandler = conf.hintErrorHandler
+
+ private def checkHintBuildSide(
+ onlyLookingAtHint: Boolean,
+ buildSide: Option[BuildSide],
+ joinType: JoinType,
+ hint: JoinHint,
+ isBroadcast: Boolean): Unit = {
+ if (onlyLookingAtHint && buildSide.isEmpty) {
+ if ((isBroadcast && hintToBroadcastLeft(hint)) ||
Review comment:
the code looks a bit messy, we can clearly show the branches:
```
def invalidBuildSideInHint(buildSide: String) = {
hintErrorHandler.joinHintNotSupported(hint.leftHint.get,
s"build $buildSide for ${joinType.sql.toLowerCase(Locale.ROOT)} join")
}
if (onlyLookingAtHint && buildSide.isEmpty) {
if (broadcast) {
// check broadcast hash join
if (hintToBroadcastLeft) invalidBuildSideInHint("left")
if (hintToBroadcastRight) invalidBuildSideInHint("right")
} else {
// check shuffle hash join
...
}
}
```
--
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] AmplabJenkins removed a comment on pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-892002628
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/141994/
--
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] SparkQA commented on pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-891783126
**[Test build #141994 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/141994/testReport)** for PR 32355 at commit [`2bb4200`](https://github.com/apache/spark/commit/2bb420046fd411b80876bfed32f0a7d0060a2a0a).
--
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] AmplabJenkins commented on pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-891702986
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/141981/
--
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] AmplabJenkins removed a comment on pull request #32355: [SPARK-35221][SQL] Add the check of supported join hints
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-893709398
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/142098/
--
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] SparkQA commented on pull request #32355: [SPARK-35221][SQL] Add the check of supported join hints
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-893919714
Kubernetes integration test unable to build dist.
exiting with code: 1
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46628/
--
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] SparkQA commented on pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-892018611
**[Test build #141986 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/141986/testReport)** for PR 32355 at commit [`307c8ac`](https://github.com/apache/spark/commit/307c8acd2193b30baabea8137669e61eb78a38c1).
* This patch **fails from timeout after a configured wait of `500m`**.
* This patch merges cleanly.
* This patch adds no public classes.
--
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] AmplabJenkins commented on pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-827303781
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42498/
--
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.
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] SparkQA removed a comment on pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-890998269
**[Test build #141955 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/141955/testReport)** for PR 32355 at commit [`f231e31`](https://github.com/apache/spark/commit/f231e31bcb8e119d8288f806df5afb6e0f9c2f6b).
--
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 change in pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32355:
URL: https://github.com/apache/spark/pull/32355#discussion_r620968097
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/SparkStrategies.scala
##########
@@ -139,6 +139,7 @@ abstract class SparkStrategies extends QueryPlanner[SparkPlan] {
object JoinSelection extends Strategy
with PredicateHelper
with JoinSelectionHelper {
+ private val hintErrorHandler = conf.hintErrorHandler
Review comment:
`EliminateOuterJoin` is a good point, so we can only know if the hint is valid or not at the planner.
It's a bit unfortunate to see the hint code being widespread, @maryannxue do you have any thoughts?
--
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.
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] AmplabJenkins commented on pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-827420714
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/137978/
--
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.
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] AmplabJenkins removed a comment on pull request #32355: [SPARK-35221][SQL] Add join hint is supported check
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-892318789
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/46524/
--
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 #32355: [SPARK-35221][SQL] Add the check of supported join hints
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-894064278
thanks, merging 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] ulysses-you commented on pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
ulysses-you commented on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-827333232
thanks for review @maropu @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.
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] AmplabJenkins removed a comment on pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-827420714
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/137978/
--
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.
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 closed pull request #32355: [SPARK-35221][SQL] Add the check of supported join hints
Posted by GitBox <gi...@apache.org>.
cloud-fan closed pull request #32355:
URL: https://github.com/apache/spark/pull/32355
--
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 change in pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32355:
URL: https://github.com/apache/spark/pull/32355#discussion_r681629000
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/JoinHintSuite.scala
##########
@@ -597,4 +597,82 @@ class JoinHintSuite extends PlanTest with SharedSparkSession with AdaptiveSparkP
assert(df9.collect().size == df10.collect().size)
}
}
+
+ test("SPARK-35221: Add join hint build side check") {
+ withSQLConf(SQLConf.AUTO_BROADCASTJOIN_THRESHOLD.key -> "-1",
+ SQLConf.PREFER_SORTMERGEJOIN.key -> "true") {
+ Seq("left_outer", "left_semi", "left_anti").foreach { joinType =>
+ val hintAppender = new LogAppender(s"join hint build side check for $joinType")
+ withLogAppender(hintAppender, level = Some(Level.WARN)) {
+ assertShuffleMergeJoin(
+ df1.hint("BROADCAST").join(df2, $"a1" === $"b1", joinType))
+ assertBroadcastHashJoin(
+ df1.join(df2.hint("BROADCAST"), $"a1" === $"b1", joinType), BuildRight)
+
+ assertShuffleMergeJoin(
+ df1.hint("SHUFFLE_HASH").join(df2, $"a1" === $"b1", joinType))
+ assertShuffleHashJoin(
+ df1.join(df2.hint("SHUFFLE_HASH"), $"a1" === $"b1", joinType), BuildRight)
+ }
+
+ val logs = hintAppender.loggingEvents.map(_.getRenderedMessage)
+ .filter(_.contains("is not supported in the query:"))
+ assert(logs.nonEmpty)
+ logs.forall(_.contains(s"build left for ${joinType.split("_").mkString(" ")} join."))
+ }
+
+ Seq("right_outer").foreach { joinType =>
+ val hintAppender = new LogAppender(s"join hint build side check for $joinType")
+ withLogAppender(hintAppender, level = Some(Level.WARN)) {
+ assertBroadcastHashJoin(
+ df1.hint("BROADCAST").join(df2, $"a1" === $"b1", joinType), BuildLeft)
+ assertShuffleMergeJoin(
+ df1.join(df2.hint("BROADCAST"), $"a1" === $"b1", joinType))
+
+ assertShuffleHashJoin(
+ df1.hint("SHUFFLE_HASH").join(df2, $"a1" === $"b1", joinType), BuildLeft)
+ assertShuffleMergeJoin(
+ df1.join(df2.hint("SHUFFLE_HASH"), $"a1" === $"b1", joinType))
+ }
+ val logs = hintAppender.loggingEvents.map(_.getRenderedMessage)
+ .filter(_.contains("is not supported in the query:"))
+ assert(logs.nonEmpty)
+ logs.forall(_.contains(s"build right for ${joinType.split("_").mkString(" ")} join."))
+ }
+
+ Seq("inner", "cross").foreach { joinType =>
+ val hintAppender = new LogAppender(s"join hint build side check for $joinType")
+ withLogAppender(hintAppender, level = Some(Level.WARN)) {
+ assertBroadcastHashJoin(
+ df1.hint("BROADCAST").join(df2, $"a1" === $"b1", joinType), BuildLeft)
+ assertBroadcastHashJoin(
+ df1.join(df2.hint("BROADCAST"), $"a1" === $"b1", joinType), BuildRight)
+
+ assertShuffleHashJoin(
+ df1.hint("SHUFFLE_HASH").join(df2, $"a1" === $"b1", joinType), BuildLeft)
+ assertShuffleHashJoin(
+ df1.join(df2.hint("SHUFFLE_HASH"), $"a1" === $"b1", joinType), BuildRight)
+ }
+ val logs = hintAppender.loggingEvents.map(_.getRenderedMessage)
+ .filter(_.contains("is not supported in the query:"))
+ assert(logs.isEmpty)
+ }
+ }
+ }
+
+ test("SPARK-35221: Add join hint non equi-join check") {
+ val hintAppender = new LogAppender(s"join hint check for equi-join")
+ withLogAppender(hintAppender, level = Some(Level.WARN)) {
+ assertBroadcastNLJoin(
+ df1.hint("SHUFFLE_HASH").join(df2, $"a1" !== $"b1"), BuildRight)
+ }
+ withLogAppender(hintAppender, level = Some(Level.WARN)) {
+ assertBroadcastNLJoin(
+ df1.join(df2.hint("MERGE"), $"a1" !== $"b1"), BuildRight)
+ }
+ val logs = hintAppender.loggingEvents.map(_.getRenderedMessage)
+ .filter(_.contains("is not supported in the query:"))
Review comment:
ditto
--
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] AmplabJenkins commented on pull request #32355: [SPARK-35221][SQL] Add the check of supported join hints
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-893755677
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/142100/
--
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] AmplabJenkins commented on pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-891743197
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/46497/
--
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] AmplabJenkins commented on pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-891549708
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/46492/
--
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] AmplabJenkins commented on pull request #32355: [SPARK-35221][SQL] Add join hint is supported check
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-892318789
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/46524/
--
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] SparkQA removed a comment on pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-891523414
**[Test build #141981 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/141981/testReport)** for PR 32355 at commit [`11c5700`](https://github.com/apache/spark/commit/11c57008d2b61ac3ea258ca4edc0777e9a8fb7cc).
--
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] AmplabJenkins removed a comment on pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-827303781
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42498/
--
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.
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] AmplabJenkins removed a comment on pull request #32355: [SPARK-35221][SQL] Add the check of supported join hints
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-893570887
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/46610/
--
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] SparkQA removed a comment on pull request #32355: [SPARK-35221][SQL] Add the check of supported join hints
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-893887130
**[Test build #142116 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/142116/testReport)** for PR 32355 at commit [`f1e7ba0`](https://github.com/apache/spark/commit/f1e7ba082bfb557c67adad8f6c5c4ec7af622167).
--
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] AmplabJenkins removed a comment on pull request #32355: [SPARK-35221][SQL] Add join hint is supported check
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-892411354
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/142012/
--
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] SparkQA commented on pull request #32355: [SPARK-35221][SQL] Add join hint is supported check
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-892401658
**[Test build #142012 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/142012/testReport)** for PR 32355 at commit [`2bb4200`](https://github.com/apache/spark/commit/2bb420046fd411b80876bfed32f0a7d0060a2a0a).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
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] SparkQA commented on pull request #32355: [SPARK-35221][SQL] Add the check of supported join hints
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-893567340
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46612/
--
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] AmplabJenkins commented on pull request #32355: [SPARK-35221][SQL] Add join hint is supported check
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-892411354
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/142012/
--
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] AmplabJenkins commented on pull request #32355: [SPARK-35221][SQL] Add the check of supported join hints
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-894000251
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/142116/
--
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] ulysses-you commented on a change in pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
ulysses-you commented on a change in pull request #32355:
URL: https://github.com/apache/spark/pull/32355#discussion_r622046617
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/HintErrorLogger.scala
##########
@@ -42,6 +45,17 @@ object HintErrorLogger extends HintErrorHandler with Logging {
logWarning(s"A join hint $hint is specified but it is not part of a join relation.")
}
+ override def joinBuildSideNotSupported(joinType: JoinType, joinHint: JoinHint): Unit = {
+ assert(joinHint.leftHint.isDefined || joinHint.rightHint.isDefined)
+ val (hint, buildSide) = if (joinHint.leftHint.isDefined) {
+ (joinHint.leftHint.get, "left")
+ } else {
+ (joinHint.rightHint.get, "right")
+ }
+ logWarning(s"A join hint $hint is specified but it is not supported with build $buildSide " +
Review comment:
This msg format and method follow that `joinNotFoundForJoinHint` msg. Seems that one is better ?
--
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.
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] SparkQA commented on pull request #32355: [SPARK-35221][SQL] Add the check of supported join hints
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-893731382
**[Test build #142100 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/142100/testReport)** for PR 32355 at commit [`5ac7952`](https://github.com/apache/spark/commit/5ac79523c67f167894b7be859e51742d4f1b23a8).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
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] AmplabJenkins commented on pull request #32355: [SPARK-35221][SQL] Add the check of supported join hints
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-893612710
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/46612/
--
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] AmplabJenkins commented on pull request #32355: [SPARK-35221][SQL] Add the check of supported join hints
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-893923084
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/46628/
--
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] SparkQA commented on pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-891835078
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46506/
--
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] SparkQA removed a comment on pull request #32355: [SPARK-35221][SQL] Add the check of supported join hints
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-893529896
**[Test build #142100 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/142100/testReport)** for PR 32355 at commit [`5ac7952`](https://github.com/apache/spark/commit/5ac79523c67f167894b7be859e51742d4f1b23a8).
--
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] AmplabJenkins commented on pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-891085931
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/46466/
--
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] AmplabJenkins commented on pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-892002628
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/141994/
--
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] SparkQA commented on pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-890998269
**[Test build #141955 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/141955/testReport)** for PR 32355 at commit [`f231e31`](https://github.com/apache/spark/commit/f231e31bcb8e119d8288f806df5afb6e0f9c2f6b).
--
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] ulysses-you commented on a change in pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
ulysses-you commented on a change in pull request #32355:
URL: https://github.com/apache/spark/pull/32355#discussion_r681664564
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/JoinHintSuite.scala
##########
@@ -597,4 +597,82 @@ class JoinHintSuite extends PlanTest with SharedSparkSession with AdaptiveSparkP
assert(df9.collect().size == df10.collect().size)
}
}
+
+ test("SPARK-35221: Add join hint build side check") {
+ withSQLConf(SQLConf.AUTO_BROADCASTJOIN_THRESHOLD.key -> "-1",
+ SQLConf.PREFER_SORTMERGEJOIN.key -> "true") {
+ Seq("left_outer", "left_semi", "left_anti").foreach { joinType =>
+ val hintAppender = new LogAppender(s"join hint build side check for $joinType")
+ withLogAppender(hintAppender, level = Some(Level.WARN)) {
+ assertShuffleMergeJoin(
+ df1.hint("BROADCAST").join(df2, $"a1" === $"b1", joinType))
+ assertBroadcastHashJoin(
+ df1.join(df2.hint("BROADCAST"), $"a1" === $"b1", joinType), BuildRight)
+
+ assertShuffleMergeJoin(
+ df1.hint("SHUFFLE_HASH").join(df2, $"a1" === $"b1", joinType))
+ assertShuffleHashJoin(
+ df1.join(df2.hint("SHUFFLE_HASH"), $"a1" === $"b1", joinType), BuildRight)
+ }
+
+ val logs = hintAppender.loggingEvents.map(_.getRenderedMessage)
+ .filter(_.contains("is not supported in the query:"))
Review comment:
`Hint (strategy=shuffle_hash) is not supported in the query: build right for right outer join.`
--
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] AmplabJenkins commented on pull request #32355: [SPARK-35221][SQL] Add join hint build side check
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32355:
URL: https://github.com/apache/spark/pull/32355#issuecomment-891249986
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/141955/
--
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