Skip to content

Conversation

@imback82
Copy link
Contributor

@imback82 imback82 commented Nov 2, 2019

What changes were proposed in this pull request?

Add AlterTableAddPartitionStatement and make ALTER TABLE ... ADD PARTITION go through the same catalog/table resolution framework of v2 commands.

Why are the changes needed?

It's important to make all the commands have the same table resolution behavior, to avoid confusing end-users. e.g.

USE my_catalog
DESC t // success and describe the table t from my_catalog
ALTER TABLE t ADD PARTITION (id=1) // report table not found as there is no table t in the session catalog

Does this PR introduce any user-facing change?

Yes. When running ALTER TABLE ... ADD PARTITION, Spark fails the command if the current catalog is set to a v2 catalog, or the table name specified a v2 catalog.

How was this patch tested?

Unit tests

| ALTER TABLE tableIdentifier (partitionSpec)?
SET SERDEPROPERTIES tablePropertyList #setTableSerDe
| ALTER TABLE tableIdentifier ADD (IF NOT EXISTS)?
| ALTER (TABLE | VIEW) multipartIdentifier ADD (IF NOT EXISTS)?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am combining these two since ALTER VIEW is not supported for ADD PARTITION and this rule is compatible with the view rule.

spec -> location
}
} else {
// Alter View: the location clauses are not allowed.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I also removed this check in ASTBuilder.scala since view check is already done at the top.

@SparkQA
Copy link

SparkQA commented Nov 2, 2019

Test build #113132 has finished for PR 26369 at commit 7a2464d.

  • This patch fails SparkR unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
  • case class AlterTableAddPartitionStatement(

@imback82
Copy link
Contributor Author

imback82 commented Nov 2, 2019

retest this please

@SparkQA
Copy link

SparkQA commented Nov 3, 2019

Test build #113138 has finished for PR 26369 at commit 7a2464d.

  • This patch fails SparkR unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
  • case class AlterTableAddPartitionStatement(

@imback82
Copy link
Contributor Author

imback82 commented Nov 3, 2019

retest this please

@SparkQA
Copy link

SparkQA commented Nov 3, 2019

Test build #113155 has finished for PR 26369 at commit 7a2464d.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
  • case class AlterTableAddPartitionStatement(

@imback82
Copy link
Contributor Author

imback82 commented Nov 3, 2019

retest this please

@SparkQA
Copy link

SparkQA commented Nov 3, 2019

Test build #113174 has finished for PR 26369 at commit 7a2464d.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
  • case class AlterTableAddPartitionStatement(

@imback82
Copy link
Contributor Author

imback82 commented Nov 4, 2019

cc: @cloud-fan @viirya

@cloud-fan
Copy link
Contributor

thanks, merging to master!

@cloud-fan cloud-fan closed this in d4ea211 Nov 4, 2019
@viirya
Copy link
Member

viirya commented Nov 4, 2019

lgtm

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.

5 participants