[SPARK-50795][SQL] Store timestamp as long type in describe LinkedHashMap
#49513
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
asl3
changed the title
long timestamp type in describe linkedhashmap to improve of date formatlong timestamp type in describe linkedhashmap
asl3
changed the title
long timestamp type in describe linkedhashmaplong timestamp type in describe LinkedHashMap
asl3
changed the title
long timestamp type in describe LinkedHashMaplong type in describe LinkedHashMap
cloud-fan
reviewed
Jan 16, 2025
...core/src/main/scala/org/apache/spark/sql/execution/command/DescribeRelationJsonCommand.scala
Outdated
Show resolved
Hide resolved
cloud-fan
approved these changes
Jan 16, 2025
cloud-fan
reviewed
Jan 16, 2025
| val formattedValue = renamedKey match { | ||
| case "created_time" | "last_access" => | ||
| value match { | ||
| case JLong(timestamp) if timestamp > 0 => |
There was a problem hiding this comment.
This can be negative for dates before 1970-01-01. We should allow it
cloud-fan
reviewed
Jan 16, 2025
...core/src/main/scala/org/apache/spark/sql/execution/command/DescribeRelationJsonCommand.scala
Outdated
Show resolved
Hide resolved
…/DescribeRelationJsonCommand.scala
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala
Outdated
Show resolved
Hide resolved
...core/src/main/scala/org/apache/spark/sql/execution/command/DescribeRelationJsonCommand.scala
Outdated
Show resolved
Hide resolved
MaxGekk
reviewed
Jan 16, 2025
| @@ -80,20 +82,18 @@ trait MetadataMapSupport { | |||
| .mkString("[", ", ", "]") | |||
| case JInt(value) => value.toString | |||
| case JDouble(value) => value.toString | |||
| case JLong(value) => | |||
| if (timestampKeys.contains(key)) { | |||
| new Date(value).toString | |||
There was a problem hiding this comment.
Could you use Iso8601TimestampFormatter instead of the legacy class Date.
There was a problem hiding this comment.
This is the behavior for DESCRIBE TABLE without AS JSON, see 36d23ef#diff-f2a04f920c41d18a7d387216f86405bfdc6fb09c44ebe1bb09312ba7dde55333L136
Here we just revert it back.
|
thanks, merging to master/4.0! |
cloud-fan
added a commit
that referenced
this pull request
Jan 16, 2025
…dHashMap ### What changes were proposed in this pull request? When storing table metadata in the `describe` LinkedHashMap object, we retain the timestamp as a `long`data type (instead of converting to a formatted date `string` type) to allow flexibility and extensibility of `describe` date format. Formatting the date fields is delegated to the caller (e.g. describe table, describe as json, describe column, etc.). Example date for describe table: `Mon Nov 01 12:00:00 UTC 2021` Example date for describe as json: `2021-11-01T12:00:00Z` ### Why are the changes needed? Improve extensibility of `describe` and ensure backwards compatibility ### Does this PR introduce _any_ user-facing change? Affects the `describe` output date format ### How was this patch tested? Added `describe table` tests for date format ### Was this patch authored or co-authored using generative AI tooling? No Closes #49513 from asl3/asl3/describetable-dateplaintext. Lead-authored-by: Amanda Liu <[email protected]> Co-authored-by: Wenchen Fan <[email protected]> Signed-off-by: Wenchen Fan <[email protected]> (cherry picked from commit 8bbec5d) Signed-off-by: Wenchen Fan <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
When storing table metadata in the
describeLinkedHashMap object, we retain the timestamp as alongdata type (instead of converting to a formatted datestringtype) to allow flexibility and extensibility ofdescribedate format. Formatting the date fields is delegated to the caller (e.g. describe table, describe as json, describe column, etc.).Example date for describe table:
Mon Nov 01 12:00:00 UTC 2021Example date for describe as json:
2021-11-01T12:00:00ZWhy are the changes needed?
Improve extensibility of
describeand ensure backwards compatibilityDoes this PR introduce any user-facing change?
Affects the
describeoutput date formatHow was this patch tested?
Added
describe tabletests for date formatWas this patch authored or co-authored using generative AI tooling?
No