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.

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

[SPARK-14506] [SQL] HiveClientImpl's toHiveTable misses a table property for external tables #12275

Closed
wants to merge 1 commit into from

Conversation

yhuai
Copy link
Contributor

@yhuai yhuai commented Apr 9, 2016

What changes were proposed in this pull request?

For an external table's metadata (in Hive's representation), its table type needs to be EXTERNAL_TABLE. Also, there needs to be a field called EXTERNAL set in the table property with a value of TRUE (for a MANAGED_TABLE it will be FALSE) based on https://github.com/apache/hive/blob/release-1.2.1/metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java#L1095-L1105. HiveClientImpl's toHiveTable misses to set this table property.

How was this patch tested?

Added a new test.

@SparkQA
Copy link

SparkQA commented Apr 9, 2016

Test build #55441 has finished for PR 12275 at commit a732e70.

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

@yhuai
Copy link
Contributor Author

yhuai commented Apr 9, 2016

test this please

@SparkQA
Copy link

SparkQA commented Apr 9, 2016

Test build #55444 has finished for PR 12275 at commit a732e70.

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

@yhuai
Copy link
Contributor Author

yhuai commented Apr 9, 2016

test this please

@SparkQA
Copy link

SparkQA commented Apr 9, 2016

Test build #55448 has finished for PR 12275 at commit a732e70.

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

@yhuai
Copy link
Contributor Author

yhuai commented Apr 9, 2016

@andrewor14 want to review this?

hiveTable.setProperty("EXTERNAL", "TRUE")
HiveTableType.EXTERNAL_TABLE
case CatalogTableType.MANAGED_TABLE =>
hiveTable.setProperty("EXTERNAL", "FALSE")
Copy link
Contributor

Choose a reason for hiding this comment

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

we don't ned to set this to false in other cases?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like we do not need to set it for other cases. This flag is only used for MANAGED_TABLE and EXTERNAL_TABLE.

@andrewor14
Copy link
Contributor

LGTM

@andrewor14
Copy link
Contributor

Merged into master.

@asfgit asfgit closed this in 3fb09af Apr 10, 2016
@yhuai yhuai deleted the SPARK-14506 branch April 10, 2016 19:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants