[SPARK-33452][SQL] Support v2 SHOW PARTITIONS #30398

MaxGekk · 2020-11-17T10:34:40Z

What changes were proposed in this pull request?

Remove V2 logical node ShowPartitionsStatement , and replace it by V2 ShowPartitions.
Implement V2 execution node ShowPartitionsExec similar to V1 ShowPartitionsCommand.

Why are the changes needed?

To have feature parity with Datasource V1.

Does this PR introduce any user-facing change?

Yes.

Before the change, SHOW PARTITIONS fails in V2 table catalogs with the exception:

org.apache.spark.sql.AnalysisException: SHOW PARTITIONS is only supported with v1 tables.
   at org.apache.spark.sql.catalyst.analysis.ResolveSessionCatalog.org$apache$spark$sql$catalyst$analysis$ResolveSessionCatalog$$parseV1Table(ResolveSessionCatalog.scala:628)
   at org.apache.spark.sql.catalyst.analysis.ResolveSessionCatalog$$anonfun$apply$1.applyOrElse(ResolveSessionCatalog.scala:466)

How was this patch tested?

By running the following test suites:

Modified ShowPartitionsParserSuite where ShowPartitionsStatement is replaced by V2 ShowPartitions.
v2.ShowPartitionsSuite

SparkQA · 2020-11-17T11:47:20Z

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

SparkQA · 2020-11-17T12:09:24Z

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

SparkQA · 2020-11-17T12:28:46Z

Test build #131219 has finished for PR 30398 at commit c62b30b.

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

SparkQA · 2020-11-17T18:46:46Z

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

SparkQA · 2020-11-17T18:56:51Z

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

SparkQA · 2020-11-17T19:11:23Z

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

SparkQA · 2020-11-17T19:16:39Z

Test build #131230 has finished for PR 30398 at commit 7e06381.

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

SparkQA · 2020-11-17T19:20:33Z

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

SparkQA · 2020-11-17T21:25:44Z

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

SparkQA · 2020-11-17T21:54:46Z

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

SparkQA · 2020-11-17T22:16:30Z

Test build #131231 has finished for PR 30398 at commit 9cbbbe1.

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

SparkQA · 2020-11-17T22:31:15Z

Test build #131234 has finished for PR 30398 at commit ade84ba.

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

SparkQA · 2020-11-18T12:38:08Z

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

SparkQA · 2020-11-18T12:51:37Z

Test build #131277 has finished for PR 30398 at commit 8700ed5.

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

SparkQA · 2020-11-18T13:05:06Z

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

SparkQA · 2020-11-25T22:06:42Z

Test build #131805 has finished for PR 30398 at commit 1cd1e34.

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

MaxGekk · 2020-11-26T07:02:15Z

jenkins, retest this, please

SparkQA · 2020-11-26T08:55:46Z

Test build #131834 has finished for PR 30398 at commit 1cd1e34.

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

MaxGekk · 2020-11-26T09:04:24Z

jenkins, retest this, please

SparkQA · 2020-11-26T13:08:07Z

Test build #131839 has finished for PR 30398 at commit 1cd1e34.

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

MaxGekk · 2020-11-26T16:11:48Z

jenkins, retest this, please

SparkQA · 2020-11-26T18:36:43Z

Test build #131852 has finished for PR 30398 at commit 1cd1e34.

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

MaxGekk · 2020-11-26T18:42:28Z

jenkins, retest this, please

SparkQA · 2020-11-26T23:49:37Z

Test build #131853 has finished for PR 30398 at commit 1cd1e34.

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

cloud-fan · 2020-11-27T08:06:46Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala

+      UnresolvedPartitionSpec(visitNonOptionalPartitionSpec(specCtx), None)
+    }
+    ShowPartitions(
+      UnresolvedTableOrView(multiPart.getOrElse(Seq.empty[String])),


If it doesn't support view, why not use UnresolvedTable?

To output proper error as for V1.

see https://github.com/apache/spark/pull/30398/files#diff-583171e935b2dc349378063a5841c5b98b30a2d57ac3743a9eccfe7bffcb8f2aR1011-R1012

V1:

spark/sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala

Lines 992 to 994 in 23e9920

if (table.tableType == VIEW) {

throw new AnalysisException(s"SHOW PARTITIONS is not allowed on a view: $tableIdentWithDB")

}

then we should improve the v2 error message like #30475

One of purpose of this PR is to create v2 implementation compatible to v1. I would prefer to avoid changing v1 impl at least in this PR since v1 is out of the scope.

I replaced by UnresolvedTableOrView by UnresolvedTable, and

removed some checks from CheckAnalysis.checkShowPartitions()

…c-v2

SparkQA · 2020-11-28T14:28:49Z

Test build #131900 has finished for PR 30398 at commit d3c344a.

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

…c-v2

cloud-fan · 2020-11-30T07:16:40Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala

+      UnresolvedPartitionSpec(visitNonOptionalPartitionSpec(specCtx), None)
+    }
+    ShowPartitions(
+      UnresolvedTable(multiPart.getOrElse(Seq.empty[String]), "SHOW PARTITIONS"),


multiPart.getOrElse(Seq.empty[String]) this looks weird. How can the table name be Seq.empty? And I don't see other parser rules doing the same thing. They are simply using visitMultipartIdentifier(ctx.multipartIdentifier). Why do we diverge here?

cloud-fan

LGTM except one comment

…ions-exec-v2 # Conflicts: # sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolvePartitionSpec.scala

cloud-fan · 2020-11-30T13:45:52Z

GA passed, merging to master, thanks!

SparkQA · 2020-11-30T18:49:37Z

Test build #131979 has finished for PR 30398 at commit dc211e6.

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

…r's` comments ### What changes were proposed in this pull request? The pr aims to fix the outdated `logical plan name` in `AstBuilder's` comments. ### Why are the changes needed? - After the pr #33609, the name of the logical plan below has been changed: `AlterTableAddColumns` -> `AddColumns` `AlterTableRenameColumn` -> `RenameColumn` `AlterTableAlterColumn` -> `AlterColumn` `AlterTableDropColumns` -> `DropColumns` - After the pr #30398 The name of the logical plan `ShowPartitionsStatement` has been changed to `ShowPartitions`. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Only update comments. ### Was this patch authored or co-authored using generative AI tooling? No. Closes #47562 from panbingkun/fix_astbuilder. Lead-authored-by: panbingkun <[email protected]> Co-authored-by: panbingkun <[email protected]> Signed-off-by: yangjie01 <[email protected]>

…r's` comments ### What changes were proposed in this pull request? The pr aims to fix the outdated `logical plan name` in `AstBuilder's` comments. ### Why are the changes needed? - After the pr apache#33609, the name of the logical plan below has been changed: `AlterTableAddColumns` -> `AddColumns` `AlterTableRenameColumn` -> `RenameColumn` `AlterTableAlterColumn` -> `AlterColumn` `AlterTableDropColumns` -> `DropColumns` - After the pr apache#30398 The name of the logical plan `ShowPartitionsStatement` has been changed to `ShowPartitions`. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Only update comments. ### Was this patch authored or co-authored using generative AI tooling? No. Closes apache#47562 from panbingkun/fix_astbuilder. Lead-authored-by: panbingkun <[email protected]> Co-authored-by: panbingkun <[email protected]> Signed-off-by: yangjie01 <[email protected]>

MaxGekk added 4 commits November 17, 2020 10:40

Add ShowPartitionsExec

65daf5d

Add logical node ShowPartitions

ef84a73

ShowPartitionsStatement -> ShowPartitions

6934fa1

Resolve ShowPartitions -> ShowPartitionsExec

c62b30b

github-actions bot added the SQL label Nov 17, 2020

MaxGekk added 4 commits November 17, 2020 18:35

Remove verify(s"SHOW PARTITIONS $t")

d1190c7

Pass UnresolvedTableOrView to ShowPartitions

e086c32

Throw an exception for non-partitioned table

7e06381

Fix ShowPartitionsParserSuite

9cbbbe1

MaxGekk added 3 commits November 17, 2020 22:55

Ignore test in PartitionedTablePerfStatsSuite

04c9317

Check partition spec

ed915c9

Remove a comment

ade84ba

MaxGekk added 2 commits November 18, 2020 10:37

pass Table to ShowPartitionsExec

3a457f9

Change type of partition spec in ShowPartitions

8700ed5

Revert a test in v1/ShowPartitionsSuite

dfbad25

cloud-fan reviewed Nov 27, 2020

View reviewed changes

MaxGekk added 2 commits November 28, 2020 11:06

Merge remote-tracking branch 'origin/master' into show-partitions-exe…

555ef3f

…c-v2

Fix merge

0fe4a61

MaxGekk mentioned this pull request Nov 28, 2020

[SPARK-33515][SQL] Improve exception messages while handling UnresolvedTable #30461

Closed

UnresolvedTableOrView -> UnresolvedTable

d3c344a

Merge remote-tracking branch 'origin/master' into show-partitions-exe…

37d7801

…c-v2

cloud-fan reviewed Nov 30, 2020

View reviewed changes

cloud-fan approved these changes Nov 30, 2020

View reviewed changes

MaxGekk added 3 commits November 30, 2020 12:53

Address Wenchen's review comment

fbc5820

Merge remote-tracking branch 'remotes/origin/master' into show-partit…

d16aec7

…ions-exec-v2 # Conflicts: # sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolvePartitionSpec.scala

Remove unused param tableName from convertToPartIdent()

dc211e6

cloud-fan closed this in 0a612b6 Nov 30, 2020

MaxGekk deleted the show-partitions-exec-v2 branch February 19, 2021 15:03

panbingkun mentioned this pull request Aug 1, 2024

[SPARK-49076][SQL] Fix the outdated logical plan name in AstBuilder's comments #47562

Closed

	if (table.tableType == VIEW) {
	throw new AnalysisException(s"SHOW PARTITIONS is not allowed on a view: $tableIdentWithDB")
	}

[SPARK-33452][SQL] Support v2 SHOW PARTITIONS #30398

[SPARK-33452][SQL] Support v2 SHOW PARTITIONS #30398

Uh oh!

Conversation

MaxGekk commented Nov 17, 2020

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

SparkQA commented Nov 17, 2020

Uh oh!

SparkQA commented Nov 17, 2020

Uh oh!

SparkQA commented Nov 17, 2020

Uh oh!

SparkQA commented Nov 17, 2020

Uh oh!

SparkQA commented Nov 17, 2020

Uh oh!

SparkQA commented Nov 17, 2020

Uh oh!

SparkQA commented Nov 17, 2020

Uh oh!

SparkQA commented Nov 17, 2020

Uh oh!

SparkQA commented Nov 17, 2020

Uh oh!

SparkQA commented Nov 17, 2020

Uh oh!

SparkQA commented Nov 17, 2020

Uh oh!

SparkQA commented Nov 17, 2020

Uh oh!

SparkQA commented Nov 18, 2020

Uh oh!

SparkQA commented Nov 18, 2020

Uh oh!

SparkQA commented Nov 18, 2020

Uh oh!

SparkQA commented Nov 25, 2020

Uh oh!

MaxGekk commented Nov 26, 2020

Uh oh!

SparkQA commented Nov 26, 2020

Uh oh!

MaxGekk commented Nov 26, 2020

Uh oh!

SparkQA commented Nov 26, 2020

Uh oh!

MaxGekk commented Nov 26, 2020

Uh oh!

SparkQA commented Nov 26, 2020

Uh oh!

MaxGekk commented Nov 26, 2020

Uh oh!

SparkQA commented Nov 26, 2020

Uh oh!

cloud-fan Nov 27, 2020

Choose a reason for hiding this comment

Uh oh!

MaxGekk Nov 27, 2020

Choose a reason for hiding this comment

Uh oh!

MaxGekk Nov 27, 2020

Choose a reason for hiding this comment

Uh oh!

MaxGekk Nov 27, 2020

Choose a reason for hiding this comment

Uh oh!

cloud-fan Nov 27, 2020

Choose a reason for hiding this comment

Uh oh!

MaxGekk Nov 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MaxGekk Nov 28, 2020

Choose a reason for hiding this comment

Uh oh!

MaxGekk Nov 27, 2020 •

edited

Loading

cloud-fan Nov 30, 2020 •

edited

Loading