[SPARK-39552][SQL] Unify v1 and v2 DESCRIBE TABLE
#36946
Conversation
MaxGekk
changed the title
MaxGekk
changed the title
MaxGekk
changed the title
DESCRIBE TABLE
|
@cloud-fan @imback82 Please, review this PR. |
| val nameToField = table.schema.map(f => (f.name, f)).toMap | ||
| rows ++= table.partitioning | ||
| .map(_.asInstanceOf[IdentityTransform]) | ||
| .flatMap(_.ref.fieldNames()) |
There was a problem hiding this comment.
I think it's more tricky here, as the reference can be a nested field, e.g. a.b. We can still keep the output in a v1 compatible way, but the code to find its type and comment will be a bit more complicated, as we need to use StructType.findNestedField
There was a problem hiding this comment.
I added a test for v2 implementation: partitioning by nested columns. Just in case, v1 doesn't support partitioning by nested columns. Also I fixed v2 impl to pass the new test.
| Row("data", "string", null), | ||
| Row("# Partition Information", "", ""), | ||
| Row("# col_name", "data_type", "comment"), | ||
| Row("id", "bigint", null), |
There was a problem hiding this comment.
just for curiosity: what's different between v1 and v2 DESC TABLE for this test DESCRIBE TABLE EXTENDED of a partitioned table?
There was a problem hiding this comment.
v2 (after the PR):
+----------------------------+----------------------------+---------------------------------------------------+
|col_name |data_type |comment |
+----------------------------+----------------------------+---------------------------------------------------+
|id |bigint |null |
|data |string |null |
|# Partition Information | | |
|# col_name |data_type |comment |
|id |bigint |null |
| | | |
|# Metadata Columns | | |
|index |int |Metadata column used to conflict with a data column|
|_partition |string |Partition key used to store the row |
| | | |
|# Detailed Table Information| | |
|Name |test_catalog.ns.table | |
|Comment |this is a test table | |
|Location |file:/tmp/testcat/table_name| |
|Provider |_ | |
|Owner |maximgekk | |
|Table Properties |[bar=baz] | |
+----------------------------+----------------------------+---------------------------------------------------+
v1 in memory:
+----------------------------+----------------------------+-------+
|col_name |data_type |comment|
+----------------------------+----------------------------+-------+
|data |string |null |
|id |bigint |null |
|# Partition Information | | |
|# col_name |data_type |comment|
|id |bigint |null |
| | | |
|# Detailed Table Information| | |
|Database |ns | |
|Table |table | |
|Created Time |Wed Jun 22 09:37:48 PDT 2022| |
|Last Access |UNKNOWN | |
|Created By |Spark 3.4.0-SNAPSHOT | |
|Type |EXTERNAL | |
|Provider |parquet | |
|Comment |this is a test table | |
|Table Properties |[bar=baz] | |
|Location |file:/tmp/testcat/table_name| |
|Partition Provider |Catalog | |
+----------------------------+----------------------------+-------+
There was a problem hiding this comment.
v1 (hive):
+----------------------------+----------------------------------------------------------+-------+
|col_name |data_type |comment|
+----------------------------+----------------------------------------------------------+-------+
|data |string |null |
|id |bigint |null |
|# Partition Information | | |
|# col_name |data_type |comment|
|id |bigint |null |
| | | |
|# Detailed Table Information| | |
|Database |ns | |
|Table |table | |
|Owner |maximgekk | |
|Created Time |Wed Jun 22 09:39:42 PDT 2022 | |
|Last Access |UNKNOWN | |
|Created By |Spark 3.4.0-SNAPSHOT | |
|Type |EXTERNAL | |
|Provider |hive | |
|Comment |this is a test table | |
|Table Properties |[transient_lastDdlTime=1655915982] | |
|Location |file:/tmp/testcat/table_name | |
|Serde Library |org.apache.hadoop.hive.serde2.lazy.LazySimpleSerDe | |
|InputFormat |org.apache.hadoop.mapred.TextInputFormat | |
|OutputFormat |org.apache.hadoop.hive.ql.io.HiveIgnoreKeyTextOutputFormat| |
|Storage Properties |[serialization.format=1] | |
|Partition Provider |Catalog | |
+----------------------------+----------------------------------------------------------+-------+
There was a problem hiding this comment.
I see, can we at least include Table Type in v2 command? It's simply checking if the table has a reserved EXTERNAL table property.
| rows += toCatalystRow(s"# ${output(0).name}", output(1).name, output(2).name) | ||
| rows ++= table.partitioning | ||
| .map(_.asInstanceOf[IdentityTransform].ref.fieldNames()) | ||
| .flatMap(table.schema.findNestedField(_)) |
There was a problem hiding this comment.
what's the v1 DESC TABLE behavior for malformed tables? e.g. partition column does not exist in the table schema? do we fail or silently ignore it?
There was a problem hiding this comment.
I have checked that v1 should trigger an assert in the case:
- describePartitionInfo()
- table.partitionSchema
- assert(partitionFields.map(_.name) == partitionColumnNames)
There was a problem hiding this comment.
Can we do the same here? assert that table.schema.findNestedField does not return None.
| .map { fieldNames => | ||
| val nestedField = table.schema.findNestedField(fieldNames) | ||
| assert(nestedField.isDefined, | ||
| s"Not found the partition column ${fieldNames.map(quoteIfNeeded).mkString(".")} " + |
There was a problem hiding this comment.
nit: we can reuse MultipartIdentifierHelper.quoted
| nestedField.get | ||
| }.map { case (path, field) => | ||
| toCatalystRow( | ||
| (path :+ field.name).map(quoteIfNeeded(_)).mkString("."), |
|
Merging to master. Thank you, @cloud-fan for review. |
2 participants
What changes were proposed in this pull request?
In the PR, I propose to change output of v2
DESCRIBE TABLE, and make it the same as v1. In particular:Also the PR moves/adds some tests to the base traits:
and addresses review comments in #35265.
Why are the changes needed?
The changes unify outputs of v1 and v2 implementations, and make the migration process from the version v1 to v2 easier for Spark SQL users.
Does this PR introduce any user-facing change?
Yes, it changes outputs of v2
DESCRIBE TABLE.How was this patch tested?
By running the
DESCRIBE TABLEtest suites:and related test suites: