andygrove opened a new issue, #6329:
URL: https://github.com/apache/datafusion-comet/issues/6329
### Describe the bug
Comet passes the session timezone to native code as the raw string from each
expression's `timeZoneId`. Native kernels parse it with arrow's `Tz::from_str`,
which accepts only IANA zone names and fixed offsets written as `+HH`, `+HHMM`
or `+HH:MM`. Spark resolves the ID with `ZoneId.of(id, ZoneId.SHORT_IDS)` and
accepts more: `Z`, offsets like `+8` and `+08:00:00`, prefixed offsets like
`GMT+8` and `UTC+08:00`, and short IDs such as `PST` and `IST`. The
`spark.sql.session.timeZone` documentation lists `Z` and `(+|-)HH:mm:ss`
explicitly. With any of these, the query fails at execution time instead of
falling back.
This isn't limited to the timezone functions. It covers most of what you can
do with a timestamp: `CAST(ts AS STRING)` and `CAST(ts AS DATE)` (and so
`year`, `month` and `dayofmonth` of a timestamp), `hour`, `minute` and
`second`, `CAST(string AS TIMESTAMP)`, `CAST(date AS TIMESTAMP)`,
`unix_timestamp` of a date, casts between `TIMESTAMP` and `TIMESTAMP_NTZ`, and
`df.show()`. `GMT+8` in particular is a common production setting.
### Steps to reproduce
On `main` at `764936187`, with the default config, on Spark 3.5 and 4.1:
```sql
CREATE TABLE events USING parquet AS
SELECT * FROM VALUES (TIMESTAMP'2024-01-15T18:30:45Z'),
(TIMESTAMP'2024-06-30T23:30:00Z') AS v(ts);
SET spark.sql.session.timeZone=GMT+8;
SELECT CAST(ts AS STRING), CAST(ts AS DATE), hour(ts) FROM events;
```
Spark returns `2024-01-16 02:30:45, 2024-01-16, 2` and `2024-07-01 07:30:00,
2024-07-01, 7`. Comet fails with `Parser error: Invalid timezone "GMT+8":
failed to parse timezone`, and so does `spark.table("events").show()`. The same
happens with `UTC+08:00`, `+8`, `+08:00:00`, `Z`, `PST` and `IST`, while
`+08:00` works. `CAST(string AS TIMESTAMP)` reports it as `[INTERNAL_ERROR]
Invalid timezone string: GMT+8`.
### Expected behavior
The same results as Spark, or a clean fallback to Spark for a timezone Comet
can't represent.
### Additional context
We could normalize the session timezone once, on the JVM side, before
serializing it. `DateTimeUtils.getZoneId(id).normalized()` turns `GMT+8`,
`UTC+08:00`, `+8` and `+08:00:00` into `+08:00`, `PST` into
`America/Los_Angeles`, and `Z`, `UTC` and `Etc/UTC` into `Z`, which we'd pass
as `UTC`. Anything the native parser still can't take, such as an offset with
seconds or a region newer than chrono-tz's tzdata, could fall back. If one
helper did this at every serde site that currently calls
`timeZoneId.getOrElse("UTC")`, it would also settle #2730.
`from_utc_timestamp`, `to_utc_timestamp` and `convert_timezone` already mark
themselves incompatible because of this parser gap for their timezone argument
(#2013), but nothing covers the session timezone.
--
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]
