adriangb commented on issue #25084: URL: https://github.com/apache/datafusion/issues/25084#issuecomment-5623270357
Update: this needs **two** upstream fixes, not one. I had assumed https://github.com/apache/arrow-rs/pull/11038 would close it. It does not. There are two independent code paths here, and they fail with two different errors. Verified on DataFusion `da89c7c85b` (55.0.0): ```sql -- path 1: the timestamp cast kernel -> arrow-cast/src/cast/mod.rs SELECT '2024-03-10 02:30:00'::timestamp AT TIME ZONE 'America/New_York'; -- Arrow error: Cast error: Cannot cast timezone to different timezone -- path 2: the string parser -> arrow-cast/src/parse.rs SELECT '2024-03-10 02:30:00' AT TIME ZONE 'America/New_York'; -- Arrow error: Parser error: Error parsing timestamp from '2024-03-10 02:30:00': error computing timezone offset SET datafusion.execution.time_zone = 'America/New_York'; SELECT '2024-03-10 02:30:00'::timestamptz; -- Arrow error: Parser error: Error parsing timestamp from '2024-03-10 02:30:00': error computing timezone offset ``` To a user those are the same query written three ways. Only the first is fixed by #11038; the other two go through `string_to_datetime`, which still resolves the local time with `LocalResult::single()` in three places. That is https://github.com/apache/arrow-rs/issues/11039. Note the third form especially — `'...'::timestamptz` under a named session time zone is the most natural way to write this, and it is entirely on the parser path, so #11038 does not help it at all. Landing #11038 alone would leave the engine self-inconsistent: adding an explicit `::timestamp` in the middle of an expression would make it start working. **So https://github.com/apache/arrow-rs/issues/11039 is a hard blocker for this issue, not a follow-up.** Both arrow-rs changes plus an arrow version bump are needed before this can be closed. On the first path, #11038 itself is in good shape. I validated it against 97,162 cases covering every transition of every IANA zone in 1890-1995 and 2023-2025 (15,871 in a gap, 14,370 ambiguous): zero disagreements with an independent implementation on the same tzdata, and agreement with PostgreSQL 17 and DuckDB/ICU wherever their tzdata versions agree. Offsets resolve per row, confirmed on arrays that straddle a transition. No existing DataFusion sqllogictest expectation changes. One consequence worth knowing before the bump lands: DataFusion has **no** test coverage for this behaviour today. There is no `Cannot cast timezone to different timezone` expectation anywhere in `test_files/`, so nothing in the suite would catch a regression here — including a per-array-instead-of-per-row offset bug. Coverage is being added in https://github.com/apache/datafusion/pull/25164. -- 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]
