Skip to content

[SPARK-19240][SQL][TEST] add test for setting location for managed table - #16597

Closed
cloud-fan wants to merge 1 commit into
apache:masterfrom
cloud-fan:set-location
Closed

[SPARK-19240][SQL][TEST] add test for setting location for managed table#16597
cloud-fan wants to merge 1 commit into
apache:masterfrom
cloud-fan:set-location

Conversation

@cloud-fan

@cloud-fancloud-fan commented Jan 16, 2017

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

SET LOCATION can also work on managed table(or table created without custom path), the behavior is a little weird, but as we have already supported it, we should add a test to explicitly show the behavior.

How was this patch tested?

N/A

@cloud-fan

Copy link
Copy Markdown
ContributorAuthor

cc @yhuai@gatorsmile

@cloud-fancloud-fan changed the title [SPARK-18885][SQL] SET LOCATION is not allowed for tables created without custom table path[SPARK-19240][SQL] SET LOCATION is not allowed for tables created without custom table pathJan 16, 2017
@SparkQA

Copy link
Copy Markdown

Test build #71438 has finished for PR 16597 at commit 35520b0.

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

@gatorsmilegatorsmile left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Based on the current way, we should not allow users to change the location for the managed table. The definition of EXTERNAL is different between Hive and Spark. When users provide a path in Spark SQL, we do not drop the data. This is our rule. Following this rule, if we allow it, we also need to change the type to EXTERNAL.

LGTM cc @yhuai

@yhuai

Copy link
Copy Markdown
Contributor

I am not sure if it is worth breaking this behavior. If the table is a managed table, it is possible that existing behavior allows users to move a table from one managed place to another managed place (e.g. the location of a database is changed). It is not clear that breaking this behavior can give us any benefit.

@cloud-fancloud-fan changed the title [SPARK-19240][SQL] SET LOCATION is not allowed for tables created without custom table path[SPARK-19240][SQL][TEST] add test for setting location for managed tableJan 17, 2017
@SparkQA

Copy link
Copy Markdown

Test build #71484 has started for PR 16597 at commit a5687f8.

@gatorsmile

gatorsmile commented Jan 17, 2017

Copy link
Copy Markdown
Member

Just FYI, this only tests the behaviors of InMemoryCatalog. I will port it to HiveDDLSuite in #16592

The added test case LGTM to me.

@gatorsmile

Copy link
Copy Markdown
Member

retest this please

@SparkQA

Copy link
Copy Markdown

Test build #71491 has finished for PR 16597 at commit a5687f8.

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

@cloud-fan

Copy link
Copy Markdown
ContributorAuthor

thanks for the review, merging to master!

uzadude pushed a commit to uzadude/spark that referenced this pull request Jan 27, 2017
## What changes were proposed in this pull request?
SET LOCATION can also work on managed table(or table created without custom path), the behavior is a little weird, but as we have already supported it, we should add a test to explicitly show the behavior.
## How was this patch tested?
N/A
Author: Wenchen Fan <wenchen@databricks.com>
Closesapache#16597 from cloud-fan/set-location.
cmonkey pushed a commit to cmonkey/spark that referenced this pull request Feb 15, 2017
## What changes were proposed in this pull request?
SET LOCATION can also work on managed table(or table created without custom path), the behavior is a little weird, but as we have already supported it, we should add a test to explicitly show the behavior.
## How was this patch tested?
N/A
Author: Wenchen Fan <wenchen@databricks.com>
Closesapache#16597 from cloud-fan/set-location.
Sign up for freeto 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

@cloud-fan@SparkQA@yhuai@gatorsmile