test: PostgreSQL differential coverage for timestamps with time zone - #25164
Open
adriangb wants to merge 1 commit into
Open
test: PostgreSQL differential coverage for timestamps with time zone#25164adriangb wants to merge 1 commit into
adriangb wants to merge 1 commit into
Conversation
Adds `test_files/pg_compat/pg_compat_timestamptz.slt`, the first pg_compat file
covering timestamps with time zones. Until now not one of the differential
files exercised `timestamptz` at all, so nothing in CI checked that DataFusion
and PostgreSQL agree on any of it.
The file contains only queries where the two engines genuinely agree. It is
organized into 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, 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`. That turned out to
help rather than hurt: 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 coincide only when the session zone equals the column's
zone, so each block aligns them deliberately.
Expected output was generated from real PostgreSQL with the sqllogictest
`--complete` mode, so PostgreSQL's answer is the expectation and DataFusion has
to match it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
adriangb
force-pushed
the
test-timezone-characterization-suite
branch
from
September 10, 2026 19:34
2c3caeb to
2b0a37f
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #25164 +/- ##
==========================================
+ Coverage 81.60% 81.90% +0.30%
==========================================
Files 1123 1132 +9
Lines 408898 420570 +11672
Branches 408898 420570 +11672
==========================================
+ Hits 333670 344482 +10812
- Misses 55625 55769 +144
- Partials 19603 20319 +716 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
This PR closes no issue. It adds differential CI coverage for an area that had none.
Related (this PR does not fix any of them; queries that hit them are deliberately excluded from the file):
datafusion.execution.time_zoneis not used for basic time zone inference #13212Timestamp(_, None)to a named timezone errors on DST boundaries #25084date_binanddate_truncdisagree on timezone-aware timestamps #25167Rationale for this change
DataFusion has a working differential harness against a real PostgreSQL —
test_files/pg_compat/pg_compat_*.slt, run in CI by thesqllogictest-postgresjob againstpostgres:15— and not one of its files covered timestamps with time zones.That is the one area where "does DataFusion agree with PostgreSQL?" is hardest to answer by reading code and easiest to answer by running both engines. Right now the parts the two engines do agree on are agreed by accident: nothing in CI would notice if a refactor moved
date_trunc, day-valued interval arithmetic, ortimestamptzliteral parsing off PostgreSQL's answer.This PR puts the shared subset under continuous differential test, so that agreement stops being something we assert in a review comment and becomes something the build checks.
What changes are included in this PR?
One new test file. No production code is touched.
datafusion/sqllogictest/test_files/pg_compat/pg_compat_timestamptz.slt(~700 lines)Three blocks, each setting both engines' session zone explicitly (
SET TimeZonefor PostgreSQL,SET datafusion.execution.time_zonefor DataFusion) and building its table in that same zone: UTC,America/Denver, thenAsia/KolkataandAmerica/Phoenix. Covered:timestamptzliteral parsing with and without an explicit offset,date_part/extracton tz-aware values,date_truncat day/month/hour,date_binwith an explicit origin,+/-withINTERVALacross both DST transitions including the'1 day'vs'24 hours'distinction, ordering and comparison, andto_char-style rendering.Two constraints shaped the file, and are documented in its header:
timestamptzwire type (cell_to_stringwould hitunimplemented!), so no query may return a tz-aware value. Every result is projected down to a tz-naivetimestamp, abigint, abooleanortext. This turned out to be a feature rather than a limitation: comparing viadate_part('epoch', ...)::biginttests the instant directly, with no rendering in the way.timestampand renders atimestamptzusing its sessionTimeZone, whereas DataFusion uses the time zone carried by the value. The two coincide only when the session zone equals the column's zone, so each block aligns them deliberately. Anything outside that alignment is a divergence and is excluded from this file rather than pinned here.Scope note. An earlier revision of this PR also contained a large DataFusion-only characterization suite (
test_files/datetime/timestamps_timezone.slt) that records current behaviour including known bugs. The two files have very different risk profiles — this one contains only queries where the two engines genuinely agree, so a failure here is always a real regression — so the characterization file has been moved to #25175 and this PR is now the pg_compat file alone.What is the testing strategy for this PR?
This PR is tests.
Expected output was generated from a real PostgreSQL with the sqllogictest
--completemode (PG_COMPAT=true PG_URI=... --complete, the same pathcargo xtask ci step test postgresuses), so PostgreSQL's answer is the expectation and DataFusion has to match it. Every generated result was then read and sanity-checked. Divergences found during that process were removed from this file rather than pinned; they are recorded in the companion characterization PR with both engines' answers.Verified locally against
postgres:15, the image CI uses:PG_COMPAT=true PG_URI=... cargo test --profile ci -p datafusion-sqllogictest --features postgres --test sqllogictests -- pg_compat— all 7 pg_compat files green against PostgreSQL 15PG_COMPAT, i.e. the file evaluated by DataFusion — greencargo test -p datafusion-sqllogictest --test sqllogictests— full suite green (506 files)cargo fmt --all,typos datafusion/ docs/— cleanAre there any user-facing changes?
No. This PR adds a single test file. No production code, no public API, and no behaviour is changed.
🤖 Generated with Claude Code