[SPARK-36850][SQL] Migrate CreateTableStatement to v2 command framework #34060

huaxingao · 2021-09-21T16:52:03Z

What changes were proposed in this pull request?

Migrate CreateTableStatement to v2 command framework

Why are the changes needed?

Migrate to the standard V2 framework

Does this PR introduce any user-facing change?

no

How was this patch tested?

existing tests

SparkQA · 2021-09-21T17:55:18Z

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

SparkQA · 2021-09-21T18:40:49Z

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

SparkQA · 2021-09-21T21:39:04Z

Test build #143477 has finished for PR 34060 at commit 1965f77.

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

huaxingao · 2021-09-22T03:36:38Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala

This is only temporary and will be removed after CreateTableAsSelect, ReplaceTable and ReplaceTableAsSelect are migrated to v2 command framework.

huaxingao · 2021-09-22T03:36:48Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/v2Commands.scala

V2CreateTablePlan can't be deleted yet because it is also extended by CreateTableAsSelect, ReplaceTable and ReplaceTableAsSelect. I use V2CreateTablePlanX for this new trait for now.

Do we have to add this? Can we let the new CreateTable extends V2CreateTablePlan and implement tableName?

def tableName = { assert(name.isResolved) name.asInstanceOf[ResolvedDBObjectName].nameParts.asIdentifier }

huaxingao · 2021-09-22T03:37:21Z

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

The original case create: V2CreateTablePlan will be removed and this will be changed to
case create: V2CreateTablePlan after the migration of CreateTableAsSelect, ReplaceTable and ReplaceTableAsSelect is done.

SparkQA · 2021-09-28T04:07:30Z

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

SparkQA · 2021-09-28T05:06:30Z

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

SparkQA · 2021-09-28T08:16:04Z

Test build #143667 has finished for PR 34060 at commit 9e42d67.

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

SparkQA · 2021-10-06T17:54:02Z

Test build #143889 has started for PR 34060 at commit 0af17cd.

SparkQA · 2021-10-06T18:34:57Z

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

SparkQA · 2021-10-06T19:17:15Z

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

SparkQA · 2021-10-15T00:27:08Z

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

SparkQA · 2021-10-15T01:13:00Z

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

SparkQA · 2021-10-15T04:34:00Z

Test build #144280 has finished for PR 34060 at commit e146c17.

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

dchvn · 2021-11-16T04:36:10Z

Hi @huaxingao , any update? I am trying to migrate ReplaceTableStatement and CreateTableAsSelectStatement by learn from your PR

huaxingao · 2021-11-17T18:02:00Z

@dchvn Sorry for the late reply. I will work on this now.

SparkQA · 2021-11-17T19:16:45Z

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

SparkQA · 2021-11-17T20:14:43Z

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

…rces/rules.scala Co-authored-by: Wenchen Fan <[email protected]>

SparkQA · 2021-11-26T08:55:24Z

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

SparkQA · 2021-11-26T09:38:50Z

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

SparkQA · 2021-11-26T13:07:50Z

Test build #145661 has finished for PR 34060 at commit 39320f4.

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

cloud-fan · 2021-11-29T11:42:52Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/trees/TreeNode.scala

      redactMapString(map, maxFields)
+    case t: TableSpec =>
+      redactMapString(t.options, maxFields)
+      redactMapString(t.properties, maxFields)


I think we need to return a copy of TableSpec with options and properties redacted.

Sorry I didn't get this right. Fixed.

cloud-fan · 2021-11-29T11:47:21Z

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

 import org.apache.spark.sql.catalyst.expressions.objects.StaticInvoke
 import org.apache.spark.sql.catalyst.parser.{CatalystSqlParser, ParseException}
-import org.apache.spark.sql.catalyst.plans.logical.{AlterColumn, AnalysisOnlyCommand, AppendData, Assignment, CreateTableAsSelect, CreateTableStatement, CreateV2Table, DeleteAction, DeleteFromTable, DescribeRelation, DropTable, InsertAction, LocalRelation, LogicalPlan, MergeIntoTable, OneRowRelation, Project, SetTableLocation, SetTableProperties, ShowTableProperties, SubqueryAlias, UnsetTableProperties, UpdateAction, UpdateTable}
+import org.apache.spark.sql.catalyst.plans.logical.{AlterColumn, AnalysisOnlyCommand, AppendData, Assignment, CreateTable => CatalystCreateTable, CreateTableAsSelect, DeleteAction, DeleteFromTable, DescribeRelation, DropTable, InsertAction, LocalRelation, LogicalPlan, MergeIntoTable, OneRowRelation, Project, SetTableLocation, SetTableProperties, ShowTableProperties, SubqueryAlias, UnsetTableProperties, UpdateAction, UpdateTable}


nit: can we change import org.apache.spark.sql.execution.datasources.CreateTable to import org.apache.spark.sql.execution.datasources.CreateTableV1 and keep the CreateTable here unchanged?

SparkQA · 2021-11-29T23:08:32Z

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

SparkQA · 2021-11-30T00:08:54Z

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

SparkQA · 2021-11-30T03:14:08Z

Test build #145733 has finished for PR 34060 at commit 8fdf059.

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

cloud-fan · 2021-11-30T04:47:55Z

thanks, merging to master!

huaxingao · 2021-11-30T07:12:24Z

Thanks!

cloud-fan · 2021-12-09T17:10:58Z

sql/core/src/main/scala/org/apache/spark/sql/streaming/DataStreamWriter.scala

-        originalMultipartIdentifier,
-        df.schema.asNullable,
-        partitioningColumns.getOrElse(Nil).asTransforms.toSeq,
+      val tableProperties = TableSpec(


typo: this should be tableSpec

Will have a follow up to fix this.

…ndition for CTAS and RTAS ### What changes were proposed in this pull request? fixed a few problems: 1. addressed this [comment](#34060 (comment)) 2. combined several `xxxOnlySupportedWithV2TableError` 3. in CTAS and RTAS, the `if isSessionCatalog(catalog)` should not be on the pattern, it should be `if (isSessionCatalog(catalog) && !isV2Provider(provider))`. Otherwise, `c.partitioning ++ c.tableSpec.bucketSpec.map(_.asTransform)` is not done for non SessionCatalog case. I tried this `c.partitioning ++ c.tableSpec.bucketSpec.map(_.asTransform)` inside `AstBuilder` but it failed [here]( https://github.com/apache/spark/blob/master/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala#L850) so I kept this in `ResolveSessionCatalog` ### Why are the changes needed? code cleaning up and bug fixing ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? Existing tests Closes #34857 from huaxingao/followup. Lead-authored-by: Huaxin Gao <[email protected]> Co-authored-by: Huaxin Gao <[email protected]> Signed-off-by: Wenchen Fan <[email protected]>

huaxingao marked this pull request as draft September 21, 2021 16:52

github-actions bot added SQL STRUCTURED STREAMING labels Sep 21, 2021

huaxingao commented Sep 22, 2021

View reviewed changes

huaxingao changed the title ~~[WIP] Migrate CreateTableStatement to v2 command framework~~ [SPARK-36850][SQL][WIP] Migrate CreateTableStatement to v2 command framework Sep 25, 2021

huaxingao marked this pull request as ready for review September 28, 2021 02:44

huaxingao changed the title ~~[SPARK-36850][SQL][WIP] Migrate CreateTableStatement to v2 command framework~~ [SPARK-36850][SQL] Migrate CreateTableStatement to v2 command framework Sep 28, 2021

huaxingao force-pushed the create_table branch from 1965f77 to 9e42d67 Compare September 28, 2021 03:03

huaxingao marked this pull request as draft September 29, 2021 00:21

huaxingao marked this pull request as ready for review October 6, 2021 23:22

huaxingao force-pushed the create_table branch from 0af17cd to e146c17 Compare October 14, 2021 23:35

huaxingao force-pushed the create_table branch from e146c17 to 6ffae91 Compare November 17, 2021 18:00

huaxingao and others added 11 commits November 25, 2021 23:20

rebase

95ef4bf

use CatalogAndIdentifier to resolve table catalog

bb8cf82

rebase

9f59ece

add case class TableProperties

6114516

address comments

8374054

fix build failure

1b38e2f

address comments

a54240d

Update sql/core/src/main/scala/org/apache/spark/sql/execution/datasou…

8eda282

…rces/rules.scala Co-authored-by: Wenchen Fan <[email protected]>

DataSourceCreateTable -> CreateTableV1

267466e

address comments

4cf26bc

rebase

39320f4

huaxingao force-pushed the create_table branch from 97481b3 to 39320f4 Compare November 26, 2021 07:26

cloud-fan reviewed Nov 29, 2021

View reviewed changes

address comments

8fdf059

cloud-fan approved these changes Nov 30, 2021

View reviewed changes

cloud-fan closed this in 98b0c80 Nov 30, 2021

huaxingao deleted the create_table branch November 30, 2021 07:12

Peng-Lei mentioned this pull request Dec 2, 2021

[SPARK-37330][SQL] Migrate ReplaceTableStatement to v2 command #34764

Closed

cloud-fan reviewed Dec 9, 2021

View reviewed changes

huaxingao mentioned this pull request Dec 10, 2021

[SPARK-36850][SQL][FOLLOWUP] Simplify exception code and fix wrong condition for CTAS and RTAS #34857

Closed

[SPARK-36850][SQL] Migrate CreateTableStatement to v2 command framework #34060

[SPARK-36850][SQL] Migrate CreateTableStatement to v2 command framework #34060

Uh oh!

Conversation

huaxingao commented Sep 21, 2021 • 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!

SparkQA commented Sep 21, 2021

Uh oh!

SparkQA commented Sep 21, 2021

Uh oh!

SparkQA commented Sep 21, 2021

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 Sep 28, 2021

Uh oh!

SparkQA commented Sep 28, 2021

Uh oh!

SparkQA commented Sep 28, 2021

Uh oh!

SparkQA commented Oct 6, 2021

Uh oh!

SparkQA commented Oct 6, 2021

Uh oh!

SparkQA commented Oct 6, 2021

Uh oh!

SparkQA commented Oct 15, 2021

Uh oh!

SparkQA commented Oct 15, 2021

Uh oh!

SparkQA commented Oct 15, 2021

Uh oh!

dchvn commented Nov 16, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

huaxingao commented Nov 17, 2021

Uh oh!

SparkQA commented Nov 17, 2021

Uh oh!

SparkQA commented Nov 17, 2021

Uh oh!

SparkQA commented Nov 26, 2021

Uh oh!

SparkQA commented Nov 26, 2021

Uh oh!

SparkQA commented Nov 26, 2021

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 29, 2021

Uh oh!

SparkQA commented Nov 30, 2021

Uh oh!

SparkQA commented Nov 30, 2021

Uh oh!

cloud-fan commented Nov 30, 2021

Uh oh!

huaxingao commented Nov 30, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

huaxingao commented Sep 21, 2021 •

edited

Loading

dchvn commented Nov 16, 2021 •

edited

Loading