Skip to content

Conversation

@brkyvz
Copy link
Contributor

@brkyvz brkyvz commented May 2, 2020

What changes were proposed in this pull request?

SHOW TBLPROPERTIES does not get the correct table properties for tables using the Session Catalog. This PR fixes that, by explicitly falling back to the V1 implementation if the table is in fact a V1 table. We also hide the reserved table properties for V2 tables, as users do not have control over setting these table properties. Henceforth, if they cannot be set or controlled by the user, then they shouldn't be displayed as such.

Why are the changes needed?

Shows the incorrect table properties, i.e. only what exists in the Hive MetaStore for V2 tables that may have table properties outside of the MetaStore.

Does this PR introduce any user-facing change?

Fixes a bug

How was this patch tested?

Regression test

@brkyvz
Copy link
Contributor Author

brkyvz commented May 2, 2020

It would be great to merge this into Spark 3.0. cc @cloud-fan

@SparkQA
Copy link

SparkQA commented May 2, 2020

Test build #122185 has finished for PR 28434 at commit d32a6c4.

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

val t1 = "tbl"
withTable(t1) {
sql(s"CREATE TABLE $t1 (id bigint, data string) USING $v2Format TBLPROPERTIES " +
s"(key='v', key2='v2')")
Copy link
Member

Choose a reason for hiding this comment

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

's' is not needed

Seq(Row("key", "v"), Row("key2", "v2"))
)

checkAnswer(
Copy link
Member

Choose a reason for hiding this comment

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

This can fit to 100 chars?

checkAnswer(
sql(s"SHOW TBLPROPERTIES $t1"),
Seq(Row("key", "v"), Row("key2", "v2"))
)
Copy link
Member

Choose a reason for hiding this comment

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

Put ')' at the end of previous line

@SparkQA
Copy link

SparkQA commented May 4, 2020

Test build #122236 has finished for PR 28434 at commit 9828286.

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

@cloud-fan
Copy link
Contributor

thanks, merging to master/3.0!

@cloud-fan cloud-fan closed this in 02a319d May 4, 2020
cloud-fan pushed a commit that referenced this pull request May 4, 2020
…session catalog

## What changes were proposed in this pull request?

SHOW TBLPROPERTIES does not get the correct table properties for tables using the Session Catalog. This PR fixes that, by explicitly falling back to the V1 implementation if the table is in fact a V1 table. We also hide the reserved table properties for V2 tables, as users do not have control over setting these table properties. Henceforth, if they cannot be set or controlled by the user, then they shouldn't be displayed as such.

### Why are the changes needed?

Shows the incorrect table properties, i.e. only what exists in the Hive MetaStore for V2 tables that may have table properties outside of the MetaStore.

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

Fixes a bug

### How was this patch tested?

Regression test

Closes #28434 from brkyvz/ddlCommands.

Authored-by: Burak Yavuz <[email protected]>
Signed-off-by: Wenchen Fan <[email protected]>
(cherry picked from commit 02a319d)
Signed-off-by: Wenchen Fan <[email protected]>
huaxingao pushed a commit to huaxingao/spark that referenced this pull request May 4, 2020
…session catalog

## What changes were proposed in this pull request?

SHOW TBLPROPERTIES does not get the correct table properties for tables using the Session Catalog. This PR fixes that, by explicitly falling back to the V1 implementation if the table is in fact a V1 table. We also hide the reserved table properties for V2 tables, as users do not have control over setting these table properties. Henceforth, if they cannot be set or controlled by the user, then they shouldn't be displayed as such.

### Why are the changes needed?

Shows the incorrect table properties, i.e. only what exists in the Hive MetaStore for V2 tables that may have table properties outside of the MetaStore.

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

Fixes a bug

### How was this patch tested?

Regression test

Closes apache#28434 from brkyvz/ddlCommands.

Authored-by: Burak Yavuz <[email protected]>
Signed-off-by: Wenchen Fan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants