[SPARK-33426][SQL][TESTS] Unify Hive SHOW TABLES tests #30340

MaxGekk · 2020-11-11T19:39:41Z

What changes were proposed in this pull request?

Create the separate test suite org.apache.spark.sql.hive.execution.command.ShowTablesSuite.
Re-use V1 SHOW TABLES tests added by [SPARK-33382][SQL][TESTS] Unify datasource v1 and v2 SHOW TABLES tests #30287 in the Hive test suites.
Add new test case for the pattern 'table_name_1*|table_name_2*' in the common test suite.

Why are the changes needed?

To test V1 + common SHOW TABLES tests in Hive.

Does this PR introduce any user-facing change?

No

How was this patch tested?

By running v1/v2 and Hive v1 ShowTablesSuite:

$  build/sbt -Phive-2.3 -Phive-thriftserver "test:testOnly *ShowTablesSuite"

MaxGekk · 2020-11-11T19:40:06Z

@cloud-fan @HyukjinKwon Could you take a look at this PR, please.

SparkQA · 2020-11-11T20:25:06Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35553/

SparkQA · 2020-11-11T20:46:35Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35553/

SparkQA · 2020-11-12T00:06:10Z

Test build #130947 has finished for PR 30340 at commit 4fb6ddc.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

cloud-fan · 2020-11-12T04:59:25Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/command/ShowTablesSuite.scala

+  override protected val spark: SparkSession = TestHive.sparkSession
+  protected override def beforeAll(): Unit = {}
+
+  test("show Hive tables") {


is it specific to hive tables? if not we should put it in the base trait.

They are not. The difference is probably in omitting explicit USING HIVE.

that's not something SHOW TABLES should care about. Let's simply add a test in the base trait for complex patterns and this suite just extends the base trait.

cloud-fan · 2020-11-12T05:00:33Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/command/ShowTablesSuite.scala

+        "SHOW TABLES IN default 'show1*'",
+        ShowRow("default", "show1a", false) :: Nil)
+      runShowTablesSql(
+        "SHOW TABLES IN default 'show1*|show2*'",


I think this is the only place that improves test coverage. Maybe we should just add one more test in the base trait to test complicated patterns.

Yeh, it seems only this test case brings some benefits. Let me move it the common trait, and remove all those duplicate checks.

MaxGekk · 2020-11-12T07:52:58Z

sql/core/src/test/scala/org/apache/spark/sql/execution/command/ShowTablesSuite.scala

  }

  test("show tables with a pattern") {
-    withNamespace(s"$catalog.ns1", s"$catalog.ns2") {


ns2 is not used in the test actually

I will revert ns2 back to check that tables from the namespace are not showed.

MaxGekk · 2020-11-12T07:53:39Z

sql/core/src/test/scala/org/apache/spark/sql/execution/command/ShowTablesSuite.scala

    val df = spark.sql(sqlText)
    assert(df.schema === showSchema)
-    assert(df.collect() === getRows(expected))
+    assert(df.collect().toSet === getRows(expected).toSet)


After modifications in show tables with a pattern, the order of rows was changed.

how about checkAnswer(df, getRows(expected))

Replaced it by checkAnswer()

SparkQA · 2020-11-12T08:05:02Z

Test build #130978 has finished for PR 30340 at commit 8dfd9c7.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2020-11-12T08:36:07Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35584/

MaxGekk · 2020-11-12T08:52:36Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/command/ShowTablesSuite.scala

+  override def version: String = "Hive V1"
+  override def defaultUsing: String = "USING HIVE"
+  override protected val spark: SparkSession = TestHive.sparkSession
+  protected override def beforeAll(): Unit = {}


I had to override this otherwise tests failed with:

SHOW TABLES Hive V1: change current catalog and namespace with USE statements *** FAILED *** (17 milliseconds) [info] java.lang.IllegalStateException: LiveListenerBus is stopped. [info] at org.apache.spark.scheduler.LiveListenerBus.addToQueue(LiveListenerBus.scala:97) [info] at org.apache.spark.scheduler.LiveListenerBus.addToStatusQueue(LiveListenerBus.scala:80) ... [info] Cause: java.lang.IllegalStateException: LiveListenerBus is stopped. [info] at org.apache.spark.scheduler.LiveListenerBus.addToQueue(LiveListenerBus.scala:97) [info] at org.apache.spark.scheduler.LiveListenerBus.addToStatusQueue(LiveListenerBus.scala:80) [info] at org.apache.spark.sql.internal.SharedState.<init>(SharedState.scala:103) [info] at org.apache.spark.sql.hive.test.TestHiveSharedState.<init>(TestHive.scala:99) [info] at org.apache.spark.sql.hive.test.TestHiveSparkSession.$anonfun$sharedState$1(TestHive.scala:222) [info] at scala.Option.getOrElse(Option.scala:189) [info] at org.apache.spark.sql.hive.test.TestHiveSparkSession.sharedState$lzycompute(TestHive.scala:222) [info] at org.apache.spark.sql.hive.test.TestHiveSparkSession.sharedState(TestHive.scala:221) [info] at org.apache.spark.sql.hive.test.TestHiveSparkSession.sharedState(TestHive.scala:174) [info] at org.apache.spark.sql.internal.BaseSessionStateBuilder.build(BaseSessionStateBuilder.scala:333) [info] at org.apache.spark.sql.hive.test.TestHiveSparkSession.sessionState$lzycompute(TestHive.scala:227) [info] at org.apache.spark.sql.hive.test.TestHiveSparkSession.sessionState(TestHive.scala:226) [info] at org.apache.spark.sql.hive.test.TestHiveSparkSession.$anonfun$sql$1(TestHive.scala:240) [info] at org.apache.spark.sql.SparkSession.withActive(SparkSession.scala:768) [info] at org.apache.spark.sql.hive.test.TestHiveSparkSession.sql(TestHive.scala:239)

MaxGekk · 2020-11-12T08:55:06Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/command/ShowTablesSuite.scala

+class ShowTablesSuite extends v1.ShowTablesSuite {
+  override def version: String = "Hive V1"
+  override def defaultUsing: String = "USING HIVE"
+  override protected val spark: SparkSession = TestHive.sparkSession


We could take this from TestHiveSingleton but when I mix the trait:

[error] /Users/maximgekk/proj/show-tables-hive-tests/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/command/ShowTablesSuite.scala:24:7: class ShowTablesSuite inherits conflicting members: [error] method spark in trait SharedSparkSessionBase of type => org.apache.spark.sql.SparkSession and [error] value spark in trait TestHiveSingleton of type org.apache.spark.sql.SparkSession [error] (Note: this can be resolved by declaring an override in class ShowTablesSuite.) [error] class ShowTablesSuite extends v1.ShowTablesSuite with TestHiveSingleton { [error] ^ [error] one error found

SparkQA · 2020-11-12T08:57:49Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35584/

SparkQA · 2020-11-12T10:43:50Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35590/

SparkQA · 2020-11-12T11:06:46Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35590/

SparkQA · 2020-11-12T11:32:46Z

Test build #130984 has finished for PR 30340 at commit fb74566.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
trait ShowTablesSuite extends QueryTest with SharedSparkSession

SparkQA · 2020-11-12T12:11:47Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35597/

SparkQA · 2020-11-12T12:42:47Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35600/

SparkQA · 2020-11-12T12:46:09Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35597/

SparkQA · 2020-11-12T13:04:38Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35600/

SparkQA · 2020-11-12T15:37:20Z

Test build #130991 has finished for PR 30340 at commit bd0f785.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
trait ShowTablesSuite extends QueryTest with SQLTestUtils
trait ShowTablesTests extends CommonShowTablesSuite
class ShowTablesSuite extends CommonShowTablesSuite with SharedSparkSession
class ShowTablesSuite extends v1.ShowTablesTests with TestHiveSingleton

SparkQA · 2020-11-12T17:04:11Z

Test build #130994 has finished for PR 30340 at commit 7eab687.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
trait ShowTablesSuiteBase extends QueryTest with SQLTestUtils
case class ShowRow(namespace: String, table: String, isTemporary: Boolean)
trait ShowTablesSuiteBase extends command.ShowTablesSuiteBase
class ShowTablesSuite extends command.ShowTablesSuiteBase with SharedSparkSession
class ShowTablesSuite extends v1.ShowTablesSuiteBase with TestHiveSingleton

cloud-fan · 2020-11-12T17:37:03Z

retest this please

SparkQA · 2020-11-12T18:38:54Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35621/

SparkQA · 2020-11-12T19:02:58Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35621/

SparkQA · 2020-11-12T22:27:42Z

Test build #131015 has finished for PR 30340 at commit 7eab687.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
trait ShowTablesSuiteBase extends QueryTest with SQLTestUtils
case class ShowRow(namespace: String, table: String, isTemporary: Boolean)
trait ShowTablesSuiteBase extends command.ShowTablesSuiteBase
class ShowTablesSuite extends command.ShowTablesSuiteBase with SharedSparkSession
class ShowTablesSuite extends v1.ShowTablesSuiteBase with TestHiveSingleton

cloud-fan · 2020-11-13T05:14:54Z

thanks, merging to master!

MaxGekk added 2 commits November 11, 2020 22:25

Add Hive ShowTablesSuite

65fd9c3

Set the default namespace

4fb6ddc

github-actions bot added the SQL label Nov 11, 2020

cloud-fan reviewed Nov 12, 2020

View reviewed changes

Move Hive tests to the common trait

8dfd9c7

MaxGekk commented Nov 12, 2020

View reviewed changes

MaxGekk added 2 commits November 12, 2020 11:28

Revert back ns2 in a test

c856b89

Use checkAnswer() in runShowTablesSql()

fb74566

MaxGekk commented Nov 12, 2020

View reviewed changes

cloud-fan approved these changes Nov 12, 2020

View reviewed changes

Avoid the hask with overriding the beforeAll() method

bd0f785

Refactoring: rename classes/traits

7eab687

cloud-fan approved these changes Nov 12, 2020

View reviewed changes

cloud-fan closed this in 539c2de Nov 13, 2020

MaxGekk deleted the show-tables-hive-tests branch February 19, 2021 15:03

[SPARK-33426][SQL][TESTS] Unify Hive SHOW TABLES tests #30340

[SPARK-33426][SQL][TESTS] Unify Hive SHOW TABLES tests #30340

Uh oh!

Conversation

MaxGekk commented Nov 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

MaxGekk commented Nov 11, 2020

Uh oh!

SparkQA commented Nov 11, 2020

Uh oh!

SparkQA commented Nov 11, 2020

Uh oh!

SparkQA commented Nov 12, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Nov 12, 2020

Uh oh!

SparkQA commented Nov 12, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Nov 12, 2020

Uh oh!

SparkQA commented Nov 12, 2020

Uh oh!

SparkQA commented Nov 12, 2020

Uh oh!

SparkQA commented Nov 12, 2020

Uh oh!

SparkQA commented Nov 12, 2020

Uh oh!

SparkQA commented Nov 12, 2020

Uh oh!

SparkQA commented Nov 12, 2020

Uh oh!

SparkQA commented Nov 12, 2020

Uh oh!

SparkQA commented Nov 12, 2020

Uh oh!

SparkQA commented Nov 12, 2020

Uh oh!

cloud-fan commented Nov 12, 2020

Uh oh!

SparkQA commented Nov 12, 2020

Uh oh!

SparkQA commented Nov 12, 2020

Uh oh!

SparkQA commented Nov 12, 2020

Uh oh!

cloud-fan commented Nov 13, 2020

Uh oh!

Reviewers

Assignees

Labels

MaxGekk commented Nov 11, 2020 •

edited

Loading