Skip to content

Conversation

@imback82
Copy link
Contributor

What changes were proposed in this pull request?

This PR proposes to migrate ALTER TABLE ... RECOVER PARTITIONS to use UnresolvedTable to resolve the table identifier. This allows consistent resolution rules (temp view first, etc.) to be applied for both v1/v2 commands. More info about the consistent resolution rule proposal can be found in JIRA or proposal doc.

Note that ALTER TABLE ... RECOVER PARTITIONS is not supported for v2 tables.

Why are the changes needed?

The PR makes the resolution consistent behavior consistent. For example,

sql("CREATE DATABASE test")
sql("CREATE TABLE spark_catalog.test.t (id bigint, val string) USING csv PARTITIONED BY (id)")
sql("CREATE TEMPORARY VIEW t AS SELECT 2")
sql("USE spark_catalog.test")
sql("ALTER TABLE t RECOVER PARTITIONS") // works fine

, but after this PR:

sql("ALTER TABLE t RECOVER PARTITIONS")
org.apache.spark.sql.AnalysisException: t is a temp view. 'ALTER TABLE ... RECOVER PARTITIONS' expects a table; line 1 pos 0

, which is the consistent behavior with other commands.

Does this PR introduce any user-facing change?

After this PR, ALTER TABLE t RECOVER PARTITIONS in the above example is resolved to a temp view t first instead of spark_catalog.test.t.

How was this patch tested?

Updated existing tests.

@github-actions github-actions bot added the SQL label Dec 15, 2020
@SparkQA
Copy link

SparkQA commented Dec 15, 2020

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37394/

@SparkQA
Copy link

SparkQA commented Dec 15, 2020

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37394/

@imback82
Copy link
Contributor Author

cc @cloud-fan (GA passed)

@SparkQA
Copy link

SparkQA commented Dec 15, 2020

Test build #132793 has finished for PR 30773 at commit 852e169.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
  • case class AlterTableRecoverPartitions(child: LogicalPlan) extends Command

@cloud-fan
Copy link
Contributor

thanks, merging to master!

@cloud-fan cloud-fan closed this in 366beda Dec 15, 2020
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.

3 participants