[SPARK-14132][SPARK-14133][SQL] Alter table partition DDLs #12220

andrewor14 · 2016-04-06T22:32:45Z

What changes were proposed in this pull request?

This implements a few alter table partition commands using the SessionCatalog. In particular:

ALTER TABLE ... ADD PARTITION ...
ALTER TABLE ... DROP PARTITION ...
ALTER TABLE ... RENAME PARTITION ... TO ...

The following operations are not supported, and an AnalysisException with a helpful error message will be thrown if the user tries to use them:

ALTER TABLE ... EXCHANGE PARTITION ...
ALTER TABLE ... ARCHIVE PARTITION ...
ALTER TABLE ... UNARCHIVE PARTITION ...
ALTER TABLE ... TOUCH ...
ALTER TABLE ... COMPACT ...
ALTER TABLE ... CONCATENATE
MSCK REPAIR TABLE ...

How was this patch tested?

DDLSuite, DDLCommandSuite and HiveDDLCommandSuite

…-ddl

andrewor14 · 2016-04-06T23:49:03Z

@yhuai

SparkQA · 2016-04-06T23:52:16Z

Test build #55158 has finished for PR 12220 at commit eedb529.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- final class VectorizedParquetInputFormat extends ParquetInputFormat[InternalRow]

…-ddl

It seems that Hive supports dropping partitions based on partial specs, where not all partitioned columns are accompanied with specified values in the spec. Ironically there's no API to achieve this using the Hive client, so we need to implement it ourselves in Spark (see HiveClientImpl.scala). Additionally two tests in HiveCompatibilitySuite use features that we explicitly do not allow, so those tests are now added to the blacklist.

SparkQA · 2016-04-07T22:48:28Z

Test build #55245 has finished for PR 12220 at commit 85c8b8f.

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

SparkQA · 2016-04-07T23:46:17Z

Test build #55274 has finished for PR 12220 at commit 2a37c75.

This patch fails to build.
This patch merges cleanly.
This patch adds no public classes.

…-ddl

and also push ignoreIfNotExists into HiveClient.

SparkQA · 2016-04-08T02:47:32Z

Test build #55286 has finished for PR 12220 at commit 220141d.

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

yhuai · 2016-04-08T20:13:17Z

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

+ * The syntax of this command is:
+ * {{{
+ *   ALTER TABLE table DROP [IF EXISTS] PARTITION spec1[, PARTITION spec2, ...] [PURGE];
+ *   ALTER VIEW view DROP [IF EXISTS] PARTITION spec1[, PARTITION spec2, ...];


I think we do not support ALTER VIEW view DROP [IF EXISTS] PARTITION spec1[, PARTITION spec2, ...]; because the semantic of having partitions defined with a view is not defined?

A previous patch already throws an exception for us when this happens. I changed the exception to be AnalysisException so we can write the test easier. Eventually all of these exceptions will be made consistent. This is mainly just a documentation + test change.

andrewor14 · 2016-04-08T20:46:53Z

Looks like #12169 already added the check in the parser so we don't have to add it ourselves. The last commit just removes the bad comment and adds two tests for the views.

…-ddl

yhuai · 2016-04-08T21:17:33Z

Actually, I have another question. If the table is an external table, we should not delete the data when we drop the partition. What do we do for this case?

yhuai · 2016-04-08T21:29:08Z

OK. I think metastore will not delete the data for external tables even if deleteData is set to true (https://github.com/apache/hive/blob/release-0.13.1/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java#L2239-L2251). Do we have tests for external tables? If not, can we add tests? Thanks!

andrewor14 · 2016-04-08T21:41:24Z

OK, let's do that in a separate patch.

SparkQA · 2016-04-08T22:05:50Z

Test build #55390 has finished for PR 12220 at commit 3fb9a87.

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

yhuai · 2016-04-08T22:08:15Z

test this please

yhuai · 2016-04-08T22:20:13Z

sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala

+        val dropOptions = new PartitionDropOptions
+        dropOptions.ifExists = ignoreIfNotExists
+        dropOptions.purgeData = purge
+        client.dropPartition(db, table, hivePartition.getValues, dropOptions)


Oh, just realized that PURGE was added in Hive 0.14. I think we need to either not support it or add tests to VersionsSuite to make sure it works.

SparkQA · 2016-04-08T22:25:25Z

Test build #55391 has finished for PR 12220 at commit 12cf973.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- class RandomForestClassificationModel(TreeEnsembleModels, JavaMLWritable, JavaMLReadable):
- class HasVarianceCol(Params):
- class RandomForestRegressionModel(TreeEnsembleModels, JavaMLWritable, JavaMLReadable):
- case class DeserializeToObject(
- case class SerializeFromObject(
- public abstract class ColumnVector implements AutoCloseable
- case class DeserializeToObject(
- case class SerializeFromObject(

SparkQA · 2016-04-08T23:42:58Z

Test build #55396 has finished for PR 12220 at commit 12cf973.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- class RandomForestClassificationModel(TreeEnsembleModels, JavaMLWritable, JavaMLReadable):
- class HasVarianceCol(Params):
- class RandomForestRegressionModel(TreeEnsembleModels, JavaMLWritable, JavaMLReadable):
- case class DeserializeToObject(
- case class SerializeFromObject(
- public abstract class ColumnVector implements AutoCloseable
- case class DeserializeToObject(
- case class SerializeFromObject(

gatorsmile · 2016-04-09T16:29:09Z

@andrewor14 I know you are busy. Feel free to let me know if you need me to submit test cases to verify if the data is not deleted when we drop the partition of an external table.

…-ddl

andrewor14 · 2016-04-12T01:24:27Z

@gatorsmile that would be great, thanks!

yhuai · 2016-04-12T01:56:44Z

LGTM!

gatorsmile · 2016-04-12T02:22:26Z

Sure, will do it after this is merged. Thanks!

SparkQA · 2016-04-12T03:02:35Z

Test build #55565 has finished for PR 12220 at commit e3f08b3.

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

yhuai · 2016-04-12T03:57:46Z

Merging to master!

gatorsmile · 2016-04-13T14:15:21Z

sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala

+      // The provided spec here can be a partial spec, i.e. it will match all partitions
+      // whose specs are supersets of this partial spec. E.g. If a table has partitions
+      // (b='1', c='1') and (b='1', c='2'), a partial spec of (b='1') will match both.
+      val matchingParts = client.getPartitions(hiveTable, s.asJava).asScala


I am afraid there is a bug in getPartitions. This function could return a wrong result when the partitioning column names are wrong. For example, if we pass a='0', it will return all the partitions. We drop all the partitions in this case. The expected return should be an empty set, right?

Two possible solutions:

get the whole list, and filter it out by ourselves.

check if the specs contains any column that is not part of table partitioning at the beginning.

Will try to submit a PR today and fix the issue based on the solution 2? Please let me know if you want to do it. Thanks! @andrewor14 @yhuai

Hive does not have such an issue. It detects the error and reports

hive> ALTER TABLE extTable_with_partitions DROP PARTITION (a='0'); FAILED: SemanticException Column a not found

I think SessionCatalog should take care the check.

It's better to avoid of fetching all partition specs to the client side. If there is a large table, fetching all partition spec will be very slow.

You meant that a was not a partitioning column, right?

Will do it soon. Thanks!

https://issues.apache.org/jira/browse/SPARK-14603 is the JIRA for the work of using SessionCatalog to check if a metadata operation is valid or not.

Yeah, will use this JIRA to handle all the cases. Thanks!

Andrew Or added 9 commits April 6, 2016 13:47

Throw exception for MSCK REPAIR TABLE

b1ea87e

Throw exception on (un)archive partition

1690596

Implement add partitions

e24e007

Throw exception for exchange partition

18de2b3

Implement drop partitions

7058676

Throw exception for touch/compact/concatenate

cccc669

Implement rename partition

cdbf251

Fix style

74f3a08

Merge branch 'master' of github.com:apache/spark into alter-partition…

eedb529

…-ddl

Andrew Or added 2 commits April 7, 2016 13:25

Merge branch 'master' of github.com:apache/spark into alter-partition…

7588b59

…-ddl

Fix tests

2a37c75

Andrew Or added 3 commits April 7, 2016 16:52

Merge branch 'master' of github.com:apache/spark into alter-partition…

d794db1

…-ddl

Fix compile after rebase

73597b2

Correct drop partitions purge behavior

220141d

and also push ignoreIfNotExists into HiveClient.

yhuai reviewed Apr 8, 2016
View reviewed changes

Merge branch 'master' of github.com:apache/spark into alter-partition…

12cf973

…-ddl

yhuai reviewed Apr 8, 2016
View reviewed changes

Andrew Or added 2 commits April 11, 2016 17:03

Merge branch 'master' of github.com:apache/spark into alter-partition…

08a76f6

…-ddl

Don't purge

e3f08b3

asfgit closed this in 83fb964 Apr 12, 2016

andrewor14 deleted the alter-partition-ddl branch April 12, 2016 17:44

gatorsmile reviewed Apr 13, 2016
View reviewed changes

gatorsmile mentioned this pull request Apr 13, 2016

[SPARK-14388][SQL] Implement CREATE TABLE #12271

Closed

[SPARK-14132][SPARK-14133][SQL] Alter table partition DDLs #12220

[SPARK-14132][SPARK-14133][SQL] Alter table partition DDLs #12220

Uh oh!

Conversation

andrewor14 commented Apr 6, 2016

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

andrewor14 commented Apr 6, 2016

Uh oh!

SparkQA commented Apr 6, 2016

Uh oh!

SparkQA commented Apr 7, 2016

Uh oh!

SparkQA commented Apr 7, 2016

Uh oh!

SparkQA commented Apr 8, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andrewor14 commented Apr 8, 2016

Uh oh!

yhuai commented Apr 8, 2016

Uh oh!

yhuai commented Apr 8, 2016

Uh oh!

andrewor14 commented Apr 8, 2016

Uh oh!

SparkQA commented Apr 8, 2016

Uh oh!

yhuai commented Apr 8, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Apr 8, 2016

Uh oh!

SparkQA commented Apr 8, 2016

Uh oh!

gatorsmile commented Apr 9, 2016

Uh oh!

andrewor14 commented Apr 12, 2016

Uh oh!

yhuai commented Apr 12, 2016

Uh oh!

gatorsmile commented Apr 12, 2016

Uh oh!

SparkQA commented Apr 12, 2016

Uh oh!

yhuai commented Apr 12, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants