[SPARK-32405][SQL] Apply table options while creating tables in JDBC Table Catalog #30154

huaxingao · 2020-10-26T22:56:06Z

What changes were proposed in this pull request?

Currently in JDBCTableCatalog, we ignore the table options when creating table.

    // TODO (SPARK-32405): Apply table options while creating tables in JDBC Table Catalog
    if (!properties.isEmpty) {
      logWarning("Cannot create JDBC table with properties, these properties will be " +
        "ignored: " + properties.asScala.map { case (k, v) => s"$k=$v" }.mkString("[", ", ", "]"))
    }

Why are the changes needed?

need to apply the table options when we create table

Does this PR introduce any user-facing change?

no

How was this patch tested?

add new test

SparkQA · 2020-10-26T23:37:23Z

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

SparkQA · 2020-10-26T23:49:46Z

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

SparkQA · 2020-10-27T00:07:25Z

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

SparkQA · 2020-10-27T00:18:40Z

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

SparkQA · 2020-10-27T00:57:40Z

Test build #130302 has finished for PR 30154 at commit edf98d0.

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

huaxingao · 2020-10-27T01:42:55Z

retest this please

SparkQA · 2020-10-27T02:29:33Z

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

SparkQA · 2020-10-27T02:58:46Z

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

SparkQA · 2020-10-27T04:17:16Z

Test build #130306 has finished for PR 30154 at commit edf98d0.

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

huaxingao · 2020-10-27T05:13:09Z

...ore/src/main/scala/org/apache/spark/sql/execution/datasources/v2/jdbc/JDBCTableCatalog.scala

@@ -123,8 +123,14 @@ class JDBCTableCatalog extends TableCatalog with Logging {
        "ignored: " + properties.asScala.map { case (k, v) => s"$k=$v" }.mkString("[", ", ", "]"))


I haven't removed the Warning yet. The properties here could be provider, owner, location, comment or TBLPROPERTIES key value pairs. I only implemented comment. Seems to me these are not options in standard database CREATE TABLE syntax. I am not sure if we need to support all of them.

I think a more reasonable behavior is to fail if unsupported table properties are given. JDBC V2 is new in this release so we still have a chance to change it.

For these 4 builtin properties, I think we only need to support comment. We should ignore provider for now. And after the CRETE TABLE unification, we can force users to not specify USING when creating table in JDBC v2.

OK, so we will support comment and ignore provider, owner, and location for now.

For supported table properties, do we need to make them database specific? for example, ENGINE and CHARSET are supported table properties for MySQL but not supported by other databases.

Instead of having a complete list of supported table properties for each of the databases, it might be better to send whatever table properties without checking and let it fail at database side if unsupported.

yea, that sounds better, although it needs more work to update every dialect.

huaxingao · 2020-10-27T05:15:25Z

cc @cloud-fan @maropu @MaxGekk @ScrapCodes

sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala

sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JdbcUtils.scala

cloud-fan · 2020-10-29T07:01:08Z

We also need to retrieve the table comment in TableCatalog.loadTable

huaxingao · 2020-10-30T01:14:56Z

We also need to retrieve the table comment in TableCatalog.loadTable

So we will retrieve table comment using a query and then set this comment in JDBCOptions in JDBCTable, right? This query is different for different databases, for example, MySQL has SELECT table_comment FROM INFORMATION_SCHEMA.TABLES WHERE table_name='tablename', DB2 has SELECT remarks from syscat.tables where tabname='tablename'

cloud-fan · 2020-10-30T06:34:52Z

This query is different for different databases

ah, it's so complicated. Let's forget about it for this PR.

SparkQA · 2020-10-31T02:08:20Z

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

SparkQA · 2020-10-31T02:29:57Z

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

SparkQA · 2020-10-31T03:35:15Z

Test build #130480 has finished for PR 30154 at commit 74469a6.

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

huaxingao · 2020-10-31T03:59:06Z

retest this please

SparkQA · 2020-10-31T04:44:32Z

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

SparkQA · 2020-10-31T05:05:29Z

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

SparkQA · 2020-10-31T06:33:07Z

Test build #130485 has finished for PR 30154 at commit 74469a6.

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

...ore/src/main/scala/org/apache/spark/sql/execution/datasources/v2/jdbc/JDBCTableCatalog.scala

sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JdbcUtils.scala

SparkQA · 2020-11-03T03:10:14Z

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

SparkQA · 2020-11-03T03:26:23Z

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

SparkQA · 2020-11-03T03:38:35Z

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

SparkQA · 2020-11-03T03:47:22Z

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

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

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

Test build #130640 has finished for PR 30154 at commit bb9c4fc.

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

SparkQA · 2020-11-05T22:52:18Z

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

SparkQA · 2020-11-05T23:20:58Z

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

SparkQA · 2020-11-06T00:40:08Z

Test build #130670 has finished for PR 30154 at commit 39057aa.

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

cloud-fan · 2020-11-06T05:50:50Z

...ore/src/main/scala/org/apache/spark/sql/execution/datasources/v2/jdbc/JDBCTableCatalog.scala

