Skip to content

[SPARK-33898][SQL][FOLLOWUP] Unify the v2 behavior with v1 for SHOW CREATE TABLE command #34773

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
Closed
Peng-Lei wants to merge 1 commit into from
Closed

[SPARK-33898][SQL][FOLLOWUP] Unify the v2 behavior with v1 for SHOW CREATE TABLE command #34773

Peng-Lei wants to merge 1 commit into from

Conversation

@Peng-Lei
Copy link
Contributor

@Peng-Lei Peng-Lei commented Dec 2, 2021

What changes were proposed in this pull request?

  1. Move the SHOW CREATE TABLE w/ char/varchar to CharVarcharDDLTestBase
  2. Fix the behavior different with v1 command that about the TBLPROPERTIES

Why are the changes needed?

Before the #PR merge. We should handle some different behavior or bugs between v1 and v2 command.

Does this PR introduce any user-facing change?

No

How was this patch tested?

existed test case.

@github-actions github-actions bot added the SQL label Dec 2, 2021
@Peng-Lei
Copy link
Contributor Author

Peng-Lei commented Dec 2, 2021

At now. There is a issue left that when use v2 command show create table $t and t is a V1Table. At then the result of SHOW CREATE TABLE $t will lost the USING info. Because there is no provider info in V1Table.properties, just in the metadata provider

@SparkQA
Copy link

SparkQA commented Dec 2, 2021

Kubernetes integration test unable to build dist.

exiting with code: 1
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50313/

@SparkQA
Copy link

SparkQA commented Dec 2, 2021

Test build #145838 has finished for PR 34773 at commit aa3bc5b.

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

LuciferYang
LuciferYang reviewed Dec 2, 2021
View reviewed changes
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/ShowCreateTableExec.scala
}

builder ++= "TBLPROPERTIES"
builder ++= "TBLPROPERTIES "
Copy link
Contributor

@LuciferYang LuciferYang Dec 2, 2021

Choose a reason for hiding this comment

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

Is this change related to Fix the behavior different with v1 command that about the TBLPROPERTIES?

Copy link
Contributor Author

@Peng-Lei Peng-Lei Dec 3, 2021

Choose a reason for hiding this comment

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

Yeah. Rright. Thank you for your review.

LuciferYang
LuciferYang approved these changes Dec 2, 2021
View reviewed changes
Copy link
Contributor

@LuciferYang LuciferYang left a comment

Choose a reason for hiding this comment

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

LGTM (non-binding)

@Peng-Lei Peng-Lei mentioned this pull request Jan 5, 2022
Closed
@cloud-fan
Copy link
Contributor

cloud-fan commented Jan 5, 2022

thanks, merging to master!

@cloud-fan cloud-fan closed this in 4741ecf Jan 5, 2022
@cloud-fan
Copy link
Contributor

cloud-fan commented Jan 5, 2022

t now. There is a issue left that when use v2 command show create table $t and t is a V1Table. At then the result of SHOW CREATE TABLE $t will lost the USING info. Because there is no provider info in V1Table.properties, just in the metadata provider

You are right. I think we should fix V1Table.properties to generate these built-in table properties like "provider".

@Peng-Lei Peng-Lei mentioned this pull request Jan 7, 2022
Closed
cloud-fan pushed a commit that referenced this pull request Jan 12, 2022
@Peng-Lei @cloud-fan
[SPARK-37827][SQL] Put the some built-in table properties into V1Tabl…
d7d31d0 
…e.propertie to adapt to V2 command

### What changes were proposed in this pull request?
Add V2 table build-in properties into V1Table.properties to adapt to V2 command.

### Why are the changes needed?
[#discuss](#34773 (comment))

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

### How was this patch tested?
Add ut testcase and existed ut testcase.

Closes #35131 from Peng-Lei/SPARK-37827.

Authored-by: PengLei <[email protected]>
Signed-off-by: Wenchen Fan <[email protected]>
dchvn pushed a commit to dchvn/spark that referenced this pull request Jan 19, 2022
@Peng-Lei @dchvn
[SPARK-37827][SQL] Put the some built-in table properties into V1Tabl…
f7e6225 
…e.propertie to adapt to V2 command

### What changes were proposed in this pull request?
Add V2 table build-in properties into V1Table.properties to adapt to V2 command.

### Why are the changes needed?
[#discuss](apache#34773 (comment))

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

### How was this patch tested?
Add ut testcase and existed ut testcase.

Closes apache#35131 from Peng-Lei/SPARK-37827.

Authored-by: PengLei <[email protected]>
Signed-off-by: Wenchen Fan <[email protected]>
scottsand-db pushed a commit to scottsand-db/delta that referenced this pull request Jan 25, 2022
@Peng-Lei @scottsand-db
[AUTO][SPARK-37827][SQL] Put the some built-in table properties into …
58039a4 
…V1Table.propertie to adapt to V2 command

### What changes were proposed in this pull request?
Add V2 table build-in properties into V1Table.properties to adapt to V2 command.

### Why are the changes needed?
[#discuss](apache/spark#34773 (comment))

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

### How was this patch tested?
Add ut testcase and existed ut testcase.

Authored-by: PengLei <[email protected]>
(cherry picked from commit d7d31d0606170847721e5c604ea7993fb202596f)

Author: PengLei <[email protected]>
Author: Bricks Bot <[email protected]>
Author: Wenchen Fan <[email protected]>
Author: Wenchen Fan <[email protected]>
Author: Max Gekk <[email protected]>

#33239 is resolved by bricks-bot/cherry-pick-master-d7d31d0606170847721e5c604ea7993fb202596f.

GitOrigin-RevId: 361704dc1813dd13025e76ce123b466844095977
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@LuciferYang LuciferYang LuciferYang approved these changes

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

Assignees

No one assigned

Labels

SQL

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Peng-Lei @SparkQA @cloud-fan @LuciferYang