adriangb opened a new pull request, #25115: URL: https://github.com/apache/datafusion/pull/25115
## Which issue does this PR close? - Closes https://github.com/apache/datafusion/issues/25084. ## Rationale for this change Casting a timezone-naive timestamp to a **named** timezone fails whenever the wall clock value falls on a daylight saving boundary: ```sql SET datafusion.execution.time_zone = 'America/New_York'; SELECT '2024-11-03T01:30:00'::timestamp::timestamptz; -- ambiguous ("fall back" hour) Arrow error: Cast error: Cannot cast timezone to different timezone SELECT '2024-03-10T02:30:00'::timestamp::timestamptz; -- nonexistent ("spring forward" hour) Arrow error: Cast error: Cannot cast timezone to different timezone ``` `TRY_CAST` turns the same values into `NULL`. Unambiguous times and fixed-offset timezones are unaffected. It reproduces on real columns as well as literals. arrow-cast resolves the offset with `offset_from_local_datetime(..).single()`, which is `None` for both `LocalResult::Ambiguous` and `LocalResult::None`. PostgreSQL 17 and DuckDB 1.5 both resolve these deterministically, and they agree with each other: | input (`America/New_York`) | PostgreSQL / DuckDB | DataFusion before | DataFusion after | |---|---|---|---| | `2024-11-03T01:30:00` (ambiguous) | `2024-11-03 01:30:00-05` | error | `2024-11-03T01:30:00-05:00` | | `2024-03-10T02:30:00` (nonexistent) | `2024-03-10 03:30:00-04` | error | `2024-03-10T03:30:00-04:00` | This matters beyond explicit casts: the fix for https://github.com/apache/datafusion/issues/13212 (see https://github.com/apache/datafusion/pull/25094) makes type coercion insert exactly this cast for `timestamptz - timestamp`, so without this change any such query under a named session timezone starts erroring on DST-boundary rows. ## What changes are included in this PR? - New module `datafusion_common::timezone_cast` implementing the PostgreSQL/DuckDB convention: - **ambiguous** local times resolve to the **later** instant, i.e. the post-transition (standard) offset; - **nonexistent** local times shift **forward** by the size of the gap, which is the same as interpreting the wall clock reading with the pre-transition offset (recovered by probing the offset 24 hours earlier). - The two DataFusion cast entry points, `ColumnarValue::cast_to` (arrays, i.e. `CastExpr`) and `ScalarValue::cast_to_with_options` (scalars, i.e. constant folding), route the one type pair `Timestamp(_, None) -> Timestamp(_, Some(tz))` through it. Everything else, including the unit conversion and `CastOptions::safe` handling, is still delegated to arrow's kernel, and the error message for a value that still cannot be resolved is unchanged. I chose to do this in DataFusion rather than arrow-rs because which instant an ambiguous wall clock time maps to is a SQL-engine semantic choice (matching PostgreSQL/DuckDB) rather than something arrow's kernel should decide by default. If arrow-rs later grows a `CastOptions` knob for this, the module can shrink to a call into it. ## What is the testing strategy for this PR? - 9 unit tests in `datafusion/common/src/timezone_cast.rs`: unambiguous, ambiguous and gap values for `America/New_York` and `Australia/Sydney` (southern hemisphere, opposite transition order), a fixed offset, null preservation, a unit-changing cast, and equality with `arrow::compute::cast` for values where arrow already succeeds. - New `datafusion/sqllogictest/test_files/datetime/cast_timestamp_dst.slt`: the reproductions from the issue as literals (constant-folded scalar path) and as table columns (array path), all four time units, `TRY_CAST`, NULL inputs, `to_unixtime` checks of the resolved instants, `Australia/Sydney`, and a fixed-offset `+08:00` control showing unchanged behaviour. Expected epoch seconds were computed independently rather than copied from the runner. - The full unfiltered sqllogictest suite passes; no existing expectation changed. The existing `statement error` for `TIMESTAMPTZ '2023-03-12 02:00:00 America/Los_Angeles'` is untouched: that is the string parser, not the cast. ## Are there any user-facing changes? Yes, in the sense that casts which used to error (or return `NULL` under `TRY_CAST`) now return a value, following the PostgreSQL/DuckDB convention described above. No public API changes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
