Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[SPARK-36902][SQL] Migrate CreateTableAsSelectStatement to v2 command #34667

Closed
wants to merge 9 commits into from
Closed

[SPARK-36902][SQL] Migrate CreateTableAsSelectStatement to v2 command #34667

wants to merge 9 commits into from

Conversation

dchvn
Copy link
Contributor

@dchvn dchvn commented Nov 19, 2021

What changes were proposed in this pull request?

Migrate CreateTableAsSelectStatement to v2 command

Why are the changes needed?

Migrate CreateTableAsSelectStatement to v2 command

Does this PR introduce any user-facing change?

No

How was this patch tested?

existing tests

@github-actions github-actions bot added the SQL label Nov 19, 2021
@dchvn
Copy link
Contributor Author

dchvn commented Nov 19, 2021

CC @huaxingao FYI, Please take a look when you find some time, thanks!

@SparkQA
Copy link

SparkQA commented Nov 19, 2021

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

@SparkQA
Copy link

SparkQA commented Nov 19, 2021

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

@SparkQA
Copy link

SparkQA commented Nov 19, 2021

Test build #145451 has finished for PR 34667 at commit 145f22b.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
  • trait V2CreateTablePlanX extends LogicalPlan

@dchvn dchvn changed the title [WIP][SPARK-36902][SQL] Migrate CreateTableAsSelectStatement to v2 command [SPARK-36902][SQL] Migrate CreateTableAsSelectStatement to v2 command Nov 30, 2021
@dchvn dchvn marked this pull request as ready for review November 30, 2021 10:53
@SparkQA
Copy link

SparkQA commented Nov 30, 2021

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

@SparkQA
Copy link

SparkQA commented Nov 30, 2021

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

@SparkQA
Copy link

SparkQA commented Nov 30, 2021

Test build #145767 has finished for PR 34667 at commit b5c2787.

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

@dchvn
Copy link
Contributor Author

dchvn commented Dec 1, 2021

CC @huaxingao @cloud-fan FYI, thanks!

@SparkQA
Copy link

SparkQA commented Dec 2, 2021

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

@SparkQA
Copy link

SparkQA commented Dec 2, 2021

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

@SparkQA
Copy link

SparkQA commented Dec 2, 2021

Test build #145837 has finished for PR 34667 at commit 729cc22.

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

@SparkQA
Copy link

SparkQA commented Dec 3, 2021

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

@SparkQA
Copy link

SparkQA commented Dec 3, 2021

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

cloud-fan
cloud-fan reviewed Dec 3, 2021
View reviewed changes
sql/core/src/main/scala/org/apache/spark/sql/DataFrameWriter.scala Outdated

val tableSpec = TableSpec(
bucketSpec = None,
properties = Map(TableCatalog.PROP_PROVIDER -> source) ++ location,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should not include the location property. We already specify it in TableSpec.location

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, we should put Map.empty here. provide is also specified in TableSpec.provider

@SparkQA
Copy link

SparkQA commented Dec 3, 2021

Test build #145876 has finished for PR 34667 at commit cba6519.

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

@SparkQA
Copy link

SparkQA commented Dec 3, 2021

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

@SparkQA
Copy link

SparkQA commented Dec 3, 2021

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

@SparkQA
Copy link

SparkQA commented Dec 3, 2021

Test build #145890 has finished for PR 34667 at commit 5186f81.

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

@cloud-fan
Copy link
Contributor

thanks, merging to master!

@cloud-fan cloud-fan closed this in 688fa23 Dec 3, 2021
@dchvn
Copy link
Contributor Author

dchvn commented Dec 3, 2021

thanks!

@dchvn dchvn deleted the migrate-CTAS branch December 3, 2021 14:36
@dongjoon-hyun
Copy link
Member

Hi, @dchvn and @cloud-fan .
Could you check if this breaks Java doc?

[error] sbt.inc.Doc$JavadocGenerationFailed

cloud-fan
cloud-fan reviewed Dec 17, 2021
View reviewed changes
sql/core/src/main/scala/org/apache/spark/sql/DataFrameWriter.scala
Map.empty,
Some(source),
df.queryExecution.analyzed,
tableSpec,
Map.empty,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@huaxingao can you fix this similar bug as well when you open the PR?

@huaxingao huaxingao mentioned this pull request Dec 23, 2021
Closed
cloud-fan pushed a commit that referenced this pull request Dec 27, 2021
@huaxingao @cloud-fan
[SPARK-37496][SQL][FOLLOWUP] DataFrameWriter.saveAsTable should take …
6c69262 
…write options in CTAS and RTAS

### What changes were proposed in this pull request?
`DataFrameWriter.saveAsTable` should pass `extraOptions.toMap` as the write options in CTAS and RTAS

### Why are the changes needed?
bug fixing
please see #34754 (comment) #34667 (comment)

### Does this PR introduce _any_ user-facing change?
no

### How was this patch tested?
new test

Closes #34997 from huaxingao/write_option.

Authored-by: Huaxin Gao <huaxin_gao@apple.com>
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
@CTTY CTTY mentioned this pull request Jul 12, 2022
Merged
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cloud-fan cloud-fan cloud-fan approved these changes

Assignees
No one assigned
Labels
SQL
Projects
None yet
Milestone
No milestone
4 participants
@dchvn @SparkQA @cloud-fan @dongjoon-hyun