[SPARK-37031][SQL][TESTS] Unify v1 and v2 DESCRIBE NAMESPACE tests #34305

imback82 · 2021-10-17T18:23:52Z

What changes were proposed in this pull request?

Move DESCRIBE DATABASE/NAMESPACE parsing tests to DescribeNamespaceParserSuite.
Put common DESCRIBE NAMESPACE tests into one trait org.apache.spark.sql.execution.command. DescribeNamespaceSuiteBase, and put datasource specific tests to the v1.DescribeNamespaceSuite and v2.DescribeNamespaceSuite.

The changes follow the approach of #30287.

Why are the changes needed?

The unification will allow to run common DESCRIBE NAMESPACE tests for both DSv1/Hive DSv1 and DSv2
We can detect missing features and differences between DSv1 and DSv2 implementations.

Does this PR introduce any user-facing change?

No

How was this patch tested?

Existing unit tests and new tests.

SparkQA · 2021-10-17T19:14:58Z

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

imback82 · 2021-10-17T19:37:05Z

cc @cloud-fan @MaxGekk

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

SparkQA · 2021-10-17T20:14:52Z

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

SparkQA · 2021-10-17T23:36:51Z

Test build #144332 has finished for PR 34305 at commit d44f56e.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
trait CommandTestUtils extends SQLTestUtils
abstract class DDLSuite extends QueryTest with CommandTestUtils
class DescribeNamespaceParserSuite extends AnalysisTest
trait DescribeNamespaceSuiteBase extends QueryTest with DDLCommandTestUtils with CommandTestUtils
trait DescribeNamespaceSuiteBase extends command.DescribeNamespaceSuiteBase
class DescribeNamespaceSuite extends command.DescribeNamespaceSuiteBase with CommandSuiteBase

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

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

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

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

SparkQA · 2021-10-24T19:40:30Z

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

SparkQA · 2021-10-24T20:24:00Z

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

SparkQA · 2021-10-25T00:05:09Z

Test build #144568 has finished for PR 34305 at commit 2640444.

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

cloud-fan · 2021-10-25T08:49:55Z

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

+
+    assert(message.contains(s"$notFoundMsgPrefix '$ns' not found"))
+
+    sql(s"DROP NAMESPACE IF EXISTS $catalog.$ns")


We don't need to test DROP NAMESPACE here

This is testing whether IF EXISTS is respected when the namespace doesn't exist (existing test). Do we still want to remove it?

Well, it should belong to DropNamespaceSuite if we have one.

OK seems we don't have existing tests cover this case. Let's add it back with a TODO to move it to DropNameSpaceSuite in the future when we have one.

cloud-fan · 2021-10-25T08:52:05Z

...ore/src/test/scala/org/apache/spark/sql/execution/command/DescribeNamespaceParserSuite.scala

