adriangb opened a new pull request, #25171: URL: https://github.com/apache/datafusion/pull/25171
## Which issue does this PR close? <!-- No issue is closed by this PR. --> - Related to https://github.com/apache/datafusion/issues/25166 ## Rationale for this change Two places describe the `TIMESTAMP WITH TIME ZONE` type mapping incorrectly or not at all. 1. The comment in `datafusion/sql/src/planner.rs` says the mapping produces `Timestamp<TimeUnit, Some(Time Zone)>`. That stopped being true in https://github.com/apache/datafusion/pull/18359, which changed `datafusion.execution.time_zone` from `String` to `Option<String>` defaulting to `None` and mechanically dropped the `Some(...)` wrapper without updating the two comment lines above it. With the default configuration the arm produces `Timestamp(unit, None)`, so the comment claims the opposite of what the code does. 2. `docs/source/user-guide/sql/data_types.md` documents the SQL-to-Arrow type mapping but has no row for `TIMESTAMP WITH TIME ZONE` / `TIMESTAMPTZ` at all, so a user has no documented way to find out what those types map to. The existing `TIMESTAMP` row also does not mention that `TIMESTAMP(p)` is accepted or which precisions are valid. Whether the timezone-naive default is the right behavior is being discussed in https://github.com/apache/datafusion/issues/25166. This PR only makes the documentation match the current code; it deliberately does not change any behavior. ## What changes are included in this PR? - `datafusion/sql/src/planner.rs`: correct the `SQLDataType::Timestamp` comment to say `Timestamp<TimeUnit, Time Zone>`, note that the configured time zone is an `Option` that is unset by default, and point at https://github.com/apache/datafusion/issues/25166. No code change. - `docs/source/user-guide/sql/data_types.md`: add the missing `TIMESTAMP WITH TIME ZONE` / `TIMESTAMPTZ` row to the Date/Time Types table, document the optional `(p)` precision and the units it selects, and note that the timezone component comes from `datafusion.execution.time_zone`, which is unset by default, with a link to the open issue. ## What is the testing strategy for this PR? No tests are added: this is a comment fix plus a documentation fix, with no behavior change. Every mapping written in the docs was verified against a `cargo build --bin datafusion-cli` build of this branch rather than read off the source: ``` > select arrow_typeof('2000-01-01T00:00:00'::TIMESTAMP); -- Timestamp(ns) > select arrow_typeof('2000-01-01T00:00:00'::TIMESTAMP(0)); -- Timestamp(s) > select arrow_typeof('2000-01-01T00:00:00'::TIMESTAMP(3)); -- Timestamp(ms) > select arrow_typeof('2000-01-01T00:00:00'::TIMESTAMP(6)); -- Timestamp(µs) > select arrow_typeof('2000-01-01T00:00:00'::TIMESTAMP(9)); -- Timestamp(ns) > select arrow_typeof('2000-01-01T00:00:00'::TIMESTAMP(1)); -- Error: Unsupported SQL type TIMESTAMP(1) -- default configuration (datafusion.execution.time_zone unset) > select arrow_typeof('2000-01-01T00:00:00'::TIMESTAMPTZ); -- Timestamp(ns) > select arrow_typeof(CAST('2000-01-01T00:00:00' AS TIMESTAMP WITH TIME ZONE)); -- Timestamp(ns) > select arrow_typeof(CAST('2000-01-01T00:00:00' AS TIMESTAMP(6) WITH TIME ZONE)); -- Timestamp(µs) > select arrow_typeof('2000-01-01T00:00:00'::TIMESTAMPTZ(3)); -- Timestamp(ms) > SET datafusion.execution.time_zone = 'America/New_York'; > select arrow_typeof('2000-01-01T00:00:00'::TIMESTAMPTZ); -- Timestamp(ns, "America/New_York") > select arrow_typeof(CAST('2000-01-01T00:00:00' AS TIMESTAMP(3) WITH TIME ZONE)); -- Timestamp(ms, "America/New_York") > select arrow_typeof('2000-01-01T00:00:00'::TIMESTAMP); -- Timestamp(ns) ``` The same mappings were confirmed through the DDL path (`CREATE TABLE t(a TIMESTAMP, b TIMESTAMPTZ, c TIMESTAMP(6), d TIMESTAMP(0) WITH TIME ZONE)`). `./ci/scripts/doc_prettier_check.sh`, `cargo fmt --all` and `cargo clippy --all-targets -- -D warnings` all pass, and the Sphinx docs build cleanly (the only warning is the pre-existing missing generated `_static/data/deps.svg`, unrelated to this change). ## Are there any user-facing changes? Documentation only. There are no behavior changes and no 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]
