[SPARK-30679][SQL] REPLACE TABLE can omit the USING clause #27402

cloud-fan · 2020-01-30T11:29:42Z

What changes were proposed in this pull request?

This PR updates the parser rule to allow REPLACE TABLE statement to omit the USING clause.

This is extracted from #27345

Why are the changes needed?

CREATE TABLE can omit the USING clause, and REPLACE TABLE should be consistent.

Does this PR introduce any user-facing change?

yes, now it's easier to write REPLACE TABLE command.

How was this patch tested?

a new test

cloud-fan · 2020-01-30T11:30:13Z

cc @rdblue @brkyvz @gengliangwang

SparkQA · 2020-01-30T15:35:32Z

Test build #117562 has finished for PR 27402 at commit ac30945.

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

brkyvz · 2020-01-30T18:32:40Z

I actually don't like the fact that you can be accidentally changing the format your table is stored in, but if people think that's fine, I won't oppose this change

rdblue · 2020-01-30T20:07:18Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala

-    val provider = ctx.tableProvider.multipartIdentifier.getText
+    val defaultProvider = conf.defaultDataSourceName
+    val provider =
+      Option(ctx.tableProvider).map(_.multipartIdentifier.getText).getOrElse(defaultProvider)


The default provider should not be set here. The purpose of the statement logical plans is to carry only what was expressed in SQL. Replacing with a default loses the fact that this query had no provider specified.

Instead, this needs to update the statement plan and pass provider as an option.

rdblue · 2020-01-30T20:08:01Z

I'm -1 on this because it incorrectly returns a statement that fills in information not in the user's SQL query. Defaults should be set later.

cloud-fan · 2020-01-31T04:35:16Z

Hi @rdblue , I just remind that, by design the default provider should be determined by the catalog implementation, not Spark.

I think you are right that the statement should hold an Option and we shouldn't set the "provider" property if USING clause is omitted when calling TableCatalog.createTable. I'll fix REPLACE TABLE here, as well as CREATE TABLE. These two are tightly coupled because of CREATE OR REPLACE TABLE.

SparkQA · 2020-01-31T08:05:02Z

Test build #117619 has finished for PR 27402 at commit a91e4e5.

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

gengliangwang · 2020-01-31T08:27:42Z

I actually don't like the fact that you can be accidentally changing the format your table is stored in, but if people think that's fine, I won't oppose this change

I am +1 with @brkyvz

gengliangwang · 2020-01-31T08:27:58Z

retest this please

cloud-fan · 2020-01-31T08:40:41Z

Seems no one is against allowing to omit the USING clause, but the question is what the default value should be. I see two options:

let catalog implementation to decide the default value. If the catalog doesn't implement StagingTableCatalog, then Spark will simply drop and re-create the table. It's hard for the catalog to retain the provider when createTable is called, as the table is already dropped.
Spark sets the default value as the provider of the existing table. This makes sure that when the USING clause is omitted, we won't accidentally change the table format.

I'll go with option 2.

SparkQA · 2020-01-31T10:42:11Z

Test build #117641 has finished for PR 27402 at commit a91e4e5.

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

cloud-fan · 2020-01-31T11:49:11Z

sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/DDLParserSuite.scala

@@ -49,26 +49,6 @@ class DDLParserSuite extends AnalysisTest {
    comparePlans(parsePlan(sql), expected, checkAnalysis = false)
  }

