Skip to content

[SPARK-38350][SQL] Make the table name output of V1/V2 "desc extended table" consistent #35681

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

[SPARK-38350][SQL] Make the table name output of V1/V2 "desc extended table" consistent #35681

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...e/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DataSourceV2Strategy.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ class DataSourceV2Strategy(session: SparkSession) extends Strategy with Predicat
if (partitionSpec.nonEmpty) {
throw QueryCompilationErrors.describeDoesNotSupportPartitionForV2TablesError()
}
DescribeTableExec(output, r.table, isExtended) :: Nil
DescribeTableExec(output, r, isExtended) :: Nil

case DescribeColumn(_: ResolvedTable, column, isExtended, output) =>
column match {
Expand Down
10 changes: 8 additions & 2 deletions ...core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DescribeTableExec.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,17 @@ import scala.collection.JavaConverters._
import scala.collection.mutable.ArrayBuffer

import org.apache.spark.sql.catalyst.InternalRow
import org.apache.spark.sql.catalyst.analysis.ResolvedTable
import org.apache.spark.sql.catalyst.expressions.Attribute
import org.apache.spark.sql.connector.catalog.{CatalogV2Util, SupportsMetadataColumns, Table}

case class DescribeTableExec(
output: Seq[Attribute],
table: Table,
resolvedTable: ResolvedTable,
isExtended: Boolean) extends LeafV2CommandExec {

private val table: Table = resolvedTable.table

override protected def run(): Seq[InternalRow] = {
val rows = new ArrayBuffer[InternalRow]()
addSchema(rows)
Expand All @@ -43,7 +47,9 @@ case class DescribeTableExec(
private def addTableDetails(rows: ArrayBuffer[InternalRow]): Unit = {
rows += emptyRow()
rows += toCatalystRow("# Detailed Table Information", "", "")
rows += toCatalystRow("Name", table.name(), "")
rows += toCatalystRow("Catalog", resolvedTable.catalog.name(), "")
rows += toCatalystRow("Database", resolvedTable.identifier.namespace().mkString("."), "")
Copy link

@superdupershant superdupershant Mar 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is namespace() returning a list? Should this be a single identifier?

Copy link
Member Author

@gengliangwang gengliangwang Mar 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, namespace() returns a list. It's a feature of DS V2. Usually the length should be 1.

Copy link

@superdupershant superdupershant Mar 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so if it returns a list will the Database name contain periods?

Copy link
Member Author

@gengliangwang gengliangwang Mar 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the length of the namespace is 1, there is no any period

Copy link

@superdupershant superdupershant Mar 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we spoke online with Wenchen and Gengliang, didn't realize the namespace is a multi part name.

rows += toCatalystRow("Table", resolvedTable.identifier.name(), "")

CatalogV2Util.TABLE_RESERVED_PROPERTIES.foreach(propKey => {
if (table.properties.containsKey(propKey)) {
Expand Down
8 changes: 5 additions & 3 deletions sql/core/src/test/scala/org/apache/spark/sql/connector/DataSourceV2SQLSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,13 +120,13 @@ class DataSourceV2SQLSuite
}

test("DescribeTable extended using v2 catalog") {
spark.sql("CREATE TABLE testcat.table_name (id bigint, data string)" +
spark.sql("CREATE TABLE testcat.default.table_name (id bigint, data string)" +
" USING foo" +
" PARTITIONED BY (id)" +
" TBLPROPERTIES ('bar'='baz')" +
" COMMENT 'this is a test table'" +
" LOCATION 'file:/tmp/testcat/table_name'")
val descriptionDf = spark.sql("DESCRIBE TABLE EXTENDED testcat.table_name")
val descriptionDf = spark.sql("DESCRIBE TABLE EXTENDED testcat.default.table_name")
Copy link

@superdupershant superdupershant Mar 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this also fix SHOW TABLE EXTENDED?

Copy link
Member Author

@gengliangwang gengliangwang Mar 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no V2 version of SHOW TABLE EXTENDED yet.

Copy link

@superdupershant superdupershant Mar 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sense

assert(descriptionDf.schema.map(field => (field.name, field.dataType))
=== Seq(
("col_name", StringType),
Expand All @@ -146,7 +146,9 @@ class DataSourceV2SQLSuite
Array("_partition", "string", "Partition key used to store the row"),
Array("", "", ""),
Array("# Detailed Table Information", "", ""),
Array("Name", "testcat.table_name", ""),
Array("Catalog", "testcat", ""),
Array("Database", "default", ""),
Array("Table", "table_name", ""),
Array("Comment", "this is a test table", ""),
Array("Location", "file:/tmp/testcat/table_name", ""),
Array("Provider", "foo", ""),
Expand Down