Skip to content

[SPARK-16004] [SQL] Correctly display "Last Access Time" of CatalogTable #13720

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 10 commits into from
Closed

[SPARK-16004] [SQL] Correctly display "Last Access Time" of CatalogTable #13720

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
358ac0d
improve the disply of CatalogTable information
Jun 16, 2016
0fe835d
fix the tests failure
Jun 17, 2016
25510f5
Merge remote-tracking branch 'upstream/master' into SPARK-16004
Jun 17, 2016
26b9d79
Merge remote-tracking branch 'upstream/master' into SPARK-16004
Jun 20, 2016
8e7b271
Merge remote-tracking branch 'upstream/master' into SPARK-16004
Jun 21, 2016
90c3ff0
revert user name
Jun 21, 2016
da8b77c
Merge remote-tracking branch 'upstream/master' into SPARK-16004
Jun 24, 2016
4c9e295
revert comment to use null
Jun 24, 2016
e93b72a
remove space
Jun 24, 2016
a72800c
add test cases
Jun 27, 2016
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
3 changes: 2 additions & 1 deletion 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 @@ -180,7 +180,8 @@ case class CatalogTable(
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}",
"Last Access: " +
(if (lastAccessTime == -1) "UNKNOWN" else new Date(lastAccessTime).toString),
Copy link
Member

@srowen srowen Jun 21, 2016

Choose a reason for hiding this comment

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

I don't feel strongly about it, but seems like elsewhere an empty string is used for no values. This seems slightly preferable.

Copy link
Contributor Author

@bomeng bomeng Jun 21, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Here is the code from Hive (it is using 0 as initial last access value):
File: MetaDataFormatUtils.java

private static String formatDate(long timeInSeconds) {
if (timeInSeconds != 0) {
Date date = new Date(timeInSeconds * 1000);
return date.toString();
}
return "UNKNOWN";
}

Copy link
Member

@srowen srowen Jun 25, 2016

Choose a reason for hiding this comment

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

Yes, but the rest of this code doesn't use that logic. It returns "" in code around this area.

Copy link
Contributor

@cloud-fan cloud-fan Jun 28, 2016

Choose a reason for hiding this comment

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

I prefer empty string here, cc @yhuai @liancheng what do you think?

Copy link
Contributor

@liancheng liancheng Jun 28, 2016

Choose a reason for hiding this comment

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

Empty string looks fine to me.

Copy link
Member

@gatorsmile gatorsmile May 23, 2017

Choose a reason for hiding this comment

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

Please follow this?

s"Type: ${tableType.name}",
if (schema.nonEmpty) s"Schema: ${schema.mkString("[", ", ", "]")}" else "",
if (partitionColumnNames.nonEmpty) s"Partition Columns: $partitionColumns" else "",
Expand Down
3 changes: 2 additions & 1 deletion 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 @@ -468,7 +468,8 @@ case class DescribeTableCommand(table: TableIdentifier, isExtended: Boolean, isF
append(buffer, "Database:", table.database, "")
append(buffer, "Owner:", table.owner, "")
append(buffer, "Create Time:", new Date(table.createTime).toString, "")
append(buffer, "Last Access Time:", new Date(table.lastAccessTime).toString, "")
append(buffer, "Last Access Time:",
if (table.lastAccessTime == -1) "UNKNOWN" else new Date(table.lastAccessTime).toString, "")
append(buffer, "Location:", table.storage.locationUri.getOrElse(""), "")
append(buffer, "Table Type:", table.tableType.name, "")

Expand Down
17 changes: 17 additions & 0 deletions sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,23 @@ class DDLSuite extends QueryTest with SharedSQLContext with BeforeAndAfterEach {
}
}

test("Describe Table") {
Copy link
Member

@gatorsmile gatorsmile May 23, 2017

Choose a reason for hiding this comment

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

This test case does not verify the issue you fixed, I think

val tabName = "tab1"
withTable(tabName) {
sql(s"CREATE TABLE $tabName(a int comment 'test')")

assert(sql(s"DESC $tabName").collect().length == 1)

assert(
sql(s"DESC FORMATTED $tabName").collect()
.exists(_.getString(0) == "# Storage Information"))

assert(
sql(s"DESC EXTENDED $tabName").collect()
.exists(_.getString(0) == "# Detailed Table Information"))
}
}

test("Alter/Describe Database") {
withTempDir { tmpDir =>
val path = tmpDir.toString
Expand Down