adriangb opened a new pull request, #25164: URL: https://github.com/apache/datafusion/pull/25164
## Which issue does this PR close? This PR closes no issue. It adds regression-prevention coverage for the area those issues live in. Related to: - https://github.com/apache/datafusion/issues/13212 - https://github.com/apache/datafusion/issues/12892 - https://github.com/apache/datafusion/issues/12218 - https://github.com/apache/datafusion/issues/10368 - https://github.com/apache/datafusion/issues/10602 - https://github.com/apache/datafusion/issues/25084 - https://github.com/apache/datafusion/issues/25095 ## Rationale for this change DataFusion has a long tail of timezone correctness bugs. They keep recurring, and the reason is structural: almost nothing in the test suite pins the behaviour down, so a change to timezone semantics can land without producing a single test diff. Concretely, on today's `main`: - `SET datafusion.execution.time_zone` appears roughly 40 times in the entire sqllogictest corpus, and nearly all of those are concentrated in two files (`to_timestamp_timezone.slt` and `set_variable.slt`). - DST-boundary dates are essentially absent. Nothing exercises an ambiguous local time (a wall clock that occurs twice) or a non-existent one (a wall clock inside the spring-forward gap). - There is a working differential harness against real PostgreSQL — `test_files/pg_compat/pg_compat_*.slt`, run in CI by the `sqllogictest-postgres` job — and **not one of its files covered timestamps with time zones**. The result is that the semantics people actually hit in production (what `::timestamptz` produces, whether a naive literal in a `WHERE` is read in the session zone or the column's zone, whether `date_bin` and `date_trunc` agree, what happens at 01:30 on the first Sunday in November) are undefined by the test suite. A refactor can change any of them silently, and a fix for one can regress another. This PR does not change any behaviour. It writes the current behaviour down, so that from now on **any change to timezone semantics shows up as a test diff** and has to be argued for rather than discovered by a user. Where today's behaviour is wrong, or disagrees with PostgreSQL, the expected output still records what DataFusion does — with a comment directly above saying so and linking the issue. When a fix lands, the diff is the specification of what changed. The second file goes further: it puts the subset of timezone behaviour that DataFusion and PostgreSQL genuinely share under continuous differential test against a real PostgreSQL 17 instance in CI, so that agreement is not something we assert in a comment but something the build checks. ## What changes are included in this PR? Two new sqllogictest files. No production code is touched. **1. `datafusion/sqllogictest/test_files/datetime/timestamps_timezone.slt`** (~1500 lines, 18 labelled sections) A characterization suite. Every case is exercised both as a literal and, where it matters, as a real CTAS column — several of these behaviours only reproduce on columns. Sections: - **0–4** — `arrow_typeof` and value of `'...'::timestamp`, `'...'::timestamptz`, `TIMESTAMP '...'` and `TIMESTAMP WITH TIME ZONE '...'` under session zone unset / `+00:00` / `+05:30` / `America/Denver` / `Europe/Brussels`, for literals and for columns. Records that with the session zone **unset**, `::timestamptz` yields a tz-**naive** `Timestamp(ns)`. - **5** — `AT TIME ZONE` on tz-naive and tz-aware input, type and value. - **6** — casts in all four directions: naive→named, named→naive, named→other named, naive→fixed offset (plus named→fixed offset). - **7** — round trips, including `t = t::timestamptz::timestamp` under UTC and non-UTC session zones. - **8** — comparison and equality between a tz-aware column and a tz-naive literal and vice versa, with `EXPLAIN` of the filter so that any change to the `unwrap_cast` / `simplify_expressions` rewrite is visible in the plan. - **9–11** — `date_bin`, `date_trunc`, `date_part`, `extract`, `to_char`, `from_unixtime`, `to_unixtime`, `to_timestamp*`, `to_local_time`, `now`, `current_date`, `current_time`, `make_date` on tz-aware input. - **12** — timestamp ± interval across both DST transitions, and the `INTERVAL '1 day'` vs `INTERVAL '24 hours'` distinction in both directions. - **13–15** — `MIN`/`MAX`/`GROUP BY`/`ORDER BY`/`DISTINCT` over a tz-aware column; joins on tz-aware keys including a mixed `+00:00` / `America/Denver` join; `UNION`/`CASE`/`COALESCE`/`greatest` coercion across mixed zones. - **16** — the DST boundaries specifically: US fall-back `2024-11-03 01:30:00` and spring-forward `2024-03-10 02:30:00` in `America/Denver`, EU `2024-10-27 02:30:00` and `2024-03-31 02:30:00` in `Europe/Brussels`, each via column cast, `AT TIME ZONE`, string literal and `::timestamptz`. - **17** — a zone with no DST (`America/Phoenix`) and a half-hour zone (`Asia/Kolkata`). **2. `datafusion/sqllogictest/test_files/pg_compat/pg_compat_timestamptz.slt`** (~700 lines) The first `pg_compat` file covering timestamps with time zones. Three blocks, each setting both engines' session zone explicitly (`SET TimeZone` for PostgreSQL, `SET datafusion.execution.time_zone` for DataFusion) and building its table in that same zone: UTC, `America/Denver`, then `Asia/Kolkata` and `America/Phoenix`. Two constraints shaped it, and are documented in the file header: - The Postgres runner has no renderer for the `timestamptz` wire type (`cell_to_string` would hit `unimplemented!`), so no query may *return* a tz-aware value. Every result is projected down to a tz-naive `timestamp`, a `bigint`, a `boolean` or `text`. This turned out to be a feature rather than a limitation: comparing via `date_part('epoch', ...)` tests the instant directly, with no rendering in the way. - PostgreSQL resolves a bare `timestamp` and renders a `timestamptz` using its **session** `TimeZone`, whereas DataFusion uses the time zone carried by the **value**. The two only coincide when the session zone equals the column's zone, so each block aligns them deliberately. Everything that falls outside that alignment is a divergence and lives in file 1 instead, not here. ## What is the testing strategy for this PR? This PR *is* tests. Both files were generated with the sqllogictest `--complete` mode rather than hand-written, and then every generated result was read and sanity-checked; several were wrong and were rewritten (e.g. queries that tripped DataFusion's projection-name-uniqueness rule, and a `- INTERVAL '1 day'` case that did not actually cross a DST boundary). `pg_compat_timestamptz.slt` was generated **from real PostgreSQL 17** (`PG_COMPAT=true PG_URI=... --complete`, the same path `cargo xtask ci step test postgres` uses) and then validated against DataFusion, so PostgreSQL's answer is the expected output and DataFusion has to match it. Divergences found during that process were removed from the pg_compat file and recorded in the characterization file with both engines' answers. Verified locally: - `cargo test -p datafusion-sqllogictest --test sqllogictests` — full suite green (507 files) - `PG_COMPAT=true PG_URI=... cargo test --features postgres --test sqllogictests -- pg_compat` — all 7 pg_compat files green against PostgreSQL 17 - `cargo fmt --all` - `cargo clippy --all-targets -- -D warnings` ### Divergences from PostgreSQL found and recorded Every one of these is recorded in `timestamps_timezone.slt` with both answers and a comment; none is fixed here. 1. **`::timestamptz` with the session zone unset is tz-naive.** PostgreSQL always yields `timestamp with time zone`. 2. **`timestamptz AT TIME ZONE zone` returns a tz-aware value in DataFusion**, relabelled into `zone`; PostgreSQL drops the zone and returns a naive `timestamp` holding the wall clock in `zone`. Composed with a cast — the standard PostgreSQL idiom — `('2024-07-01T18:00:00Z'::timestamptz AT TIME ZONE 'America/Denver')::timestamp` gives `2024-07-01 12:00:00` in PostgreSQL and `2024-07-02T00:00:00` in DataFusion. 3. **`timestamptz::timestamp` always renders in UTC** and ignores `datafusion.execution.time_zone`; PostgreSQL renders in the session `TimeZone`. Under `TimeZone='America/Denver'`, `'2024-07-01T18:00:00Z'::timestamptz::timestamp` is `2024-07-01 12:00:00` in PostgreSQL and `2024-07-01T18:00:00` in DataFusion. 4. **The naive↔aware round trip is not the identity** under a non-UTC session zone, as a consequence of 3. 5. **A tz-naive literal compared against a tz-aware column is coerced into the column's zone**, not the session zone. `SELECT ts FROM t WHERE ts > '2024-07-01 06:00:00'` over a Denver-typed column returns 1 row in DataFusion and 2 in PostgreSQL under `TimeZone='UTC'`. The session zone has no effect on this path at all. 6. **A tz-aware literal loses its offset when the session zone is unset.** `WHERE ts = '2024-07-01T06:00:00Z'::timestamptz` against a Denver column returns the **12:00Z** row, not the 06:00Z one — the `Z` is silently discarded and the wall clock re-read in the column's zone. Setting a session zone fixes it. Same root cause in `CASE`: a branch written as `'2024-07-01T00:00:00Z'::timestamptz` comes back as `2024-07-01T06:00:00Z` once coerced to a Denver branch. 7. **`date_trunc` truncates in the value's zone**; PostgreSQL truncates in the session `TimeZone` (or an explicit third argument, which DataFusion does not accept). `date_bin`, by contrast, agrees with PostgreSQL. 8. **`date_bin` and `date_trunc` disagree with each other inside DataFusion** for the same input, whenever the zone offset is not a whole multiple of the stride — `date_bin` bins on the UTC instant, `date_trunc` truncates locally. Most visible on `Asia/Kolkata`, where `date_trunc('day', ts)` lands on local midnight and `date_bin(INTERVAL '1 day', ts)` lands on 05:30 local. 9. **`date_part('timezone_hour')` and `date_part('timezone_minute')` are rejected** — `Execution error: Date part 'timezone_hour' not supported`. PostgreSQL supports both (0/0 for UTC, -6/0 for Denver in summer, 5/30 for Kolkata). There is currently no way to ask a tz-aware value for its own offset. 10. **Day-valued interval arithmetic is DST-aware in the value's zone**, whereas PostgreSQL scopes it to the session `TimeZone`. When the two are aligned the engines agree exactly, including `'1 day'` vs `'24 hours'` across both transitions — that agreement is now pinned in the pg_compat file. 11. **Ambiguous and non-existent local times fail outright** (see below). 12. Minor type-level differences, recorded for completeness: `now()` is tz-naive with the session zone unset, `current_time` is `Time64(ns)` where PostgreSQL has `time with time zone`, and `from_unixtime`/`to_timestamp*` return naive values unless a session zone is set. ### The DST-boundary behaviour, precisely DataFusion cannot represent an ambiguous or non-existent local time in a named zone **by any route**. Column cast, `AT TIME ZONE`, string literal and `::timestamptz` under a session zone all error. The column path and the literal path fail with *different* errors for the same value: - column path — `Arrow error: Cast error: Cannot cast timezone to different timezone` - literal path — folded by `simplify_expressions`, then `Arrow error: Parser error: ... error computing timezone offset` PostgreSQL resolves all four cases: `2024-11-03 01:30:00` Denver → `01:30:00-07` (the second, standard-time occurrence); `2024-03-10 02:30:00` Denver → `03:30:00-06` (shifted forward out of the gap); `2024-10-27 02:30:00` Brussels → `02:30:00+01`; `2024-03-31 02:30:00` Brussels → `03:30:00+02`. This is https://github.com/apache/datafusion/issues/25084, and https://github.com/apache/arrow-rs/pull/11038 is the arrow-rs fix. Note that the failure is **not** limited to columns — literals fail too, just with a different message. Both paths, and both error texts, are pinned in the file so the fix will show up as an exact diff. Once the values are built from explicit UTC instants the boundary is navigable, and that behaviour is characterized too: both occurrences of 01:30 render with the correct differing offsets, `to_local_time` collapses them onto the same naive value, and `+ INTERVAL '1 hour'` walks through the repeated hour rather than over it. ### Divergences that look like they may not be covered by an existing issue - **#9, `timezone_hour` / `timezone_minute` unsupported.** A plain gap rather than a wrong answer, but it removes the only portable way to read a value's own UTC offset. - **#6, the `Z` in a tz-aware literal being discarded** when the session zone is unset. This is downstream of "unset session zone makes `::timestamptz` naive", but the user-visible symptom — a `WHERE` clause with an explicit `Z` silently matching the wrong row, and a `CASE` branch shifting by the zone offset — is sharper than what https://github.com/apache/datafusion/issues/25095 currently describes, and worth calling out separately. - **#8, `date_bin` vs `date_trunc` disagreeing with each other.** This is an internal inconsistency rather than a PostgreSQL divergence (each is individually defensible), but the two functions returning different answers for the same input and the same declared return type is the kind of thing that should be a deliberate decision rather than an accident. I have not filed issues for these; happy to if reviewers agree they are worth tracking. ## Are there any user-facing changes? No. This PR adds test files only. No production code, no public API, and no behaviour is changed. 🤖 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]
