[WIP] add support for show partitions command #1051

JassAbidi · 2022-04-02T16:54:08Z

Description

This PR adds support for SHOW PARTITIONS command to list the partition names of a delta table.

Resolves #996

How was this patch tested?

Tests to test this PR were added in ShowDeltaPartitionsSuite

Does this PR introduce any user-facing changes?

No

[SC-24892] Add typesafe bintray repo for sbt-mima-plugin

update fork

catch up to master

update master

update with master

update fork branch

update fork

fork update

update fork

sync with master

add more unit tests

JassAbidi · 2022-04-02T17:02:02Z

@zsxwing this PR is not finished yet (the partition spec is not supported yet). But I wanted to double-check the implementation with you since I also introduced some changes to the Describe Detail command to avoid duplicating code. Please let me know if there are changes that I need to do before tackling the partition spec work.

allisonport-db

Took a precursory look, let me know what you think about the suggested refactoring.

allisonport-db · 2022-04-06T00:21:03Z

core/src/main/antlr4/io/delta/sql/parser/DeltaSqlBase.g4

+    ;
+
+partitionVal
+    : identifier (EQ constant)?


Why is (EQ constant) optional?

allisonport-db · 2022-04-06T00:23:05Z

core/src/main/antlr4/io/delta/sql/parser/DeltaSqlBase.g4

 // Add keywords here so that people's queries don't break if they have a column name as one of
 // these tokens
 nonReserved
    : VACUUM | RETAIN | HOURS | DRY | RUN
    | CONVERT | TO | DELTA | PARTITIONED | BY
    | DESC | DESCRIBE | LIMIT | DETAIL
    | GENERATE | FOR | TABLE | CHECK | EXISTS | OPTIMIZE
-    | RESTORE | AS | OF
+    | RESTORE | AS | OF | SHOW | PARTITIONS


What about PARTITION?

allisonport-db · 2022-04-06T01:27:33Z

core/src/main/scala/org/apache/spark/sql/delta/commands/ShowDeltaPartitionsCommand.scala

+ * A command for users to list the partition names of a delta table. If partition spec is specified,
+ * partitions that match the spec are returned. Otherwise an empty result set is returned.


Shouldn't all the partitions be returned if partition_spec isn't provided?

allisonport-db · 2022-04-06T01:30:39Z

core/src/main/antlr4/io/delta/sql/parser/DeltaSqlBase.g4

@@ -75,6 +75,8 @@ statement
    : VACUUM (path=STRING | table=qualifiedName)
        (RETAIN number HOURS)? (DRY RUN)?                               #vacuumTable
    | (DESC | DESCRIBE) DETAIL (path=STRING | table=qualifiedName)      #describeDeltaDetail
+    | SHOW PARTITIONS (path=STRING | table=qualifiedName)
+        partitionSpec?                                                  #showPartitions


I think maybe making this more like (PARTITIONED BY '(' colTypeList ')')? might be more readable? partitionSpec is a bit convoluted

allisonport-db · 2022-04-06T18:38:06Z

core/src/main/scala/io/delta/sql/parser/DeltaSqlParser.scala

+    val path = Option(ctx.path).map(string)
+    val tableIdentifier = Option(ctx.table).map(visitTableIdentifier)
+    ShowDeltaPartitionsCommand(path, tableIdentifier)


Can we do it without the variables like the other commands?

allisonport-db · 2022-04-06T21:31:07Z

core/src/main/scala/org/apache/spark/sql/delta/commands/ShowDeltaPartitionsCommand.scala

