You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by andrewor14 <gi...@git.apache.org> on 2016/04/07 00:32:46 UTC

[GitHub] spark pull request: [SPARK-14132][SPARK-14133][SQL] Alter table pa...

GitHub user andrewor14 opened a pull request:

    https://github.com/apache/spark/pull/12220

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

    ## 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
    ```
    
    ## How was this patch tested?
    
    `DDLSuite`, `DDLCommandSuite` and `HiveDDLCommandSuite`

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/andrewor14/spark alter-partition-ddl

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/spark/pull/12220.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #12220
    
----
commit b1ea87ed2757dfa5fa48e772575a20959050ef8f
Author: Andrew Or <an...@databricks.com>
Date:   2016-04-06T20:47:45Z

    Throw exception for MSCK REPAIR TABLE

commit 1690596460f93377c8192763666a7767b2a17bda
Author: Andrew Or <an...@databricks.com>
Date:   2016-04-06T21:07:48Z

    Throw exception on (un)archive partition

commit e24e007a1a8e6dfc03c7477788af7fbc6bdd6a11
Author: Andrew Or <an...@databricks.com>
Date:   2016-04-06T21:45:54Z

    Implement add partitions

commit 18de2b39dd25bfd0caf42b83326523e7c46d66b2
Author: Andrew Or <an...@databricks.com>
Date:   2016-04-06T21:48:08Z

    Throw exception for exchange partition

commit 70586761f48fc98ea6c8131445c0606e3e3b129d
Author: Andrew Or <an...@databricks.com>
Date:   2016-04-06T22:13:08Z

    Implement drop partitions

commit cccc66914aa9a379183a2fca7181dbae03907d0d
Author: Andrew Or <an...@databricks.com>
Date:   2016-04-06T22:14:31Z

    Throw exception for touch/compact/concatenate

commit cdbf251d06a19c3656b7d93f390418c685dd85c2
Author: Andrew Or <an...@databricks.com>
Date:   2016-04-06T22:20:15Z

    Implement rename partition

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14132][SPARK-14133][SQL] Alter table pa...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/12220#issuecomment-208655091
  
    **[Test build #55565 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55565/consoleFull)** for PR 12220 at commit [`e3f08b3`](https://github.com/apache/spark/commit/e3f08b35efa502f34aabcbba266350e4bfefb0c9).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14132][SPARK-14133][SQL] Alter table pa...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/12220#issuecomment-207649169
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/55396/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14132][SPARK-14133][SQL] Alter table pa...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/12220#issuecomment-207631456
  
    **[Test build #55391 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55391/consoleFull)** for PR 12220 at commit [`12cf973`](https://github.com/apache/spark/commit/12cf973da405d6209f0af6842613cf546bcf8471).
     * 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(`


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14132][SPARK-14133][SQL] Alter table pa...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/12220#issuecomment-206624966
  
    **[Test build #55158 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55158/consoleFull)** for PR 12220 at commit [`eedb529`](https://github.com/apache/spark/commit/eedb5297ce2736e75c3181efeadaa5d6c09e54f1).
     * This patch **fails Spark unit tests**.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `final class VectorizedParquetInputFormat extends ParquetInputFormat[InternalRow] `


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14132][SPARK-14133][SQL] Alter table pa...

Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:

    https://github.com/apache/spark/pull/12220#issuecomment-207598920
  
    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.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14132][SPARK-14133][SQL] Alter table pa...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/12220#issuecomment-207648979
  
    **[Test build #55396 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55396/consoleFull)** for PR 12220 at commit [`12cf973`](https://github.com/apache/spark/commit/12cf973da405d6209f0af6842613cf546bcf8471).
     * 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(`


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14132][SPARK-14133][SQL] Alter table pa...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/12220#issuecomment-207625025
  
    **[Test build #55390 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55390/consoleFull)** for PR 12220 at commit [`3fb9a87`](https://github.com/apache/spark/commit/3fb9a87cc786b23c5bad6a651e4163afbae0a045).
     * This patch **fails Spark unit tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14132][SPARK-14133][SQL] Alter table pa...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/12220#issuecomment-207186730
  
    **[Test build #55286 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55286/consoleFull)** for PR 12220 at commit [`220141d`](https://github.com/apache/spark/commit/220141dc748d8278513a7fece270f26be3f33717).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14132][SPARK-14133][SQL] Alter table pa...

Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on the pull request:

    https://github.com/apache/spark/pull/12220#issuecomment-207609848
  
    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?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14132][SPARK-14133][SQL] Alter table pa...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/12220#issuecomment-207149433
  
    **[Test build #55286 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55286/consoleFull)** for PR 12220 at commit [`220141d`](https://github.com/apache/spark/commit/220141dc748d8278513a7fece270f26be3f33717).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14132][SPARK-14133][SQL] Alter table pa...

Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the pull request:

    https://github.com/apache/spark/pull/12220#issuecomment-208667326
  
    Sure, will do it after this is merged. Thanks!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14132][SPARK-14133][SQL] Alter table pa...

Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:

    https://github.com/apache/spark/pull/12220#discussion_r59097205
  
    --- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala ---
    @@ -367,9 +367,27 @@ private[hive] class HiveClientImpl(
       override def dropPartitions(
           db: String,
           table: String,
    -      specs: Seq[ExternalCatalog.TablePartitionSpec]): Unit = withHiveState {
    +      specs: Seq[ExternalCatalog.TablePartitionSpec],
    +      ignoreIfNotExists: Boolean,
    +      purge: Boolean): Unit = withHiveState {
         // TODO: figure out how to drop multiple partitions in one call
    -    specs.foreach { s => client.dropPartition(db, table, s.values.toList.asJava, true) }
    +    val hiveTable = client.getTable(db, table, true /* throw exception */)
    +    specs.foreach { s =>
    +      // 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
    +      if (matchingParts.isEmpty && !ignoreIfNotExists) {
    +        throw new AnalysisException(
    +          s"partition to drop '$s' does not exist in table '$table' database '$db'")
    +      }
    +      matchingParts.foreach { hivePartition =>
    +        val dropOptions = new PartitionDropOptions
    +        dropOptions.ifExists = ignoreIfNotExists
    +        dropOptions.purgeData = purge
    +        client.dropPartition(db, table, hivePartition.getValues, dropOptions)
    --- End diff --
    
    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.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14132][SPARK-14133][SQL] Alter table pa...

Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:

    https://github.com/apache/spark/pull/12220#issuecomment-206624499
  
    @yhuai


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14132][SPARK-14133][SQL] Alter table pa...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/12220#issuecomment-207187148
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/55286/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14132][SPARK-14133][SQL] Alter table pa...

Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on the pull request:

    https://github.com/apache/spark/pull/12220#issuecomment-207625555
  
    test this please


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14132][SPARK-14133][SQL] Alter table pa...

Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on the pull request:

    https://github.com/apache/spark/pull/12220#issuecomment-207606698
  
    LGTM pending jenkins


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14132][SPARK-14133][SQL] Alter table pa...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/12220#issuecomment-208683450
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14132][SPARK-14133][SQL] Alter table pa...

Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:

    https://github.com/apache/spark/pull/12220#discussion_r59593862
  
    --- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala ---
    @@ -367,9 +367,25 @@ private[hive] class HiveClientImpl(
       override def dropPartitions(
           db: String,
           table: String,
    -      specs: Seq[ExternalCatalog.TablePartitionSpec]): Unit = withHiveState {
    +      specs: Seq[ExternalCatalog.TablePartitionSpec],
    +      ignoreIfNotExists: Boolean): Unit = withHiveState {
         // TODO: figure out how to drop multiple partitions in one call
    -    specs.foreach { s => client.dropPartition(db, table, s.values.toList.asJava, true) }
    +    val hiveTable = client.getTable(db, table, true /* throw exception */)
    +    specs.foreach { s =>
    +      // 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
    --- End diff --
    
    yeah. True


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14132][SPARK-14133][SQL] Alter table pa...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/12220#issuecomment-207601969
  
    **[Test build #55391 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55391/consoleFull)** for PR 12220 at commit [`12cf973`](https://github.com/apache/spark/commit/12cf973da405d6209f0af6842613cf546bcf8471).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14132][SPARK-14133][SQL] Alter table pa...

Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:

    https://github.com/apache/spark/pull/12220#discussion_r59596474
  
    --- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala ---
    @@ -367,9 +367,25 @@ private[hive] class HiveClientImpl(
       override def dropPartitions(
           db: String,
           table: String,
    -      specs: Seq[ExternalCatalog.TablePartitionSpec]): Unit = withHiveState {
    +      specs: Seq[ExternalCatalog.TablePartitionSpec],
    +      ignoreIfNotExists: Boolean): Unit = withHiveState {
         // TODO: figure out how to drop multiple partitions in one call
    -    specs.foreach { s => client.dropPartition(db, table, s.values.toList.asJava, true) }
    +    val hiveTable = client.getTable(db, table, true /* throw exception */)
    +    specs.foreach { s =>
    +      // 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
    --- End diff --
    
    Yeah, will use this JIRA to handle all the cases. Thanks!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14132][SPARK-14133][SQL] Alter table pa...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/spark/pull/12220


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14132][SPARK-14133][SQL] Alter table pa...

Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:

    https://github.com/apache/spark/pull/12220#discussion_r59308833
  
    --- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala ---
    @@ -367,9 +367,27 @@ private[hive] class HiveClientImpl(
       override def dropPartitions(
           db: String,
           table: String,
    -      specs: Seq[ExternalCatalog.TablePartitionSpec]): Unit = withHiveState {
    +      specs: Seq[ExternalCatalog.TablePartitionSpec],
    +      ignoreIfNotExists: Boolean,
    +      purge: Boolean): Unit = withHiveState {
         // TODO: figure out how to drop multiple partitions in one call
    -    specs.foreach { s => client.dropPartition(db, table, s.values.toList.asJava, true) }
    +    val hiveTable = client.getTable(db, table, true /* throw exception */)
    +    specs.foreach { s =>
    +      // 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
    +      if (matchingParts.isEmpty && !ignoreIfNotExists) {
    +        throw new AnalysisException(
    +          s"partition to drop '$s' does not exist in table '$table' database '$db'")
    +      }
    +      matchingParts.foreach { hivePartition =>
    +        val dropOptions = new PartitionDropOptions
    +        dropOptions.ifExists = ignoreIfNotExists
    +        dropOptions.purgeData = purge
    +        client.dropPartition(db, table, hivePartition.getValues, dropOptions)
    --- End diff --
    
    removed


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14132][SPARK-14133][SQL] Alter table pa...

Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on the pull request:

    https://github.com/apache/spark/pull/12220#issuecomment-207612627
  
    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!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14132][SPARK-14133][SQL] Alter table pa...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/12220#issuecomment-207599354
  
    **[Test build #55390 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55390/consoleFull)** for PR 12220 at commit [`3fb9a87`](https://github.com/apache/spark/commit/3fb9a87cc786b23c5bad6a651e4163afbae0a045).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14132][SPARK-14133][SQL] Alter table pa...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/12220#issuecomment-207626796
  
    **[Test build #55396 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55396/consoleFull)** for PR 12220 at commit [`12cf973`](https://github.com/apache/spark/commit/12cf973da405d6209f0af6842613cf546bcf8471).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14132][SPARK-14133][SQL] Alter table pa...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/12220#issuecomment-207097736
  
    **[Test build #55245 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55245/consoleFull)** for PR 12220 at commit [`85c8b8f`](https://github.com/apache/spark/commit/85c8b8f8fda68bcfd1f551e952040d281a5ca38c).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14132][SPARK-14133][SQL] Alter table pa...

Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the pull request:

    https://github.com/apache/spark/pull/12220#issuecomment-207815780
  
    @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.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14132][SPARK-14133][SQL] Alter table pa...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/12220#issuecomment-207649166
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14132][SPARK-14133][SQL] Alter table pa...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/12220#issuecomment-206625094
  
    Merged build finished. Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14132][SPARK-14133][SQL] Alter table pa...

Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:

    https://github.com/apache/spark/pull/12220#discussion_r59082635
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala ---
    @@ -333,23 +367,32 @@ case class AlterTableExchangePartition(
      * unless 'purge' is true, but the metadata is completely lost.
      * An error message will be issued if the partition does not exist, unless 'ifExists' is true.
      * Note: purge is always false when the target is a view.
    + *
    + * 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, ...];
    --- End diff --
    
    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?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14132][SPARK-14133][SQL] Alter table pa...

Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:

    https://github.com/apache/spark/pull/12220#discussion_r59555113
  
    --- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala ---
    @@ -367,9 +367,25 @@ private[hive] class HiveClientImpl(
       override def dropPartitions(
           db: String,
           table: String,
    -      specs: Seq[ExternalCatalog.TablePartitionSpec]): Unit = withHiveState {
    +      specs: Seq[ExternalCatalog.TablePartitionSpec],
    +      ignoreIfNotExists: Boolean): Unit = withHiveState {
         // TODO: figure out how to drop multiple partitions in one call
    -    specs.foreach { s => client.dropPartition(db, table, s.values.toList.asJava, true) }
    +    val hiveTable = client.getTable(db, table, true /* throw exception */)
    +    specs.foreach { s =>
    +      // 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
    --- End diff --
    
    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. The expected return should be an empty set, right?
    
    Two possible solutions:
    
    1. get the whole list, and filter it out by ourselves. Do you want me to write a test case and fix this issue?
    2. check if the specs contains any column that is not part of table partitioning at the beginning.  @andrewor14 @yhuai 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14132][SPARK-14133][SQL] Alter table pa...

Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on the pull request:

    https://github.com/apache/spark/pull/12220#issuecomment-208693034
  
    Merging to master!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14132][SPARK-14133][SQL] Alter table pa...

Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:

    https://github.com/apache/spark/pull/12220#discussion_r59591625
  
    --- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala ---
    @@ -367,9 +367,25 @@ private[hive] class HiveClientImpl(
       override def dropPartitions(
           db: String,
           table: String,
    -      specs: Seq[ExternalCatalog.TablePartitionSpec]): Unit = withHiveState {
    +      specs: Seq[ExternalCatalog.TablePartitionSpec],
    +      ignoreIfNotExists: Boolean): Unit = withHiveState {
         // TODO: figure out how to drop multiple partitions in one call
    -    specs.foreach { s => client.dropPartition(db, table, s.values.toList.asJava, true) }
    +    val hiveTable = client.getTable(db, table, true /* throw exception */)
    +    specs.foreach { s =>
    +      // 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
    --- End diff --
    
    I think SessionCatalog should take care the check.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14132][SPARK-14133][SQL] Alter table pa...

Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:

    https://github.com/apache/spark/pull/12220#discussion_r59084949
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala ---
    @@ -307,24 +308,57 @@ case class AlterTableSerDeProperties(
      * 'partitionSpecsAndLocs': the syntax of ALTER VIEW is identical to ALTER TABLE,
      * EXCEPT that it is ILLEGAL to specify a LOCATION clause.
      * An error message will be issued if the partition exists, unless 'ifNotExists' is true.
    + *
    + * The syntax of this command is:
    + * {{{
    + *   ALTER TABLE table ADD [IF NOT EXISTS] PARTITION spec [LOCATION 'loc1']
    + *   ALTER VIEW view ADD [IF NOT EXISTS] PARTITION spec
    --- End diff --
    
    Let's also remove `ALTER VIEW view ADD [IF NOT EXISTS] PARTITION spec`.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14132][SPARK-14133][SQL] Alter table pa...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/12220#issuecomment-207138911
  
    **[Test build #55274 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55274/consoleFull)** for PR 12220 at commit [`2a37c75`](https://github.com/apache/spark/commit/2a37c75fe19fd985ec386455570d69b922e17ac8).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14132][SPARK-14133][SQL] Alter table pa...

Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on the pull request:

    https://github.com/apache/spark/pull/12220#issuecomment-208662098
  
    LGTM!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14132][SPARK-14133][SQL] Alter table pa...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/12220#issuecomment-207139617
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/55274/
    Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14132][SPARK-14133][SQL] Alter table pa...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/12220#issuecomment-207124681
  
    **[Test build #55245 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55245/consoleFull)** for PR 12220 at commit [`85c8b8f`](https://github.com/apache/spark/commit/85c8b8f8fda68bcfd1f551e952040d281a5ca38c).
     * This patch **fails Spark unit tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14132][SPARK-14133][SQL] Alter table pa...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/12220#issuecomment-207631671
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/55391/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14132][SPARK-14133][SQL] Alter table pa...

Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:

    https://github.com/apache/spark/pull/12220#issuecomment-208654664
  
    @gatorsmile that would be great, thanks!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14132][SPARK-14133][SQL] Alter table pa...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/12220#issuecomment-207631670
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14132][SPARK-14133][SQL] Alter table pa...

Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:

    https://github.com/apache/spark/pull/12220#discussion_r59595797
  
    --- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala ---
    @@ -367,9 +367,25 @@ private[hive] class HiveClientImpl(
       override def dropPartitions(
           db: String,
           table: String,
    -      specs: Seq[ExternalCatalog.TablePartitionSpec]): Unit = withHiveState {
    +      specs: Seq[ExternalCatalog.TablePartitionSpec],
    +      ignoreIfNotExists: Boolean): Unit = withHiveState {
         // TODO: figure out how to drop multiple partitions in one call
    -    specs.foreach { s => client.dropPartition(db, table, s.values.toList.asJava, true) }
    +    val hiveTable = client.getTable(db, table, true /* throw exception */)
    +    specs.foreach { s =>
    +      // 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
    --- End diff --
    
    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.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14132][SPARK-14133][SQL] Alter table pa...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/12220#issuecomment-208683294
  
    **[Test build #55565 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55565/consoleFull)** for PR 12220 at commit [`e3f08b3`](https://github.com/apache/spark/commit/e3f08b35efa502f34aabcbba266350e4bfefb0c9).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14132][SPARK-14133][SQL] Alter table pa...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/12220#issuecomment-207139613
  
    **[Test build #55274 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55274/consoleFull)** for PR 12220 at commit [`2a37c75`](https://github.com/apache/spark/commit/2a37c75fe19fd985ec386455570d69b922e17ac8).
     * This patch **fails to build**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14132][SPARK-14133][SQL] Alter table pa...

Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:

    https://github.com/apache/spark/pull/12220#discussion_r59591995
  
    --- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala ---
    @@ -367,9 +367,25 @@ private[hive] class HiveClientImpl(
       override def dropPartitions(
           db: String,
           table: String,
    -      specs: Seq[ExternalCatalog.TablePartitionSpec]): Unit = withHiveState {
    +      specs: Seq[ExternalCatalog.TablePartitionSpec],
    +      ignoreIfNotExists: Boolean): Unit = withHiveState {
         // TODO: figure out how to drop multiple partitions in one call
    -    specs.foreach { s => client.dropPartition(db, table, s.values.toList.asJava, true) }
    +    val hiveTable = client.getTable(db, table, true /* throw exception */)
    +    specs.foreach { s =>
    +      // 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
    --- End diff --
    
    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.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14132][SPARK-14133][SQL] Alter table pa...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/12220#issuecomment-207125100
  
    Merged build finished. Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14132][SPARK-14133][SQL] Alter table pa...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/12220#issuecomment-206598759
  
    **[Test build #55158 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55158/consoleFull)** for PR 12220 at commit [`eedb529`](https://github.com/apache/spark/commit/eedb5297ce2736e75c3181efeadaa5d6c09e54f1).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14132][SPARK-14133][SQL] Alter table pa...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/12220#issuecomment-207139616
  
    Merged build finished. Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14132][SPARK-14133][SQL] Alter table pa...

Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:

    https://github.com/apache/spark/pull/12220#issuecomment-207618996
  
    OK, let's do that in a separate patch.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14132][SPARK-14133][SQL] Alter table pa...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/12220#issuecomment-207187142
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14132][SPARK-14133][SQL] Alter table pa...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/12220#issuecomment-206625095
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/55158/
    Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14132][SPARK-14133][SQL] Alter table pa...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/12220#issuecomment-207625167
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/55390/
    Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14132][SPARK-14133][SQL] Alter table pa...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/12220#issuecomment-207125103
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/55245/
    Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14132][SPARK-14133][SQL] Alter table pa...

Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:

    https://github.com/apache/spark/pull/12220#discussion_r59593022
  
    --- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala ---
    @@ -367,9 +367,25 @@ private[hive] class HiveClientImpl(
       override def dropPartitions(
           db: String,
           table: String,
    -      specs: Seq[ExternalCatalog.TablePartitionSpec]): Unit = withHiveState {
    +      specs: Seq[ExternalCatalog.TablePartitionSpec],
    +      ignoreIfNotExists: Boolean): Unit = withHiveState {
         // TODO: figure out how to drop multiple partitions in one call
    -    specs.foreach { s => client.dropPartition(db, table, s.values.toList.asJava, true) }
    +    val hiveTable = client.getTable(db, table, true /* throw exception */)
    +    specs.foreach { s =>
    +      // 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
    --- End diff --
    
    You meant that `a` was not a partitioning column, right?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14132][SPARK-14133][SQL] Alter table pa...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/12220#issuecomment-207625166
  
    Merged build finished. Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14132][SPARK-14133][SQL] Alter table pa...

Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:

    https://github.com/apache/spark/pull/12220#discussion_r59559335
  
    --- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala ---
    @@ -367,9 +367,25 @@ private[hive] class HiveClientImpl(
       override def dropPartitions(
           db: String,
           table: String,
    -      specs: Seq[ExternalCatalog.TablePartitionSpec]): Unit = withHiveState {
    +      specs: Seq[ExternalCatalog.TablePartitionSpec],
    +      ignoreIfNotExists: Boolean): Unit = withHiveState {
         // TODO: figure out how to drop multiple partitions in one call
    -    specs.foreach { s => client.dropPartition(db, table, s.values.toList.asJava, true) }
    +    val hiveTable = client.getTable(db, table, true /* throw exception */)
    +    specs.foreach { s =>
    +      // 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
    --- End diff --
    
    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
    ```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14132][SPARK-14133][SQL] Alter table pa...

Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:

    https://github.com/apache/spark/pull/12220#discussion_r59593951
  
    --- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala ---
    @@ -367,9 +367,25 @@ private[hive] class HiveClientImpl(
       override def dropPartitions(
           db: String,
           table: String,
    -      specs: Seq[ExternalCatalog.TablePartitionSpec]): Unit = withHiveState {
    +      specs: Seq[ExternalCatalog.TablePartitionSpec],
    +      ignoreIfNotExists: Boolean): Unit = withHiveState {
         // TODO: figure out how to drop multiple partitions in one call
    -    specs.foreach { s => client.dropPartition(db, table, s.values.toList.asJava, true) }
    +    val hiveTable = client.getTable(db, table, true /* throw exception */)
    +    specs.foreach { s =>
    +      // 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
    --- End diff --
    
    Will do it soon. Thanks!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14132][SPARK-14133][SQL] Alter table pa...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/12220#issuecomment-208683451
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/55565/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org