LuciferYang commented on code in PR #57110:
URL: https://github.com/apache/spark/pull/57110#discussion_r3541502944
##########
sql/api/src/main/scala/org/apache/spark/sql/catalyst/util/SparkDateTimeUtils.scala:
##########
@@ -43,10 +43,20 @@ trait SparkDateTimeUtils {
def getZoneId(timeZoneId: String): ZoneId = {
try {
- // To support the (+|-)h:mm format because it was supported before Spark
3.0.
- var formattedZoneId =
singleHourTz.matcher(timeZoneId).replaceFirst("$10$2:")
- // To support the (+|-)hh:m format because it was supported before Spark
3.0.
- formattedZoneId =
singleMinuteTz.matcher(formattedZoneId).replaceFirst("$1$2:0$3")
+ // The (+|-)h:mm and (+|-)hh:m formats (supported before Spark 3.0) both
start with a sign
Review Comment:
Good catch, and thanks — your question surfaced a real regression I missed.
The two patterns are `(\+|\-)(\d):` and `(\+|\-)(\d\d):(\d)$` — neither is
`^`-anchored. So the current code doesn't only normalize a leading sign offset;
it also rewrites an offset that appears **after** a `GMT`/`UTC`/`UT` prefix.
For example `GMT+8:00` is rewritten to `GMT+08:00`, which `ZoneId.of(...,
SHORT_IDS)` then accepts. My `charAt(0) == '+'/'-'` guard skips that rewrite,
so the following inputs now throw `DateTimeException` instead of resolving:
- `GMT+8:00` → `GMT+08:00` (before) / throws (after)
- `GMT-5:30`, `UTC+8:0`, `UT+8:00` — same
These forms fall under the `zone_id` grammar documented in
`parseTimestampString` (the `UTC+/GMT+/UT+` prefix with a `+|-hh[:]mm` suffix),
so this is a user-visible behavior change, not just an untested edge. My
premise for the optimization — that the regexes only act on the leading sign —
was simply wrong, and it can't be fixed by tweaking the guard without
re-scanning for a mid-string `[+-]\d:`, which would negate the point.
So I'm inclined to close 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