+  test("describe namespace") {
+    val sql1 = "DESCRIBE NAMESPACE EXTENDED a.b"
+    val sql2 = "DESCRIBE NAMESPACE a.b"
+    val sql3 = "DESCRIBE NAMESPACE cat.`database`"


let's test that DESCRIBE DATABASE, DESCRIBE SCHEMA also work

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

…/DescribeNamespaceSuiteBase.scala

SparkQA · 2021-10-25T09:05:37Z

Test build #144583 has finished for PR 34305 at commit cecd0a6.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2021-10-25T09:12:02Z

Kubernetes integration test unable to build dist.

exiting with code: 1
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49054/

SparkQA · 2021-10-25T17:02:51Z

Test build #144595 has started for PR 34305 at commit a8820ab.

imback82 · 2021-10-26T15:14:15Z

retest this please

SparkQA · 2021-10-26T15:18:50Z

Test build #144624 has started for PR 34305 at commit a8820ab.

SparkQA · 2021-10-26T15:59:49Z

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

SparkQA · 2021-10-26T16:59:52Z

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

imback82 · 2021-10-26T17:07:32Z

GA shows tests are passing: https://github.com/imback82/spark-4/actions/runs/1382032814

cloud-fan · 2021-10-27T04:15:35Z

thanks, merging to master!

### What changes were proposed in this pull request? 1. Move SHOW TBLPROPERTIES parsing tests to `ShowTblPropertiesParserSuite`. 2. Define the class `command.ShowTblPropertiesSuiteBase` that is parent of `v1.ShowTblPropertiesSuiteBase` and `v2.ShowTblPropertiesSuite`. 3. Define the class `v1.ShowTblPropertiesSuiteBase` that is parent of `v1.ShowTblPropertiesSuite` and `hive.execution.command.ShowTblPropertiesSuite`. 4. move test case from `DataSourceV2SQLSessionCatalogSuite` to `command.ShowTblPropertiesSuiteBase` to run for V2/V1/HIVE V1 5. move some test case from `HiveCommandSuite` to `command.ShowTblPropertiesSuiteBase` 6. move some test case from `HiveCommandSuite` to `v1.ShowTblPropertiesSuiteBase ` to run for V1/HIVE V1 7. Add two test case for `v1.ShowTblPropertiesSuite ` for view. The changes follow the approach of [#30287](#30287) [#34305](#34305) ### Why are the changes needed? 1. The unification will allow to run common `SHOW TBLPROPERTIES` tests for both DSv1/Hive DSv1 and DSv2 2. We can detect missing features and differences between DSv1 and DSv2 implementations. ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? By running v1/v2 and Hive v1 ShowTblPropertiesSuite: $ build/sbt -Phive-2.3 -Phive-thriftserver "test:testOnly *ShowTblPropertiesSuite" Closes #34476 from Peng-Lei/SPARK-37195. Authored-by: PengLei <[email protected]> Signed-off-by: Wenchen Fan <[email protected]>

### What changes were proposed in this pull request? unify test case: 1. Move the testcase from `DDLParserSuite.scala` to `ShowCreateTableParserSuite.scala`. 2. Move the testcase from `DataSourceV2SQLSuite` to `v2.ShowCreateTableSuite`. If the test case also work with V1/Hive, then move to ShowCreateTableSuiteBase 3. Move the testcase from `sql/ShowCreateTableSuite.scala` to `v1.ShowCreateTableSuite` If the test case also work with Hive, then move to `v1.ShowCreateTableSuiteBase` If the test case also work with V2/Hive, then move to `ShowCreateTableSuiteBase` 4. Move the testcase from `HiveShowCreateTableSuite` to `hive.ShowCreateTableSuite` If the test case also work with V1/V2, then move to `ShowCreateTableSuiteBase` If the test case also work with V1, then move to `v1.ShowCreateTableSuiteBase` 5. Use `getShowCreateDDL` instead of `checkCreateTable` to check the result. fix diff behavior: 1. Add one space after `create table xxx` for the command `SHOW CREATE TABLE AS SERDE` to unify the output. 2. Add one space after `OPTIONS` for v2 command `SHOW CREATE TABLE` to unify the output. The changes follow the approach of [#30287](#30287) [#34305](#34305) ### Why are the changes needed? 1. The unification will allow to run common `SHOW CREATE TABLE` tests for both DSv1/Hive DSv1 and DSv2 2. We can detect missing features and differences between DSv1 and DSv2 implementations. ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? $ build/sbt -Phive-2.3 -Phive-thriftserver "test:testOnly *ShowCreateTableSuite" Closes #34719 from Peng-Lei/SPARK-37381. Authored-by: PengLei <[email protected]> Signed-off-by: Wenchen Fan <[email protected]>

### What changes were proposed in this pull request? unify test case: 1. Move the testcase from `DDLParserSuite.scala` to `ShowCreateTableParserSuite.scala`. 2. Move the testcase from `DataSourceV2SQLSuite` to `v2.ShowCreateTableSuite`. If the test case also work with V1/Hive, then move to ShowCreateTableSuiteBase 3. Move the testcase from `sql/ShowCreateTableSuite.scala` to `v1.ShowCreateTableSuite` If the test case also work with Hive, then move to `v1.ShowCreateTableSuiteBase` If the test case also work with V2/Hive, then move to `ShowCreateTableSuiteBase` 4. Move the testcase from `HiveShowCreateTableSuite` to `hive.ShowCreateTableSuite` If the test case also work with V1/V2, then move to `ShowCreateTableSuiteBase` If the test case also work with V1, then move to `v1.ShowCreateTableSuiteBase` 5. Use `getShowCreateDDL` instead of `checkCreateTable` to check the result. fix diff behavior: 1. Add one space after `create table xxx` for the command `SHOW CREATE TABLE AS SERDE` to unify the output. 2. Add one space after `OPTIONS` for v2 command `SHOW CREATE TABLE` to unify the output. The changes follow the approach of [apache#30287](apache#30287) [apache#34305](apache#34305) ### Why are the changes needed? 1. The unification will allow to run common `SHOW CREATE TABLE` tests for both DSv1/Hive DSv1 and DSv2 2. We can detect missing features and differences between DSv1 and DSv2 implementations. ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? $ build/sbt -Phive-2.3 -Phive-thriftserver "test:testOnly *ShowCreateTableSuite" Closes apache#34719 from Peng-Lei/SPARK-37381. Authored-by: PengLei <[email protected]> Signed-off-by: Wenchen Fan <[email protected]>

initial commit

d44f56e

github-actions bot added the SQL label Oct 17, 2021

imback82 commented Oct 17, 2021

View reviewed changes

sql/core/src/test/scala/org/apache/spark/sql/execution/command/DescribeNamespaceSuiteBase.scala Outdated Show resolved Hide resolved

cloud-fan reviewed Oct 18, 2021

View reviewed changes

sql/core/src/test/scala/org/apache/spark/sql/execution/command/DescribeNamespaceSuiteBase.scala Outdated Show resolved Hide resolved

cloud-fan reviewed Oct 18, 2021

View reviewed changes

sql/core/src/test/scala/org/apache/spark/sql/execution/command/v1/DescribeNamespaceSuite.scala Outdated Show resolved Hide resolved

cloud-fan reviewed Oct 18, 2021

View reviewed changes

sql/core/src/test/scala/org/apache/spark/sql/execution/command/v1/DescribeNamespaceSuite.scala Outdated Show resolved Hide resolved

MaxGekk reviewed Oct 18, 2021

View reviewed changes

sql/core/src/test/scala/org/apache/spark/sql/execution/command/DescribeNamespaceSuiteBase.scala Outdated Show resolved Hide resolved

sql/core/src/test/scala/org/apache/spark/sql/execution/command/v2/DescribeNamespaceSuite.scala Outdated Show resolved Hide resolved

imback82 added 4 commits October 24, 2021 11:04

Merge branch 'master' into migrate_desc_namespace

ab5532c

address PR comments

69ab022

minor fix

002b4e7

update variable name

2640444

cloud-fan reviewed Oct 25, 2021

View reviewed changes

sql/core/src/test/scala/org/apache/spark/sql/execution/command/DescribeNamespaceSuiteBase.scala Show resolved Hide resolved

cloud-fan added 2 commits October 25, 2021 17:00

Update DescribeNamespaceParserSuite.scala

174aedd

Update sql/core/src/test/scala/org/apache/spark/sql/execution/command…

cecd0a6

…/DescribeNamespaceSuiteBase.scala

cloud-fan approved these changes Oct 25, 2021

View reviewed changes

imback82 added 2 commits October 25, 2021 09:22

fix compilation error

a5c1492

add back with TODO

a8820ab

cloud-fan closed this in d2aeba6 Oct 27, 2021

Peng-Lei mentioned this pull request Nov 3, 2021

[SPARK-37195][SQL][TESTS] Unify v1 and v2 SHOW TBLPROPERTIES tests #34476

Closed

Peng-Lei mentioned this pull request Nov 26, 2021

[SPARK-37381][SQL] Unify v1 and v2 SHOW CREATE TABLE tests #34719

Closed


		assert(message.contains(s"$notFoundMsgPrefix '$ns' not found"))

		sql(s"DROP NAMESPACE IF EXISTS $catalog.$ns")

[SPARK-37031][SQL][TESTS] Unify v1 and v2 DESCRIBE NAMESPACE tests #34305

[SPARK-37031][SQL][TESTS] Unify v1 and v2 DESCRIBE NAMESPACE tests #34305

Uh oh!

Conversation

imback82 commented Oct 17, 2021

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!

SparkQA commented Oct 17, 2021

Uh oh!

imback82 commented Oct 17, 2021

Uh oh!

Uh oh!

SparkQA commented Oct 17, 2021

Uh oh!

SparkQA commented Oct 17, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

SparkQA commented Oct 24, 2021

Uh oh!

SparkQA commented Oct 24, 2021

Uh oh!

SparkQA commented Oct 25, 2021

Uh oh!

cloud-fan Oct 25, 2021

Choose a reason for hiding this comment

Uh oh!

imback82 Oct 25, 2021

Choose a reason for hiding this comment

Uh oh!

cloud-fan Oct 25, 2021

Choose a reason for hiding this comment

Uh oh!

cloud-fan Oct 25, 2021

Choose a reason for hiding this comment

Uh oh!

cloud-fan Oct 25, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

SparkQA commented Oct 25, 2021

Uh oh!

SparkQA commented Oct 25, 2021

Uh oh!

SparkQA commented Oct 25, 2021

Uh oh!

imback82 commented Oct 26, 2021

Uh oh!

SparkQA commented Oct 26, 2021

Uh oh!

SparkQA commented Oct 26, 2021

Uh oh!

SparkQA commented Oct 26, 2021

Uh oh!

imback82 commented Oct 26, 2021

Uh oh!

cloud-fan commented Oct 27, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants