Skip to content

[SPARK-20067] [SQL] Unify and Clean Up Desc Commands Using Catalog Interface #17394

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
gatorsmile wants to merge 15 commits into from
Closed

[SPARK-20067] [SQL] Unify and Clean Up Desc Commands Using Catalog Interface #17394

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
2ebeac8
fix.
gatorsmile Mar 23, 2017
8720919
improve
gatorsmile Mar 23, 2017
d999147
revert
gatorsmile Mar 23, 2017
3859097
address comments.
gatorsmile Mar 24, 2017
1d72079
fixed the test case
gatorsmile Mar 25, 2017
68bb05c
fix.
gatorsmile Mar 26, 2017
ac3f351
migrate the test case
gatorsmile Mar 26, 2017
bef1134
remove :
gatorsmile Mar 28, 2017
29817ea
test case fix.
gatorsmile Mar 28, 2017
6c56041
address comments.
gatorsmile Mar 28, 2017
36b501e
fix.
gatorsmile Mar 29, 2017
e116018
address comments.
gatorsmile Apr 1, 2017
a6db8a3
address comments.
gatorsmile Apr 1, 2017
43668be
fix the test cases
gatorsmile Apr 2, 2017
862a4d7
address comments.
gatorsmile Apr 4, 2017
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
63 changes: 36 additions & 27 deletions sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,15 +91,20 @@ case class CatalogTablePartition(
storage: CatalogStorageFormat,
parameters: Map[String, String] = Map.empty) {

override def toString: String = {
private def toStringSeq: Seq[String] = {
val specString = spec.map { case (k, v) => s"$k=$v" }.mkString(", ")
val output =
Seq(
s"Partition Values: [$specString]",
s"$storage",
s"Partition Parameters:{${parameters.map(p => p._1 + "=" + p._2).mkString(", ")}}")
Seq(
s"Partition Values: [$specString]",
s"$storage",
s"Partition Parameters:{${parameters.map(p => p._1 + "=" + p._2).mkString(", ")}}")
}

override def toString: String = {
toStringSeq.filter(_.nonEmpty).mkString("CatalogPartition(\n\t", "\n\t", ")")
}

output.filter(_.nonEmpty).mkString("CatalogPartition(\n\t", "\n\t", ")")
def simpleString: String = {
Copy link
Contributor

@cloud-fan cloud-fan Mar 31, 2017

Choose a reason for hiding this comment

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

when shall we call simpleString?

Copy link
Member Author

@gatorsmile gatorsmile Mar 31, 2017

Choose a reason for hiding this comment

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

Show Table command will call it.

toStringSeq.filter(_.nonEmpty).mkString("", "\n", "")
}

/** Return the partition location, assuming it is specified. */
Expand Down Expand Up @@ -261,7 +266,7 @@ case class CatalogTable(
locationUri, inputFormat, outputFormat, serde, compressed, properties))
}

override def toString: String = {
private def toStringSeq: Seq[String] = {
val tableProperties = properties.map(p => p._1 + "=" + p._2).mkString("[", ", ", "]")
val partitionColumns = partitionColumnNames.map(quoteIdentifier).mkString("[", ", ", "]")
val bucketStrings = bucketSpec match {
Expand All @@ -273,28 +278,32 @@ case class CatalogTable(
if (bucketColumnNames.nonEmpty) s"Bucket Columns: $bucketColumnsString" else "",
if (sortColumnNames.nonEmpty) s"Sort Columns: $sortColumnsString" else ""
)

case _ => Nil
}
Copy link
Member Author

@gatorsmile gatorsmile Mar 26, 2017

Choose a reason for hiding this comment

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

We should de-duplicate the codes.


val output =
Seq(s"Table: ${identifier.quotedString}",
if (owner.nonEmpty) s"Owner: $owner" else "",
s"Created: ${new Date(createTime).toString}",
s"Last Access: ${new Date(lastAccessTime).toString}",
s"Type: ${tableType.name}",
if (schema.nonEmpty) s"Schema: ${schema.mkString("[", ", ", "]")}" else "",
if (provider.isDefined) s"Provider: ${provider.get}" else "",
if (partitionColumnNames.nonEmpty) s"Partition Columns: $partitionColumns" else ""
) ++ bucketStrings ++ Seq(
viewText.map("View: " + _).getOrElse(""),
comment.map("Comment: " + _).getOrElse(""),
if (properties.nonEmpty) s"Properties: $tableProperties" else "",
if (stats.isDefined) s"Statistics: ${stats.get.simpleString}" else "",
s"$storage",
if (tracksPartitionsInCatalog) "Partition Provider: Catalog" else "")

output.filter(_.nonEmpty).mkString("CatalogTable(\n\t", "\n\t", ")")
Seq(s"Table: ${identifier.quotedString}",
if (owner.nonEmpty) s"Owner: $owner" else "",
s"Created: ${new Date(createTime).toString}",
s"Last Access: ${new Date(lastAccessTime).toString}",
s"Type: ${tableType.name}",
if (provider.isDefined) s"Provider: ${provider.get}" else "",
if (partitionColumnNames.nonEmpty) s"Partition Columns: $partitionColumns" else ""
) ++ bucketStrings ++ Seq(
viewText.map("View: " + _).getOrElse(""),
comment.map("Comment: " + _).getOrElse(""),
if (properties.nonEmpty) s"Properties: $tableProperties" else "",
if (stats.isDefined) s"Statistics: ${stats.get.simpleString}" else "",
s"$storage",
if (tracksPartitionsInCatalog) "Partition Provider: Catalog" else "",
if (schema.nonEmpty) s"Schema: ${schema.treeString}" else "")
}

override def toString: String = {
toStringSeq.filter(_.nonEmpty).mkString("CatalogTable(\n", "\n", ")")
}

def simpleString: String = {
toStringSeq.filter(_.nonEmpty).mkString("", "\n", "")
}
}

Expand Down
7 changes: 4 additions & 3 deletions sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -560,7 +560,8 @@ case class DescribeTableCommand(

private def describeExtendedTableInfo(table: CatalogTable, buffer: ArrayBuffer[Row]): Unit = {
append(buffer, "", "", "")
append(buffer, "# Detailed Table Information", table.toString, "")
append(buffer, "# Detailed Table Information", "", "")
append(buffer, table.simpleString, "", "")
}

private def describeFormattedTableInfo(table: CatalogTable, buffer: ArrayBuffer[Row]): Unit = {
Expand Down Expand Up @@ -728,7 +729,7 @@ case class ShowTablesCommand(
val tableName = tableIdent.table
val isTemp = catalog.isTemporaryTable(tableIdent)
if (isExtended) {
val information = catalog.getTempViewOrPermanentTableMetadata(tableIdent).toString
val information = catalog.getTempViewOrPermanentTableMetadata(tableIdent).simpleString
Row(database, tableName, isTemp, s"$information\n")
} else {
Row(database, tableName, isTemp)
Expand All @@ -745,7 +746,7 @@ case class ShowTablesCommand(
val database = table.database.getOrElse("")
val tableName = table.table
val isTemp = catalog.isTemporaryTable(table)
val information = partition.toString
val information = partition.simpleString
Seq(Row(database, tableName, isTemp, s"$information\n"))
}
}
Expand Down
130 changes: 70 additions & 60 deletions sql/core/src/test/resources/sql-tests/results/describe.sql.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,13 @@ DESCRIBE t
-- !query 2 schema
struct<col_name:string,data_type:string,comment:string>
-- !query 2 output
# Partition Information
# col_name data_type comment
a string
b int
c string
c string
d string
# Partition Information
# col_name data_type comment
c string
d string


Expand All @@ -38,13 +38,13 @@ DESC t
-- !query 3 schema
struct<col_name:string,data_type:string,comment:string>
-- !query 3 output
# Partition Information
# col_name data_type comment
a string
b int
c string
c string
d string
# Partition Information
# col_name data_type comment
c string
d string


Expand All @@ -53,13 +53,13 @@ DESC TABLE t
-- !query 4 schema
struct<col_name:string,data_type:string,comment:string>
-- !query 4 output
# Partition Information
# col_name data_type comment
a string
b int
c string
c string
d string
# Partition Information
# col_name data_type comment
c string
d string


Expand All @@ -68,67 +68,74 @@ DESC FORMATTED t
-- !query 5 schema
struct<col_name:string,data_type:string,comment:string>
-- !query 5 output
# Detailed Table Information
a string
b int
c string
d string
# Partition Information
# Storage Information
# col_name data_type comment
Comment: table_comment
Compressed: No
Created:
c string
d string

# Detailed Table Information
Database: default
Owner:
Created:
Last Access:
Location: sql/core/spark-warehouse/t
Owner:
Partition Provider: Catalog
Storage Desc Parameters:
Table Parameters:
Table Type: MANAGED
a string
b int
c string
c string
d string
d string
Comment: table_comment
Table Parameters:

# Storage Information
Compressed: No
Storage Desc Parameters:
Partition Provider: Catalog


-- !query 6
DESC EXTENDED t
-- !query 6 schema
struct<col_name:string,data_type:string,comment:string>
-- !query 6 output
# Detailed Table Information CatalogTable(
Table: `default`.`t`
Created:
Last Access:
Type: MANAGED
Schema: [StructField(a,StringType,true), StructField(b,IntegerType,true), StructField(c,StringType,true), StructField(d,StringType,true)]
Provider: parquet
Partition Columns: [`c`, `d`]
Comment: table_comment
Storage(Location: sql/core/spark-warehouse/t)
Partition Provider: Catalog)
# Partition Information
# col_name data_type comment
a string
Copy link
Contributor

@cloud-fan cloud-fan Apr 1, 2017

Choose a reason for hiding this comment

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

we should add a header here # col_name data_type comment.

Copy link
Member Author

@gatorsmile gatorsmile Apr 1, 2017

Choose a reason for hiding this comment

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

Done.

b int
c string
d string
# Partition Information
# col_name data_type comment
c string
d string
d string

# Detailed Table Information
Table: `default`.`t`
Created:
Last Access:
Type: MANAGED
Provider: parquet
Partition Columns: [`c`, `d`]
Comment: table_comment
Storage(Location: sql/core/spark-warehouse/t)
Partition Provider: Catalog
Schema: root
|-- a: string (nullable = true)
|-- b: integer (nullable = true)
|-- c: string (nullable = true)
|-- d: string (nullable = true)
Copy link
Member Author

@gatorsmile gatorsmile Mar 24, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

@cloud-fan Do we still need Schema, if the above info already has it? The only missing part is the nullability, which can be put in comment or add a separate column.

Copy link
Contributor

@cloud-fan cloud-fan Mar 25, 2017

Choose a reason for hiding this comment

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

yea we do not need, but I think CatalogTable.toString should still have the schema. Actually why do we have DESC EXTENDED? I think DESC FORMATTED is good enough

Copy link
Member Author

@gatorsmile gatorsmile Mar 26, 2017

Choose a reason for hiding this comment

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

Below is what Programming Hive said:

Replacing EXTENDED with FORMATTED provides more readable but also more verbose output.

For most end-users, they should use DESC FORMATTED .

Copy link
Member Author

@gatorsmile gatorsmile Mar 26, 2017

Choose a reason for hiding this comment

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

Maybe we can keep the output of DESC EXTENDED as it is. Then, we need to revisit DESC FORMATTED to ensure it includes all the user-facing metadata info.



-- !query 7
DESC t PARTITION (c='Us', d=1)
Copy link
Contributor

@cloud-fan cloud-fan Apr 2, 2017

Choose a reason for hiding this comment

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

This has the same result with DESC t, is it same with Hive?

Copy link
Member Author

@gatorsmile gatorsmile Apr 2, 2017

Choose a reason for hiding this comment

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

Yes.

hive> DESC page_view PARTITION(dt='part1', country='part2');
OK
viewtime            	int                 	                    
userid              	bigint              	                    
page_url            	string              	                    
referrer_url        	string              	                    
ip                  	string              	IP Address of the User
dt                  	string              	                    
country             	string              	                    
	 	 
# Partition Information	 	 
# col_name            	data_type           	comment             
	 	 
dt                  	string              	                    
country             	string              	                    
Time taken: 0.12 seconds, Fetched: 13 row(s)
hive> DESC page_view;
OK
viewtime            	int                 	                    
userid              	bigint              	                    
page_url            	string              	                    
referrer_url        	string              	                    
ip                  	string              	IP Address of the User
dt                  	string              	                    
country             	string              	                    
	 	 
# Partition Information	 	 
# col_name            	data_type           	comment             
	 	 
dt                  	string              	                    
country             	string              	                    
Time taken: 0.022 seconds, Fetched: 13 row(s)

-- !query 7 schema
struct<col_name:string,data_type:string,comment:string>
-- !query 7 output
# Partition Information
# col_name data_type comment
a string
b int
c string
c string
d string
# Partition Information
# col_name data_type comment
c string
d string


Expand All @@ -137,42 +144,45 @@ DESC EXTENDED t PARTITION (c='Us', d=1)
-- !query 8 schema
struct<col_name:string,data_type:string,comment:string>
-- !query 8 output
# Partition Information
# col_name data_type comment
Detailed Partition Information CatalogPartition(
Partition Values: [c=Us, d=1]
Storage(Location: sql/core/spark-warehouse/t/c=Us/d=1)
Partition Parameters:{})
a string
b int
c string
d string
# Partition Information
# col_name data_type comment
c string
d string
d string

Detailed Partition Information CatalogPartition(
Partition Values: [c=Us, d=1]
Storage(Location: sql/core/spark-warehouse/t/c=Us/d=1)
Partition Parameters:{})


-- !query 9
DESC FORMATTED t PARTITION (c='Us', d=1)
-- !query 9 schema
struct<col_name:string,data_type:string,comment:string>
-- !query 9 output
# Detailed Partition Information
# Partition Information
# Storage Information
# col_name data_type comment
Compressed: No
Database: default
Location: sql/core/spark-warehouse/t/c=Us/d=1
Partition Parameters:
Partition Value: [Us, 1]
Storage Desc Parameters:
Table: t
a string
b int
c string
d string
# Partition Information
# col_name data_type comment
c string
d string
d string

# Detailed Partition Information
Partition Value: [Us, 1]
Database: default
Table: t
Location: sql/core/spark-warehouse/t/c=Us/d=1
Partition Parameters:

# Storage Information
Compressed: No
Storage Desc Parameters:


-- !query 10
Expand Down
Loading