Skip to content

Conversation

@MaxGekk
Copy link
Member

@MaxGekk MaxGekk commented Mar 17, 2025

What changes were proposed in this pull request?

In the PR, I propose to support new data type TIME in Literal's methods such as:

  • default()
  • sql()
  • toString()
  • componentTypeToDataType()

Why are the changes needed?

To output literals of the TIME data type in human readable format. In particular, the column names are formatted by default with internal representation. See the example below where 43200000000 is a column name:

spark-sql (default)> desc select time'12:00';
43200000000         	time(6)

Does this PR introduce any user-facing change?

Yes. After the changes, TIME literals are in human readable format. For the example above:

spark-sql (default)> desc select time'12:00';
TIME '12:00:00'     	time(6)

How was this patch tested?

By running the modified test suites:

$ build/sbt "test:testOnly *LiteralExpressionSuite"
$ build/sbt "sql/testOnly org.apache.spark.sql.SQLQueryTestSuite -- -z time.sql"

Was this patch authored or co-authored using generative AI tooling?

No.

@github-actions github-actions bot added the SQL label Mar 17, 2025
@MaxGekk MaxGekk changed the title [WIP][SQL] Support the TIME data type in Literal methods [WIP][SPARK-51541][SQL] Support the TIME data type in Literal methods Mar 18, 2025
@MaxGekk MaxGekk changed the title [WIP][SPARK-51541][SQL] Support the TIME data type in Literal methods [SPARK-51541][SQL] Support the TIME data type in Literal methods Mar 18, 2025
@MaxGekk MaxGekk marked this pull request as ready for review March 18, 2025 07:00
Copy link
Member

@yaooqinn yaooqinn left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@LuciferYang LuciferYang left a comment

Choose a reason for hiding this comment

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

LGTM

@MaxGekk
Copy link
Member Author

MaxGekk commented Mar 18, 2025

Merging to master. Thank you, @yaooqinn @LuciferYang for review.

@MaxGekk MaxGekk closed this in 0670e4f Mar 18, 2025
Copy link
Contributor

@beliefer beliefer left a comment

Choose a reason for hiding this comment

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

LGTM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants