fix: AT TIME ZONE on a timezone-aware timestamp returns a naive timestamp - #25165
fix: AT TIME ZONE on a timezone-aware timestamp returns a naive timestamp#25165adriangb wants to merge 4 commits into
AT TIME ZONE on a timezone-aware timestamp returns a naive timestamp#25165Conversation
Records DataFusion's current behaviour for `<tz-aware> AT TIME ZONE zone`, including the reproducer from apache#12218, DST transitions on a real multi-row column, chaining, fixed offsets and precision handling. PostgreSQL returns a timezone-*naive* `timestamp` here, holding the wall clock in `zone`; DataFusion returns a timezone-aware value that merely relabels the display zone. These expectations are flipped in the next commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`expr AT TIME ZONE 'tz'` was unconditionally lowered to `CAST(expr AS Timestamp(Nanosecond, Some(tz)))`. That is right for a timezone-naive input -- arrow's `Timestamp(_, None) -> Timestamp(_, Some(tz))` cast reads the naive value as local time in `tz` -- but wrong for a timezone-aware one, where it merely relabels the display zone. PostgreSQL (and DuckDB) make the operator asymmetric: `timestamptz AT TIME ZONE zone` returns a `timestamp` (naive) holding the wall clock in `zone`. Because DataFusion kept the value timezone-aware, casting the result to `::timestamp` produced the UTC wall clock instead of the zone's, which is what apache#12218 reports. The SQL planner now types the input and branches on it. The naive case is unchanged. The aware case relabels the instant into `tz` with the same cast and then drops the timezone while keeping the displayed value, which is exactly `to_local_time`. `datafusion-sql` must not depend on `datafusion-functions`, so that second half goes through a new `ExprPlanner::plan_at_time_zone` hook -- the same shape as `plan_extract` lowering `EXTRACT` to `date_part` -- implemented by `DatetimeFunctionPlanner`. Sessions without it get a clear planning error instead of the old silent mislowering. `AT TIME ZONE` also no longer forces `Nanosecond`: it keeps the input's `TimeUnit` when the input is a timestamp. Closes apache#12218 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Adds an `AT TIME ZONE` entry to the SQL operators guide covering both directions (naive -> aware, aware -> naive), precision preservation, chaining and the ISO-vs-POSIX sign convention for fixed-offset strings. `to_local_time`'s own examples all apply `AT TIME ZONE` to timezone-*naive* values, so they are unaffected by the semantic change; its description now says so, and notes that wrapping an already timezone-aware `AT TIME ZONE` in `to_local_time` is redundant. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #25165 +/- ##
==========================================
+ Coverage 81.60% 81.91% +0.31%
==========================================
Files 1123 1132 +9
Lines 408898 421160 +12262
Branches 408898 421160 +12262
==========================================
+ Hits 333670 344992 +11322
- Misses 55625 55772 +147
- Partials 19603 20396 +793 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Heads up on merge order: #25175 adds a timezone characterization suite ( Whichever of the two merges second will need to update the other, and CI on it will be red until that happens. Concretely, after this PR lands the two queries in SECTION 5b become: and the Nothing else in that file conflicts — its other |
|
Self-review: a QA pass on my own PR. I ran every check below against three sources:
Terms: an aware timestamp carries a timezone ( 1. The type branch runs before type coercion, so the fix misses some inputs
SET datafusion.execution.time_zone = 'UTC';
CREATE TABLE t AS SELECT '2024-01-01T12:00:00Z'::timestamptz AS aware,
'2024-01-01 12:00:00'::timestamp AS naive, true AS b;
SELECT arrow_typeof(CASE WHEN b THEN naive ELSE aware END) FROM t;
-- Timestamp(ns, "UTC") <- the real type is aware
SELECT arrow_typeof((CASE WHEN b THEN naive ELSE aware END) AT TIME ZONE 'America/Denver'),
(CASE WHEN b THEN naive ELSE aware END) AT TIME ZONE 'America/Denver' FROM t;
-- Timestamp(ns, "America/Denver") 2024-01-01T05:00:00-07:00 <- the OLD, wrong shape
SELECT arrow_typeof((CASE WHEN b THEN aware ELSE naive END) AT TIME ZONE 'America/Denver') FROM t;
-- Timestamp(ns) <- correct, only because the arms swap orderPostgreSQL 17.11 on the same shape: So the result depends on the order of the Options, in my order of preference:
I do not want to merge this without at least option 2. A silent, order-dependent hole is worse than the old uniform bug. 2. The description says PostgreSQL and DuckDB agree on every case. They do notThe claim covers "fixed offsets". DuckDB rejects a fixed-offset string outright: PostgreSQL accepts it and reads it POSIX-style (west-positive), so it returns The two engines cannot arbitrate this one. The description must say so. It must not claim agreement. The divergence already exists on main and is out of scope here (#25170), but it must not hide behind a blanket claim. 3. The
|
Review of this PR found that the fix does not reach every expression. The type
branch calls `Expr::get_type` in the SQL planner, which runs before the type
coercion analyzer. For a `CASE`, `get_type` reports the first non-null `THEN`
arm and ignores coercion.
So these two have the same coerced input type, `Timestamp(ns, "UTC")`, and give
different answers:
CASE WHEN b THEN naive ELSE aware END AT TIME ZONE 'America/Denver'
-> Timestamp(ns, "America/Denver") 2024-01-01T05:00:00-07:00
CASE WHEN b THEN aware ELSE naive END AT TIME ZONE 'America/Denver'
-> Timestamp(ns) 2024-01-01T05:00:00
PostgreSQL gives the naive `2024-01-01 05:00:00` for both. `coalesce` is not
affected, because `verify_function_arguments` coerces before the planner sees
the type.
A correct fix has to dispatch after coercion, which is a larger change than
this PR. Pinning the behaviour so it is visible rather than silent.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Which issue does this PR close?
Warning
Known limitation, found in review of this PR: the fix does not reach every expression.
The type branch calls
Expr::get_typein the SQL planner, and the planner runs before the typecoercion analyzer. For a
CASE,get_typereports the first non-nullTHENarm and ignorescoercion. So the two queries below have the same coerced input type,
Timestamp(ns, "UTC"),and give different answers:
PostgreSQL 17 gives the naive
2024-01-01 05:00:00for both.coalesceis not affected, becauseverify_function_argumentscoerces before the planner reads the type.A correct fix must dispatch after coercion, which is a larger change than this PR. The behaviour is
pinned in
datetime/timestamps.sltso it is visible rather than silent. Maintainers: please saywhether you want that fixed here before merge, or tracked separately.
Rationale for this change
Start here: what a user gets today
Two terms run through this description:
Timestamp(unit, Some(tz)). SQL spells ittimestamptz.Timestamp(unit, None). SQL spells ittimestamp.This is the case from the issue. Noon UTC is 05:00 in Denver.
2024-01-01T12:00:002024-01-01T05:00:002024-01-01 05:00:002024-01-01 05:00:00DataFusion returns the UTC wall clock, so it drops the timezone the user asks for. Drop the final cast and the cause appears:
2024-01-01T05:00:00-07:00Timestamp(ns, "America/Denver")2024-01-01T05:00:00Timestamp(ns)2024-01-01 05:00:00timestamp without time zone2024-01-01 05:00:00TIMESTAMPThe displayed value looks right today, but the type is wrong. The result stays aware, so the next operation on it reads the UTC wall clock again.
Overview
This is a breaking semantic change. It changes the result type of
AT TIME ZONE, and so the result value of anything downstream, when the input is an aware timestamp.AT TIME ZONEis asymmetric in PostgreSQL. It always returns the other kind of timestamp:DataFusion implements the first half only. This PR adds the second half. The first half does not change.
Why the current lowering is wrong
DataFusion lowers both halves to
CAST(expr AS Timestamp(Nanosecond, Some(tz))).tz.What changes are included in this PR?
SqlToRelnow types the input ofAT TIME ZONEand branches on it. The new function issql_at_time_zone_to_exprindatafusion/sql/src/expr/mod.rs.Timestamp(unit, None), or any type that is not a timestamp: no change. The planner emitsCAST(expr AS Timestamp(unit, Some(tz))).Timestamp(unit, Some(_)): the planner emits the same cast, then wraps it in a step that drops the timezone and keeps the wall clock. That step isto_local_time.The planner hook
datafusion-sqlmust not depend ondatafusion-functions, so the second step goes through a new trait method:ExprPlanner::plan_at_time_zoneindatafusion/expr/src/planner.rs,DatetimeFunctionPlannerindatafusion/functions/src/datetime/planner.rs,PlannerResult::Originalbody, so out-of-tree implementations still compile.plan_extractlowersEXTRACTtodate_partthrough the same seam, so this follows an established pattern. The alternative was a name lookup throughContextProvider::get_function_meta("to_local_time"). The hook keeps the function name out of the SQL planner.The type branch stays in
datafusion-sql, becauseExpr::get_typeneeds the schema and anExprPlannerdoes not have one. The hook receives the expression after the cast, and the trait doc states that contract.A session that registers no such planner now gets a plan error, where before it got a wrong plan.
The
TimeUnitchangeAT TIME ZONEno longer forcesNanosecond. It keeps the input'sTimeUnitwhen the input is a timestamp.Timestamp(µs, "UTC")Timestamp(ns, "America/Denver")Timestamp(µs)Timestamp(µs, None)Timestamp(ns, "America/Denver")Timestamp(µs, "America/Denver")Timestamp(s, "UTC")Timestamp(ns, "America/Denver")Timestamp(s)Utf8,Date32, and other non-timestampsTimestamp(ns, "America/Denver")Timestamp(ns, "America/Denver")PostgreSQL keeps the precision of its input too.
timestamptz '2024-01-01 12:00:00.123456Z' AT TIME ZONE 'America/Denver'returns2024-01-01 05:00:00.123456.Not changed
SparkFunctionPlannerimplementsplan_extractandplan_substringonly, andwith_spark_featuresinserts it at index 0 of the list thatwith_default_featuresbuilds. Soplan_at_time_zonefalls through toDatetimeFunctionPlanner, and no Spark-specific function or planner behaviour changes. A Spark-flavoured session that uses DataFusion's SQL planner does get the newAT TIME ZONEsemantics, but Spark SQL has noAT TIME ZONEsyntax. It usesfrom_utc_timestampandto_utc_timestamp. Nothing underdatafusion/spark/implements, tests or documents the operator.AT LOCAL. PostgreSQL 17 supports it. sqlparser 0.62.0 has noAtLocalAST node and rejects the syntax, so there is nothing to route.'2000-12-01T04:04:12-05:00' AT TIME ZONE 'x'still takes the naive path, becauseUtf8is not a timestamp type. That keeps today's behaviour: DataFusion honours the offset inside the string. PostgreSQL resolves the unknown-typed literal totimestampand drops the offset. This gap is older than this PR and is separate from Casting existing timestamp to timestamp again strips timezone information #12218.What is the testing strategy for this PR?
Field research
I measured two reference engines rather than trust the documentation:
docker run --rm -e POSTGRES_PASSWORD=pw -p 55452:5432 postgres:17, withSET TimeZone='UTC',SET TimeZone='UTC'.The DataFusion columns come from a
datafusion-clibuild ofmainand of this branch, withSET datafusion.execution.time_zone = 'UTC'.The four core cases
'2024-01-01 12:00:00'::timestamp AT TIME ZONE 'America/Denver'2024-01-01T12:00:00-07:00Timestamp(ns, "America/Denver")2024-01-01 19:00:00+00timestamp with time zone2024-01-01 19:00:00+00TIMESTAMP WITH TIME ZONE'2024-01-01T12:00:00Z'::timestamptz AT TIME ZONE 'America/Denver'2024-01-01T05:00:00-07:00Timestamp(ns, "America/Denver")2024-01-01T05:00:00Timestamp(ns)2024-01-01 05:00:00timestamp without time zone2024-01-01 05:00:00TIMESTAMP(<aware> AT TIME ZONE 'America/Denver')::timestamp2024-01-01T12:00:002024-01-01T05:00:002024-01-01 05:00:002024-01-01 05:00:00<aware> AT TIME ZONE 'America/Denver' AT TIME ZONE 'Europe/Brussels'2024-01-01T13:00:00+01:00= 12:00 UTC
2024-01-01T05:00:00+01:00= 04:00 UTC
2024-01-01 04:00:00+002024-01-01 04:00:00+00The naive row agrees across all four columns.
12:00in Denver is19:00UTC. Only the display convention differs.The chained row is the one that changes value, not just type. Today DataFusion relabels twice and the instant never moves. With this PR the Denver wall clock (05:00) becomes a Brussels wall clock, which is 04:00 UTC. PostgreSQL and DuckDB both return 04:00 UTC.
Both 2024 DST transitions in
America/Denver, aware input2024-01-01T12:00:00Z2024-01-01T05:00:00-07:002024-01-01T05:00:002024-01-01 05:00:002024-01-01 05:00:002024-03-10T08:59:00Z2024-03-10T01:59:00-07:002024-03-10T01:59:002024-03-10 01:59:002024-03-10 01:59:002024-03-10T09:00:00Z2024-03-10T03:00:00-06:002024-03-10T03:00:002024-03-10 03:00:002024-03-10 03:00:002024-07-01T12:00:00Z2024-07-01T06:00:00-06:002024-07-01T06:00:002024-07-01 06:00:002024-07-01 06:00:002024-11-03T07:59:00Z2024-11-03T01:59:00-06:002024-11-03T01:59:002024-11-03 01:59:002024-11-03 01:59:002024-11-03T08:00:00Z2024-11-03T01:00:00-07:002024-11-03T01:00:002024-11-03 01:00:002024-11-03 01:00:00The rows cover the spring-forward gap (
01:59jumps to03:00) and the fall-back overlap (01:59MDT, then01:00MST). All four columns agree on the wall clock. Only the DataFusion type changes.The DST gap and overlap from the other side, naive input
This is the branch that this PR does not touch. It is here so the table stays honest.
'2024-03-10 02:30:00'::timestamp AT TIME ZONE 'America/Denver'Arrow error: Cast error: Cannot cast timezone to different timezone2024-03-10 09:30:00+002024-03-10 09:30:00+00'2024-11-03 01:30:00'::timestamp AT TIME ZONE 'America/Denver'2024-11-03 08:30:00+002024-11-03 08:30:00+00A bare
arrow_casttoTimestamp(Nanosecond, Some("America/Denver"))reproduces the error, so it comes from the arrow cast and not from the SQL planner. This PR does not change it.The case the two engines cannot arbitrate: a fixed-offset string
<aware> AT TIME ZONE '+05:30'2024-01-01T17:30:00+05:30Timestamp(ns, "+05:30")2024-01-01T17:30:00Timestamp(ns)2024-01-01 06:30:00timestamp without time zoneNot implemented Error: Unknown TimeZone '+05:30'!DataFusion follows arrow and ISO 8601, where
+05:30is east of UTC. PostgreSQL applies the POSIX convention to an offset spelled as a string and reads+05:30as west of UTC. DuckDB rejects the string and accepts IANA names only. So DataFusion's reading matches neither engine.PostgreSQL does agree with DataFusion when the offset carries a type:
This divergence already exists on main. It is separate from #12218 and out of scope here. It is tracked in #25170. This PR changes only the result type for this case, not the value.
Tests
New
sqllogictestcoverage indatafusion/sqllogictest/test_files/datetime/timestamps.slt. The first commit adds it as a characterization of today's behaviour. The second commit flips it, so the diff shows exactly what changes. Every case assertsarrow_typeofnext to the value, and I checked every expected value against PostgreSQL 17 by hand. I did not regenerate them with--complete. The cases are:::timestampreproducer from Casting existing timestamp to timestamp again strips timezone information #12218,AT TIME ZONEapplications,now(),America/Denver.Two planner tests in
datafusion/sql/tests/sql_integration.rs: the plan shape of the naive lowering, and the error when noExprPlannerprovidesto_local_time.Green locally:
sqllogictestsuite, with zero changed expectations in the files that main already has,cargo test -p datafusion-sql,cargo test -p datafusion-functions to_local_time,cargo fmt --allandcargo clippy --all-targets -- -D warnings,--features avro,json,backtrace,extended_tests,recursive_protection,parquet_encryption).One caveat on the zero-change result.
AT TIME ZONEappears in exactly one test file on main, and every use there has aUtf8or naive inner expression. So the corpus had no coverage of the broken case at all, which is why the bug survived from #9647 in DataFusion 37. "Zero changed expectations" is a weak safety signal here, not a strong one. The field research above carries the argument instead.Are there any user-facing changes?
Yes. This is a breaking change to
AT TIME ZONEsemantics. Theapi changelabel is applied.The changes, in order of impact:
AT TIME ZONEkeeps the input'sTimeUnitinstead of a forcedNanosecond.SELECT aware AT TIME ZONE 'America/Denver' FROM tnow names the columnto_local_time(t.aware).datafusion-functionsnow gets a plan error forAT TIME ZONEon an aware input, where before it got a wrong plan.ExprPlannergains aplan_at_time_zonemethod. It has a default body, so out-of-tree implementations still compile.AT TIME ZONEsection indocs/source/user-guide/sql/operators.md. Theto_local_timedescription says how the two relate.A naive input does not change. Its value and its timezone label stay the same.
Maintainer input wanted: does this land as a straight behaviour fix, or behind a config flag with a deprecation period? I deliberately added no flag. The current behaviour has no defensible justification, and a flag means two timestamp typings through the planner for a long time. But this changes results for anyone who relies on the old shape, so I would rather you decide than assume.
Merge order: #25175 adds a timezone characterization suite (
test_files/datetime/timestamps_timezone.slt) whose SECTION 5b pins the exact behaviour this PR changes, both thearrow_typeofand the value. Whichever of the two merges second must update the other, and CI on it stays red until that happens. After this PR lands, the two queries in SECTION 5b become:The
DIVERGES FROM POSTGRESQL (type, not instant)comment above them can go, because this PR removes that divergence. Nothing else in that file conflicts. Its otherAT TIME ZONEcases all apply the operator to a naive input.🤖 Generated with Claude Code