-  test("SPARK-30098: create table without provider should " +


The default provider shouldn't be set by the parser. The newly added test case will test the default provider.

SparkQA · 2020-01-31T15:33:49Z

Test build #117651 has finished for PR 27402 at commit 1874e39.

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

SparkQA · 2020-01-31T21:12:59Z

Test build #117667 has finished for PR 27402 at commit 647851c.

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

cloud-fan · 2020-02-03T04:45:08Z

retest this please

HyukjinKwon · 2020-02-18T07:30:20Z

retest this please

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

Test build #118619 has finished for PR 27402 at commit 647851c.

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

cloud-fan · 2020-02-20T15:40:53Z

It's not a breaking change to allow to omit the provider in REPLACE TABLE, so I'll leave it to 3.1.

The CREATE TABLE behavior should be fixed before 3.0: #27650

github-actions · 2020-05-31T00:22:14Z

We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!

SparkQA · 2020-06-01T07:49:35Z

Test build #123361 has finished for PR 27402 at commit 647851c.

This patch fails Spark unit tests.
This patch does not merge cleanly.
This patch adds no public classes.

github-actions · 2020-09-10T00:41:04Z

We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!

dongjoon-hyun added the SQL label Jan 30, 2020

rdblue reviewed Jan 30, 2020

View reviewed changes

REPLACE TABLE can omit the USING clause

dc965dd

cloud-fan force-pushed the rtas branch from a91e4e5 to 1874e39 Compare January 31, 2020 11:46

cloud-fan commented Jan 31, 2020

View reviewed changes

address comment

647851c

cloud-fan force-pushed the rtas branch from 1874e39 to 647851c Compare January 31, 2020 16:30

github-actions bot added the Stale label May 31, 2020

github-actions bot closed this Jun 1, 2020

cloud-fan removed the Stale label Jun 1, 2020

cloud-fan reopened this Jun 1, 2020

github-actions bot added the Stale label Sep 10, 2020

cloud-fan removed the Stale label Sep 10, 2020

cloud-fan closed this Dec 1, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-30679][SQL] REPLACE TABLE can omit the USING clause #27402

[SPARK-30679][SQL] REPLACE TABLE can omit the USING clause #27402

cloud-fan commented Jan 30, 2020

cloud-fan commented Jan 30, 2020

SparkQA commented Jan 30, 2020

brkyvz commented Jan 30, 2020

rdblue Jan 30, 2020

rdblue commented Jan 30, 2020

cloud-fan commented Jan 31, 2020 •

edited

SparkQA commented Jan 31, 2020

gengliangwang commented Jan 31, 2020

gengliangwang commented Jan 31, 2020

cloud-fan commented Jan 31, 2020

SparkQA commented Jan 31, 2020

cloud-fan Jan 31, 2020

SparkQA commented Jan 31, 2020

SparkQA commented Jan 31, 2020

cloud-fan commented Feb 3, 2020

HyukjinKwon commented Feb 18, 2020

SparkQA commented Feb 18, 2020

cloud-fan commented Feb 20, 2020

github-actions bot commented May 31, 2020

SparkQA commented Jun 1, 2020

github-actions bot commented Sep 10, 2020

[SPARK-30679][SQL] REPLACE TABLE can omit the USING clause #27402

[SPARK-30679][SQL] REPLACE TABLE can omit the USING clause #27402

Conversation

cloud-fan commented Jan 30, 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?

cloud-fan commented Jan 30, 2020

SparkQA commented Jan 30, 2020

brkyvz commented Jan 30, 2020

rdblue Jan 30, 2020

Choose a reason for hiding this comment

rdblue commented Jan 30, 2020

cloud-fan commented Jan 31, 2020 • edited

SparkQA commented Jan 31, 2020

gengliangwang commented Jan 31, 2020

gengliangwang commented Jan 31, 2020

cloud-fan commented Jan 31, 2020

SparkQA commented Jan 31, 2020

cloud-fan Jan 31, 2020

Choose a reason for hiding this comment

SparkQA commented Jan 31, 2020

SparkQA commented Jan 31, 2020

cloud-fan commented Feb 3, 2020

HyukjinKwon commented Feb 18, 2020

SparkQA commented Feb 18, 2020

cloud-fan commented Feb 20, 2020

github-actions bot commented May 31, 2020

SparkQA commented Jun 1, 2020

github-actions bot commented Sep 10, 2020

cloud-fan commented Jan 31, 2020 •

edited