+          case "provider" =>
+          case "owner" => // owner is ignored. It is default to current user name.
+          case "location" =>
+            throw new AnalysisException("Cannot create JDBC table with property location.")


let's be more user-facing: CREATE TABLE ... LOCATION ... is not supported in the JDBC catalog.

…Table Catalog

SparkQA · 2020-11-06T07:36:32Z

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

SparkQA · 2020-11-06T07:58:36Z

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

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

Test build #130696 has finished for PR 30154 at commit ecc9a4e.

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

huaxingao · 2020-11-06T16:18:02Z

retest this please

SparkQA · 2020-11-06T17:06:02Z

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

SparkQA · 2020-11-06T17:34:53Z

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

SparkQA · 2020-11-06T19:08:23Z

Test build #130731 has finished for PR 30154 at commit ecc9a4e.

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

cloud-fan · 2020-11-09T07:02:10Z

thanks, merging to master!

huaxingao · 2020-11-09T17:09:10Z

Thank you! @cloud-fan

huaxingao commented Oct 27, 2020

View reviewed changes

cloud-fan reviewed Oct 28, 2020

View reviewed changes

sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala Show resolved Hide resolved

cloud-fan reviewed Oct 29, 2020

View reviewed changes

sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JdbcUtils.scala Show resolved Hide resolved

cloud-fan reviewed Nov 2, 2020

View reviewed changes

...ore/src/main/scala/org/apache/spark/sql/execution/datasources/v2/jdbc/JDBCTableCatalog.scala Show resolved Hide resolved

cloud-fan reviewed Nov 2, 2020

View reviewed changes

sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JdbcUtils.scala Show resolved Hide resolved

huaxingao force-pushed the table_options branch from e267481 to d4b1cf9 Compare November 3, 2020 02:36

cloud-fan reviewed Nov 5, 2020

View reviewed changes

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

cloud-fan reviewed Nov 6, 2020

View reviewed changes

huaxingao added 8 commits November 5, 2020 22:09

[SPARK-32405][SQL] Apply table options while creating tables in JDBC …

46d8f71

…Table Catalog

add import

3ea5b3e

add table properties to createTableOptions

7ef9c44

address comments

38d2cac

rebase

0041038

add CREATE TABLE with property test in docker test

d7d4d91

address comments

cb1023d

rebase and address comments

ecc9a4e

huaxingao force-pushed the table_options branch from 39057aa to ecc9a4e Compare November 6, 2020 06:50

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

cloud-fan approved these changes Nov 9, 2020

View reviewed changes

cloud-fan closed this in bfb257f Nov 9, 2020

huaxingao deleted the table_options branch November 9, 2020 17:09

		@@ -123,8 +123,14 @@ class JDBCTableCatalog extends TableCatalog with Logging {
		"ignored: " + properties.asScala.map { case (k, v) => s"$k=$v" }.mkString("[", ", ", "]"))

[SPARK-32405][SQL] Apply table options while creating tables in JDBC Table Catalog #30154

[SPARK-32405][SQL] Apply table options while creating tables in JDBC Table Catalog #30154

Conversation

huaxingao commented Oct 26, 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?

SparkQA commented Oct 26, 2020

SparkQA commented Oct 26, 2020

SparkQA commented Oct 27, 2020

SparkQA commented Oct 27, 2020

SparkQA commented Oct 27, 2020

huaxingao commented Oct 27, 2020

SparkQA commented Oct 27, 2020

SparkQA commented Oct 27, 2020

SparkQA commented Oct 27, 2020

huaxingao Oct 27, 2020

Choose a reason for hiding this comment

cloud-fan Oct 29, 2020

Choose a reason for hiding this comment

huaxingao Oct 30, 2020

Choose a reason for hiding this comment

huaxingao Oct 30, 2020

Choose a reason for hiding this comment

cloud-fan Oct 30, 2020

Choose a reason for hiding this comment

huaxingao commented Oct 27, 2020

cloud-fan commented Oct 29, 2020

huaxingao commented Oct 30, 2020

cloud-fan commented Oct 30, 2020

SparkQA commented Oct 31, 2020

SparkQA commented Oct 31, 2020

SparkQA commented Oct 31, 2020

huaxingao commented Oct 31, 2020

SparkQA commented Oct 31, 2020

SparkQA commented Oct 31, 2020

SparkQA commented Oct 31, 2020

SparkQA commented Nov 3, 2020

SparkQA commented Nov 3, 2020

SparkQA commented Nov 3, 2020

SparkQA commented Nov 3, 2020

SparkQA commented Nov 5, 2020

SparkQA commented Nov 5, 2020

SparkQA commented Nov 5, 2020

SparkQA commented Nov 6, 2020

cloud-fan Nov 6, 2020

Choose a reason for hiding this comment

SparkQA commented Nov 6, 2020

SparkQA commented Nov 6, 2020

SparkQA commented Nov 6, 2020

huaxingao commented Nov 6, 2020

SparkQA commented Nov 6, 2020

SparkQA commented Nov 6, 2020

SparkQA commented Nov 6, 2020

cloud-fan commented Nov 9, 2020

huaxingao commented Nov 9, 2020