+  }
+}
+
+abstract class DeltaCommandWithoutViewsSupport(


So first, I noticed DeltaCommand uses DeltaErrors.viewNotSupportedException(operationName), we should be able to use this error instead (and delete the command specific ones in DeltaErrors.)

I also noticed a lot of this code overlaps with DeltaCommand.getDeltaLog, which for some reason neither DescribeDeltaDetailsCommand nor DescribeDeltaHistoryCommand extends. In fact it seems to me DescribeDeltaHistoryCommand basically copies exactly getDeltaLog.

What I'm currently thinking needs to happen is...

DescribeDeltaHistoryCommand should extend DeltaCommand and use DeltaCommand.getDeltaLog

We should see if we can combine this code here (as used by DescribeDeltaDetailsCommand) with DeltaCommand.getDeltaLog to minimize overlap. I'm currently thinking something like ...

trait DeltaCommand { ... def getPathAndTableMetadata(...) { ... } def getDeltaLog(...) { path, metadata = getPathAndTableMetadata(...) // check if delta table and throw error if so .... }

Do we need to support ShowDeltaPartitionsCommand for non-delta tables (@zsxwing I'm thinking no)? Right now we aren't checking if it's a Delta table before creating the DeltaLog. Assuming we only need to support Delta tables, we should then be able to use DeltaCommand.getDeltaLog here since both usages of getPathAndTableMetadata ignores tableMetadata and only uses basePath to create a DeltaLog.

(1) and (2) should be done in a separate PR. I think in this PR we can just revert the changes to DescribeDeltaDetailsCommand, and use DeltaCommand.getDeltaLog.

We should also update DeltaCommand.getDeltaLog to throw the more helpful error for temporary views, as we have done in DescribeDeltaDetailCommand (good reason for reducing code duplication 😄 ).

What do you think @zsxwing?

LMK if you'd be willing to tackle 1&2 in a separate PR.

YannByron · 2022-07-04T08:43:46Z

I am not sure if we can not support this sql command using path.
If so, we don't need to extend the antlr grammar and sql parser and can reuse the spark syntax directly. and make the DeltaTableV2 to extend the SupportsPartitionManagement interface.

zsxwing · 2022-07-05T05:01:59Z

@YannByron We have to create our own parser as we don't plan to follow Spark's current show partitions format. It's coming from legacy hive format and it's not user friendly. See #996 for the examples I put in the description.

YannByron · 2022-07-06T04:03:19Z

@zsxwing thanks for you explanation. I see delta defines the new output format for show partitions and that's nice.

scottsand-db · 2022-08-26T21:16:55Z

@JassAbidi - any update? are you still interested in working on this?

zsxwing · 2022-10-11T18:55:37Z

@JassAbidi are you still working on this?

nate-kuhl · 2022-10-26T19:46:15Z

My team could certainly make use of this feature - is there anything I can do to help move it along ?

allisonport-db · 2022-10-26T23:00:32Z

@nate-kuhl This PR doesn't seem to be active, you're welcome to pick up this work instead. In my review above I outlined a refactoring I think should be done first before implementing this if you would like to work on it.

nate-kuhl · 2022-10-28T04:00:11Z

@allisonport-db I can give it a shot ! I guess I should open a new PR ?

allisonport-db · 2022-10-28T17:44:18Z

@nate-kuhl since we need to do a refactoring (2 PRs) you should create a new PR. Feel free to open the PR early and ask on it if you have any questions

JassAbidi and others added 20 commits October 23, 2019 21:48

Merge pull request #1 from delta-io/master

37174b3

[SC-24892] Add typesafe bintray repo for sbt-mima-plugin

Merge branch 'master' of https://github.com/delta-io/delta

17748af

Merge pull request #2 from delta-io/master

5fb6a77

update fork

Merge pull request #3 from delta-io/master

4ec9fec

catch up to master

Merge pull request #4 from delta-io/master

67cacce

update master

Merge pull request #5 from delta-io/master

0feb9c6

update with master

Merge pull request #6 from delta-io/master

cf7a9c9

update fork branch

Merge pull request #7 from delta-io/master

4289d09

update fork

Merge pull request #8 from delta-io/master

7041c1a

update fork

Merge pull request #9 from delta-io/master

eee16c7

update fork

Merge pull request #10 from delta-io/master

cad3c3d

fork update

Merge pull request #11 from delta-io/master

9e96cd7

update fork

Merge pull request #13 from delta-io/master

c8eb27c

update fork

Merge pull request #14 from delta-io/master

5717ee3

update fork

Merge pull request #15 from delta-io/master

f5c35cd

update fork

Merge pull request #16 from delta-io/master

68ef72a

sync with master

stach changes

b10840c

add support for show partitions

739588c

remove space

a748cda

support null and empty partition columns

8504fca

add more unit tests

modify comment for the test table

524228e

allisonport-db self-assigned this Apr 4, 2022

allisonport-db reviewed Apr 6, 2022

View reviewed changes

zsxwing mentioned this pull request Oct 11, 2022

[BUG] AnalysisException: Table spark_catalog.my_db.my_table does not support partition management. #1430

Closed

		* A command for users to list the partition names of a delta table. If partition spec is specified,
		* partitions that match the spec are returned. Otherwise an empty result set is returned.

[WIP] add support for show partitions command #1051

Are you sure you want to change the base?

[WIP] add support for show partitions command #1051

Conversation

JassAbidi commented Apr 2, 2022

Description

How was this patch tested?

Does this PR introduce any user-facing changes?

Uh oh!

JassAbidi commented Apr 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

allisonport-db left a comment

Choose a reason for hiding this comment

Uh oh!

allisonport-db Apr 6, 2022

Choose a reason for hiding this comment

Uh oh!

allisonport-db Apr 6, 2022

Choose a reason for hiding this comment

Uh oh!

allisonport-db Apr 6, 2022

Choose a reason for hiding this comment

Uh oh!

allisonport-db Apr 6, 2022

Choose a reason for hiding this comment

Uh oh!

allisonport-db Apr 6, 2022

Choose a reason for hiding this comment

Uh oh!

allisonport-db Apr 6, 2022

Choose a reason for hiding this comment

Uh oh!

allisonport-db Apr 6, 2022

Choose a reason for hiding this comment

Uh oh!

YannByron commented Jul 4, 2022

Uh oh!

zsxwing commented Jul 5, 2022

Uh oh!

YannByron commented Jul 6, 2022

Uh oh!

scottsand-db commented Aug 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zsxwing commented Oct 11, 2022

Uh oh!

nate-kuhl commented Oct 26, 2022

Uh oh!

allisonport-db commented Oct 26, 2022

Uh oh!

nate-kuhl commented Oct 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

allisonport-db commented Oct 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

JassAbidi commented Apr 2, 2022 •

edited

Loading

scottsand-db commented Aug 26, 2022 •

edited

Loading

nate-kuhl commented Oct 28, 2022 •

edited

Loading

allisonport-db commented Oct 28, 2022 •

edited

Loading