Skip to content

Conversation

@huaxingao
Copy link
Contributor

What changes were proposed in this pull request?

Add ShowPartitionsStatement and make SHOW PARTITIONS 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.

Does this PR introduce any user-facing change?

Yes. When running SHOW PARTITIONS, 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.

@huaxingao
Copy link
Contributor Author

@viirya
Could you please review? Thanks a lot!

Copy link
Member

@viirya viirya left a comment

Choose a reason for hiding this comment

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

This looks fine to me. Looks like loadTable can provide partitioning info so V2 implementation can also be implemented now? @cloud-fan @rdblue

@rdblue
Copy link
Contributor

rdblue commented Oct 22, 2019

This looks fine to me. Looks like loadTable can provide partitioning info so V2 implementation can also be implemented now?

The partitioning provided by loadTable are the transforms and partition columns, not the actual table partitions. SHOW PARTITIONS can't be implemented until we have an API for getting partition metadata.


test("SHOW PARTITIONS") {
val sql1 = "SHOW PARTITIONS t1"
val sql2 = "SHOW PARTITIONS db1.t1"
Copy link
Member

Choose a reason for hiding this comment

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

Let's add a case of a.b.c.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @viirya for your review. The comment has been addressed.

@SparkQA
Copy link

SparkQA commented Oct 22, 2019

Test build #112414 has finished for PR 26198 at commit 003ab44.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
  • case class ShowPartitionsStatement(tableName: Seq[String],

@SparkQA
Copy link

SparkQA commented Oct 22, 2019

Test build #112436 has finished for PR 26198 at commit 157b755.

  • This patch fails due to an unknown error code, -9.
  • This patch merges cleanly.
  • This patch adds no public classes.

@cloud-fan
Copy link
Contributor

the changes LGTM, let's fix the conflict

@SparkQA
Copy link

SparkQA commented Oct 22, 2019

Test build #112478 has finished for PR 26198 at commit 248513c.

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

@huaxingao
Copy link
Contributor Author

Conflicts fixed @viirya

@viirya
Copy link
Member

viirya commented Oct 22, 2019

Thanks. Merging this to master!

@viirya viirya closed this in 3bf5355 Oct 22, 2019
@huaxingao
Copy link
Contributor Author

Thanks all for the help!

@huaxingao huaxingao deleted the spark-29539 branch October 22, 2019 22:06
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.

6 participants