You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "stevomitric (via GitHub)" <gi...@apache.org> on 2026/05/21 14:10:24 UTC
[PR] [SPARK-56965][SQL] Add SQL parser support for TIMESTAMP_NTZ(p) and TIMESTAMP_LTZ(p) [spark]
stevomitric opened a new pull request, #56041:
URL: https://github.com/apache/spark/pull/56041
### What changes were proposed in this pull request?
Adds SQL parser support for parameterized nanosecond-precision timestamp types introduced in SPARK-56876. The parser now accepts:
- TIMESTAMP_NTZ(p) -> TimestampNTZNanosType(p)
- TIMESTAMP_LTZ(p) -> TimestampLTZNanosType(p)
- TIMESTAMP(p) WITHOUT TIME ZONE (alias for TIMESTAMP_NTZ(p))
- TIMESTAMP(p) WITH LOCAL TIME ZONE (alias for TIMESTAMP_LTZ(p))
- TIMESTAMP(p) (resolves via spark.sql.timestampType session default)
with `p` from `[7, 9]`. Out-of-range precision throws `INVALID_TIMESTAMP_PRECISION`; negative precision is rejected by the grammar as `PARSE_SYNTAX_ERROR`.
The new syntax is gated behind a new internal preview flag `spark.sql.timestampNanosTypes.enabled` (default false).
Unparameterized TIMESTAMP, TIMESTAMP_NTZ, TIMESTAMP_LTZ, and the WITH/WITHOUT TIME ZONE variants continue to return the existing microsecond types - no behavior change.
Part of SPIP SPARK-56822 (https://issues.apache.org/jira/browse/SPARK-56822).
### Why are the changes needed?
SPARK-56876 added TimestampNTZNanosType / TimestampLTZNanosType to the type system but explicitly left out SQL/DDL integration - users cannot declare these types in CREATE TABLE, CAST, or Column.cast(String) today. This PR is the parser sub-task of the SPIP and wires those spellings through DataTypeAstBuilder, behind a preview flag so the surface is opt-in until the cast/runtime sub-tasks land.
### Does this PR introduce _any_ user-facing change?
Yes made the parser changes to allow timestamps with precision.
### How was this patch tested?
Extended DataTypeParserSuite
### Was this patch authored or co-authored using generative AI tooling?
Yes
--
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-56965][SQL] Add SQL parser support for TIMESTAMP_NTZ(p) and TIMESTAMP_LTZ(p) [spark]
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk closed pull request #56041: [SPARK-56965][SQL] Add SQL parser support for TIMESTAMP_NTZ(p) and TIMESTAMP_LTZ(p)
URL: https://github.com/apache/spark/pull/56041
--
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-56965][SQL] Add SQL parser support for TIMESTAMP_NTZ(p) and TIMESTAMP_LTZ(p) [spark]
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on PR #56041:
URL: https://github.com/apache/spark/pull/56041#issuecomment-4524483303
+1, LGTM. Merging to master.
Thank you, @stevomitric.
--
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-56965][SQL] Add SQL parser support for TIMESTAMP_NTZ(p) and TIMESTAMP_LTZ(p) [spark]
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on code in PR #56041:
URL: https://github.com/apache/spark/pull/56041#discussion_r3283094579
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/DataTypeParserSuite.scala:
##########
@@ -241,4 +289,53 @@ class DataTypeParserSuite extends SparkFunSuite with SQLHelper {
condition = "PARSE_SYNTAX_ERROR",
parameters = Map("error" -> "'WITH'", "hint" -> ""))
}
+
+ test("invalid precision of the nanos timestamp data type") {
Review Comment:
This test covers the two zone-suffixed forms - TIMESTAMP(6) WITHOUT TIME ZONE -> TIMESTAMP_NTZ error and TIMESTAMP(10) WITH LOCAL TIME ZONE -> TIMESTAMP_LTZ error - but does not exercise the bare TIMESTAMP(6) / TIMESTAMP(10) form under each SQLConf.TIMESTAMP_TYPE setting, which is the path that routes through SqlApiConf.get.timestampType in DataTypeAstBuilder.scala.
Alternatively, the assertions could be added to the existing test("Set default timestamp type") block at line 159, since that test already wraps both values of SQLConf.TIMESTAMP_TYPE and was modified by this PR to add the success-case parse("timestamp(9)") assertions — the invalid-precision counterpart would sit naturally next to them.
--
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-56965][SQL] Add SQL parser support for TIMESTAMP_NTZ(p) and TIMESTAMP_LTZ(p) [spark]
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on code in PR #56041:
URL: https://github.com/apache/spark/pull/56041#discussion_r3282911422
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala:
##########
@@ -644,6 +644,19 @@ object SQLConf {
.booleanConf
.createWithDefaultFunction(() => Utils.isTesting)
+ val TIMESTAMP_NANOS_TYPES_ENABLED =
+ buildConf("spark.sql.timestampNanosTypes.enabled")
+ .internal()
+ .doc("When true, the SQL parser accepts the parameterized nanosecond-precision " +
Review Comment:
The description is too restrictive (especially taking into account its name). The config shall disable other entry points like creating df from Java types: java.time.LocalDateTime.
--
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-56965][SQL] Add SQL parser support for TIMESTAMP_NTZ(p) and TIMESTAMP_LTZ(p) [spark]
Posted by "stevomitric (via GitHub)" <gi...@apache.org>.
stevomitric commented on code in PR #56041:
URL: https://github.com/apache/spark/pull/56041#discussion_r3287647897
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/DataTypeParserSuite.scala:
##########
@@ -241,4 +289,53 @@ class DataTypeParserSuite extends SparkFunSuite with SQLHelper {
condition = "PARSE_SYNTAX_ERROR",
parameters = Map("error" -> "'WITH'", "hint" -> ""))
}
+
+ test("invalid precision of the nanos timestamp data type") {
Review Comment:
Added both 6 and 10 precision checks.
--
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-56965][SQL] Add SQL parser support for TIMESTAMP_NTZ(p) and TIMESTAMP_LTZ(p) [spark]
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on PR #56041:
URL: https://github.com/apache/spark/pull/56041#issuecomment-4540710738
@stevomitric Could you backport this to branch-4.x, please.
--
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-56965][SQL] Add SQL parser support for TIMESTAMP_NTZ(p) and TIMESTAMP_LTZ(p) [spark]
Posted by "stevomitric (via GitHub)" <gi...@apache.org>.
stevomitric commented on PR #56041:
URL: https://github.com/apache/spark/pull/56041#issuecomment-4509095549
@MaxGekk please take a look at this PR.
--
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-56965][SQL] Add SQL parser support for TIMESTAMP_NTZ(p) and TIMESTAMP_LTZ(p) [spark]
Posted by "stevomitric (via GitHub)" <gi...@apache.org>.
stevomitric commented on code in PR #56041:
URL: https://github.com/apache/spark/pull/56041#discussion_r3287511201
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala:
##########
@@ -644,6 +644,19 @@ object SQLConf {
.booleanConf
.createWithDefaultFunction(() => Utils.isTesting)
+ val TIMESTAMP_NANOS_TYPES_ENABLED =
+ buildConf("spark.sql.timestampNanosTypes.enabled")
+ .internal()
+ .doc("When true, the SQL parser accepts the parameterized nanosecond-precision " +
Review Comment:
Makes sense. I generalized the doc and had to gate the check on DataTypes as well.
--
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-56965][SQL] Add SQL parser support for TIMESTAMP_NTZ(p) and TIMESTAMP_LTZ(p) [spark]
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on PR #56041:
URL: https://github.com/apache/spark/pull/56041#issuecomment-4519763054
@stevomitric Could you re-check the test failure. It might be related to your changes:
```
[info] SparkConfigBindingPolicySuite:
...
[info] - Config enforcement for bindingPolicy *** FAILED *** (18 milliseconds)
[info] The following configs do not have bindingPolicy field set. You need to define it by using .withBindingPolicy(ConfigBindingPolicy.SESSION/PERSISTED) when you build the config entry.
```
--
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-56965][SQL] Add SQL parser support for TIMESTAMP_NTZ(p) and TIMESTAMP_LTZ(p) [spark]
Posted by "stevomitric (via GitHub)" <gi...@apache.org>.
stevomitric commented on PR #56041:
URL: https://github.com/apache/spark/pull/56041#issuecomment-4519947581
@MaxGekk should be fixed now - config binding policy was the issue.
--
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