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 2020/08/05 09:33:21 UTC

[GitHub] [spark] LantaoJin opened a new pull request #29359: [SPARK-32537][SQL][TEST] Add a CTEHintSuite for test coverage

LantaoJin opened a new pull request #29359:
URL: https://github.com/apache/spark/pull/29359


   ### What changes were proposed in this pull request?
   Add a new test suite `CTEHintSuite`
   
   ### Why are the changes needed?
   This ticket is to address the below comments to help us understand the test coverage of SQL HINT for CTE.
   https://github.com/apache/spark/pull/29062#discussion_r463247491
   https://github.com/apache/spark/pull/29062#discussion_r463248167
   
   
   ### Does this PR introduce _any_ user-facing change?
   No
   
   
   ### How was this patch tested?
   Add a test suite.
   


----------------------------------------------------------------
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 #29359: [SPARK-32537][SQL][TEST] Add a CTEHintSuite for test coverage

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29359:
URL: https://github.com/apache/spark/pull/29359#issuecomment-669922047






----------------------------------------------------------------
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 #29359: [SPARK-32537][SQL][TEST] Add a CTEHintSuite for test coverage

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29359:
URL: https://github.com/apache/spark/pull/29359#issuecomment-669796804


   **[Test build #127134 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127134/testReport)** for PR 29359 at commit [`6f8a2cc`](https://github.com/apache/spark/commit/6f8a2cc74b91bbf8a15ff60a6e36e5281caaa4ca).


----------------------------------------------------------------
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 #29359: [SPARK-32537][SQL][TEST] Add a CTEHintSuite for test coverage

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29359:
URL: https://github.com/apache/spark/pull/29359#issuecomment-669800599


   **[Test build #127135 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127135/testReport)** for PR 29359 at commit [`ce94127`](https://github.com/apache/spark/commit/ce941270ccb3a2080c22d037f940bb8ed4191df2).


----------------------------------------------------------------
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 #29359: [SPARK-32537][SQL][TEST] Add a CTEHintSuite for test coverage

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29359:
URL: https://github.com/apache/spark/pull/29359#issuecomment-669092955






----------------------------------------------------------------
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] LantaoJin commented on a change in pull request #29359: [SPARK-32537][SQL][TEST] Add a CTEHintSuite for test coverage

Posted by GitBox <gi...@apache.org>.
LantaoJin commented on a change in pull request #29359:
URL: https://github.com/apache/spark/pull/29359#discussion_r466232143



##########
File path: sql/core/src/test/scala/org/apache/spark/sql/CTEHintSuite.scala
##########
@@ -0,0 +1,158 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql
+
+import org.apache.log4j.Level
+
+import org.apache.spark.sql.catalyst.plans.PlanTest
+import org.apache.spark.sql.catalyst.plans.logical.{BROADCAST, HintInfo, Join, JoinHint, Repartition, RepartitionByExpression, ResolvedHint, SHUFFLE_HASH, SHUFFLE_MERGE, SHUFFLE_REPLICATE_NL}
+import org.apache.spark.sql.execution.adaptive.AdaptiveSparkPlanHelper
+import org.apache.spark.sql.test.SharedSparkSession
+
+class CTEHintSuite extends PlanTest with SharedSparkSession with AdaptiveSparkPlanHelper {

Review comment:
       I will remove it. I just copy it from `JoinHintSuite` :P




----------------------------------------------------------------
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 #29359: [SPARK-32537][SQL][TEST] Add a CTEHintSuite for test coverage

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29359:
URL: https://github.com/apache/spark/pull/29359#issuecomment-670364861






----------------------------------------------------------------
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 #29359: [SPARK-32537][SQL][TEST] Add a CTEHintSuite for test coverage

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29359:
URL: https://github.com/apache/spark/pull/29359#issuecomment-670368548


   **[Test build #127187 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127187/testReport)** for PR 29359 at commit [`4256339`](https://github.com/apache/spark/commit/42563390f9b34c87c9aea1038e047b59c68dcaba).


----------------------------------------------------------------
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 #29359: [SPARK-32537][SQL][TEST] Add a CTEHintSuite for test coverage

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29359:
URL: https://github.com/apache/spark/pull/29359#issuecomment-670484582






----------------------------------------------------------------
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 #29359: [SPARK-32537][SQL][TEST] Add a CTEHintSuite for test coverage

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29359:
URL: https://github.com/apache/spark/pull/29359#issuecomment-670369069






----------------------------------------------------------------
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 #29359: [SPARK-32537][SQL][TEST] Add a CTEHintSuite for test coverage

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29359:
URL: https://github.com/apache/spark/pull/29359#issuecomment-669921066


   **[Test build #127135 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127135/testReport)** for PR 29359 at commit [`ce94127`](https://github.com/apache/spark/commit/ce941270ccb3a2080c22d037f940bb8ed4191df2).
    * This patch passes all tests.
    * This patch merges cleanly.
    * This patch adds the following public classes _(experimental)_:
     * `class CTEHintSuite extends QueryTest with SharedSparkSession `


----------------------------------------------------------------
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 #29359: [SPARK-32537][SQL][TEST] Add a CTEHintSuite for test coverage

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29359:
URL: https://github.com/apache/spark/pull/29359#issuecomment-670329082


   **[Test build #127176 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127176/testReport)** for PR 29359 at commit [`4256339`](https://github.com/apache/spark/commit/42563390f9b34c87c9aea1038e047b59c68dcaba).


----------------------------------------------------------------
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 #29359: [SPARK-32537][SQL][TEST] Add a CTEHintSuite for test coverage

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29359:
URL: https://github.com/apache/spark/pull/29359#issuecomment-670364665


   **[Test build #127176 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127176/testReport)** for PR 29359 at commit [`4256339`](https://github.com/apache/spark/commit/42563390f9b34c87c9aea1038e047b59c68dcaba).
    * This patch **fails due to an unknown error code, -9**.
    * 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] AmplabJenkins commented on pull request #29359: [SPARK-32537][SQL][TEST] Add a CTEHintSuite for test coverage

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29359:
URL: https://github.com/apache/spark/pull/29359#issuecomment-669199951






----------------------------------------------------------------
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 #29359: [SPARK-32537][SQL][TEST] Add a CTEHintSuite for test coverage

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29359:
URL: https://github.com/apache/spark/pull/29359#issuecomment-671196968






----------------------------------------------------------------
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 #29359: [SPARK-32537][SQL][TEST] Add a CTEHintSuite for test coverage

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29359:
URL: https://github.com/apache/spark/pull/29359#issuecomment-670484582






----------------------------------------------------------------
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] LantaoJin commented on a change in pull request #29359: [SPARK-32537][SQL][TEST] Add a CTEHintSuite for test coverage

Posted by GitBox <gi...@apache.org>.
LantaoJin commented on a change in pull request #29359:
URL: https://github.com/apache/spark/pull/29359#discussion_r466824955



##########
File path: sql/core/src/test/scala/org/apache/spark/sql/CTEHintSuite.scala
##########
@@ -0,0 +1,178 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql
+
+import org.apache.log4j.Level
+
+import org.apache.spark.sql.catalyst.plans.logical._
+import org.apache.spark.sql.test.SharedSparkSession
+
+class CTEHintSuite extends QueryTest with SharedSparkSession {
+
+  def verifyCoalesceHint(df: DataFrame): Unit = {

Review comment:
       `ResolveCoalesceHints` accepts names "COALESCE", "REPARTITION", and "REPARTITION_BY_RANGE".
   https://github.com/apache/spark/blob/9a35b93c8a02939bf02437f47501d55223b6a283/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveHints.scala#L174




----------------------------------------------------------------
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 #29359: [SPARK-32537][SQL][TEST] Add a CTEHintSuite for test coverage

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29359:
URL: https://github.com/apache/spark/pull/29359#issuecomment-669922047






----------------------------------------------------------------
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 #29359: [SPARK-32537][SQL][TEST] Add a CTEHintSuite for test coverage

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29359:
URL: https://github.com/apache/spark/pull/29359#issuecomment-671133228


   **[Test build #127251 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127251/testReport)** for PR 29359 at commit [`a7c4d32`](https://github.com/apache/spark/commit/a7c4d32894d75696ee9a891219d9c99314ef05ed).


----------------------------------------------------------------
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] maropu commented on a change in pull request #29359: [SPARK-32537][SQL][TEST] Add a CTEHintSuite for test coverage

Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #29359:
URL: https://github.com/apache/spark/pull/29359#discussion_r466219770



##########
File path: sql/core/src/test/scala/org/apache/spark/sql/CTEHintSuite.scala
##########
@@ -0,0 +1,158 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql
+
+import org.apache.log4j.Level
+
+import org.apache.spark.sql.catalyst.plans.PlanTest
+import org.apache.spark.sql.catalyst.plans.logical.{BROADCAST, HintInfo, Join, JoinHint, Repartition, RepartitionByExpression, ResolvedHint, SHUFFLE_HASH, SHUFFLE_MERGE, SHUFFLE_REPLICATE_NL}

Review comment:
       nit: I think its okay to fold it: `import org.apache.spark.sql.catalyst.plans.logical._`

##########
File path: sql/core/src/test/scala/org/apache/spark/sql/CTEHintSuite.scala
##########
@@ -0,0 +1,158 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql
+
+import org.apache.log4j.Level
+
+import org.apache.spark.sql.catalyst.plans.PlanTest
+import org.apache.spark.sql.catalyst.plans.logical.{BROADCAST, HintInfo, Join, JoinHint, Repartition, RepartitionByExpression, ResolvedHint, SHUFFLE_HASH, SHUFFLE_MERGE, SHUFFLE_REPLICATE_NL}
+import org.apache.spark.sql.execution.adaptive.AdaptiveSparkPlanHelper
+import org.apache.spark.sql.test.SharedSparkSession
+
+class CTEHintSuite extends PlanTest with SharedSparkSession with AdaptiveSparkPlanHelper {
+
+  def verifyCoalesceHint(df: DataFrame): Unit = {
+    val optimized = df.queryExecution.optimizedPlan

Review comment:
       Could you test it in an analyzed plan, too?

##########
File path: sql/core/src/test/scala/org/apache/spark/sql/CTEHintSuite.scala
##########
@@ -0,0 +1,158 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql
+
+import org.apache.log4j.Level
+
+import org.apache.spark.sql.catalyst.plans.PlanTest
+import org.apache.spark.sql.catalyst.plans.logical.{BROADCAST, HintInfo, Join, JoinHint, Repartition, RepartitionByExpression, ResolvedHint, SHUFFLE_HASH, SHUFFLE_MERGE, SHUFFLE_REPLICATE_NL}
+import org.apache.spark.sql.execution.adaptive.AdaptiveSparkPlanHelper
+import org.apache.spark.sql.test.SharedSparkSession
+
+class CTEHintSuite extends PlanTest with SharedSparkSession with AdaptiveSparkPlanHelper {
+
+  def verifyCoalesceHint(df: DataFrame): Unit = {
+    val optimized = df.queryExecution.optimizedPlan
+    val repartitions = optimized collect {
+      case r: Repartition => r
+      case r: RepartitionByExpression => r
+      case _: ResolvedHint => fail("ResolvedHint should not appear after optimize.")
+    }
+    assert(repartitions.nonEmpty)
+  }
+
+  def verifyJoinHint(df: DataFrame, expectedHints: Seq[JoinHint]): Unit = {
+    val optimized = df.queryExecution.optimizedPlan

Review comment:
       ditto

##########
File path: sql/core/src/test/scala/org/apache/spark/sql/CTEHintSuite.scala
##########
@@ -0,0 +1,158 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql
+
+import org.apache.log4j.Level
+
+import org.apache.spark.sql.catalyst.plans.PlanTest
+import org.apache.spark.sql.catalyst.plans.logical.{BROADCAST, HintInfo, Join, JoinHint, Repartition, RepartitionByExpression, ResolvedHint, SHUFFLE_HASH, SHUFFLE_MERGE, SHUFFLE_REPLICATE_NL}
+import org.apache.spark.sql.execution.adaptive.AdaptiveSparkPlanHelper
+import org.apache.spark.sql.test.SharedSparkSession
+
+class CTEHintSuite extends PlanTest with SharedSparkSession with AdaptiveSparkPlanHelper {

Review comment:
       we need `AdaptiveSparkPlanHelper`?




----------------------------------------------------------------
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] maropu commented on a change in pull request #29359: [SPARK-32537][SQL][TEST] Add a CTEHintSuite for test coverage

Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #29359:
URL: https://github.com/apache/spark/pull/29359#discussion_r466473690



##########
File path: sql/core/src/test/scala/org/apache/spark/sql/CTEHintSuite.scala
##########
@@ -0,0 +1,178 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql
+
+import org.apache.log4j.Level
+
+import org.apache.spark.sql.catalyst.plans.logical._
+import org.apache.spark.sql.test.SharedSparkSession
+
+class CTEHintSuite extends QueryTest with SharedSparkSession {
+
+  def verifyCoalesceHint(df: DataFrame): Unit = {
+    def checkPlanContainsRepartition(plan: LogicalPlan): Unit = {
+      val repartitions = plan collect {
+        case r: Repartition => r
+        case r: RepartitionByExpression => r
+        case _: ResolvedHint => fail("ResolvedHint should not appear after optimize.")

Review comment:
       Is this error message correct even if the test is for an analyzed plan? https://github.com/apache/spark/pull/29359/files#diff-0d893e4fe9a621d3e084c2913c533e00R38




----------------------------------------------------------------
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 #29359: [SPARK-32537][SQL][TEST] Add a CTEHintSuite for test coverage

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29359:
URL: https://github.com/apache/spark/pull/29359#issuecomment-669198930


   **[Test build #127092 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127092/testReport)** for PR 29359 at commit [`773eacd`](https://github.com/apache/spark/commit/773eacdb3f2275863e3e521a6aa9a94178c5ce7c).
    * This patch passes all tests.
    * This patch merges cleanly.
    * This patch adds the following public classes _(experimental)_:
     * `class CTEHintSuite extends PlanTest with SharedSparkSession with AdaptiveSparkPlanHelper `


----------------------------------------------------------------
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 #29359: [SPARK-32537][SQL][TEST] Add a CTEHintSuite for test coverage

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29359:
URL: https://github.com/apache/spark/pull/29359#issuecomment-669800599


   **[Test build #127135 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127135/testReport)** for PR 29359 at commit [`ce94127`](https://github.com/apache/spark/commit/ce941270ccb3a2080c22d037f940bb8ed4191df2).


----------------------------------------------------------------
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 #29359: [SPARK-32537][SQL][TEST] Add a CTEHintSuite for test coverage

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29359:
URL: https://github.com/apache/spark/pull/29359#issuecomment-669796804


   **[Test build #127134 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127134/testReport)** for PR 29359 at commit [`6f8a2cc`](https://github.com/apache/spark/commit/6f8a2cc74b91bbf8a15ff60a6e36e5281caaa4ca).


----------------------------------------------------------------
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 #29359: [SPARK-32537][SQL][TEST] Add a CTEHintSuite for test coverage

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29359:
URL: https://github.com/apache/spark/pull/29359#issuecomment-670329082


   **[Test build #127176 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127176/testReport)** for PR 29359 at commit [`4256339`](https://github.com/apache/spark/commit/42563390f9b34c87c9aea1038e047b59c68dcaba).


----------------------------------------------------------------
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] maropu commented on a change in pull request #29359: [SPARK-32537][SQL][TEST] Add a CTEHintSuite for test coverage

Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #29359:
URL: https://github.com/apache/spark/pull/29359#discussion_r466475175



##########
File path: sql/core/src/test/scala/org/apache/spark/sql/CTEHintSuite.scala
##########
@@ -0,0 +1,178 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql
+
+import org.apache.log4j.Level
+
+import org.apache.spark.sql.catalyst.plans.logical._
+import org.apache.spark.sql.test.SharedSparkSession
+
+class CTEHintSuite extends QueryTest with SharedSparkSession {
+
+  def verifyCoalesceHint(df: DataFrame): Unit = {
+    def checkPlanContainsRepartition(plan: LogicalPlan): Unit = {
+      val repartitions = plan collect {
+        case r: Repartition => r
+        case r: RepartitionByExpression => r
+        case _: ResolvedHint => fail("ResolvedHint should not appear after optimize.")
+      }
+      assert(repartitions.nonEmpty)
+    }
+    val analyzed = df.queryExecution.analyzed
+    val optimized = df.queryExecution.optimizedPlan
+    checkPlanContainsRepartition(analyzed)
+    checkPlanContainsRepartition(optimized)
+  }
+
+  def verifyJoinHint(df: DataFrame, expectedHints: Seq[JoinHint]): Unit = {
+    val analyzed = df.queryExecution.analyzed
+    val resolvedHints = analyzed collect {
+      case r: ResolvedHint => r
+    }
+    assert(resolvedHints.nonEmpty)
+    val optimized = df.queryExecution.optimizedPlan
+    val joinHints = optimized collect {
+      case Join(_, _, _, _, hint) => hint
+      case _: ResolvedHint => fail("ResolvedHint should not appear after optimize.")
+    }
+    assert(joinHints == expectedHints)
+  }
+
+  def verifyJoinHintWithWarnings(
+      df: => DataFrame,
+      expectedHints: Seq[JoinHint],
+      warnings: Seq[String]): Unit = {
+    val logAppender = new LogAppender("join hints")
+    withLogAppender(logAppender) {
+      verifyJoinHint(df, expectedHints)
+    }
+    val warningMessages = logAppender.loggingEvents
+      .filter(_.getLevel == Level.WARN)
+      .map(_.getRenderedMessage)
+      .filter(_.contains("hint"))
+    assert(warningMessages.size == warnings.size)
+    warnings.foreach { w =>
+      assert(warningMessages.contains(w))
+    }
+  }
+
+  def msgNoJoinForJoinHint(strategy: String): String =
+    s"A join hint (strategy=$strategy) is specified but it is not part of a join relation."
+
+  def msgJoinHintOverridden(strategy: String): String =
+    s"Hint (strategy=$strategy) is overridden by another hint and will not take effect."
+
+  test("Resolve coalesce hint in CTE") {
+    // COALESCE,
+    // REPARTITION,
+    // REPARTITION_BY_RANGE
+    withTable("t") {
+      sql("CREATE TABLE t USING PARQUET AS SELECT 1 AS id")
+      verifyCoalesceHint(
+        sql("WITH cte AS (SELECT /*+ COALESCE(1) */ * FROM t) SELECT * FROM cte"))
+      verifyCoalesceHint(
+        sql("WITH cte AS (SELECT /*+ REPARTITION(3) */ * FROM t) SELECT * FROM cte"))
+      verifyCoalesceHint(
+        sql("WITH cte AS (SELECT /*+ REPARTITION(id) */ * FROM t) SELECT * FROM cte"))
+      verifyCoalesceHint(
+        sql("WITH cte AS (SELECT /*+ REPARTITION(3, id) */ * FROM t) SELECT * FROM cte"))
+      verifyCoalesceHint(
+        sql("WITH cte AS (SELECT /*+ REPARTITION_BY_RANGE(id) */ * FROM t) SELECT * FROM cte"))
+      verifyCoalesceHint(
+        sql("WITH cte AS (SELECT /*+ REPARTITION_BY_RANGE(3, id) */ * FROM t) SELECT * FROM cte"))
+    }
+  }
+
+  test("Resolve join hint in CTE") {
+    // BROADCAST,
+    // SHUFFLE_MERGE,
+    // SHUFFLE_HASH,
+    // SHUFFLE_REPLICATE_NL
+    withTable("t", "s") {
+      sql("CREATE TABLE a USING PARQUET AS SELECT 1 AS a1")
+      sql("CREATE TABLE b USING PARQUET AS SELECT 1 AS b1")
+      sql("CREATE TABLE c USING PARQUET AS SELECT 1 AS c1")
+      verifyJoinHint(
+        sql(
+          """
+            |WITH cte AS (
+            |  SELECT /*+ BROADCAST(a) */ * FROM a JOIN b ON a.a1 = b.b1
+            |)
+            |SELECT * FROM cte
+            |""".stripMargin),
+        JoinHint(
+          Some(HintInfo(strategy = Some(BROADCAST))),
+          None) :: Nil
+      )
+      verifyJoinHint(
+        sql(
+          """
+            |WITH cte AS (
+            |  SELECT /*+ SHUFFLE_HASH(a) */ * FROM a JOIN b ON a.a1 = b.b1
+            |)
+            |SELECT * FROM cte
+            |""".stripMargin),
+        JoinHint(
+          Some(HintInfo(strategy = Some(SHUFFLE_HASH))),
+          None) :: Nil
+      )
+      verifyJoinHintWithWarnings(
+        sql(
+          """
+            |WITH cte AS (
+            |  SELECT /*+ SHUFFLE_HASH MERGE(a, c) BROADCAST(a, b)*/ * FROM a, b, c
+            |  WHERE a.a1 = b.b1 AND b.b1 = c.c1
+            |)
+            |SELECT * FROM cte
+            |""".stripMargin),
+        JoinHint(
+          None,
+          Some(HintInfo(strategy = Some(SHUFFLE_MERGE)))) ::
+          JoinHint(
+            Some(HintInfo(strategy = Some(SHUFFLE_MERGE))),
+            Some(HintInfo(strategy = Some(BROADCAST)))) :: Nil,
+          msgNoJoinForJoinHint("shuffle_hash") ::
+            msgJoinHintOverridden("broadcast") :: Nil
+      )
+      verifyJoinHint(
+        sql(
+          """
+            |WITH cte AS (
+            |  SELECT /*+ SHUFFLE_REPLICATE_NL(a) SHUFFLE_HASH(b) */ * FROM a JOIN b ON a.a1 = b.b1
+            |)
+            |SELECT * FROM cte
+            |""".stripMargin),
+        JoinHint(
+          Some(HintInfo(strategy = Some(SHUFFLE_REPLICATE_NL))),
+          Some(HintInfo(strategy = Some(SHUFFLE_HASH)))) :: Nil
+      )
+    }
+  }
+
+  test("SPARK-32237: Hint in CTE") {
+    withTable("t") {
+      sql("CREATE TABLE t USING PARQUET AS SELECT 1 AS id")
+      checkAnswer(
+        sql(s"""
+               |WITH cte AS (SELECT /*+ REPARTITION(3) */ * FROM t)
+               |SELECT * FROM cte
+        """.stripMargin),

Review comment:
       nit format: 
   ```
         checkAnswer(
           sql(
             s"""
                |WITH cte AS (SELECT /*+ REPARTITION(3) */ * FROM t)
                |SELECT * FROM cte
                |""".stripMargin),
           Row(1) :: Nil)
   ```




----------------------------------------------------------------
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 #29359: [SPARK-32537][SQL][TEST] Add a CTEHintSuite for test coverage

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29359:
URL: https://github.com/apache/spark/pull/29359#issuecomment-671133337






----------------------------------------------------------------
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 #29359: [SPARK-32537][SQL][TEST] Add a CTEHintSuite for test coverage

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29359:
URL: https://github.com/apache/spark/pull/29359#issuecomment-669092378


   **[Test build #127092 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127092/testReport)** for PR 29359 at commit [`773eacd`](https://github.com/apache/spark/commit/773eacdb3f2275863e3e521a6aa9a94178c5ce7c).


----------------------------------------------------------------
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 #29359: [SPARK-32537][SQL][TEST] Add a CTEHintSuite for test coverage

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29359:
URL: https://github.com/apache/spark/pull/29359#issuecomment-671133228


   **[Test build #127251 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127251/testReport)** for PR 29359 at commit [`a7c4d32`](https://github.com/apache/spark/commit/a7c4d32894d75696ee9a891219d9c99314ef05ed).


----------------------------------------------------------------
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 #29359: [SPARK-32537][SQL][TEST] Add a CTEHintSuite for test coverage

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29359:
URL: https://github.com/apache/spark/pull/29359#issuecomment-669797538






----------------------------------------------------------------
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] LantaoJin commented on pull request #29359: [SPARK-32537][SQL][TEST] Add a CTEHintSuite for test coverage

Posted by GitBox <gi...@apache.org>.
LantaoJin commented on pull request #29359:
URL: https://github.com/apache/spark/pull/29359#issuecomment-669749776


   cc @gatorsmile @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] cloud-fan closed pull request #29359: [SPARK-32537][SQL][TEST] Add a CTEHintSuite for test coverage

Posted by GitBox <gi...@apache.org>.
cloud-fan closed pull request #29359:
URL: https://github.com/apache/spark/pull/29359


   


----------------------------------------------------------------
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 #29359: [SPARK-32537][SQL][TEST] Add a CTEHintSuite for test coverage

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29359:
URL: https://github.com/apache/spark/pull/29359#issuecomment-669801085






----------------------------------------------------------------
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 #29359: [SPARK-32537][SQL][TEST] Add a CTEHintSuite for test coverage

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29359:
URL: https://github.com/apache/spark/pull/29359#issuecomment-670368548


   **[Test build #127187 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127187/testReport)** for PR 29359 at commit [`4256339`](https://github.com/apache/spark/commit/42563390f9b34c87c9aea1038e047b59c68dcaba).


----------------------------------------------------------------
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 #29359: [SPARK-32537][SQL][TEST] Add a CTEHintSuite for test coverage

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29359:
URL: https://github.com/apache/spark/pull/29359#issuecomment-671133337






----------------------------------------------------------------
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 #29359: [SPARK-32537][SQL][TEST] Add a CTEHintSuite for test coverage

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29359:
URL: https://github.com/apache/spark/pull/29359#issuecomment-670364861






----------------------------------------------------------------
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] LantaoJin commented on pull request #29359: [SPARK-32537][SQL][TEST] Add a CTEHintSuite for test coverage

Posted by GitBox <gi...@apache.org>.
LantaoJin commented on pull request #29359:
URL: https://github.com/apache/spark/pull/29359#issuecomment-670367525


   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.

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 #29359: [SPARK-32537][SQL][TEST] Add a CTEHintSuite for test coverage

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29359:
URL: https://github.com/apache/spark/pull/29359#issuecomment-669912839


   **[Test build #127134 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127134/testReport)** for PR 29359 at commit [`6f8a2cc`](https://github.com/apache/spark/commit/6f8a2cc74b91bbf8a15ff60a6e36e5281caaa4ca).
    * This patch passes all tests.
    * This patch merges cleanly.
    * This patch adds the following public classes _(experimental)_:
     * `class CTEHintSuite extends PlanTest with SharedSparkSession `


----------------------------------------------------------------
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 #29359: [SPARK-32537][SQL][TEST] Add a CTEHintSuite for test coverage

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29359:
URL: https://github.com/apache/spark/pull/29359#issuecomment-669092378


   **[Test build #127092 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127092/testReport)** for PR 29359 at commit [`773eacd`](https://github.com/apache/spark/commit/773eacdb3f2275863e3e521a6aa9a94178c5ce7c).


----------------------------------------------------------------
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] LantaoJin commented on a change in pull request #29359: [SPARK-32537][SQL][TEST] Add a CTEHintSuite for test coverage

Posted by GitBox <gi...@apache.org>.
LantaoJin commented on a change in pull request #29359:
URL: https://github.com/apache/spark/pull/29359#discussion_r466827455



##########
File path: sql/core/src/test/scala/org/apache/spark/sql/CTEHintSuite.scala
##########
@@ -0,0 +1,178 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql
+
+import org.apache.log4j.Level
+
+import org.apache.spark.sql.catalyst.plans.logical._
+import org.apache.spark.sql.test.SharedSparkSession
+
+class CTEHintSuite extends QueryTest with SharedSparkSession {
+
+  def verifyCoalesceHint(df: DataFrame): Unit = {

Review comment:
       Changed to `def verifyCoalesceOrRepartitionHint`




----------------------------------------------------------------
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 #29359: [SPARK-32537][SQL][TEST] Add a CTEHintSuite for test coverage

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29359:
URL: https://github.com/apache/spark/pull/29359#issuecomment-669913821






----------------------------------------------------------------
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 #29359: [SPARK-32537][SQL][TEST] Add a CTEHintSuite for test coverage

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29359:
URL: https://github.com/apache/spark/pull/29359#issuecomment-669913821






----------------------------------------------------------------
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 #29359: [SPARK-32537][SQL][TEST] Add a CTEHintSuite for test coverage

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29359:
URL: https://github.com/apache/spark/pull/29359#issuecomment-670329485






----------------------------------------------------------------
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 commented on a change in pull request #29359: [SPARK-32537][SQL][TEST] Add a CTEHintSuite for test coverage

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #29359:
URL: https://github.com/apache/spark/pull/29359#discussion_r466240736



##########
File path: sql/core/src/test/scala/org/apache/spark/sql/CTEHintSuite.scala
##########
@@ -0,0 +1,158 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql
+
+import org.apache.log4j.Level
+
+import org.apache.spark.sql.catalyst.plans.PlanTest
+import org.apache.spark.sql.catalyst.plans.logical.{BROADCAST, HintInfo, Join, JoinHint, Repartition, RepartitionByExpression, ResolvedHint, SHUFFLE_HASH, SHUFFLE_MERGE, SHUFFLE_REPLICATE_NL}
+import org.apache.spark.sql.execution.adaptive.AdaptiveSparkPlanHelper
+import org.apache.spark.sql.test.SharedSparkSession
+
+class CTEHintSuite extends PlanTest with SharedSparkSession with AdaptiveSparkPlanHelper {
+
+  def verifyCoalesceHint(df: DataFrame): Unit = {
+    val optimized = df.queryExecution.optimizedPlan
+    val repartitions = optimized collect {
+      case r: Repartition => r
+      case r: RepartitionByExpression => r
+      case _: ResolvedHint => fail("ResolvedHint should not appear after optimize.")
+    }
+    assert(repartitions.nonEmpty)
+  }
+
+  def verifyJoinHint(df: DataFrame, expectedHints: Seq[JoinHint]): Unit = {
+    val optimized = df.queryExecution.optimizedPlan
+    val joinHints = optimized collect {
+      case Join(_, _, _, _, hint) => hint
+      case _: ResolvedHint => fail("ResolvedHint should not appear after optimize.")
+    }
+    assert(joinHints == expectedHints)
+  }
+
+  def verifyJoinHintWithWarnings(
+      df: => DataFrame,
+      expectedHints: Seq[JoinHint],
+      warnings: Seq[String]): Unit = {
+    val logAppender = new LogAppender("join hints")
+    withLogAppender(logAppender) {
+      verifyJoinHint(df, expectedHints)
+    }
+    val warningMessages = logAppender.loggingEvents
+      .filter(_.getLevel == Level.WARN)
+      .map(_.getRenderedMessage)
+      .filter(_.contains("hint"))
+    assert(warningMessages.size == warnings.size)
+    warnings.foreach { w =>
+      assert(warningMessages.contains(w))
+    }
+  }
+
+  def msgNoJoinForJoinHint(strategy: String): String =
+    s"A join hint (strategy=$strategy) is specified but it is not part of a join relation."
+
+  def msgJoinHintOverridden(strategy: String): String =
+    s"Hint (strategy=$strategy) is overridden by another hint and will not take effect."
+
+  test("Resolve coalesce hint in CTE") {
+    // COALESCE,
+    // REPARTITION,
+    // REPARTITION_BY_RANGE
+    withTable("t") {
+      sql("CREATE TABLE t USING PARQUET AS SELECT 1 AS id")
+      verifyCoalesceHint(
+        sql("WITH cte AS (SELECT /*+ COALESCE(1) */ * FROM t) SELECT * FROM cte"))
+      verifyCoalesceHint(
+        sql("WITH cte AS (SELECT /*+ REPARTITION(3) */ * FROM t) SELECT * FROM cte"))
+      verifyCoalesceHint(
+        sql("WITH cte AS (SELECT /*+ REPARTITION(id) */ * FROM t) SELECT * FROM cte"))
+      verifyCoalesceHint(
+        sql("WITH cte AS (SELECT /*+ REPARTITION(3, id) */ * FROM t) SELECT * FROM cte"))
+      verifyCoalesceHint(
+        sql("WITH cte AS (SELECT /*+ REPARTITION_BY_RANGE(id) */ * FROM t) SELECT * FROM cte"))
+      verifyCoalesceHint(
+        sql("WITH cte AS (SELECT /*+ REPARTITION_BY_RANGE(3, id) */ * FROM t) SELECT * FROM cte"))
+    }
+  }
+
+  test("Resolve join hint in CTE") {
+    // BROADCAST,
+    // SHUFFLE_MERGE,
+    // SHUFFLE_HASH,
+    // SHUFFLE_REPLICATE_NL
+    withTable("t", "s") {
+      sql("CREATE TABLE a USING PARQUET AS SELECT 1 AS a1")
+      sql("CREATE TABLE b USING PARQUET AS SELECT 1 AS b1")
+      sql("CREATE TABLE c USING PARQUET AS SELECT 1 AS c1")
+      verifyJoinHint(
+        sql(
+          """
+            |WITH cte AS (
+            |  SELECT /*+ BROADCAST(a) */ * FROM a JOIN b ON a.a1 = b.b1
+            |)
+            |SELECT * FROM cte
+            |""".stripMargin),
+        JoinHint(
+          Some(HintInfo(strategy = Some(BROADCAST))),
+          None) :: Nil
+      )
+      verifyJoinHint(
+        sql(
+          """
+            |WITH cte AS (
+            |  SELECT /*+ SHUFFLE_HASH(a) */ * FROM a JOIN b ON a.a1 = b.b1
+            |)
+            |SELECT * FROM cte
+            |""".stripMargin),
+        JoinHint(
+          Some(HintInfo(strategy = Some(SHUFFLE_HASH))),
+          None) :: Nil
+      )
+      verifyJoinHintWithWarnings(
+        sql(
+          """
+            |WITH cte AS (
+            |  SELECT /*+ SHUFFLE_HASH MERGE(a, c) BROADCAST(a, b)*/ * FROM a, b, c
+            |  WHERE a.a1 = b.b1 AND b.b1 = c.c1
+            |)
+            |SELECT * FROM cte
+            |""".stripMargin),
+        JoinHint(
+          None,
+          Some(HintInfo(strategy = Some(SHUFFLE_MERGE)))) ::
+          JoinHint(
+            Some(HintInfo(strategy = Some(SHUFFLE_MERGE))),
+            Some(HintInfo(strategy = Some(BROADCAST)))) :: Nil,
+          msgNoJoinForJoinHint("shuffle_hash") ::
+            msgJoinHintOverridden("broadcast") :: Nil
+      )
+      verifyJoinHint(
+        sql(
+          """
+            |WITH cte AS (
+            |  SELECT /*+ SHUFFLE_REPLICATE_NL(a) SHUFFLE_HASH(b) */ * FROM a JOIN b ON a.a1 = b.b1
+            |)
+            |SELECT * FROM cte
+            |""".stripMargin),
+        JoinHint(
+          Some(HintInfo(strategy = Some(SHUFFLE_REPLICATE_NL))),
+          Some(HintInfo(strategy = Some(SHUFFLE_HASH)))) :: Nil
+      )
+    }
+  }
+}

Review comment:
       can we move https://github.com/apache/spark/pull/29062/files#diff-5a2e7f03d14856c8769fd3ddea8742bdR3564 to this new suite?




----------------------------------------------------------------
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] maropu commented on a change in pull request #29359: [SPARK-32537][SQL][TEST] Add a CTEHintSuite for test coverage

Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #29359:
URL: https://github.com/apache/spark/pull/29359#discussion_r466472027



##########
File path: sql/core/src/test/scala/org/apache/spark/sql/CTEHintSuite.scala
##########
@@ -0,0 +1,178 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql
+
+import org.apache.log4j.Level
+
+import org.apache.spark.sql.catalyst.plans.logical._
+import org.apache.spark.sql.test.SharedSparkSession
+
+class CTEHintSuite extends QueryTest with SharedSparkSession {
+
+  def verifyCoalesceHint(df: DataFrame): Unit = {

Review comment:
       nit: `Coalesce` -> `Repartition`?




----------------------------------------------------------------
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 #29359: [SPARK-32537][SQL][TEST] Add a CTEHintSuite for test coverage

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29359:
URL: https://github.com/apache/spark/pull/29359#issuecomment-671196982


   Test FAILed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/127251/
   Test FAILed.


----------------------------------------------------------------
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 #29359: [SPARK-32537][SQL][TEST] Add a CTEHintSuite for test coverage

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29359:
URL: https://github.com/apache/spark/pull/29359#issuecomment-669199951






----------------------------------------------------------------
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 commented on a change in pull request #29359: [SPARK-32537][SQL][TEST] Add a CTEHintSuite for test coverage

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #29359:
URL: https://github.com/apache/spark/pull/29359#discussion_r467064091



##########
File path: sql/core/src/test/scala/org/apache/spark/sql/CTEHintSuite.scala
##########
@@ -0,0 +1,181 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql
+
+import org.apache.log4j.Level
+
+import org.apache.spark.sql.catalyst.plans.logical._
+import org.apache.spark.sql.test.SharedSparkSession
+
+class CTEHintSuite extends QueryTest with SharedSparkSession {
+
+  def verifyCoalesceOrRepartitionHint(df: DataFrame): Unit = {
+    def checkContainsRepartition(plan: LogicalPlan): Unit = {
+      val repartitions = plan collect {
+        case r: Repartition => r
+        case r: RepartitionByExpression => r
+      }
+      assert(repartitions.nonEmpty)
+    }
+    val analyzed = df.queryExecution.analyzed
+    val optimized = df.queryExecution.optimizedPlan
+    checkContainsRepartition(analyzed)
+    checkContainsRepartition(optimized)
+    optimized collect {
+      case _: ResolvedHint => fail("ResolvedHint should not appear after optimize.")
+    }
+  }
+
+  def verifyJoinHint(df: DataFrame, expectedHints: Seq[JoinHint]): Unit = {
+    val analyzed = df.queryExecution.analyzed
+    val resolvedHints = analyzed collect {
+      case r: ResolvedHint => r
+    }
+    assert(resolvedHints.nonEmpty)
+    val optimized = df.queryExecution.optimizedPlan
+    val joinHints = optimized collect {
+      case Join(_, _, _, _, hint) => hint
+      case _: ResolvedHint => fail("ResolvedHint should not appear after optimize.")
+    }
+    assert(joinHints == expectedHints)
+  }
+
+  def verifyJoinHintWithWarnings(
+      df: => DataFrame,
+      expectedHints: Seq[JoinHint],
+      warnings: Seq[String]): Unit = {
+    val logAppender = new LogAppender("join hints")
+    withLogAppender(logAppender) {
+      verifyJoinHint(df, expectedHints)
+    }
+    val warningMessages = logAppender.loggingEvents
+      .filter(_.getLevel == Level.WARN)
+      .map(_.getRenderedMessage)
+      .filter(_.contains("hint"))
+    assert(warningMessages.size == warnings.size)
+    warnings.foreach { w =>
+      assert(warningMessages.contains(w))
+    }
+  }
+
+  def msgNoJoinForJoinHint(strategy: String): String =
+    s"A join hint (strategy=$strategy) is specified but it is not part of a join relation."
+
+  def msgJoinHintOverridden(strategy: String): String =
+    s"Hint (strategy=$strategy) is overridden by another hint and will not take effect."
+
+  test("Resolve coalesce hint in CTE") {
+    // COALESCE,
+    // REPARTITION,
+    // REPARTITION_BY_RANGE
+    withTable("t") {
+      sql("CREATE TABLE t USING PARQUET AS SELECT 1 AS id")
+      verifyCoalesceOrRepartitionHint(
+        sql("WITH cte AS (SELECT /*+ COALESCE(1) */ * FROM t) SELECT * FROM cte"))
+      verifyCoalesceOrRepartitionHint(
+        sql("WITH cte AS (SELECT /*+ REPARTITION(3) */ * FROM t) SELECT * FROM cte"))
+      verifyCoalesceOrRepartitionHint(
+        sql("WITH cte AS (SELECT /*+ REPARTITION(id) */ * FROM t) SELECT * FROM cte"))
+      verifyCoalesceOrRepartitionHint(
+        sql("WITH cte AS (SELECT /*+ REPARTITION(3, id) */ * FROM t) SELECT * FROM cte"))
+      verifyCoalesceOrRepartitionHint(
+        sql("WITH cte AS (SELECT /*+ REPARTITION_BY_RANGE(id) */ * FROM t) SELECT * FROM cte"))
+      verifyCoalesceOrRepartitionHint(
+        sql("WITH cte AS (SELECT /*+ REPARTITION_BY_RANGE(3, id) */ * FROM t) SELECT * FROM cte"))
+    }
+  }
+
+  test("Resolve join hint in CTE") {
+    // BROADCAST,
+    // SHUFFLE_MERGE,
+    // SHUFFLE_HASH,
+    // SHUFFLE_REPLICATE_NL
+    withTable("t", "s") {
+      sql("CREATE TABLE a USING PARQUET AS SELECT 1 AS a1")
+      sql("CREATE TABLE b USING PARQUET AS SELECT 1 AS b1")
+      sql("CREATE TABLE c USING PARQUET AS SELECT 1 AS c1")
+      verifyJoinHint(
+        sql(
+          """
+            |WITH cte AS (
+            |  SELECT /*+ BROADCAST(a) */ * FROM a JOIN b ON a.a1 = b.b1
+            |)
+            |SELECT * FROM cte
+            |""".stripMargin),
+        JoinHint(
+          Some(HintInfo(strategy = Some(BROADCAST))),
+          None) :: Nil
+      )
+      verifyJoinHint(
+        sql(
+          """
+            |WITH cte AS (
+            |  SELECT /*+ SHUFFLE_HASH(a) */ * FROM a JOIN b ON a.a1 = b.b1
+            |)
+            |SELECT * FROM cte
+            |""".stripMargin),
+        JoinHint(
+          Some(HintInfo(strategy = Some(SHUFFLE_HASH))),
+          None) :: Nil
+      )
+      verifyJoinHintWithWarnings(
+        sql(
+          """
+            |WITH cte AS (
+            |  SELECT /*+ SHUFFLE_HASH MERGE(a, c) BROADCAST(a, b)*/ * FROM a, b, c
+            |  WHERE a.a1 = b.b1 AND b.b1 = c.c1
+            |)
+            |SELECT * FROM cte
+            |""".stripMargin),
+        JoinHint(
+          None,
+          Some(HintInfo(strategy = Some(SHUFFLE_MERGE)))) ::
+          JoinHint(
+            Some(HintInfo(strategy = Some(SHUFFLE_MERGE))),
+            Some(HintInfo(strategy = Some(BROADCAST)))) :: Nil,
+          msgNoJoinForJoinHint("shuffle_hash") ::
+            msgJoinHintOverridden("broadcast") :: Nil
+      )
+      verifyJoinHint(
+        sql(
+          """
+            |WITH cte AS (
+            |  SELECT /*+ SHUFFLE_REPLICATE_NL(a) SHUFFLE_HASH(b) */ * FROM a JOIN b ON a.a1 = b.b1
+            |)
+            |SELECT * FROM cte
+            |""".stripMargin),
+        JoinHint(
+          Some(HintInfo(strategy = Some(SHUFFLE_REPLICATE_NL))),
+          Some(HintInfo(strategy = Some(SHUFFLE_HASH)))) :: Nil
+      )
+    }
+  }
+
+  test("SPARK-32237: Hint in CTE") {

Review comment:
       Actually we can remove it now. It's already covered by `Resolve coalesce hint in CTE`.




----------------------------------------------------------------
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 #29359: [SPARK-32537][SQL][TEST] Add a CTEHintSuite for test coverage

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29359:
URL: https://github.com/apache/spark/pull/29359#issuecomment-669801085






----------------------------------------------------------------
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 #29359: [SPARK-32537][SQL][TEST] Add a CTEHintSuite for test coverage

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29359:
URL: https://github.com/apache/spark/pull/29359#issuecomment-670329485






----------------------------------------------------------------
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 #29359: [SPARK-32537][SQL][TEST] Add a CTEHintSuite for test coverage

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29359:
URL: https://github.com/apache/spark/pull/29359#issuecomment-670369069






----------------------------------------------------------------
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 #29359: [SPARK-32537][SQL][TEST] Add a CTEHintSuite for test coverage

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29359:
URL: https://github.com/apache/spark/pull/29359#issuecomment-671196608


   **[Test build #127251 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127251/testReport)** for PR 29359 at commit [`a7c4d32`](https://github.com/apache/spark/commit/a7c4d32894d75696ee9a891219d9c99314ef05ed).
    * This patch **fails due to an unknown error code, -9**.
    * 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] AmplabJenkins commented on pull request #29359: [SPARK-32537][SQL][TEST] Add a CTEHintSuite for test coverage

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29359:
URL: https://github.com/apache/spark/pull/29359#issuecomment-669797538






----------------------------------------------------------------
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 #29359: [SPARK-32537][SQL][TEST] Add a CTEHintSuite for test coverage

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29359:
URL: https://github.com/apache/spark/pull/29359#issuecomment-671196968


   Merged build finished. Test FAILed.


----------------------------------------------------------------
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 #29359: [SPARK-32537][SQL][TEST] Add a CTEHintSuite for test coverage

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29359:
URL: https://github.com/apache/spark/pull/29359#issuecomment-670483456


   **[Test build #127187 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127187/testReport)** for PR 29359 at commit [`4256339`](https://github.com/apache/spark/commit/42563390f9b34c87c9aea1038e047b59c68dcaba).
    * 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] AmplabJenkins commented on pull request #29359: [SPARK-32537][SQL][TEST] Add a CTEHintSuite for test coverage

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29359:
URL: https://github.com/apache/spark/pull/29359#issuecomment-669092955






----------------------------------------------------------------
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 commented on pull request #29359: [SPARK-32537][SQL][TEST] Add a CTEHintSuite for test coverage

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #29359:
URL: https://github.com/apache/spark/pull/29359#issuecomment-671374450


   github action passed, merging to master, thanks!


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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