Skip to content

Conversation

@cloud-fan
Copy link
Contributor

What changes were proposed in this pull request?

This is a regression in Spark 2.2. In Spark 2.2, we introduced a new way to resolve persisted view: https://issues.apache.org/jira/browse/SPARK-18209 , but this makes the persisted view non case-preserving because we store the schema in hive metastore directly. We should follow data source table and store schema in table properties.

How was this patch tested?

new regression test

val tableLocation = if (needDefaultTableLocation) {
Some(CatalogUtils.stringToURI(defaultTablePath(tableDefinition.identifier)))
} else {
// Ideally we should not create a managed table with location, but Hive serde table can
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this change is actually small, just rearrange the if-else to make view and hive serde table fall into the same branch.

@cloud-fan
Copy link
Contributor Author

cc @gatorsmile @viirya

@viirya
Copy link
Member

viirya commented Jun 20, 2017

LGTM

@viirya
Copy link
Member

viirya commented Jun 20, 2017

Oh. I just thought there might be an issue when doing CREATE OR REPLACE VIEW viewName AS SELECT ... in CreateViewCommand.

The CatalogTable used to replace existing view is also coming from prepareTable. It doesn't contain the schema properties. And in alterTable we don't explicitly add the properties for VIEW too.

So once we replace an existing view, the schema properties will not be there anymore.

@SparkQA
Copy link

SparkQA commented Jun 20, 2017

Test build #78277 has finished for PR 18360 at commit 439c950.

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

@cloud-fan
Copy link
Contributor Author

@viirya good catch! fixed

@SparkQA
Copy link

SparkQA commented Jun 20, 2017

Test build #78289 has finished for PR 18360 at commit ff3b1aa.

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

Copy link
Member

@gatorsmile gatorsmile left a comment

Choose a reason for hiding this comment

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

LGTM

@gatorsmile
Copy link
Member

Thanks! Merging to master/2.2

@asfgit asfgit closed this in e862dc9 Jun 20, 2017
asfgit pushed a commit that referenced this pull request Jun 20, 2017
… case preserving

## What changes were proposed in this pull request?

This is a regression in Spark 2.2. In Spark 2.2, we introduced a new way to resolve persisted view: https://issues.apache.org/jira/browse/SPARK-18209 , but this makes the persisted view non case-preserving because we store the schema in hive metastore directly. We should follow data source table and store schema in table properties.

## How was this patch tested?

new regression test

Author: Wenchen Fan <[email protected]>

Closes #18360 from cloud-fan/view.

(cherry picked from commit e862dc9)
Signed-off-by: gatorsmile <[email protected]>
robert3005 pushed a commit to palantir/spark that referenced this pull request Jun 29, 2017
… case preserving

## What changes were proposed in this pull request?

This is a regression in Spark 2.2. In Spark 2.2, we introduced a new way to resolve persisted view: https://issues.apache.org/jira/browse/SPARK-18209 , but this makes the persisted view non case-preserving because we store the schema in hive metastore directly. We should follow data source table and store schema in table properties.

## How was this patch tested?

new regression test

Author: Wenchen Fan <[email protected]>

Closes apache#18360 from cloud-fan/view.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants