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 2020/09/26 23:19:55 UTC

[GitHub] [spark] imback82 opened a new pull request #29880: [SPARK-33004][SQL] Migrate DESCRIBE column to use UnresolvedTableOrView to resolve the identifier 

imback82 opened a new pull request #29880:
URL: https://github.com/apache/spark/pull/29880


   <!--
   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 proposes to migrate `DESCRIBE tbl colname` to use `UnresolvedTableOrView` to resolve the table/view identifier. This allows consistent resolution rules (temp view first, etc.) to be applied for both v1/v2 commands. More info about the consistent resolution rule proposal can be found in [JIRA](https://issues.apache.org/jira/browse/SPARK-29900) or [proposal doc](https://docs.google.com/document/d/1hvLjGA8y_W_hhilpngXVub1Ebv8RsMap986nENCFnrg/edit?usp=sharing).
   
   ### 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.
   -->
   
   The current behavior is not consistent between v1 and v2 commands when resolving a temp view.
   In v2, the `t` in the following example is resolved to a table:
   ```scala
   sql("CREATE TABLE testcat.ns.t (id bigint) USING foo")
   sql("CREATE TEMPORARY VIEW t AS SELECT 2 as i")
   sql("USE testcat.ns")
   sql("DESCRIBE t i") // 't' is resolved to testcat.ns.t
   
   Describing columns is not supported for v2 tables.;
   org.apache.spark.sql.AnalysisException: Describing columns is not supported for v2 tables.;
   ```
   whereas in v1, the `t` is resolved to a temp view:
   ```scala
   sql("CREATE DATABASE test")
   sql("CREATE TABLE spark_catalog.test.t (id bigint) USING csv")
   sql("CREATE TEMPORARY VIEW t AS SELECT 2 as i")
   sql("USE spark_catalog.test")
   sql("DESCRIBE t i").show // 't' is resolved to a temp view
   
   +---------+----------+
   |info_name|info_value|
   +---------+----------+
   | col_name|         i|
   |data_type|       int|
   |  comment|      NULL|
   +---------+----------+
   ```
   
   ### 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'.
   -->
   After this PR, `DESCRIBE t i` is resolved to a temp view `t` instead of `testcat.ns.t`.
   
   ### 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.
   -->
   Added a new test
   


----------------------------------------------------------------
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 #29880: [SPARK-33004][SQL] Migrate DESCRIBE column to use UnresolvedTableOrView to resolve the identifier

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



##########
File path: sql/core/src/test/scala/org/apache/spark/sql/connector/DataSourceV2SQLSuite.scala
##########
@@ -168,7 +168,31 @@ class DataSourceV2SQLSuite
       Array("Provider", "foo", ""),
       Array(TableCatalog.PROP_OWNER.capitalize, defaultUser, ""),
       Array("Table Properties", "[bar=baz]", "")))
+  }
 
+  test("Describe column is not supported for v2 catalog") {

Review comment:
       There doesn't seem to be a test coverage for this, so I am adding here.

##########
File path: sql/core/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveSessionCatalog.scala
##########
@@ -256,16 +256,20 @@ class ResolveSessionCatalog(
     case RenameTableStatement(TempViewOrV1Table(oldName), newName, isView) =>
       AlterTableRenameCommand(oldName.asTableIdentifier, newName.asTableIdentifier, isView)
 
-    case DescribeRelation(ResolvedTable(_, ident, _: V1Table), partitionSpec, isExtended) =>
+    case DescribeRelation(r @ ResolvedTable(_, ident, _: V1Table), partitionSpec, isExtended)
+        if isSessionCatalog(r.catalog) =>

Review comment:
       This change is for `DESCRIBE table`, but adding this check for consistency.




----------------------------------------------------------------
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 #29880: [SPARK-33004][SQL] Migrate DESCRIBE column to use UnresolvedTableOrView to resolve the identifier

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


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


----------------------------------------------------------------
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 #29880: [SPARK-33004][SQL] Migrate DESCRIBE column to use UnresolvedTableOrView to resolve the identifier

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



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/v2Commands.scala
##########
@@ -315,6 +315,17 @@ case class DescribeRelation(
   override def output: Seq[Attribute] = DescribeTableSchema.describeTableAttributes()
 }
 
+/**
+ * The logical plan of the DESCRIBE relation_name col_name command that works for v2 tables.
+ */
+case class DescribeColumn(
+    relation: LogicalPlan,
+    colNameParts: Seq[String],

Review comment:
       > Does Hive support qualified column names in `DESCRIBE`?
   
   Hive [doesn't support qualified name](https://cwiki.apache.org/confluence/display/Hive/LanguageManual+DDL#LanguageManualDDL-Describe) for column names. For example:
   
   ```
   DESCRIBE hivesampletable default.hivesampletable.clientid
   
   java.sql.SQLException: Error while processing statement: FAILED: Execution Error, return code 1 from org.apache.hadoop.hive.ql.exec.DDLTask. cannot find field default from [0:clientid, 1:querytime, 2:market, 3:deviceplatform, 4:devicemake, 5:devicemodel, 6:state, 7:country, 8:querydwelltime, 9:sessionid, 10:sessionpagevieworder]
   ```
   
   
   




----------------------------------------------------------------
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 pull request #29880: [SPARK-33004][SQL] Migrate DESCRIBE column to use UnresolvedTableOrView to resolve the identifier

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


   cc @cloud-fan 
   
   Thanks!


----------------------------------------------------------------
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 #29880: [SPARK-33004][SQL] Migrate DESCRIBE column to use UnresolvedTableOrView to resolve the identifier

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


   **[Test build #129368 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/129368/testReport)** for PR 29880 at commit [`11cfcd3`](https://github.com/apache/spark/commit/11cfcd30f5e38789698cbbfdd3e2a740685339f0).


----------------------------------------------------------------
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 #29880: [SPARK-33004][SQL] Migrate DESCRIBE column to use UnresolvedTableOrView to resolve the identifier

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


   **[Test build #129464 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/129464/testReport)** for PR 29880 at commit [`97a4deb`](https://github.com/apache/spark/commit/97a4deb99439fb6f5903c44d3ca6b74bae1c2f78).
    * 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 #29880: [SPARK-33004][SQL] Migrate DESCRIBE column to use UnresolvedTableOrView to resolve the identifier

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


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


----------------------------------------------------------------
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 #29880: [SPARK-33004][SQL] Migrate DESCRIBE column to use UnresolvedTableOrView to resolve the identifier

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






----------------------------------------------------------------
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 #29880: [SPARK-33004][SQL] Migrate DESCRIBE column to use UnresolvedTableOrView to resolve the identifier

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



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveSessionCatalog.scala
##########
@@ -318,7 +320,7 @@ class ResolveSessionCatalog(
           ignoreIfExists = c.ifNotExists)
       }
 
-    case RefreshTable(r @ ResolvedTable(_, _, _: V1Table)) if isSessionCatalog(r.catalog) =>

Review comment:
       Ah sorry I missed these existing checks. If we must change some places to make it consistent, I think it's better to always add this `isSessionCatalog` check.




----------------------------------------------------------------
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 #29880: [SPARK-33004][SQL] Migrate DESCRIBE column to use UnresolvedTableOrView to resolve the identifier

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


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


----------------------------------------------------------------
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 #29880: [SPARK-33004][SQL] Migrate DESCRIBE column to use UnresolvedTableOrView to resolve the identifier

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



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveSessionCatalog.scala
##########
@@ -256,16 +256,20 @@ class ResolveSessionCatalog(
     case RenameTableStatement(TempViewOrV1Table(oldName), newName, isView) =>
       AlterTableRenameCommand(oldName.asTableIdentifier, newName.asTableIdentifier, isView)
 
-    case DescribeRelation(ResolvedTable(_, ident, _: V1Table), partitionSpec, isExtended) =>
+    case DescribeRelation(r @ ResolvedTable(_, ident, _: V1Table), partitionSpec, isExtended)
+        if isSessionCatalog(r.catalog) =>

Review comment:
       OK. I will remove the check here and other existing places.




----------------------------------------------------------------
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 #29880: [SPARK-33004][SQL] Migrate DESCRIBE column to use UnresolvedTableOrView to resolve the identifier

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


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


----------------------------------------------------------------
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 #29880: [SPARK-33004][SQL] Migrate DESCRIBE column to use UnresolvedTableOrView to resolve the identifier

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






----------------------------------------------------------------
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 #29880: [SPARK-33004][SQL] Migrate DESCRIBE column to use UnresolvedTableOrView to resolve the identifier

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


   **[Test build #129138 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/129138/testReport)** for PR 29880 at commit [`fd3f0eb`](https://github.com/apache/spark/commit/fd3f0eb8bf1ebee3ec94608699fb8b2b8afd49cb).


----------------------------------------------------------------
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 #29880: [SPARK-33004][SQL] Migrate DESCRIBE column to use UnresolvedTableOrView to resolve the identifier

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


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


----------------------------------------------------------------
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 #29880: [SPARK-33004][SQL] Migrate DESCRIBE column to use UnresolvedTableOrView to resolve the identifier

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






----------------------------------------------------------------
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 #29880: [SPARK-33004][SQL] Migrate DESCRIBE column to use UnresolvedTableOrView to resolve the identifier

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






----------------------------------------------------------------
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 #29880: [SPARK-33004][SQL] Migrate DESCRIBE column to use UnresolvedTableOrView to resolve the identifier

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






----------------------------------------------------------------
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 #29880: [SPARK-33004][SQL] Migrate DESCRIBE column to use UnresolvedTableOrView to resolve the identifier

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


   **[Test build #129368 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/129368/testReport)** for PR 29880 at commit [`11cfcd3`](https://github.com/apache/spark/commit/11cfcd30f5e38789698cbbfdd3e2a740685339f0).


----------------------------------------------------------------
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 #29880: [SPARK-33004][SQL] Migrate DESCRIBE column to use UnresolvedTableOrView to resolve the identifier

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


   **[Test build #129138 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/129138/testReport)** for PR 29880 at commit [`fd3f0eb`](https://github.com/apache/spark/commit/fd3f0eb8bf1ebee3ec94608699fb8b2b8afd49cb).
    * 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] imback82 commented on a change in pull request #29880: [SPARK-33004][SQL] Migrate DESCRIBE column to use UnresolvedTableOrView to resolve the identifier

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



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/v2Commands.scala
##########
@@ -315,6 +315,17 @@ case class DescribeRelation(
   override def output: Seq[Attribute] = DescribeTableSchema.describeTableAttributes()
 }
 
+/**
+ * The logical plan of the DESCRIBE relation_name col_name command that works for v2 tables.
+ */
+case class DescribeColumn(
+    relation: LogicalPlan,
+    colNameParts: Seq[String],

Review comment:
       Sure, will take a look.




----------------------------------------------------------------
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 #29880: [SPARK-33004][SQL] Migrate DESCRIBE column to use UnresolvedTableOrView to resolve the identifier

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


   


----------------------------------------------------------------
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 #29880: [SPARK-33004][SQL] Migrate DESCRIBE column to use UnresolvedTableOrView to resolve the identifier

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



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/DescribeTableSchema.scala
##########
@@ -18,7 +18,7 @@
 package org.apache.spark.sql.catalyst.plans
 
 import org.apache.spark.sql.catalyst.expressions.AttributeReference
-import org.apache.spark.sql.types.{MetadataBuilder, StringType, StructField, StructType}
+import org.apache.spark.sql.types.{MetadataBuilder, StringType}
 
 private[sql] object DescribeTableSchema {

Review comment:
       we should rename it to `DescribeCommandSchema`




----------------------------------------------------------------
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 #29880: [SPARK-33004][SQL] Migrate DESCRIBE column to use UnresolvedTableOrView to resolve the identifier

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



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/v2Commands.scala
##########
@@ -315,6 +315,17 @@ case class DescribeRelation(
   override def output: Seq[Attribute] = DescribeTableSchema.describeTableAttributes()
 }
 
+/**
+ * The logical plan of the DESCRIBE relation_name col_name command that works for v2 tables.
+ */
+case class DescribeColumn(
+    relation: LogicalPlan,
+    colNameParts: Seq[String],

Review comment:
       Yea we can't break it. When we add v2 `DecribeColumExec` later, we may want to change `Seq[String]` to `Attribute` here, so that we can unify the column resolution logic for v1 and  v2 commands.




----------------------------------------------------------------
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 #29880: [SPARK-33004][SQL] Migrate DESCRIBE column to use UnresolvedTableOrView to resolve the identifier

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



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/v2Commands.scala
##########
@@ -315,6 +315,17 @@ case class DescribeRelation(
   override def output: Seq[Attribute] = DescribeTableSchema.describeTableAttributes()
 }
 
+/**
+ * The logical plan of the DESCRIBE relation_name col_name command that works for v2 tables.
+ */
+case class DescribeColumn(
+    relation: LogicalPlan,
+    colNameParts: Seq[String],

Review comment:
       > A simple idea is to put an `UnresolvedAttribute` here, and analyzer can do the work for us.
   
   Since we need to have the relation resolved first, we need to match like the following in the analyzer:
   ```scala
   case DescribeColumn(r: ResolvedTable, u: UnresolvedAttribute, _) =>
   ...
   ```
   Is that what you had in mind?




----------------------------------------------------------------
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 #29880: [SPARK-33004][SQL] Migrate DESCRIBE column to use UnresolvedTableOrView to resolve the identifier

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






----------------------------------------------------------------
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 #29880: [SPARK-33004][SQL] Migrate DESCRIBE column to use UnresolvedTableOrView to resolve the identifier

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






----------------------------------------------------------------
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 #29880: [SPARK-33004][SQL] Migrate DESCRIBE column to use UnresolvedTableOrView to resolve the identifier

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



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveSessionCatalog.scala
##########
@@ -256,16 +256,20 @@ class ResolveSessionCatalog(
     case RenameTableStatement(TempViewOrV1Table(oldName), newName, isView) =>
       AlterTableRenameCommand(oldName.asTableIdentifier, newName.asTableIdentifier, isView)
 
-    case DescribeRelation(ResolvedTable(_, ident, _: V1Table), partitionSpec, isExtended) =>
+    case DescribeRelation(r @ ResolvedTable(_, ident, _: V1Table), partitionSpec, isExtended)
+        if isSessionCatalog(r.catalog) =>

Review comment:
       hmm, I think only session catalog can return `V1Table`?




----------------------------------------------------------------
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 #29880: [SPARK-33004][SQL] Migrate DESCRIBE column to use UnresolvedTableOrView to resolve the identifier

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






----------------------------------------------------------------
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 #29880: [SPARK-33004][SQL] Migrate DESCRIBE column to use UnresolvedTableOrView to resolve the identifier

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



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/v2Commands.scala
##########
@@ -315,6 +315,17 @@ case class DescribeRelation(
   override def output: Seq[Attribute] = DescribeTableSchema.describeTableAttributes()
 }
 
+/**
+ * The logical plan of the DESCRIBE relation_name col_name command that works for v2 tables.
+ */
+case class DescribeColumn(
+    relation: LogicalPlan,
+    colNameParts: Seq[String],

Review comment:
       A simple idea is to put an `UnresolvedAttribute` here, and analyzer can do the work for us.




----------------------------------------------------------------
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 #29880: [SPARK-33004][SQL] Migrate DESCRIBE column to use UnresolvedTableOrView to resolve the identifier

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



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveSessionCatalog.scala
##########
@@ -256,16 +256,20 @@ class ResolveSessionCatalog(
     case RenameTableStatement(TempViewOrV1Table(oldName), newName, isView) =>
       AlterTableRenameCommand(oldName.asTableIdentifier, newName.asTableIdentifier, isView)
 
-    case DescribeRelation(ResolvedTable(_, ident, _: V1Table), partitionSpec, isExtended) =>
+    case DescribeRelation(r @ ResolvedTable(_, ident, _: V1Table), partitionSpec, isExtended)
+        if isSessionCatalog(r.catalog) =>
       DescribeTableCommand(ident.asTableIdentifier, partitionSpec, isExtended)
 
     // Use v1 command to describe (temp) view, as v2 catalog doesn't support view yet.
     case DescribeRelation(ResolvedView(ident), partitionSpec, isExtended) =>
       DescribeTableCommand(ident.asTableIdentifier, partitionSpec, isExtended)
 
-    case DescribeColumnStatement(tbl, colNameParts, isExtended) =>
-      val name = parseTempViewOrV1Table(tbl, "Describing columns")
-      DescribeColumnCommand(name.asTableIdentifier, colNameParts, isExtended)
+    case DescribeColumn(r @ ResolvedTable(_, _, _: V1Table), colNameParts, isExtended)
+        if isSessionCatalog(r.catalog) =>

Review comment:
       ditto




----------------------------------------------------------------
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 #29880: [SPARK-33004][SQL] Migrate DESCRIBE column to use UnresolvedTableOrView to resolve the identifier

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


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


----------------------------------------------------------------
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 #29880: [SPARK-33004][SQL] Migrate DESCRIBE column to use UnresolvedTableOrView to resolve the identifier

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


   **[Test build #129368 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/129368/testReport)** for PR 29880 at commit [`11cfcd3`](https://github.com/apache/spark/commit/11cfcd30f5e38789698cbbfdd3e2a740685339f0).
    * 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] cloud-fan commented on a change in pull request #29880: [SPARK-33004][SQL] Migrate DESCRIBE column to use UnresolvedTableOrView to resolve the identifier

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



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/v2Commands.scala
##########
@@ -315,6 +315,17 @@ case class DescribeRelation(
   override def output: Seq[Attribute] = DescribeTableSchema.describeTableAttributes()
 }
 
+/**
+ * The logical plan of the DESCRIBE relation_name col_name command that works for v2 tables.
+ */
+case class DescribeColumn(
+    relation: LogicalPlan,
+    colNameParts: Seq[String],

Review comment:
       is it possible that the column name is qualified like `catalog_name.ns1.tbl1.col`? Does Hive support qualified column names in `DESCRIBE`?




----------------------------------------------------------------
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 #29880: [SPARK-33004][SQL] Migrate DESCRIBE column to use UnresolvedTableOrView to resolve the identifier

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



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveSessionCatalog.scala
##########
@@ -256,16 +256,18 @@ class ResolveSessionCatalog(
     case RenameTableStatement(TempViewOrV1Table(oldName), newName, isView) =>
       AlterTableRenameCommand(oldName.asTableIdentifier, newName.asTableIdentifier, isView)
 
-    case DescribeRelation(ResolvedTable(_, ident, _: V1Table), partitionSpec, isExtended) =>
+    case DescribeRelation(r @ ResolvedTable(_, ident, _: V1Table), partitionSpec, isExtended) =>
       DescribeTableCommand(ident.asTableIdentifier, partitionSpec, isExtended)
 
     // Use v1 command to describe (temp) view, as v2 catalog doesn't support view yet.
     case DescribeRelation(ResolvedView(ident), partitionSpec, isExtended) =>
       DescribeTableCommand(ident.asTableIdentifier, partitionSpec, isExtended)
 
-    case DescribeColumnStatement(tbl, colNameParts, isExtended) =>
-      val name = parseTempViewOrV1Table(tbl, "Describing columns")
-      DescribeColumnCommand(name.asTableIdentifier, colNameParts, isExtended)
+    case DescribeColumn(r @ ResolvedTable(_, _, _: V1Table), colNameParts, isExtended) =>

Review comment:
       nit: `ResolvedTable(_, ident, _: V1Table)`




----------------------------------------------------------------
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 #29880: [SPARK-33004][SQL] Migrate DESCRIBE column to use UnresolvedTableOrView to resolve the identifier

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



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/v2Commands.scala
##########
@@ -315,6 +315,17 @@ case class DescribeRelation(
   override def output: Seq[Attribute] = DescribeTableSchema.describeTableAttributes()
 }
 
+/**
+ * The logical plan of the DESCRIBE relation_name col_name command that works for v2 tables.
+ */
+case class DescribeColumn(
+    relation: LogicalPlan,
+    colNameParts: Seq[String],

Review comment:
       If Hive doesn't support it, it's OK to follow Hive here. Let's make sure that other commands only allow column name without qualifies 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 #29880: [SPARK-33004][SQL] Migrate DESCRIBE column to use UnresolvedTableOrView to resolve the identifier

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


   thanks, 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] SparkQA removed a comment on pull request #29880: [SPARK-33004][SQL] Migrate DESCRIBE column to use UnresolvedTableOrView to resolve the identifier

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


   **[Test build #129138 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/129138/testReport)** for PR 29880 at commit [`fd3f0eb`](https://github.com/apache/spark/commit/fd3f0eb8bf1ebee3ec94608699fb8b2b8afd49cb).


----------------------------------------------------------------
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 #29880: [SPARK-33004][SQL] Migrate DESCRIBE column to use UnresolvedTableOrView to resolve the identifier

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


   **[Test build #129464 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/129464/testReport)** for PR 29880 at commit [`97a4deb`](https://github.com/apache/spark/commit/97a4deb99439fb6f5903c44d3ca6b74bae1c2f78).


----------------------------------------------------------------
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 #29880: [SPARK-33004][SQL] Migrate DESCRIBE column to use UnresolvedTableOrView to resolve the identifier

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


   **[Test build #129464 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/129464/testReport)** for PR 29880 at commit [`97a4deb`](https://github.com/apache/spark/commit/97a4deb99439fb6f5903c44d3ca6b74bae1c2f78).


----------------------------------------------------------------
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 #29880: [SPARK-33004][SQL] Migrate DESCRIBE column to use UnresolvedTableOrView to resolve the identifier

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



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/v2Commands.scala
##########
@@ -315,6 +315,17 @@ case class DescribeRelation(
   override def output: Seq[Attribute] = DescribeTableSchema.describeTableAttributes()
 }
 
+/**
+ * The logical plan of the DESCRIBE relation_name col_name command that works for v2 tables.
+ */
+case class DescribeColumn(
+    relation: LogicalPlan,
+    colNameParts: Seq[String],

Review comment:
       > is it possible that the column name is qualified like `catalog_name.ns1.tbl1.col`? Does Hive support qualified column names in `DESCRIBE`?
   
   @cloud-fan actually I didn't respond to your first question. Yes, Spark allows qualifying the column name (same in 2.4):
   ```
   scala> sql("CREATE TABLE t (id bigint) USING csv")
   scala> sql("describe t spark_catalog.default.t.id").show
   +---------+----------+
   |info_name|info_value|
   +---------+----------+
   | col_name|        id|
   |data_type|    bigint|
   |  comment|      NULL|
   +---------+----------+
   ```
   , whereas Hive does not support. I guess we cannot change the behavior at this point?
   




----------------------------------------------------------------
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 #29880: [SPARK-33004][SQL] Migrate DESCRIBE column to use UnresolvedTableOrView to resolve the identifier

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



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveSessionCatalog.scala
##########
@@ -256,16 +256,18 @@ class ResolveSessionCatalog(
     case RenameTableStatement(TempViewOrV1Table(oldName), newName, isView) =>
       AlterTableRenameCommand(oldName.asTableIdentifier, newName.asTableIdentifier, isView)
 
-    case DescribeRelation(ResolvedTable(_, ident, _: V1Table), partitionSpec, isExtended) =>
+    case DescribeRelation(r @ ResolvedTable(_, ident, _: V1Table), partitionSpec, isExtended) =>

Review comment:
       unnecessary change.




----------------------------------------------------------------
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 #29880: [SPARK-33004][SQL] Migrate DESCRIBE column to use UnresolvedTableOrView to resolve the identifier

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


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


----------------------------------------------------------------
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 #29880: [SPARK-33004][SQL] Migrate DESCRIBE column to use UnresolvedTableOrView to resolve the identifier

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






----------------------------------------------------------------
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 #29880: [SPARK-33004][SQL] Migrate DESCRIBE column to use UnresolvedTableOrView to resolve the identifier

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



##########
File path: sql/core/src/test/scala/org/apache/spark/sql/connector/DataSourceV2SQLSuite.scala
##########
@@ -168,7 +168,31 @@ class DataSourceV2SQLSuite
       Array("Provider", "foo", ""),
       Array(TableCatalog.PROP_OWNER.capitalize, defaultUser, ""),
       Array("Table Properties", "[bar=baz]", "")))
+  }
 
+  test("Describe column is not supported for v2 catalog") {
+    withTable("testcat.tbl") {
+      spark.sql("CREATE TABLE testcat.tbl (id bigint) USING foo")
+      val ex = intercept[AnalysisException] {
+        spark.sql("DESCRIBE testcat.tbl id")
+      }
+      assert(ex.message.contains("Describing columns is not supported for v2 tables"))

Review comment:
       If there is no API limitation to support it, can we implement describe column for v2 catalogs later? @imback82 




----------------------------------------------------------------
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 #29880: [SPARK-33004][SQL] Migrate DESCRIBE column to use UnresolvedTableOrView to resolve the identifier

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






----------------------------------------------------------------
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 #29880: [SPARK-33004][SQL] Migrate DESCRIBE column to use UnresolvedTableOrView to resolve the identifier

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


   Merged build finished. Test FAILed.


----------------------------------------------------------------
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 #29880: [SPARK-33004][SQL] Migrate DESCRIBE column to use UnresolvedTableOrView to resolve the identifier

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



##########
File path: sql/core/src/test/scala/org/apache/spark/sql/connector/DataSourceV2SQLSuite.scala
##########
@@ -168,7 +168,31 @@ class DataSourceV2SQLSuite
       Array("Provider", "foo", ""),
       Array(TableCatalog.PROP_OWNER.capitalize, defaultUser, ""),
       Array("Table Properties", "[bar=baz]", "")))
+  }
 
+  test("Describe column is not supported for v2 catalog") {
+    withTable("testcat.tbl") {
+      spark.sql("CREATE TABLE testcat.tbl (id bigint) USING foo")
+      val ex = intercept[AnalysisException] {
+        spark.sql("DESCRIBE testcat.tbl id")
+      }
+      assert(ex.message.contains("Describing columns is not supported for v2 tables"))

Review comment:
       OK, I will take a look.




----------------------------------------------------------------
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 #29880: [SPARK-33004][SQL] Migrate DESCRIBE column to use UnresolvedTableOrView to resolve the identifier

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






----------------------------------------------------------------
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 #29880: [SPARK-33004][SQL] Migrate DESCRIBE column to use UnresolvedTableOrView to resolve the identifier

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



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/v2Commands.scala
##########
@@ -315,6 +315,17 @@ case class DescribeRelation(
   override def output: Seq[Attribute] = DescribeTableSchema.describeTableAttributes()
 }
 
+/**
+ * The logical plan of the DESCRIBE relation_name col_name command that works for v2 tables.
+ */
+case class DescribeColumn(
+    relation: LogicalPlan,
+    colNameParts: Seq[String],

Review comment:
       > is it possible that the column name is qualified like `catalog_name.ns1.tbl1.col`? Does Hive support qualified column names in `DESCRIBE`?
   
   @cloud-fan actually I didn't respond to your first question. Yes, Spark allows qualifying the column name (same in 2.4):
   ```
   scala> sql("CREATE TABLE t (id bigint) USING csv")
   scala> sql("describe t spark_catalog.default.t.id").show
   +---------+----------+
   |info_name|info_value|
   +---------+----------+
   | col_name|        id|
   |data_type|    bigint|
   |  comment|      NULL|
   +---------+----------+
   ```
   , whereas Hive does not support. I guess we cannot change the behavior at this point?
   




----------------------------------------------------------------
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