You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "adriangb (via GitHub)" <gi...@apache.org> on 2026/09/10 17:23:30 UTC
[PR] feat: support `timezone`, `timezone_hour` and `timezone_minute` in `date_part` [datafusion]
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: support `timezone`, `timezone_hour` and `timezone_minute` in `date_part` [datafusion]
Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on PR #25163:
URL: https://github.com/apache/datafusion/pull/25163#issuecomment-5625321869
Heads-up on a merge-order interaction: https://github.com/apache/datafusion/pull/25175 (timezone characterization tests) currently pins the *current* behaviour of these fields as errors —
```
query error DataFusion error: Execution error: Date part 'timezone_hour' not supported
SELECT date_part('timezone_hour', ts) FROM day_denver
```
— in `datafusion/sqllogictest/test_files/datetime/timestamps_timezone.slt`, Section 9, along with a comment saying there is no way to ask a timezone-aware value for its own offset.
Once this PR lands, those two `query error` blocks become value queries (`-6` and `0` for every `day_denver` row) and that comment is no longer true. Whichever of the two merges second needs to update the other; nothing is broken by merging in either order. Noting it here so it is not a surprise.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: support `timezone`, `timezone_hour` and `timezone_minute` in `date_part` [datafusion]
Posted by "kosiew (via GitHub)" <gi...@apache.org>.
kosiew commented on code in PR #25163:
URL: https://github.com/apache/datafusion/pull/25163#discussion_r4071597656
##########
datafusion/functions/src/datetime/date_part.rs:
##########
@@ -317,6 +337,100 @@ fn is_nanosecond(part: &str) -> bool {
.unwrap_or(false)
}
+/// The `timezone`, `timezone_hour` and `timezone_minute` fields, which report
+/// the UTC offset that applies to a timezone-aware timestamp *at that instant*.
+///
+/// Because the offset of a named timezone changes with daylight saving time,
+/// these are per-row values, not a property of the type alone.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+enum TimezonePart {
+ /// `timezone`: the whole UTC offset, in seconds
+ Offset,
+ /// `timezone_hour`: whole hours of the UTC offset
+ Hour,
+ /// `timezone_minute`: whole minutes of the UTC offset, excluding the hours.
+ /// The sign follows the sign of the offset (PostgreSQL behaviour), so
+ /// `-03:30` yields `-3` hours and `-30` minutes.
+ Minute,
+}
+
+impl TimezonePart {
+ fn parse(part: &str) -> Option<Self> {
+ match part {
+ "timezone" => Some(Self::Offset),
+ "timezone_hour" => Some(Self::Hour),
+ "timezone_minute" => Some(Self::Minute),
+ _ => None,
+ }
+ }
+
+ fn name(&self) -> &'static str {
+ match self {
+ Self::Offset => "timezone",
+ Self::Hour => "timezone_hour",
+ Self::Minute => "timezone_minute",
+ }
+ }
+
+ /// Extract this field from a UTC offset expressed in seconds.
+ ///
+ /// Rust integer division truncates towards zero, which is what makes the
+ /// sign of `timezone_minute` follow the sign of the offset, matching
+ /// PostgreSQL.
+ fn project(&self, offset_seconds: i32) -> i32 {
+ match self {
+ Self::Offset => offset_seconds,
+ Self::Hour => offset_seconds / 3_600,
+ Self::Minute => (offset_seconds % 3_600) / 60,
+ }
+ }
+}
+
+/// Compute a [`TimezonePart`] for every element of a timezone-aware timestamp
+/// array.
+///
+/// Errors for any other input type, including timezone-naive timestamps, which
+/// carry no offset at all. This matches PostgreSQL, which rejects
+/// `date_part('timezone', <timestamp without time zone>)`.
+fn timezone_part(array: &dyn Array, part: TimezonePart) -> Result<ArrayRef> {
+ let Timestamp(unit, tz_opt) = array.data_type() else {
+ return exec_err!(
+ "Date part '{}' is only supported for timestamps with a timezone, got {}",
+ part.name(),
+ array.data_type()
+ );
+ };
+ let Some(tz) = parse_tz(tz_opt.as_ref())? else {
Review Comment:
Could we add a small unit test with a timestamp array that has an invalid Arrow timezone string? It would be useful to assert that the execution error preserves the timezone value and context. This is the new caller of the relocated `parse_tz` helper, and while the valid timezone cases cover the success path well, a test here would also protect the user-visible error path for externally constructed Arrow timestamp schemas.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: support `timezone`, `timezone_hour` and `timezone_minute` in `date_part` [datafusion]
Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on code in PR #25163:
URL: https://github.com/apache/datafusion/pull/25163#discussion_r4072360799
##########
datafusion/functions/src/datetime/date_part.rs:
##########
@@ -317,6 +337,100 @@ fn is_nanosecond(part: &str) -> bool {
.unwrap_or(false)
}
+/// The `timezone`, `timezone_hour` and `timezone_minute` fields, which report
+/// the UTC offset that applies to a timezone-aware timestamp *at that instant*.
+///
+/// Because the offset of a named timezone changes with daylight saving time,
+/// these are per-row values, not a property of the type alone.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+enum TimezonePart {
+ /// `timezone`: the whole UTC offset, in seconds
+ Offset,
+ /// `timezone_hour`: whole hours of the UTC offset
+ Hour,
+ /// `timezone_minute`: whole minutes of the UTC offset, excluding the hours.
+ /// The sign follows the sign of the offset (PostgreSQL behaviour), so
+ /// `-03:30` yields `-3` hours and `-30` minutes.
+ Minute,
+}
+
+impl TimezonePart {
+ fn parse(part: &str) -> Option<Self> {
+ match part {
+ "timezone" => Some(Self::Offset),
+ "timezone_hour" => Some(Self::Hour),
+ "timezone_minute" => Some(Self::Minute),
+ _ => None,
+ }
+ }
+
+ fn name(&self) -> &'static str {
+ match self {
+ Self::Offset => "timezone",
+ Self::Hour => "timezone_hour",
+ Self::Minute => "timezone_minute",
+ }
+ }
+
+ /// Extract this field from a UTC offset expressed in seconds.
+ ///
+ /// Rust integer division truncates towards zero, which is what makes the
+ /// sign of `timezone_minute` follow the sign of the offset, matching
+ /// PostgreSQL.
+ fn project(&self, offset_seconds: i32) -> i32 {
+ match self {
+ Self::Offset => offset_seconds,
+ Self::Hour => offset_seconds / 3_600,
+ Self::Minute => (offset_seconds % 3_600) / 60,
+ }
+ }
+}
+
+/// Compute a [`TimezonePart`] for every element of a timezone-aware timestamp
+/// array.
+///
+/// Errors for any other input type, including timezone-naive timestamps, which
+/// carry no offset at all. This matches PostgreSQL, which rejects
+/// `date_part('timezone', <timestamp without time zone>)`.
+fn timezone_part(array: &dyn Array, part: TimezonePart) -> Result<ArrayRef> {
+ let Timestamp(unit, tz_opt) = array.data_type() else {
+ return exec_err!(
+ "Date part '{}' is only supported for timestamps with a timezone, got {}",
+ part.name(),
+ array.data_type()
+ );
+ };
+ let Some(tz) = parse_tz(tz_opt.as_ref())? else {
Review Comment:
Thanks, added in [08d6ae0](https://github.com/apache/datafusion/pull/25163/commits/08d6ae07e066936de302d69f057da2aa5ae7ae08)
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: support `timezone`, `timezone_hour` and `timezone_minute` in `date_part` [datafusion]
Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on PR #25163:
URL: https://github.com/apache/datafusion/pull/25163#issuecomment-5626585948
**Self-review: a QA pass on my own PR.** I checked every claim in the description against a real PostgreSQL 17.11 and a real DuckDB 1.5.2, and I re-ran the build and the tests. I found no correctness bug. The sign convention, the return type and the `EXTRACT` path are all correct. Below are one user-visible gap, three nits, and then the full list of what I checked.
---
## 1. `date_part('timezone', now())` fails out of the box
This is the first thing a user tries, and it does not work with the default configuration:
```sql
> SELECT arrow_typeof(now());
Timestamp(ns)
> SELECT date_part('timezone', now());
Execution error: Date part 'timezone' is not supported for timezone-naive timestamps, got Timestamp(ns)
```
`datafusion.execution.time_zone` defaults to `None`, so `now()` is timezone-naive. A session zone fixes it:
```sql
> SET datafusion.execution.time_zone = 'Europe/Brussels';
> SELECT arrow_typeof(now()), date_part('timezone', now());
Timestamp(ns, "Europe/Brussels") 7200
```
PostgreSQL answers `date_part('timezone', now())` directly, because its `now()` is always `timestamptz`.
The root cause is https://github.com/apache/datafusion/issues/25166, not this PR, and the fix does not belong here. But the PR adds the exact function a user reaches for, so the dead end is new. Two cheap additions cover it:
- a `.slt` case for `now()` with and without a session zone, which pins the gap and turns into a diff when 25166 lands;
- one sentence in the `user_doc!` block that names `datafusion.execution.time_zone`.
## 2. The doc sentence over-states the sign rule
The doc block says: "For a negative offset both parts carry the sign". That is not exact. When the absolute offset is under one hour, the hour is `0` and loses the sign, and only the minute carries it. The code is still right — PostgreSQL does the same thing:
| engine | zone | instant | `timezone` | `timezone_hour` | `timezone_minute` |
| --- | --- | --- | --- | --- | --- |
| PostgreSQL 17.11 | `Africa/Monrovia` | 1900-01-01T12:00:00Z | -2588 | 0 | -43 |
| this PR | `Africa/Monrovia` | 1900-01-01T12:00:00Z | -2588 | 0 | -43 |
Truncation towards zero matches on the seconds too, so this is an exact match and not a near miss. No modern zone has a negative offset under one hour, so only historical LMT data reaches it.
Two suggestions:
- reword to "the minute carries the sign of the offset, and so does the hour when the offset is a whole hour or more";
- add the `Africa/Monrovia` row to `timezone_parts_match_postgres`. It is the one case the current 15-row table does not reach, and both engines agree on it.
## 3. Move `to_lowercase` inside `TimezonePart::parse`
The call site is correct today, and I confirmed mixed case works after the #24906 rebase. But the invariant is fragile:
```rust
Err(_) => match TimezonePart::parse(&part_trim.to_lowercase()) {
```
`part_normalization` strips quotes only; it does not lower the case. `DatePart::from_str` and `is_epoch` each lower the case *inside* themselves. `TimezonePart::parse` is the one function in the file that depends on its caller. A second caller would silently break `date_part('TIMEZONE_HOUR', ...)`, and no local test catches it. Move `.to_lowercase()` into `parse` so the type owns its own contract.
## 4. Two error shapes for one condition
The two messages read as two different rules:
- `Date part 'timezone' is not supported for timezone-naive timestamps, got Timestamp(ns)`
- `Date part 'timezone' is only supported for timestamps with a timezone, got Date32`
Both mean "this value has no UTC offset". One shape is easier to grep for and easier to document. Neither message tells the user what to do next. A hint helps: name `AT TIME ZONE` or `datafusion.execution.time_zone`.
## 5. Two small errors in the PR description
- The count is `349 tests`; the real number is `353`. The lib suite goes from 344 to 353.
- The "After this PR" block shows a column named `utc_offset_seconds`, but the query carries no such alias.
I fix both in the rewritten description.
---
## What I checked, and what it measured
**Sign convention, against PostgreSQL 17.11.** `SET TimeZone TO '<zone>'`, then `date_part('<part>', timestamptz '<instant>')`. All 10 rows of the table in the PR description reproduce exactly, and 6 more zone and instant pairs do too:
| zone | instant | `timezone` | `timezone_hour` | `timezone_minute` |
| --- | --- | --- | --- | --- |
| `America/St_Johns` | 2024-01-01T12:00:00Z | -12600 | -3 | -30 |
| `America/St_Johns` | 2024-07-01T12:00:00Z | -9000 | -2 | -30 |
| `Pacific/Chatham` | 2024-01-01T12:00:00Z | 49500 | 13 | 45 |
| `Pacific/Chatham` | 2024-07-01T12:00:00Z | 45900 | 12 | 45 |
| `Asia/Kolkata` | both | 19800 | 5 | 30 |
| `Asia/Kathmandu` | both | 20700 | 5 | 45 |
| `Europe/Brussels` | Jan / Jul | 3600 / 7200 | 1 / 2 | 0 / 0 |
| `America/Denver` | Jan / Jul | -25200 / -21600 | -7 / -6 | 0 / 0 |
| `Pacific/Marquesas` | both | -34200 | -9 | -30 |
| `UTC` | both | 0 | 0 | 0 |
So the negative minute is confirmed, not assumed.
**DuckDB 1.5.2 supports all three fields**, and it agrees with PostgreSQL on every zone above. Two useful differences:
- DuckDB returns `BIGINT`, not `double precision`. So an integer return type has precedent, and `Int32` here is not an outlier.
- DuckDB returns `0` for a timezone-naive timestamp; PostgreSQL raises `unit "timezone" not supported for type timestamp without time zone`. This PR follows PostgreSQL. That is the right call, but the description presents `0` as self-evidently wrong when a major engine ships it. Worth a mention rather than a silent omission.
DuckDB rejects `DATE` and `INTERVAL` input, as this PR does.
**Return type.** `return_field_from_args` gives `Float64` for `epoch`, `Int64` for `nanosecond` and `Int32` for everything else. So the claim in the description holds, and `Int32` is the consistent choice. The divergence from PostgreSQL's `double precision` is pre-existing and applies to `hour`, `minute` and the rest already. It does not need a call-out in this PR.
**`EXTRACT` needs no planner change.** Confirmed end to end. sqlparser 0.62 renders `DateTimeField::TimezoneHour` as `TIMEZONE_HOUR`, the planner forwards that string, and the `.slt` cases for all three fields pass.
**Case insensitivity after the #24906 rebase.** `date_part('TIMEZONE', ...)`, `date_part('TIMEZONE_HOUR', ...)` and `date_part('Timezone_Minute', ...)` all work, in the unit tests and in the `.slt`. See nit 3 for the fragility, not for a defect. Leading and trailing whitespace fails (`date_part(' timezone ', ...)`), exactly as it fails for every other part on `main`. Quoted spellings such as `'''timezone'''` work.
**Nullability.** `unary_opt` can turn an out-of-range value into a `NULL` even when the input field is not nullable. `date_part('hour', ...)` on the same value does the same thing, so Arrow's own kernel already behaves this way. Pre-existing and consistent, so no action.
**`preimage`.** `DatePart::from_str("timezone")` fails, so `preimage` returns `PreimageResult::None` and no filter rewrite fires. Constant folding still works: `EXPLAIN` shows `Projection: Int32(7200)`.
**Both commits build green on their own.**
- commit 1 (`refactor: move parse_tz`): `cargo check --workspace --all-targets` clean, `cargo clippy -p datafusion-functions --all-targets -- -D warnings` clean, 344 lib tests pass.
- commit 2 (`feat: ... date_part`): `cargo clippy --all-targets -- -D warnings` clean on the whole workspace, 353 lib tests pass, `datetime/date_part.slt` green.
**Generated docs are in sync.** I re-ran `print_functions_docs -- scalar` and diffed the `date_part` section against the committed file. The content matches; the only differences are the prettier reflow that `dev/update_function_docs.sh` applies. `./ci/scripts/doc_prettier_check.sh` and `typos` are both clean.
**The doc example is real.** `SELECT date_part('timezone', TIMESTAMP '2024-07-01T12:00:00' AT TIME ZONE 'Europe/Brussels')` returns `7200`, and the input type is `Timestamp(ns, "Europe/Brussels")`.
**Merge order.** https://github.com/apache/datafusion/pull/25175 pins `Date part 'timezone_hour' not supported` in its SECTION 9. Whichever PR merges second must update the other. That PR already records the collision in its own description.
🤖 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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: support `timezone`, `timezone_hour` and `timezone_minute` in `date_part` [datafusion]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #25163:
URL: https://github.com/apache/datafusion/pull/25163#issuecomment-5624419777
## [Codecov](https://app.codecov.io/gh/apache/datafusion/pull/25163?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `97.94239%` with `5 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 81.91%. Comparing base ([`1ec9ede`](https://app.codecov.io/gh/apache/datafusion/commit/1ec9ede5da5c67cbc2d3e0110a7ddbc2a47796a6?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`5b7548e`](https://app.codecov.io/gh/apache/datafusion/commit/5b7548edfa14cc76bc073b7867e0984a1ba1dd0b?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
| [Files with missing lines](https://app.codecov.io/gh/apache/datafusion/pull/25163?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [datafusion/functions/src/datetime/date\_part.rs](https://app.codecov.io/gh/apache/datafusion/pull/25163?src=pr&el=tree&filepath=datafusion%2Ffunctions%2Fsrc%2Fdatetime%2Fdate_part.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZGF0YWZ1c2lvbi9mdW5jdGlvbnMvc3JjL2RhdGV0aW1lL2RhdGVfcGFydC5ycw==) | 98.30% | [2 Missing and 2 partials :warning: ](https://app.codecov.io/gh/apache/datafusion/pull/25163?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [datafusion/functions/src/datetime/common.rs](https://app.codecov.io/gh/apache/datafusion/pull/25163?src=pr&el=tree&filepath=datafusion%2Ffunctions%2Fsrc%2Fdatetime%2Fcommon.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZGF0YWZ1c2lvbi9mdW5jdGlvbnMvc3JjL2RhdGV0aW1lL2NvbW1vbi5ycw==) | 85.71% | [0 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/datafusion/pull/25163?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #25163 +/- ##
========================================
Coverage 81.91% 81.91%
========================================
Files 1132 1132
Lines 420570 420805 +235
Branches 420570 420805 +235
========================================
+ Hits 344490 344713 +223
- Misses 55764 55768 +4
- Partials 20316 20324 +8
```
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/datafusion/pull/25163?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: support `timezone`, `timezone_hour` and `timezone_minute` in `date_part` [datafusion]
Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on code in PR #25163:
URL: https://github.com/apache/datafusion/pull/25163#discussion_r4072360799
##########
datafusion/functions/src/datetime/date_part.rs:
##########
@@ -317,6 +337,100 @@ fn is_nanosecond(part: &str) -> bool {
.unwrap_or(false)
}
+/// The `timezone`, `timezone_hour` and `timezone_minute` fields, which report
+/// the UTC offset that applies to a timezone-aware timestamp *at that instant*.
+///
+/// Because the offset of a named timezone changes with daylight saving time,
+/// these are per-row values, not a property of the type alone.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+enum TimezonePart {
+ /// `timezone`: the whole UTC offset, in seconds
+ Offset,
+ /// `timezone_hour`: whole hours of the UTC offset
+ Hour,
+ /// `timezone_minute`: whole minutes of the UTC offset, excluding the hours.
+ /// The sign follows the sign of the offset (PostgreSQL behaviour), so
+ /// `-03:30` yields `-3` hours and `-30` minutes.
+ Minute,
+}
+
+impl TimezonePart {
+ fn parse(part: &str) -> Option<Self> {
+ match part {
+ "timezone" => Some(Self::Offset),
+ "timezone_hour" => Some(Self::Hour),
+ "timezone_minute" => Some(Self::Minute),
+ _ => None,
+ }
+ }
+
+ fn name(&self) -> &'static str {
+ match self {
+ Self::Offset => "timezone",
+ Self::Hour => "timezone_hour",
+ Self::Minute => "timezone_minute",
+ }
+ }
+
+ /// Extract this field from a UTC offset expressed in seconds.
+ ///
+ /// Rust integer division truncates towards zero, which is what makes the
+ /// sign of `timezone_minute` follow the sign of the offset, matching
+ /// PostgreSQL.
+ fn project(&self, offset_seconds: i32) -> i32 {
+ match self {
+ Self::Offset => offset_seconds,
+ Self::Hour => offset_seconds / 3_600,
+ Self::Minute => (offset_seconds % 3_600) / 60,
+ }
+ }
+}
+
+/// Compute a [`TimezonePart`] for every element of a timezone-aware timestamp
+/// array.
+///
+/// Errors for any other input type, including timezone-naive timestamps, which
+/// carry no offset at all. This matches PostgreSQL, which rejects
+/// `date_part('timezone', <timestamp without time zone>)`.
+fn timezone_part(array: &dyn Array, part: TimezonePart) -> Result<ArrayRef> {
+ let Timestamp(unit, tz_opt) = array.data_type() else {
+ return exec_err!(
+ "Date part '{}' is only supported for timestamps with a timezone, got {}",
+ part.name(),
+ array.data_type()
+ );
+ };
+ let Some(tz) = parse_tz(tz_opt.as_ref())? else {
Review Comment:
Thanks, added in 08d6ae0
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: support `timezone`, `timezone_hour` and `timezone_minute` in `date_part` [datafusion]
Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb merged PR #25163:
URL: https://github.com/apache/datafusion/pull/25163
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org