adriangb opened a new pull request, #25163: URL: https://github.com/apache/datafusion/pull/25163
## Which issue does this PR close? - Part of https://github.com/apache/datafusion/issues/10368 (the third bullet: "Extracting the time offset using `date_part` might be a nice to have"). The other bullets of that issue are not addressed here, so it is deliberately not closed. ## Rationale for this change On today's `main`, asking for the UTC offset of a timezone-aware timestamp fails: ``` > SELECT date_part('timezone', '2024-07-01T12:00:00Z'::timestamptz AT TIME ZONE 'Europe/Brussels'); Execution error: Date part 'timezone' not supported ``` PostgreSQL supports three fields here, and there is no other way in DataFusion to get at the offset that applies to a value — it is information the `Timestamp` type carries but never exposes. This is particularly awkward because the offset is not a property of the type alone: for a named timezone it changes with daylight saving time, so a user cannot compute it once and reuse it. After this PR: ``` > SELECT date_part('timezone', '2024-07-01T12:00:00Z'::timestamptz AT TIME ZONE 'Europe/Brussels'); +--------------------+ | utc_offset_seconds | +--------------------+ | 7200 | +--------------------+ ``` ## What changes are included in this PR? Two commits. **1. `refactor: move parse_tz from date_trunc into datetime::common`** — no behaviour change. `parse_tz` (Arrow's optional timezone string → `arrow::array::timezone::Tz`) was private to `date_trunc.rs`; it now lives next to the other shared datetime helpers so `date_part` can reuse it instead of adding a second parser. **2. `feat: support timezone, timezone_hour and timezone_minute in date_part`** — the three PostgreSQL fields: | part | meaning | `Europe/Brussels`, July | | --- | --- | --- | | `timezone` | UTC offset in seconds | `7200` | | `timezone_hour` | whole hours of the offset | `2` | | `timezone_minute` | whole minutes of the offset, excluding the hours | `0` | Design points, and why: - **Per row, not per type.** The offset of a named timezone depends on the instant, so each value is converted with `as_datetime_with_timezone::<T>` and its offset read back. `Europe/Brussels` gives `3600` in January and `7200` in July. A single pass of `PrimitiveArray::unary_opt` computes the field directly, so there is no intermediate offset array; input nulls are preserved. - **Non-whole-hour and negative offsets.** `Asia/Kolkata` → `19800 / 5 / 30`, `Pacific/Chatham` → `45900 / 12 / 45` (and `49500 / 13 / 45` in its southern-hemisphere summer), `America/St_Johns` → `-12600 / -3 / -30`. For a negative offset **both** the hour and the minute carry the sign — this is PostgreSQL's convention, confirmed against a real server rather than guessed (see below). It falls out of Rust's truncate-towards-zero integer division. - **Timezone-naive input is an error, not an implicit UTC.** A `Timestamp(_, None)` carries no offset at all, and silently answering `0` would be wrong rather than merely unhelpful. PostgreSQL rejects it (`unit "timezone" not supported for type timestamp without time zone`) and so do we: `Date part 'timezone' is not supported for timezone-naive timestamps, got Timestamp(ns)`. Dates, times, intervals and durations get an analogous error. - **Return type is `Int32`**, matching every other `date_part` field except `epoch` (`Float64`) and `nanosecond` (`Int64`). PostgreSQL returns `double precision` for *all* fields; DataFusion already diverges from that for the integral ones, and UTC offsets are integral seconds, so `Int32` is the consistent choice here rather than a new special case. - **`EXTRACT` works too, with no planner change.** `SQLExpr::Extract` forwards the `DateTimeField` to `date_part` as its `Display` string, and sqlparser already carries `Timezone`, `TimezoneHour` and `TimezoneMinute` variants — so `EXTRACT(TIMEZONE_HOUR FROM ...)` reaches the new code as `"TIMEZONE_HOUR"` and is matched case-insensitively like the other parts. No existing field's behaviour changes: the new parts are reached only from the `_` arm of the "not an interval unit" match that previously returned `Date part '{part}' not supported`. ### PostgreSQL semantics: how they were confirmed The sign convention for `timezone_minute` on negative offsets is the kind of thing that is easy to get backwards, so it was measured rather than assumed, on `postgres:17` (17.11) in Docker, `SET TimeZone TO '<zone>'` with a `timestamptz` literal: | zone | instant | `timezone` | `timezone_hour` | `timezone_minute` | | --- | --- | --- | --- | --- | | `Europe/Brussels` | 2024-01-01T12:00:00Z | 3600 | 1 | 0 | | `Europe/Brussels` | 2024-07-01T12:00:00Z | 7200 | 2 | 0 | | `Asia/Kolkata` | 2024-07-01T12:00:00Z | 19800 | 5 | 30 | | `Asia/Kathmandu` | 2024-07-01T12:00:00Z | 20700 | 5 | 45 | | `Pacific/Chatham` | 2024-01-01T12:00:00Z | 49500 | 13 | 45 | | `Pacific/Chatham` | 2024-07-01T12:00:00Z | 45900 | 12 | 45 | | `America/St_Johns` | 2024-01-01T12:00:00Z | **-12600** | **-3** | **-30** | | `America/St_Johns` | 2024-07-01T12:00:00Z | -9000 | -2 | -30 | | `America/Denver` | 2024-01-01T12:00:00Z | -25200 | -7 | 0 | | `Pacific/Marquesas` | 2024-07-01T12:00:00Z | -34200 | -9 | -30 | So the minute follows the sign of the offset. Also confirmed on the same server: `NULL` in → `NULL` out; the part name is case-insensitive; `tz` is *not* an accepted spelling; and `date`, `time` and `interval` inputs are all rejected. ## What is the testing strategy for this PR? - **Unit tests** in `datafusion/functions/src/datetime/date_part.rs` (9 new tests). `timezone_parts_match_postgres` is a table of all 15 zone/instant combinations above, asserted for all three fields — the timestamp arrays are built by relabelling UTC instants, so these are exactly the PostgreSQL cases. The rest cover: case insensitivity (including the `EXTRACT` spelling), array input spanning a DST transition with a null row, all four `TimeUnit`s, scalar input, `NULL` input, the timezone-naive rejection, the non-timestamp rejection, and the `Int32` return field. - **sqllogictest** cases appended to `datafusion/sqllogictest/test_files/datetime/date_part.slt`: fixed offsets (`+05:30`, `-03:30`), the exact query from the issue, named zones in standard vs daylight saving time, the 45-minute `Pacific/Chatham` cases, negative non-whole-hour offsets, `EXTRACT(...)` syntax, `arrow_typeof` of all three results, `NULL`, a column straddling the exact Brussels spring-forward boundary (01:59:59 vs 03:00:00 local), and the four error cases. - Full `cargo test -p datafusion-functions` (349 tests) and the full sqllogictest suite (505 files) pass locally, plus `cargo fmt --all` and `cargo clippy --all-targets -- -D warnings`. ## Are there any user-facing changes? Yes, additive only — three new `date_part`/`EXTRACT` fields. `docs/source/user-guide/sql/scalar_functions.md` is regenerated from the `user_doc!` block via `dev/update_function_docs.sh`, adding the three parts, a note on the DST and sign behaviour and the timezone-only restriction, and a worked example. No public API changes and no behaviour change to any existing field, so no `api change` label. 🤖 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]
