adriangb opened a new pull request, #25162: URL: https://github.com/apache/datafusion/pull/25162
## Which issue does this PR close? - Closes https://github.com/apache/datafusion/issues/10344. This is the last unchecked Documentation item in the EPIC https://github.com/apache/datafusion/issues/8282. ## Rationale for this change DataFusion has a long tail of open timezone bugs: https://github.com/apache/datafusion/issues/13212, https://github.com/apache/datafusion/issues/12218, https://github.com/apache/datafusion/issues/12892, https://github.com/apache/datafusion/issues/23841, https://github.com/apache/datafusion/issues/25095, https://github.com/apache/datafusion/issues/18228, https://github.com/apache/datafusion/issues/13962. They look like seven unrelated bugs. They are the *same* confusion showing up at seven different layers: when a timestamp gains or loses a time zone, is that a **relabel** (keep the integer, change the annotation) or a **shift** (keep the instant, change the wall clock)? Arrow's cast kernel says shift. Parts of DataFusion — type coercion, `unwrap_cast_in_comparison`, `to_timestamp_*`, `::timestamptz` when `datafusion.execution.time_zone` is unset — assume relabel. That is already user-visible as a straight contradiction. On today's `main`, with no session timezone set at all: ```sql CREATE OR REPLACE TABLE c AS SELECT TIMESTAMP '2024-01-01 12:00:00' AS ts; SELECT TIMESTAMP '2024-01-01 12:00:00' = arrow_cast(1704135600, 'Timestamp(Second, Some("America/Denver"))') AS literal_cmp; -- true (the naive side is shifted into Denver) SELECT ts = arrow_cast(1704135600, 'Timestamp(Second, Some("America/Denver"))') AS column_cmp FROM c; -- false (the optimizer unwraps the cast and relabels) ``` Three PRs are in flight right now, each settling one piece of this independently: - https://github.com/apache/datafusion/pull/25094 (@kumarUjjawal) — session timezone for timestamp subtraction / coercion - https://github.com/apache/datafusion/pull/25099 (@Ruchirtripathi) — `unwrap_cast_in_comparison` dropping the timezone shift - https://github.com/apache/datafusion/pull/23925 (@nevi-me) — preserving the timezone in `to_timestamp_*` With nothing written down, those three can each settle it *differently*, and we end up with a fourth reading. This page is the shared reference that lets them agree: it states the rule once, in one place, derived from what the code actually does today. @kumarUjjawal @Ruchirtripathi @nevi-me — could you each check your change against the "The one rule" section of the new page and say whether it moves DataFusion toward or away from it? If any of you thinks the rule as written is the wrong rule, that is exactly the discussion this PR is trying to start, and it is much cheaper to have it here than after three PRs land. ## What changes are included in this PR? A new user-guide page, `docs/source/user-guide/sql/timestamps.md` ("Timestamps and Time Zones"), added to the SQL Reference toctree immediately after `data_types.md`. It is a new page rather than a section of `data_types.md` because the content is cross-cutting: it spans the type mapping, cast semantics, a session config setting, the date/time function catalogue, DST, and query recipes. `data_types.md` is a terse type-mapping reference and would be swamped by it; `operators.md` is the wrong home for a data-model discussion. Sections: 1. **The data model** — `Timestamp(unit, Some(tz))` is an instant plus a display annotation; `Timestamp(unit, None)` is a wall clock with no instant. How that maps to SQL `TIMESTAMP` / `TIMESTAMP WITH TIME ZONE`, and where it does not. 2. **The one rule** — naive → zoned *shifts*; zoned → zoned *relabels*; zoned → naive yields the **UTC** wall clock. Each with a runnable example. Then `AT TIME ZONE` expressed in the same terms, and a subsection showing where DataFusion does not apply the rule consistently today. 3. **The session time zone** — what `datafusion.execution.time_zone` does and does not affect, with the default-unset footgun called out explicitly (including the exact shape of https://github.com/apache/datafusion/issues/13962), and the type-unification rules for mixed-zone values. 4. **Local time versus the instant** — a table of which date/time functions work on the value's own local wall clock and which work on the UTC instant. The `date_trunc` / `date_bin` disagreement is spelled out; both individually match PostgreSQL, but the pair surprises everyone. 5. **Daylight saving time** — `INTERVAL '1 day'` vs `INTERVAL '24 hours'` (82800 / 86400 / 90000 seconds around the Denver transitions), and what currently happens on ambiguous and nonexistent local times. 6. **Recipes** — aggregating UTC data by local calendar day in a named zone (this produces exactly the output requested in https://github.com/apache/datafusion/issues/10602), getting a zone's local wall clock, and round-tripping safely. 7. **Differences from PostgreSQL** — a table, each row linked to its issue by full URL. Two smaller changes come with it: - `docs/source/user-guide/sql/data_types.md` gains the missing `TIMESTAMP WITH TIME ZONE` row in the Date/Time table (it was simply absent), a note on precision, and a link to the new page. - `docs/source/user-guide/sql/index.rst` gains the toctree entry. ## What is the testing strategy for this PR? Documentation only, so there is no code to test — but every claim on the page was derived from behaviour rather than from reading the source, and every SQL example is one that was actually run with its real output pasted in. Nothing on the page is invented. - All examples were run against `datafusion-cli` built from this branch (DataFusion 55.0.0, arrow-rs 59.2.0), including the error messages. - All PostgreSQL claims were verified against a real PostgreSQL 17.11 (`docker run --rm -e POSTGRES_PASSWORD=pw postgres:17`), not against the documentation: `date_trunc` / `date_bin` / `date_part` behaviour, `INTERVAL '1 day'` vs `'24 hours'` across both DST transitions, `AT TIME ZONE` on naive and aware inputs, `tstz::timestamp`, naive-vs-aware comparison, and DST-boundary resolution. - The `AT TIME ZONE`-on-an-aware-value divergence was additionally confirmed against DuckDB. - `./ci/scripts/doc_prettier_check.sh` passes. - The docs build (`make html`) was run and the new page renders; every cross-reference anchor it emits was checked to exist in the generated HTML. The build's only warning is the pre-existing missing `deps.svg`, which `build.sh` generates via `cargo depgraph` before `make html`. A note on scope: the page documents **today's** behaviour, including behaviour that is arguably wrong. Where an open issue proposes changing it, the page says so and links the issue rather than documenting a future state. In particular the `AT TIME ZONE`-on-an-aware-value rows carry an explicit note that https://github.com/apache/datafusion/issues/12218 may change them. ## Are there any user-facing changes? Yes — new user documentation. No API or behaviour 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]
