You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "shrirangmhalgi (via GitHub)" <gi...@apache.org> on 2026/06/25 05:58:36 UTC
[PR] [SPARK-57558][SQL] Add the LOCALTIME datetime value function [spark]
shrirangmhalgi opened a new pull request, #56766:
URL: https://github.com/apache/spark/pull/56766
### What changes were proposed in this pull request?
Add the ANSI `LOCALTIME` datetime value function as an alias for `current_time`, returning the current local time as `TimeType` with optional precision.
### Why are the changes needed?
ANSI SQL defines `LOCALTIME` (TIME WITHOUT TIME ZONE) alongside `CURRENT_TIME`. Spark has `localtimestamp` but not `localtime`. Since Spark's `TIME` is always WITHOUT TIME ZONE, `LOCALTIME` is the standard-named counterpart.
### Does this PR introduce _any_ user-facing change?
Yes. `SELECT LOCALTIME`, `SELECT LOCALTIME()`, and `SELECT localtime(3)` now return the current time at the requested precision.
### How was this patch tested?
- Added parser test: `LOCALTIME` resolves to CurrentTime() in ANSI mode, UnresolvedAttribute in non-ANSI mode (ExpressionParserSuite)
- Added end-to-end test: `localtime()` returns `TimeType(6)`, `localtime(3)` returns `TimeType(3)`, `localtime() equals current_time()`
- All existing tests pass (ExpressionParserSuite: 50/50, TimeExpressionsSuite: 20/20)
### Was this patch authored or co-authored using generative AI tooling?
Co-Authored using Claude Opus 4.6
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57558][SQL] Add the LOCALTIME datetime value function [spark]
Posted by "shrirangmhalgi (via GitHub)" <gi...@apache.org>.
shrirangmhalgi commented on PR #56766:
URL: https://github.com/apache/spark/pull/56766#issuecomment-4801479394
Thanks @MaxGekk / @uros-b for the review. Addressed all the comments in the latest commit:
- Added `LOCALTIME` row to `docs / sql-ref-ansi-compliance.md (reserved | non-reserved | reserved)`
- Added `since = Some("4.3.0")` to the registry entry
- Regenerated `sql-expression-schema.md` golden file
- Added bare `SELECT LOCALTIME` end-to-end assertion
- Fixed `ThriftServer` keyword list. (CI failure)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57558][SQL] Add the LOCALTIME datetime value function [spark]
Posted by "shrirangmhalgi (via GitHub)" <gi...@apache.org>.
shrirangmhalgi commented on PR #56766:
URL: https://github.com/apache/spark/pull/56766#issuecomment-4796375738
@MaxGekk Could you please review the PR? This adds `LOCALTIME` as the ANSI-standard alias for `current_time`, reusing the existing `CurrentTime` expression.
Thankyou.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57558][SQL] Add the LOCALTIME datetime value function [spark]
Posted by "shrirangmhalgi (via GitHub)" <gi...@apache.org>.
shrirangmhalgi commented on code in PR #56766:
URL: https://github.com/apache/spark/pull/56766#discussion_r3478807078
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/FunctionRegistry.scala:
##########
@@ -700,6 +700,7 @@ object FunctionRegistry {
expressionBuilder("curdate", CurDateExpressionBuilder, setAlias = true),
expression[CurrentTimestamp]("current_timestamp"),
expression[CurrentTime]("current_time"),
+ expression[CurrentTime]("localtime", since = Some("4.3.0")),
Review Comment:
Acknowledged. This mirrors how `now()` labels as `current_timestamp()`. Making `localtime()` self-label would require a separate expression class (can't override `prettyName` on a case class alias). I would prefer to leave it consistent with the existing alias pattern for now - happy to refactor in a follow-up.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57558][SQL] Add the LOCALTIME datetime value function [spark]
Posted by "shrirangmhalgi (via GitHub)" <gi...@apache.org>.
shrirangmhalgi commented on code in PR #56766:
URL: https://github.com/apache/spark/pull/56766#discussion_r3478807078
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/FunctionRegistry.scala:
##########
@@ -700,6 +700,7 @@ object FunctionRegistry {
expressionBuilder("curdate", CurDateExpressionBuilder, setAlias = true),
expression[CurrentTimestamp]("current_timestamp"),
expression[CurrentTime]("current_time"),
+ expression[CurrentTime]("localtime", since = Some("4.3.0")),
Review Comment:
Acknowledged. This mirrors how `now()` labels as `current_timestamp()`. Making `localtime()` self-label would require a separate expression class (can't override `prettyName` on a case class alias). I would prefer to leave it consistent with the existing alias pattern for now - happy to refactor in a follow-up if preferred.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57558][SQL] Add the LOCALTIME datetime value function [spark]
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on PR #56766:
URL: https://github.com/apache/spark/pull/56766#issuecomment-4808676940
+1, LGTM. Merging to master/4.x.
Thank you, @shrirangmhalgi and @HyukjinKwon @uros-b for review.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57558][SQL] Add the LOCALTIME datetime value function [spark]
Posted by "shrirangmhalgi (via GitHub)" <gi...@apache.org>.
shrirangmhalgi commented on PR #56766:
URL: https://github.com/apache/spark/pull/56766#issuecomment-4809369641
Thankyou @MaxGekk / @uros-b / @HyukjinKwon for the review.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57558][SQL] Add the LOCALTIME datetime value function [spark]
Posted by "uros-b (via GitHub)" <gi...@apache.org>.
uros-b commented on code in PR #56766:
URL: https://github.com/apache/spark/pull/56766#discussion_r3473887676
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/FunctionRegistry.scala:
##########
@@ -700,6 +700,7 @@ object FunctionRegistry {
expressionBuilder("curdate", CurDateExpressionBuilder, setAlias = true),
expression[CurrentTimestamp]("current_timestamp"),
expression[CurrentTime]("current_time"),
+ expression[CurrentTime]("localtime"),
Review Comment:
Please add `since = Some("4.3.0")`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57558][SQL] Add the LOCALTIME datetime value function [spark]
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk closed pull request #56766: [SPARK-57558][SQL] Add the LOCALTIME datetime value function
URL: https://github.com/apache/spark/pull/56766
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57558][SQL] Add the LOCALTIME datetime value function [spark]
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on code in PR #56766:
URL: https://github.com/apache/spark/pull/56766#discussion_r3473207073
##########
sql/core/src/test/scala/org/apache/spark/sql/TimeFunctionsSuiteBase.scala:
##########
@@ -108,6 +108,23 @@ abstract class TimeFunctionsSuiteBase extends SharedSparkSession {
}
}
+ test("SPARK-57558: localtime function") {
Review Comment:
Non-blocking: this test exercises only the function form (`localtime()`, `localtime(3)`), i.e. the `FunctionRegistry` path. The bare-keyword grammar path (`SELECT LOCALTIME` with no parens) is covered at the parser level in `ExpressionParserSuite`, so end-to-end coverage isn't strictly required — but a bare `SELECT LOCALTIME` assertion here would round it out. Optional; the existing `current_time` e2e test has the same shape, so omitting it is consistency-neutral.
##########
sql/api/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBaseLexer.g4:
##########
@@ -345,6 +345,7 @@ LINES: 'LINES';
LIST: 'LIST';
LOAD: 'LOAD';
LOCAL: 'LOCAL';
+LOCALTIME: 'LOCALTIME';
Review Comment:
Adding `LOCALTIME` to the lexer puts it in the grammar's `SPARK-KEYWORD-LIST`, which `SQLKeywordSuite."all keywords are documented"` cross-checks against the manually-maintained keyword table in `docs/sql-ref-ansi-compliance.md`. That table currently has no `LOCALTIME` row, so the suite will fail with `Some keywords are not documented: LOCALTIME`.
Add the row alphabetically between `LOCAL` and `LOCATION` (line ~628), identical to the existing `CURRENT_TIME` row:
```
|LOCALTIME|reserved|non-reserved|reserved|
```
This matches the implemented status (reserved in ANSI mode — absent from `ansiNonReserved`; non-reserved in default mode — present in `nonReserved`) and `LOCALTIME` being a SQL-2016 reserved word, so all three of `SQLKeywordSuite`'s documentation checks pass.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57558][SQL] Add the LOCALTIME datetime value function [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #56766:
URL: https://github.com/apache/spark/pull/56766#discussion_r3478208849
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/FunctionRegistry.scala:
##########
@@ -700,6 +700,7 @@ object FunctionRegistry {
expressionBuilder("curdate", CurDateExpressionBuilder, setAlias = true),
expression[CurrentTimestamp]("current_timestamp"),
expression[CurrentTime]("current_time"),
+ expression[CurrentTime]("localtime", since = Some("4.3.0")),
Review Comment:
Non-blocking: because `localtime` is registered as an alias for `CurrentTime` (prettyName `current_time`), `SELECT localtime()` yields a column labeled `current_time(6)` (see `sql-expression-schema.md`), whereas the parallel ANSI function `LOCALTIMESTAMP` is its own expression and self-labels as `localtimestamp()`. The aliasing is semantically correct (Spark `TIME` is always WITHOUT TIME ZONE, so `LOCALTIME` == `CURRENT_TIME`), so this is only a cosmetic naming inconsistency — worth a thought if ANSI users expect `localtime()` to self-label.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57558][SQL] Add the LOCALTIME datetime value function [spark]
Posted by "uros-b (via GitHub)" <gi...@apache.org>.
uros-b commented on code in PR #56766:
URL: https://github.com/apache/spark/pull/56766#discussion_r3473881957
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/FunctionRegistry.scala:
##########
@@ -700,6 +700,7 @@ object FunctionRegistry {
expressionBuilder("curdate", CurDateExpressionBuilder, setAlias = true),
expression[CurrentTimestamp]("current_timestamp"),
expression[CurrentTime]("current_time"),
+ expression[CurrentTime]("localtime"),
Review Comment:
The expression[CurrentTime]("localtime") alias is registered but the `sql-expression-schema` golden file was not regenerated. ExpressionsSchemaSuite asserts exact schema size/rows and will likely fail.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/FunctionRegistry.scala:
##########
@@ -700,6 +700,7 @@ object FunctionRegistry {
expressionBuilder("curdate", CurDateExpressionBuilder, setAlias = true),
expression[CurrentTimestamp]("current_timestamp"),
expression[CurrentTime]("current_time"),
+ expression[CurrentTime]("localtime"),
Review Comment:
The `expression[CurrentTime]("localtime")` alias is registered but the `sql-expression-schema` golden file was not regenerated. ExpressionsSchemaSuite asserts exact schema size/rows and will likely fail.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org