You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2021/02/24 17:27:45 UTC

[GitHub] [spark] cloud-fan opened a new pull request #31637: [SPARK-34524][SQL] Simplify v2 partition commands resolution

cloud-fan opened a new pull request #31637:
URL: https://github.com/apache/spark/pull/31637


   <!--
   Thanks for sending a pull request!  Here are some tips for you:
     1. If this is your first time, please read our contributor guidelines: https://spark.apache.org/contributing.html
     2. Ensure you have added or run the appropriate tests for your PR: https://spark.apache.org/developer-tools.html
     3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][SPARK-XXXX] Your PR title ...'.
     4. Be sure to keep the PR description updated to reflect all changes.
     5. Please write your PR title to summarize what this PR proposes.
     6. If possible, provide a concise example to reproduce the issue for a faster review.
     7. If you want to add a new configuration, please read the guideline first for naming configurations in
        'core/src/main/scala/org/apache/spark/internal/config/ConfigEntry.scala'.
   -->
   
   ### What changes were proposed in this pull request?
   <!--
   Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue. 
   If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
     1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
     2. If you fix some SQL features, you can provide some references of other DBMSes.
     3. If there is design documentation, please add the link.
     4. If there is a discussion in the mailing list, please add the link.
   -->
   This PR simplifies the resolution of v2 partition commands:
   1. Add a common trait for v2 partition commands, so that we don't need to match them one by one in the rules.
   2. Make partition spec an expression, so that it's easier to resolve them via tree node transformation.
   3. Simplify `CheckAnalysis` to only check if the table is partitioned. For partitioned tables, partition spec is always resolved, so we don't need to check it. The `SupportsAtomicPartitionManagement` check is also done in the runtime. Since Spark eagerly executes commands, exception in runtime will also be thrown at analysis time.
   
   ### Why are the changes needed?
   <!--
   Please clarify why the changes are needed. For instance,
     1. If you propose a new API, clarify the use case for a new API.
     2. If you fix a bug, you can clarify why it is a bug.
   -->
   code cleanup
   
   ### Does this PR introduce _any_ user-facing change?
   <!--
   Note that it means *any* user-facing change including all aspects such as the documentation fix.
   If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible.
   If possible, please also clarify if this is a user-facing change compared to the released Spark versions or within the unreleased branches such as master.
   If no, write 'No'.
   -->
   no
   
   ### How was this patch tested?
   <!--
   If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
   If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
   If tests were not added, please describe why they were not added and/or why it was difficult to add.
   -->
   existing tests


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [spark] SparkQA commented on pull request #31637: [SPARK-34524][SQL] Simplify v2 partition commands resolution

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31637:
URL: https://github.com/apache/spark/pull/31637#issuecomment-786613338


   **[Test build #135498 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135498/testReport)** for PR 31637 at commit [`a2c6575`](https://github.com/apache/spark/commit/a2c6575cf06660bee6245bc6a23da8713205c74c).
    * This patch passes all tests.
    * This patch merges cleanly.
    * This patch adds no public classes.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [spark] SparkQA commented on pull request #31637: [SPARK-34524][SQL] Simplify v2 partition commands resolution

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31637:
URL: https://github.com/apache/spark/pull/31637#issuecomment-786477051


   **[Test build #135498 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135498/testReport)** for PR 31637 at commit [`a2c6575`](https://github.com/apache/spark/commit/a2c6575cf06660bee6245bc6a23da8713205c74c).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [spark] SparkQA removed a comment on pull request #31637: [SPARK-34524][SQL] Simplify v2 partition commands resolution

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31637:
URL: https://github.com/apache/spark/pull/31637#issuecomment-786477051


   **[Test build #135498 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135498/testReport)** for PR 31637 at commit [`a2c6575`](https://github.com/apache/spark/commit/a2c6575cf06660bee6245bc6a23da8713205c74c).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [spark] imback82 commented on a change in pull request #31637: [SPARK-34524][SQL] Simplify v2 partition commands resolution

Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #31637:
URL: https://github.com/apache/spark/pull/31637#discussion_r583784452



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala
##########
@@ -150,6 +150,23 @@ trait CheckAnalysis extends PredicateHelper with LookupCatalog {
       case AlterTable(_, _, u: UnresolvedV2Relation, _) =>
         failAnalysis(s"Table not found: ${u.originalNameParts.quoted}")
 
+      case command: V2PartitionCommand =>
+        command.table match {
+          case r @ ResolvedTable(_, _, table, _) => table match {
+            case t: SupportsPartitionManagement =>
+              if (t.partitionSchema.isEmpty) {
+                failAnalysis(s"Table ${r.name} is not partitioned.")
+              }
+            case _ =>
+              failAnalysis(s"Table ${r.name} does not support partition management.")
+          }
+          case _ =>
+        }
+
+      // `ShowTableExtended` should have been converted to the v1 command if the table is v1.
+      case _: ShowTableExtended =>
+        throw new AnalysisException("SHOW TABLE EXTENDED is not supported for v2 tables.")

Review comment:
       Not related to this PR, but should we also move other commands that do not support v2 tables here from `DataSourceV2Strategy`?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [spark] SparkQA removed a comment on pull request #31637: [SPARK-34524][SQL] Simplify v2 partition commands resolution

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31637:
URL: https://github.com/apache/spark/pull/31637#issuecomment-785728067


   **[Test build #135462 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135462/testReport)** for PR 31637 at commit [`4bbf08d`](https://github.com/apache/spark/commit/4bbf08df553faefc32ab9f11175ca900d79d0cc9).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [spark] HyukjinKwon commented on a change in pull request #31637: [SPARK-34524][SQL] Simplify v2 partition commands resolution

Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #31637:
URL: https://github.com/apache/spark/pull/31637#discussion_r582423742



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/v2Commands.scala
##########
@@ -59,6 +59,11 @@ trait V2WriteCommand extends Command {
   def withNewTable(newTable: NamedRelation): V2WriteCommand
 }
 
+trait V2PartitionCommand extends Command {
+  def table: LogicalPlan
+  def allowPartialPartitionSpec: Boolean = false

Review comment:
       What's the partial partition sepc? It would be great if we can document what this means.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [spark] AmplabJenkins removed a comment on pull request #31637: [SPARK-34524][SQL] Simplify v2 partition commands resolution

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31637:
URL: https://github.com/apache/spark/pull/31637#issuecomment-785394440


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135438/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [spark] HyukjinKwon commented on a change in pull request #31637: [SPARK-34524][SQL] Simplify v2 partition commands resolution

Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #31637:
URL: https://github.com/apache/spark/pull/31637#discussion_r582426743



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/v2Commands.scala
##########
@@ -59,6 +59,11 @@ trait V2WriteCommand extends Command {
   def withNewTable(newTable: NamedRelation): V2WriteCommand
 }
 
+trait V2PartitionCommand extends Command {
+  def table: LogicalPlan
+  def allowPartialPartitionSpec: Boolean = false

Review comment:
       Maybe name it `requireExactMatchedPartitionSpec` or `shouldExactlyMatchPartitionSpec`? I don't feel strongly. I am good with leaving it as is too.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [spark] cloud-fan commented on pull request #31637: [SPARK-34524][SQL] Simplify v2 partition commands resolution

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #31637:
URL: https://github.com/apache/spark/pull/31637#issuecomment-785242508


   cc @MaxGekk @imback82 @HyukjinKwon 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [spark] HyukjinKwon commented on a change in pull request #31637: [SPARK-34524][SQL] Simplify v2 partition commands resolution

Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #31637:
URL: https://github.com/apache/spark/pull/31637#discussion_r582426743



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/v2Commands.scala
##########
@@ -59,6 +59,11 @@ trait V2WriteCommand extends Command {
   def withNewTable(newTable: NamedRelation): V2WriteCommand
 }
 
+trait V2PartitionCommand extends Command {
+  def table: LogicalPlan
+  def allowPartialPartitionSpec: Boolean = false

Review comment:
       Maybe name it `requireExactMatchedPartitionSpec` or `shoildExactlyMatchPartitionSpec`

##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/v2Commands.scala
##########
@@ -59,6 +59,11 @@ trait V2WriteCommand extends Command {
   def withNewTable(newTable: NamedRelation): V2WriteCommand
 }
 
+trait V2PartitionCommand extends Command {
+  def table: LogicalPlan
+  def allowPartialPartitionSpec: Boolean = false

Review comment:
       Maybe name it `requireExactMatchedPartitionSpec` or `shouldExactlyMatchPartitionSpec`




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [spark] AmplabJenkins commented on pull request #31637: [SPARK-34524][SQL] Simplify v2 partition commands resolution

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31637:
URL: https://github.com/apache/spark/pull/31637#issuecomment-785351200


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135435/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [spark] SparkQA commented on pull request #31637: [SPARK-34524][SQL] Simplify v2 partition commands resolution

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31637:
URL: https://github.com/apache/spark/pull/31637#issuecomment-786503285


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


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [spark] SparkQA commented on pull request #31637: [SPARK-34524][SQL] Simplify v2 partition commands resolution

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31637:
URL: https://github.com/apache/spark/pull/31637#issuecomment-786488186


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


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [spark] cloud-fan commented on a change in pull request #31637: [SPARK-34524][SQL] Simplify v2 partition commands resolution

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31637:
URL: https://github.com/apache/spark/pull/31637#discussion_r582590874



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala
##########
@@ -1009,35 +1005,4 @@ trait CheckAnalysis extends PredicateHelper with LookupCatalog {
       case _ =>
     }
   }
-
-  // Make sure that the `SHOW PARTITIONS` command is allowed for the table
-  private def checkShowPartitions(showPartitions: ShowPartitions): Unit = showPartitions match {
-    case ShowPartitions(rt: ResolvedTable, _, _)
-        if !rt.table.isInstanceOf[SupportsPartitionManagement] =>
-      failAnalysis("SHOW PARTITIONS cannot run for a table which does not support partitioning")
-    case ShowPartitions(ResolvedTable(_, _, partTable: SupportsPartitionManagement, _), _, _)
-        if partTable.partitionSchema().isEmpty =>

Review comment:
       fixed




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [spark] SparkQA commented on pull request #31637: [SPARK-34524][SQL] Simplify v2 partition commands resolution

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31637:
URL: https://github.com/apache/spark/pull/31637#issuecomment-785758304


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


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [spark] AmplabJenkins commented on pull request #31637: [SPARK-34524][SQL] Simplify v2 partition commands resolution

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31637:
URL: https://github.com/apache/spark/pull/31637#issuecomment-786619828


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135498/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [spark] AmplabJenkins commented on pull request #31637: [SPARK-34524][SQL] Simplify v2 partition commands resolution

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31637:
URL: https://github.com/apache/spark/pull/31637#issuecomment-785394440


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135438/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [spark] AmplabJenkins commented on pull request #31637: [SPARK-34524][SQL] Simplify v2 partition commands resolution

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31637:
URL: https://github.com/apache/spark/pull/31637#issuecomment-785356845


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/40018/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [spark] AmplabJenkins removed a comment on pull request #31637: [SPARK-34524][SQL] Simplify v2 partition commands resolution

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31637:
URL: https://github.com/apache/spark/pull/31637#issuecomment-785279071






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [spark] MaxGekk commented on a change in pull request #31637: [SPARK-34524][SQL] Simplify v2 partition commands resolution

Posted by GitBox <gi...@apache.org>.
MaxGekk commented on a change in pull request #31637:
URL: https://github.com/apache/spark/pull/31637#discussion_r582182987



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala
##########
@@ -1009,35 +1005,4 @@ trait CheckAnalysis extends PredicateHelper with LookupCatalog {
       case _ =>
     }
   }
-
-  // Make sure that the `SHOW PARTITIONS` command is allowed for the table
-  private def checkShowPartitions(showPartitions: ShowPartitions): Unit = showPartitions match {
-    case ShowPartitions(rt: ResolvedTable, _, _)
-        if !rt.table.isInstanceOf[SupportsPartitionManagement] =>
-      failAnalysis("SHOW PARTITIONS cannot run for a table which does not support partitioning")
-    case ShowPartitions(ResolvedTable(_, _, partTable: SupportsPartitionManagement, _), _, _)
-        if partTable.partitionSchema().isEmpty =>

Review comment:
       Where is the check now?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [spark] SparkQA commented on pull request #31637: [SPARK-34524][SQL] Simplify v2 partition commands resolution

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31637:
URL: https://github.com/apache/spark/pull/31637#issuecomment-785793669


   **[Test build #135462 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135462/testReport)** for PR 31637 at commit [`4bbf08d`](https://github.com/apache/spark/commit/4bbf08df553faefc32ab9f11175ca900d79d0cc9).
    * This patch **fails Spark unit tests**.
    * This patch merges cleanly.
    * This patch adds the following public classes _(experimental)_:
     * `sealed trait PartitionSpec extends LeafExpression with Unevaluable `
     * `trait V2PartitionCommand extends Command `
     * `case class TruncateTable(table: LogicalPlan) extends Command `
     * `case class TruncatePartition(`
     * `case class TruncatePartitionExec(`


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [spark] shaneknapp commented on pull request #31637: [SPARK-34524][SQL] Simplify v2 partition commands resolution

Posted by GitBox <gi...@apache.org>.
shaneknapp commented on pull request #31637:
URL: https://github.com/apache/spark/pull/31637#issuecomment-785300191


   test this please


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [spark] MaxGekk commented on a change in pull request #31637: [SPARK-34524][SQL] Simplify v2 partition commands resolution

Posted by GitBox <gi...@apache.org>.
MaxGekk commented on a change in pull request #31637:
URL: https://github.com/apache/spark/pull/31637#discussion_r582185077



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala
##########
@@ -1009,35 +1005,4 @@ trait CheckAnalysis extends PredicateHelper with LookupCatalog {
       case _ =>
     }
   }
-
-  // Make sure that the `SHOW PARTITIONS` command is allowed for the table
-  private def checkShowPartitions(showPartitions: ShowPartitions): Unit = showPartitions match {
-    case ShowPartitions(rt: ResolvedTable, _, _)
-        if !rt.table.isInstanceOf[SupportsPartitionManagement] =>
-      failAnalysis("SHOW PARTITIONS cannot run for a table which does not support partitioning")
-    case ShowPartitions(ResolvedTable(_, _, partTable: SupportsPartitionManagement, _), _, _)
-        if partTable.partitionSchema().isEmpty =>
-      failAnalysis(
-        s"SHOW PARTITIONS is not allowed on a table that is not partitioned: ${partTable.name()}")
-    case _ =>
-  }
-
-  private def checkTruncateTable(truncateTable: TruncateTable): Unit = truncateTable match {
-    case TruncateTable(rt: ResolvedTable, None) if !rt.table.isInstanceOf[TruncatableTable] =>

Review comment:
       This check and below guarded from `scala.MatchError` in `run()`. How does the contract work now?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [spark] SparkQA removed a comment on pull request #31637: [SPARK-34524][SQL] Simplify v2 partition commands resolution

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31637:
URL: https://github.com/apache/spark/pull/31637#issuecomment-785374501


   **[Test build #135438 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135438/testReport)** for PR 31637 at commit [`e85d4ef`](https://github.com/apache/spark/commit/e85d4ef872ccd975be1c3a48c4d3ad4372465eb4).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [spark] AmplabJenkins commented on pull request #31637: [SPARK-34524][SQL] Simplify v2 partition commands resolution

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31637:
URL: https://github.com/apache/spark/pull/31637#issuecomment-785789066


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/40042/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [spark] AmplabJenkins commented on pull request #31637: [SPARK-34524][SQL] Simplify v2 partition commands resolution

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31637:
URL: https://github.com/apache/spark/pull/31637#issuecomment-785279072






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [spark] SparkQA commented on pull request #31637: [SPARK-34524][SQL] Simplify v2 partition commands resolution

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31637:
URL: https://github.com/apache/spark/pull/31637#issuecomment-785331251


   **[Test build #135435 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135435/testReport)** for PR 31637 at commit [`e85d4ef`](https://github.com/apache/spark/commit/e85d4ef872ccd975be1c3a48c4d3ad4372465eb4).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [spark] SparkQA commented on pull request #31637: [SPARK-34524][SQL] Simplify v2 partition commands resolution

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31637:
URL: https://github.com/apache/spark/pull/31637#issuecomment-785374501


   **[Test build #135438 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135438/testReport)** for PR 31637 at commit [`e85d4ef`](https://github.com/apache/spark/commit/e85d4ef872ccd975be1c3a48c4d3ad4372465eb4).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [spark] cloud-fan commented on a change in pull request #31637: [SPARK-34524][SQL] Simplify v2 partition commands resolution

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31637:
URL: https://github.com/apache/spark/pull/31637#discussion_r584458526



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala
##########
@@ -150,6 +150,23 @@ trait CheckAnalysis extends PredicateHelper with LookupCatalog {
       case AlterTable(_, _, u: UnresolvedV2Relation, _) =>
         failAnalysis(s"Table not found: ${u.originalNameParts.quoted}")
 
+      case command: V2PartitionCommand =>
+        command.table match {
+          case r @ ResolvedTable(_, _, table, _) => table match {
+            case t: SupportsPartitionManagement =>
+              if (t.partitionSchema.isEmpty) {
+                failAnalysis(s"Table ${r.name} is not partitioned.")
+              }
+            case _ =>
+              failAnalysis(s"Table ${r.name} does not support partition management.")
+          }
+          case _ =>
+        }
+
+      // `ShowTableExtended` should have been converted to the v1 command if the table is v1.
+      case _: ShowTableExtended =>
+        throw new AnalysisException("SHOW TABLE EXTENDED is not supported for v2 tables.")

Review comment:
       Yea we should. I only moved this one because it broke some tests.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [spark] HyukjinKwon commented on a change in pull request #31637: [SPARK-34524][SQL] Simplify v2 partition commands resolution

Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #31637:
URL: https://github.com/apache/spark/pull/31637#discussion_r582423742



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/v2Commands.scala
##########
@@ -59,6 +59,11 @@ trait V2WriteCommand extends Command {
   def withNewTable(newTable: NamedRelation): V2WriteCommand
 }
 
+trait V2PartitionCommand extends Command {
+  def table: LogicalPlan
+  def allowPartialPartitionSpec: Boolean = false

Review comment:
       What's the partial partition sepc?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [spark] AmplabJenkins removed a comment on pull request #31637: [SPARK-34524][SQL] Simplify v2 partition commands resolution

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31637:
URL: https://github.com/apache/spark/pull/31637#issuecomment-785351200


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135435/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [spark] SparkQA commented on pull request #31637: [SPARK-34524][SQL] Simplify v2 partition commands resolution

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31637:
URL: https://github.com/apache/spark/pull/31637#issuecomment-785377367


   **[Test build #135438 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135438/testReport)** for PR 31637 at commit [`e85d4ef`](https://github.com/apache/spark/commit/e85d4ef872ccd975be1c3a48c4d3ad4372465eb4).
    * This patch **fails to build**.
    * This patch merges cleanly.
    * This patch adds the following public classes _(experimental)_:
     * `sealed trait PartitionSpec extends LeafExpression with Unevaluable `
     * `trait V2PartitionCommand extends Command `


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [spark] AmplabJenkins removed a comment on pull request #31637: [SPARK-34524][SQL] Simplify v2 partition commands resolution

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31637:
URL: https://github.com/apache/spark/pull/31637#issuecomment-785794065


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135462/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [spark] AmplabJenkins removed a comment on pull request #31637: [SPARK-34524][SQL] Simplify v2 partition commands resolution

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31637:
URL: https://github.com/apache/spark/pull/31637#issuecomment-786525905


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/40079/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [spark] AmplabJenkins removed a comment on pull request #31637: [SPARK-34524][SQL] Simplify v2 partition commands resolution

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31637:
URL: https://github.com/apache/spark/pull/31637#issuecomment-785356845


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/40018/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [spark] AmplabJenkins removed a comment on pull request #31637: [SPARK-34524][SQL] Simplify v2 partition commands resolution

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31637:
URL: https://github.com/apache/spark/pull/31637#issuecomment-785789066


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/40042/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [spark] SparkQA commented on pull request #31637: [SPARK-34524][SQL] Simplify v2 partition commands resolution

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31637:
URL: https://github.com/apache/spark/pull/31637#issuecomment-785258405


   **[Test build #135432 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135432/testReport)** for PR 31637 at commit [`e85d4ef`](https://github.com/apache/spark/commit/e85d4ef872ccd975be1c3a48c4d3ad4372465eb4).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [spark] HyukjinKwon commented on a change in pull request #31637: [SPARK-34524][SQL] Simplify v2 partition commands resolution

Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #31637:
URL: https://github.com/apache/spark/pull/31637#discussion_r582423742



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/v2Commands.scala
##########
@@ -59,6 +59,11 @@ trait V2WriteCommand extends Command {
   def withNewTable(newTable: NamedRelation): V2WriteCommand
 }
 
+trait V2PartitionCommand extends Command {
+  def table: LogicalPlan
+  def allowPartialPartitionSpec: Boolean = false

Review comment:
       What's the partial partition sepc? It would be great if we can document what this means.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [spark] SparkQA removed a comment on pull request #31637: [SPARK-34524][SQL] Simplify v2 partition commands resolution

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31637:
URL: https://github.com/apache/spark/pull/31637#issuecomment-785258405


   **[Test build #135432 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135432/testReport)** for PR 31637 at commit [`e85d4ef`](https://github.com/apache/spark/commit/e85d4ef872ccd975be1c3a48c4d3ad4372465eb4).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [spark] SparkQA commented on pull request #31637: [SPARK-34524][SQL] Simplify v2 partition commands resolution

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31637:
URL: https://github.com/apache/spark/pull/31637#issuecomment-785728067


   **[Test build #135462 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135462/testReport)** for PR 31637 at commit [`4bbf08d`](https://github.com/apache/spark/commit/4bbf08df553faefc32ab9f11175ca900d79d0cc9).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [spark] AmplabJenkins commented on pull request #31637: [SPARK-34524][SQL] Simplify v2 partition commands resolution

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31637:
URL: https://github.com/apache/spark/pull/31637#issuecomment-785794065


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135462/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [spark] SparkQA commented on pull request #31637: [SPARK-34524][SQL] Simplify v2 partition commands resolution

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31637:
URL: https://github.com/apache/spark/pull/31637#issuecomment-785356834


   Kubernetes integration test unable to build dist.
   
   exiting with code: 1
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/40018/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [spark] SparkQA commented on pull request #31637: [SPARK-34524][SQL] Simplify v2 partition commands resolution

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31637:
URL: https://github.com/apache/spark/pull/31637#issuecomment-785776628


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


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [spark] cloud-fan commented on pull request #31637: [SPARK-34524][SQL] Simplify v2 partition commands resolution

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #31637:
URL: https://github.com/apache/spark/pull/31637#issuecomment-786597803


   thanks for review, merging to master!


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [spark] cloud-fan commented on a change in pull request #31637: [SPARK-34524][SQL] Simplify v2 partition commands resolution

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31637:
URL: https://github.com/apache/spark/pull/31637#discussion_r582624173



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala
##########
@@ -1009,35 +1005,4 @@ trait CheckAnalysis extends PredicateHelper with LookupCatalog {
       case _ =>
     }
   }
-
-  // Make sure that the `SHOW PARTITIONS` command is allowed for the table
-  private def checkShowPartitions(showPartitions: ShowPartitions): Unit = showPartitions match {
-    case ShowPartitions(rt: ResolvedTable, _, _)
-        if !rt.table.isInstanceOf[SupportsPartitionManagement] =>
-      failAnalysis("SHOW PARTITIONS cannot run for a table which does not support partitioning")
-    case ShowPartitions(ResolvedTable(_, _, partTable: SupportsPartitionManagement, _), _, _)
-        if partTable.partitionSchema().isEmpty =>
-      failAnalysis(
-        s"SHOW PARTITIONS is not allowed on a table that is not partitioned: ${partTable.name()}")
-    case _ =>
-  }
-
-  private def checkTruncateTable(truncateTable: TruncateTable): Unit = truncateTable match {
-    case TruncateTable(rt: ResolvedTable, None) if !rt.table.isInstanceOf[TruncatableTable] =>

Review comment:
       I have updated the execution part, it will throw proper error instead of MatchError.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [spark] AmplabJenkins commented on pull request #31637: [SPARK-34524][SQL] Simplify v2 partition commands resolution

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31637:
URL: https://github.com/apache/spark/pull/31637#issuecomment-786525905


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/40079/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [spark] cloud-fan commented on a change in pull request #31637: [SPARK-34524][SQL] Simplify v2 partition commands resolution

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31637:
URL: https://github.com/apache/spark/pull/31637#discussion_r582590874



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala
##########
@@ -1009,35 +1005,4 @@ trait CheckAnalysis extends PredicateHelper with LookupCatalog {
       case _ =>
     }
   }
-
-  // Make sure that the `SHOW PARTITIONS` command is allowed for the table
-  private def checkShowPartitions(showPartitions: ShowPartitions): Unit = showPartitions match {
-    case ShowPartitions(rt: ResolvedTable, _, _)
-        if !rt.table.isInstanceOf[SupportsPartitionManagement] =>
-      failAnalysis("SHOW PARTITIONS cannot run for a table which does not support partitioning")
-    case ShowPartitions(ResolvedTable(_, _, partTable: SupportsPartitionManagement, _), _, _)
-        if partTable.partitionSchema().isEmpty =>

Review comment:
       If a `SupportsPartitionManagement` has empty partition schema, `ResolvePartitionSpec` fails earlier when calling `normalizePartitionSpec`




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [spark] shaneknapp commented on pull request #31637: [SPARK-34524][SQL] Simplify v2 partition commands resolution

Posted by GitBox <gi...@apache.org>.
shaneknapp commented on pull request #31637:
URL: https://github.com/apache/spark/pull/31637#issuecomment-785335005


   test this please (i'm trying to fix the -K8s github permissions)


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [spark] HyukjinKwon commented on a change in pull request #31637: [SPARK-34524][SQL] Simplify v2 partition commands resolution

Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #31637:
URL: https://github.com/apache/spark/pull/31637#discussion_r582424867



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/v2ResolutionPlans.scala
##########
@@ -103,12 +112,12 @@ case class ResolvedTable(
     catalog: TableCatalog,
     identifier: Identifier,
     table: Table,
-    outputAttributes: Seq[Attribute])
-  extends LeafNode {
+    outputAttributes: Seq[Attribute]) extends LeafNode {

Review comment:
       nit .. I wouldn't change this ..




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [spark] shaneknapp commented on pull request #31637: [SPARK-34524][SQL] Simplify v2 partition commands resolution

Posted by GitBox <gi...@apache.org>.
shaneknapp commented on pull request #31637:
URL: https://github.com/apache/spark/pull/31637#issuecomment-785360735


   > Kubernetes integration test unable to build dist.
   > 
   > exiting with code: 1
   > URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/40018/
   
   fixed posting from the k8s build, and hopefully stopped the 'build started' error spam in the logs


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [spark] SparkQA commented on pull request #31637: [SPARK-34524][SQL] Simplify v2 partition commands resolution

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31637:
URL: https://github.com/apache/spark/pull/31637#issuecomment-785261282


   **[Test build #135432 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135432/testReport)** for PR 31637 at commit [`e85d4ef`](https://github.com/apache/spark/commit/e85d4ef872ccd975be1c3a48c4d3ad4372465eb4).
    * This patch **fails to build**.
    * This patch merges cleanly.
    * This patch adds the following public classes _(experimental)_:
     * `sealed trait PartitionSpec extends LeafExpression with Unevaluable `
     * `trait V2PartitionCommand extends Command `


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [spark] cloud-fan closed pull request #31637: [SPARK-34524][SQL] Simplify v2 partition commands resolution

Posted by GitBox <gi...@apache.org>.
cloud-fan closed pull request #31637:
URL: https://github.com/apache/spark/pull/31637


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [spark] AmplabJenkins removed a comment on pull request #31637: [SPARK-34524][SQL] Simplify v2 partition commands resolution

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31637:
URL: https://github.com/apache/spark/pull/31637#issuecomment-786619828


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135498/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [spark] MaxGekk commented on a change in pull request #31637: [SPARK-34524][SQL] Simplify v2 partition commands resolution

Posted by GitBox <gi...@apache.org>.
MaxGekk commented on a change in pull request #31637:
URL: https://github.com/apache/spark/pull/31637#discussion_r582185077



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala
##########
@@ -1009,35 +1005,4 @@ trait CheckAnalysis extends PredicateHelper with LookupCatalog {
       case _ =>
     }
   }
-
-  // Make sure that the `SHOW PARTITIONS` command is allowed for the table
-  private def checkShowPartitions(showPartitions: ShowPartitions): Unit = showPartitions match {
-    case ShowPartitions(rt: ResolvedTable, _, _)
-        if !rt.table.isInstanceOf[SupportsPartitionManagement] =>
-      failAnalysis("SHOW PARTITIONS cannot run for a table which does not support partitioning")
-    case ShowPartitions(ResolvedTable(_, _, partTable: SupportsPartitionManagement, _), _, _)
-        if partTable.partitionSchema().isEmpty =>
-      failAnalysis(
-        s"SHOW PARTITIONS is not allowed on a table that is not partitioned: ${partTable.name()}")
-    case _ =>
-  }
-
-  private def checkTruncateTable(truncateTable: TruncateTable): Unit = truncateTable match {
-    case TruncateTable(rt: ResolvedTable, None) if !rt.table.isInstanceOf[TruncatableTable] =>

Review comment:
       This check and below guarded from `scala.MatchError` in `run()`. How do you guarantee that the exception does not happen?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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