[SPARK-33137][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (Postgres dialect) #30089

huaxingao · 2020-10-18T23:16:02Z

What changes were proposed in this pull request?

Override the default SQL strings in Postgres Dialect for:

ALTER TABLE UPDATE COLUMN TYPE
ALTER TABLE UPDATE COLUMN NULLABILITY

Add new docker integration test suite jdbc/v2/PostgreSQLIntegrationSuite.scala

Why are the changes needed?

supports Postgres specific ALTER TABLE syntax.

Does this PR introduce any user-facing change?

No

How was this patch tested?

Add new test PostgreSQLIntegrationSuite

SparkQA · 2020-10-18T23:55:59Z

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

SparkQA · 2020-10-19T00:07:50Z

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

SparkQA · 2020-10-19T00:17:37Z

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

SparkQA · 2020-10-19T00:29:01Z

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

SparkQA · 2020-10-19T01:52:09Z

Test build #129970 has finished for PR 30089 at commit 9ca0e1c.

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

huaxingao · 2020-10-19T02:45:36Z

cc @cloud-fan @MaxGekk thanks

cloud-fan · 2020-10-22T07:47:22Z

external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/v2/V2JDBCTest.scala

+      sql(s"ALTER TABLE $catalogName.alt_table DROP COLUMN c3")
+      val t = spark.table(s"$catalogName.alt_table")
+      val expectedSchema = catalogName match {
+        case "oracle" => new StructType().add("C2", DecimalType(10, 0))


what happens in Oracle?

Oracle uses NUMBER(10) for integer. Please see https://docs.oracle.com/cd/E19501-01/819-3659/gcmaz/

…te type and nullability of columns (PostgreSQL dialect)

huaxingao · 2020-10-22T16:14:23Z

external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/v2/V2JDBCTest.scala

    assert(msg.contains("Table not found"))
  }

+  test("SPARK-33034: ALTER TABLE ... rename column") {


MySQLIntegrationSuite fails on this test. The docker image we have for MySQL is 5.7.31. Seems MySQL 5.7 doesn't support syntax ALTER TABLE tbl_name RENAME COLUMN old_col_name TO new_col_name but 8.0 does.
https://dev.mysql.com/doc/refman/5.7/en/alter-table.html
https://dev.mysql.com/doc/refman/8.0/en/alter-table.html

We can either upgrade the MySQL version in docker test, or override the test in MySQLIntegrationSuite to assert the failure.

SparkQA · 2020-10-22T16:43:05Z

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

SparkQA · 2020-10-22T17:06:47Z

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

SparkQA · 2020-10-22T18:42:04Z

Test build #130167 has finished for PR 30089 at commit c97f12a.

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

cloud-fan · 2020-10-23T05:58:05Z

external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/v2/V2JDBCTest.scala

+      sql(s"ALTER TABLE $catalogName.alt_table RENAME COLUMN id TO C")
+      val t = spark.table(s"$catalogName.alt_table")
+      val expectedSchema = catalogName match {
+        case "oracle" => new StructType().add("C", DecimalType(10, 0)).add("C0", DecimalType(10, 0))


to work around it, shall we use string or boolean type instead? It's testing rename so the type doesn't matter.

huaxingao · 2020-10-23T15:32:51Z

@cloud-fan I removed rename column test because @ScrapCodes has a follow up #30142 to address the rename problem.

SparkQA · 2020-10-23T16:18:59Z

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

SparkQA · 2020-10-23T16:42:05Z

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

SparkQA · 2020-10-23T17:53:29Z

Test build #130208 has finished for PR 30089 at commit 89c5045.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

huaxingao · 2020-10-23T17:57:34Z

retest this please

SparkQA · 2020-10-23T18:48:23Z

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

SparkQA · 2020-10-23T19:12:36Z

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

SparkQA · 2020-10-23T20:23:50Z

Test build #130213 has finished for PR 30089 at commit 89c5045.

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

ScrapCodes · 2020-10-24T08:58:56Z

Hi @huaxingao, I wasn't aware of this parallel work here. And created another PR #30142. I am going to copy your rename test and may be you could take a look at the other PR.

...cker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/v2/DB2IntegrationSuite.scala

cloud-fan · 2020-10-26T08:30:17Z

external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/v2/V2JDBCTest.scala


+  test("SPARK-33034: ALTER TABLE ... drop column") {
+    withTable(s"$catalogName.alt_table") {
+      sql(s"CREATE TABLE $catalogName.alt_table (C1 INTEGER, C2 INTEGER, c3 INTEGER) USING _")


since it's DROP COLUMN test, the type doesn't matter. Can we use boolean or string type to avoid the oracle special case?

Changed. Sorry I didn't see this last time.

SparkQA · 2020-10-26T16:32:41Z

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

SparkQA · 2020-10-26T16:53:12Z

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

SparkQA · 2020-10-26T18:48:25Z

Test build #130294 has finished for PR 30089 at commit c9f264a.

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

cloud-fan · 2020-10-27T15:04:51Z

thanks, merging to master!

huaxingao · 2020-10-27T16:02:45Z

Thanks!

cloud-fan reviewed Oct 22, 2020

View reviewed changes

cloud-fan approved these changes Oct 22, 2020

View reviewed changes

huaxingao added 2 commits October 22, 2020 08:05

[SPARK-33137][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: upda…

8b37874

…te type and nullability of columns (PostgreSQL dialect)

rebase

c97f12a

huaxingao force-pushed the postgres_docker branch from 9ca0e1c to c97f12a Compare October 22, 2020 15:59

huaxingao commented Oct 22, 2020

View reviewed changes

cloud-fan reviewed Oct 23, 2020

View reviewed changes

remove rename test

89c5045

HyukjinKwon reviewed Oct 26, 2020

View reviewed changes

...cker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/v2/DB2IntegrationSuite.scala Show resolved Hide resolved

cloud-fan reviewed Oct 26, 2020

View reviewed changes

change data type from int to string for drop col test

c9f264a

cloud-fan closed this in f284218 Oct 27, 2020

huaxingao deleted the postgres_docker branch October 27, 2020 16:02

[SPARK-33137][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (Postgres dialect) #30089

[SPARK-33137][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (Postgres dialect) #30089

Uh oh!

Conversation

huaxingao commented Oct 18, 2020

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 18, 2020

Uh oh!

SparkQA commented Oct 19, 2020

Uh oh!

SparkQA commented Oct 19, 2020

Uh oh!

SparkQA commented Oct 19, 2020

Uh oh!

SparkQA commented Oct 19, 2020

Uh oh!

huaxingao commented Oct 19, 2020

Uh oh!

cloud-fan Oct 22, 2020

Choose a reason for hiding this comment

Uh oh!

huaxingao Oct 22, 2020

Choose a reason for hiding this comment

Uh oh!

huaxingao Oct 22, 2020

Choose a reason for hiding this comment

Uh oh!

cloud-fan Oct 23, 2020

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Oct 22, 2020

Uh oh!

SparkQA commented Oct 22, 2020

Uh oh!

SparkQA commented Oct 22, 2020

Uh oh!

cloud-fan Oct 23, 2020

Choose a reason for hiding this comment

Uh oh!

huaxingao commented Oct 23, 2020

Uh oh!

SparkQA commented Oct 23, 2020

Uh oh!

SparkQA commented Oct 23, 2020

Uh oh!

SparkQA commented Oct 23, 2020

Uh oh!

huaxingao commented Oct 23, 2020

Uh oh!

SparkQA commented Oct 23, 2020

Uh oh!

SparkQA commented Oct 23, 2020

Uh oh!

SparkQA commented Oct 23, 2020

Uh oh!

ScrapCodes commented Oct 24, 2020

Uh oh!

Uh oh!

cloud-fan Oct 26, 2020

Choose a reason for hiding this comment

Uh oh!

huaxingao Oct 26, 2020

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Oct 26, 2020

Uh oh!

SparkQA commented Oct 26, 2020

Uh oh!

SparkQA commented Oct 26, 2020

Uh oh!

cloud-fan commented Oct 27, 2020

Uh oh!

huaxingao commented Oct 27, 2020

Uh oh!

Reviewers

Assignees