You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "dtenedor (via GitHub)" <gi...@apache.org> on 2023/02/02 20:47:22 UTC

[GitHub] [spark] dtenedor opened a new pull request, #39863: [SPARK-42294][SQL] Include column default values in DESCRIBE output for V2 tables

dtenedor opened a new pull request, #39863:
URL: https://github.com/apache/spark/pull/39863

   ### What changes were proposed in this pull request?
   
   Include column default values in DESCRIBE output for V2 tables.
   
   This was previously implemented for V1 tables, but missed V2 tables.
   
   ### Why are the changes needed?
   
   DESCRIBE commands make it easier to work with tables by inspecting their metadata.
   
   ### Does this PR introduce _any_ user-facing change?
   
   Yes, it adds more information to DESCRIBE commands for tables with default column values.
   
   ### How was this patch tested?
   
   This PR adds unit test coverage.


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

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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] gengliangwang commented on pull request #39863: [SPARK-42294][SQL] Include column default values in DESCRIBE output for V2 tables

Posted by "gengliangwang (via GitHub)" <gi...@apache.org>.
gengliangwang commented on PR #39863:
URL: https://github.com/apache/spark/pull/39863#issuecomment-1414983257

   Thanks, merging to master/3.4


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

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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] dtenedor commented on a diff in pull request #39863: [SPARK-42294][SQL] Include column default values in DESCRIBE output for V2 tables

Posted by "dtenedor (via GitHub)" <gi...@apache.org>.
dtenedor commented on code in PR #39863:
URL: https://github.com/apache/spark/pull/39863#discussion_r1095104666


##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DescribeTableExec.scala:
##########
@@ -68,6 +69,16 @@ case class DescribeTableExec(
         case (key, value) => key + "=" + value
       }.mkString("[", ",", "]")
     rows += toCatalystRow("Table Properties", properties, "")
+
+    // If any columns have default values, append them to the result.
+    if (table.schema.fields.exists(_.metadata.contains(CURRENT_DEFAULT_COLUMN_METADATA_KEY))) {

Review Comment:
   Good idea, done!



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

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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] gengliangwang commented on pull request #39863: [SPARK-42294][SQL] Include column default values in DESCRIBE output for V2 tables

Posted by "gengliangwang (via GitHub)" <gi...@apache.org>.
gengliangwang commented on PR #39863:
URL: https://github.com/apache/spark/pull/39863#issuecomment-1414517429

   @dtenedor the scala 2.13 build failed: https://github.com/dtenedor/spark/actions/runs/4078836635/jobs/7029542763


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

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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] gengliangwang closed pull request #39863: [SPARK-42294][SQL] Include column default values in DESCRIBE output for V2 tables

Posted by "gengliangwang (via GitHub)" <gi...@apache.org>.
gengliangwang closed pull request #39863: [SPARK-42294][SQL] Include column default values in DESCRIBE output for V2 tables
URL: https://github.com/apache/spark/pull/39863


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

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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] dtenedor commented on pull request #39863: [SPARK-42294][SQL] Include column default values in DESCRIBE output for V2 tables

Posted by "dtenedor (via GitHub)" <gi...@apache.org>.
dtenedor commented on PR #39863:
URL: https://github.com/apache/spark/pull/39863#issuecomment-1414510531

   The CI appears to have passed, except for one pyspark test which was a flake and unrelated.


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

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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] gengliangwang commented on pull request #39863: [SPARK-42294][SQL] Include column default values in DESCRIBE output for V2 tables

Posted by "gengliangwang (via GitHub)" <gi...@apache.org>.
gengliangwang commented on PR #39863:
URL: https://github.com/apache/spark/pull/39863#issuecomment-1414358954

   @dtenedor There is V2 command ShowCreateTableExec as well. Do we need to update it?


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

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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] dtenedor commented on pull request #39863: [SPARK-42294][SQL] Include column default values in DESCRIBE output for V2 tables

Posted by "dtenedor (via GitHub)" <gi...@apache.org>.
dtenedor commented on PR #39863:
URL: https://github.com/apache/spark/pull/39863#issuecomment-1414549381

   > @dtenedor the scala 2.13 build failed: https://github.com/dtenedor/spark/actions/runs/4078836635/jobs/7029542763
   
   @gengliangwang sorry about that. I committed a fix.


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

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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] dtenedor commented on pull request #39863: [SPARK-42294][SQL] Include column default values in DESCRIBE output for V2 tables

Posted by "dtenedor (via GitHub)" <gi...@apache.org>.
dtenedor commented on PR #39863:
URL: https://github.com/apache/spark/pull/39863#issuecomment-1414400738

   > @dtenedor There is V2 command ShowCreateTableExec as well. Do we need to update it?
   @gengliangwang good question. I checked and it seems that also calls the `toDDL` method of `StructField`, so there is no need to change anything separately for V2 tables there:
   
   ```
     private def showTableDataColumns(table: Table, builder: StringBuilder): Unit = {
       val columns = CharVarcharUtils.getRawSchema(table.schema(), conf).fields.map(_.toDDL)
       builder ++= concatByMultiLines(columns)
     }
   ```


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

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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] gengliangwang commented on a diff in pull request #39863: [SPARK-42294][SQL] Include column default values in DESCRIBE output for V2 tables

Posted by "gengliangwang (via GitHub)" <gi...@apache.org>.
gengliangwang commented on code in PR #39863:
URL: https://github.com/apache/spark/pull/39863#discussion_r1095075995


##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DescribeTableExec.scala:
##########
@@ -68,6 +69,16 @@ case class DescribeTableExec(
         case (key, value) => key + "=" + value
       }.mkString("[", ",", "]")
     rows += toCatalystRow("Table Properties", properties, "")
+
+    // If any columns have default values, append them to the result.
+    if (table.schema.fields.exists(_.metadata.contains(CURRENT_DEFAULT_COLUMN_METADATA_KEY))) {

Review Comment:
   This seesm duplicated with https://github.com/apache/spark/pull/39726/files#diff-632efff0ffceb2ff47b97f39570b4f2cff24401f132f559158acaa651b27af26R651. Shall we abstract them?



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

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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] dtenedor commented on pull request #39863: [SPARK-42294][SQL] Include column default values in DESCRIBE output for V2 tables

Posted by "dtenedor (via GitHub)" <gi...@apache.org>.
dtenedor commented on PR #39863:
URL: https://github.com/apache/spark/pull/39863#issuecomment-1414352745

   Hi @gengliangwang it looks like I forgot to cover V2 tables when adding DESCRIBE support for column default values recently, this PR adds the coverage. Would you mind to help 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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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