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 3 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
6 changes: 3 additions & 3 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 @@ -283,7 +283,6 @@ case class CatalogTable(
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(
Expand All @@ -292,9 +291,10 @@ case class CatalogTable(
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 (tracksPartitionsInCatalog) "Partition Provider: Catalog" else "",
if (schema.nonEmpty) s"Schema: ${schema.treeString}" else "")

output.filter(_.nonEmpty).mkString("CatalogTable(\n\t", "\n\t", ")")
output.filter(_.nonEmpty).mkString("CatalogTable(\n", "\n", ")")
}
}

Expand Down
25 changes: 15 additions & 10 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 @@ -97,16 +97,21 @@ DESC EXTENDED t
struct<col_name:string,data_type:string,comment:string>
-- !query 6 output
# Detailed Table Information CatalogTable(
Copy link
Contributor

@cloud-fan cloud-fan Mar 24, 2017

Choose a reason for hiding this comment

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

do we need to wrap the information with CatalogTable(...)?

Copy link
Member Author

@gatorsmile gatorsmile Mar 24, 2017

Choose a reason for hiding this comment

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

Yes. Let me remove it and move it to the next line

Copy link
Member Author

@gatorsmile gatorsmile Mar 24, 2017

Choose a reason for hiding this comment

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

The output looks weird if we sort the Command, so file a PR: #17414

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

)
# Partition Information
# col_name data_type comment
a string
Expand Down
54 changes: 32 additions & 22 deletions sql/core/src/test/resources/sql-tests/results/show-tables.sql.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,32 +119,42 @@ SHOW TABLE EXTENDED LIKE 'show_t*'
struct<database:string,tableName:string,isTemporary:boolean,information:string>
-- !query 12 output
show_t3 true CatalogTable(
Table: `show_t3`
Created:
Last Access:
Type: VIEW
Schema: [StructField(e,IntegerType,true)]
Storage())
Table: `show_t3`
Created:
Last Access:
Type: VIEW
Storage()
Schema: root
|-- e: integer (nullable = true)
)

showdb show_t1 false CatalogTable(
Table: `showdb`.`show_t1`
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`]
Storage(Location: sql/core/spark-warehouse/showdb.db/show_t1)
Partition Provider: Catalog)
Table: `showdb`.`show_t1`
Created:
Last Access:
Type: MANAGED
Provider: parquet
Partition Columns: [`c`, `d`]
Storage(Location: sql/core/spark-warehouse/showdb.db/show_t1)
Partition Provider: Catalog
Schema: root
|-- a: string (nullable = true)
|-- b: integer (nullable = true)
|-- c: string (nullable = true)
|-- d: string (nullable = true)
)

showdb show_t2 false CatalogTable(
Table: `showdb`.`show_t2`
Created:
Last Access:
Type: MANAGED
Schema: [StructField(b,StringType,true), StructField(d,IntegerType,true)]
Provider: parquet
Storage(Location: sql/core/spark-warehouse/showdb.db/show_t2))
Table: `showdb`.`show_t2`
Created:
Last Access:
Type: MANAGED
Provider: parquet
Storage(Location: sql/core/spark-warehouse/showdb.db/show_t2)
Schema: root
|-- b: string (nullable = true)
|-- d: integer (nullable = true)
)


-- !query 13
Expand Down