fix(adapter-pg): correctly handle TIMESTAMPTZ reads and writes on non-UTC servers#29481
Open
matingathani wants to merge 3 commits into
Open
fix(adapter-pg): correctly handle TIMESTAMPTZ reads and writes on non-UTC servers#29481matingathani wants to merge 3 commits into
matingathani wants to merge 3 commits into
Conversation
…-UTC servers Two bugs in `packages/adapter-pg/src/conversion.ts` cause incorrect Date values when the PostgreSQL session timezone is not UTC. **Read bug** (issue prisma#26786): `normalize_timestamptz` replaced the timezone offset label with `+00:00` without adjusting the time component. For example, a session-local `2025-11-24 09:26:34.887-06` (representing 15:26:34.887 UTC) was returned as `2025-11-24T09:26:34.887+00:00`, which is a six-hour error. Fix: parse with `new Date()` to obtain the true UTC instant, then re-emit in `+00:00` form. **Write bug** (issue prisma#28629): `formatDateTime` emitted no timezone suffix, so PostgreSQL interpreted the value in the server's local timezone rather than UTC. Fix: always append `+00:00`. For `TIMESTAMP WITHOUT TIME ZONE` columns PostgreSQL silently ignores the offset, so this is a safe no-op for those columns while being critical for `TIMESTAMPTZ` columns. Unit tests in `conversion.test.ts` are updated to expect the new `+00:00` suffix on datetime writes, and new tests cover `normalize_timestamptz` with UTC-6 / UTC+5:30 / UTC+0 offsets. A functional regression test (`issues/26786-pg-timestamptz-non-utc`) sets the session timezone to `America/New_York` and verifies the full round-trip.
This was referenced Apr 17, 2026
Contributor
Summary by CodeRabbit
WalkthroughRewrite TIMESTAMPTZ normalization to produce UTC RFC3339/ISO strings (with explicit +00:00), adjust datetime formatting to always append Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes incorrect TIMESTAMPTZ handling in the PostgreSQL driver adapter when the PostgreSQL session timezone is not UTC, and adds regression coverage.
Changes:
- Fix
TIMESTAMPTZread normalization to convert the timestamp to UTC (instead of relabeling the offset). - Fix datetime parameter formatting to always include an explicit
+00:00suffix when writing. - Add/extend unit + functional regression tests covering non-UTC session timezones and round-trips.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/adapter-pg/src/conversion.ts | Correct UTC conversion for TIMESTAMPTZ reads; append +00:00 when formatting datetimes for writes. |
| packages/adapter-pg/src/tests/conversion.test.ts | Updates expectations for datetime formatting and adds normalize_timestamptz unit coverage. |
| packages/client/tests/functional/issues/26786-pg-timestamptz-non-utc/test.ts | Adds functional regression tests that force a non-UTC session timezone and verify write→read round-trip. |
| packages/client/tests/functional/issues/26786-pg-timestamptz-non-utc/prisma/_schema.ts | Adds a minimal schema with an @db.Timestamptz field for the regression suite. |
| packages/client/tests/functional/issues/26786-pg-timestamptz-non-utc/_matrix.ts | Defines the test matrix entry for PostgreSQL. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| optOut: { | ||
| from: ['sqlite', 'mysql', 'mongodb', 'cockroachdb', 'sqlserver'], | ||
| reason: 'TIMESTAMPTZ and SET timezone are PostgreSQL-specific', | ||
| }, |
There was a problem hiding this comment.
This regression suite will be expanded to run against all PostgreSQL driver-adapter flavors (notably js_neon as well as js_pg). The timezone read/write bugs fixed in this PR are also present in packages/adapter-neon/src/conversion.ts (it still swaps the offset label and still writes datetimes without a +00:00 suffix), so this new test is expected to fail under the js_neon matrix unless you either port the same fix to the Neon adapter or explicitly skipDriverAdapter for js_neon in this suite.
Suggested change
| }, | |
| }, | |
| skipDriverAdapter: ['js_neon'], |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/adapter-pg/src/__tests__/conversion.test.ts`:
- Around line 45-67: Add a unit test that exercises the array path: call the
array parser registered for ArrayColumnType.TIMESTAMPTZ_ARRAY
(customParsers[1185], wired via normalize_array(normalize_timestamptz)) with a
PostgreSQL array literal like {"2025-11-24 09:26:34.887-06","2024-06-15
12:00:00+00","2024-01-01 09:26:34.887456-06"} and assert it returns the
corresponding UTC ISO strings
["2025-11-24T15:26:34.887+00:00","2024-06-15T12:00:00.000+00:00","2024-01-01T15:26:34.887+00:00"]
(note microsecond input is ms-truncated), ensuring the array conversion path via
normalize_array(normalize_timestamptz) is covered.
In `@packages/adapter-pg/src/conversion.ts`:
- Around line 312-319: Add a unit test that passes a TIMESTAMPTZ string with
6-digit microsecond precision (e.g., "2024-01-01 09:26:34.887456-06") through
the normalize_timestamptz function and asserts the result matches the expected
ISO string with millisecond precision (microseconds truncated) and +00:00
offset; this documents the truncation behavior introduced by using
Date.toISOString() and prevents regressions. Ensure the test references
normalize_timestamptz from packages/adapter-pg/src/conversion.ts and covers
conversion of the space to 'T', offset normalization, and the loss of the last
three fractional digits. Keep the test name descriptive (e.g.,
"normalize_timestamptz truncates microseconds to milliseconds") and include an
assertion for the exact expected string produced by the current implementation.
In
`@packages/client/tests/functional/issues/26786-pg-timestamptz-non-utc/test.ts`:
- Around line 38-41: The test file currently includes an optOut block
(optOut.from listing sqlite, mysql, mongodb, cockroachdb, sqlserver) which is
redundant because _matrix.ts already restricts the suite to
Providers.POSTGRESQL; remove the redundant optOut block from the test (or
alternatively remove the Providers.POSTGRESQL restriction in _matrix.ts) so
there is a single source of truth—locate the optOut declaration in this test
file and delete it to resolve the duplication.
- Around line 16-35: The test uses prisma.$executeRawUnsafe(`SET timezone =
'America/New_York'`) outside a transaction which may run on a different pooled
connection than the subsequent prisma.event.create/findFirst calls; change the
test to run the timezone change and the create/find calls inside a single
interactive transaction so they use the same connection: start an interactive
transaction via prisma.$transaction(async (tx) => { await
tx.$executeRawUnsafe("SET LOCAL timezone = 'America/New_York'"); await
tx.event.create(...); await tx.event.findFirst(...); }) so you use SET LOCAL and
the same tx object for event.create and event.findFirst (referencing
prisma.$transaction, $executeRawUnsafe, and tx.event.create/tx.event.findFirst).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 58d99785-a8f3-4401-986c-5f519a50c83d
📒 Files selected for processing (5)
packages/adapter-pg/src/__tests__/conversion.test.tspackages/adapter-pg/src/conversion.tspackages/client/tests/functional/issues/26786-pg-timestamptz-non-utc/_matrix.tspackages/client/tests/functional/issues/26786-pg-timestamptz-non-utc/prisma/_schema.tspackages/client/tests/functional/issues/26786-pg-timestamptz-non-utc/test.ts
Comment on lines
+38
to
+41
| optOut: { | ||
| from: ['sqlite', 'mysql', 'mongodb', 'cockroachdb', 'sqlserver'], | ||
| reason: 'TIMESTAMPTZ and SET timezone are PostgreSQL-specific', | ||
| }, |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Nit: optOut is redundant with the matrix.
_matrix.ts already restricts the suite to Providers.POSTGRESQL, so listing sqlite, mysql, mongodb, cockroachdb, sqlserver in optOut.from has no effect. You can drop the optOut block, or drop the matrix restriction in favor of a single source of truth.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/client/tests/functional/issues/26786-pg-timestamptz-non-utc/test.ts`
around lines 38 - 41, The test file currently includes an optOut block
(optOut.from listing sqlite, mysql, mongodb, cockroachdb, sqlserver) which is
redundant because _matrix.ts already restricts the suite to
Providers.POSTGRESQL; remove the redundant optOut block from the test (or
alternatively remove the Providers.POSTGRESQL restriction in _matrix.ts) so
there is a single source of truth—locate the optOut declaration in this test
file and delete it to resolve the duplication.
matingathani
force-pushed
the
fix/adapter-pg-timestamptz-conversion
branch
from
d0af4bd to
8125a1a
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
packages/client/tests/functional/issues/26786-pg-timestamptz-non-utc/test.ts (2)
38-41: 🧹 Nitpick | 🔵 TrivialRedundant
optOut— matrix already restricts to PostgreSQL.
_matrix.tsdefines the matrix as[[{ provider: Providers.POSTGRESQL }]], so theoptOut.fromblock listing sqlite/mysql/mongodb/cockroachdb/sqlserver has no effect. Drop theoptOutblock (or, conversely, remove the PG restriction from_matrix.ts) to keep a single source of truth.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/client/tests/functional/issues/26786-pg-timestamptz-non-utc/test.ts` around lines 38 - 41, The optOut block in the test definition is redundant because the test matrix in _matrix.ts already restricts runs to PostgreSQL (Providers.POSTGRESQL); remove the optOut object from the test declaration in test.ts (the optOut property listing sqlite/mysql/mongodb/cockroachdb/sqlserver) so there’s a single source of truth, or alternatively remove the PostgreSQL-only constraint in _matrix.ts—ensure you modify either the optOut property in the test file or the matrix definition (Providers.POSTGRESQL) but not both.
16-35:⚠️ Potential issue | 🟠 Major
SET timezoneinbeforeEachmay run on a different pooled connection than the assertions — test can silently pass even if the fix is reverted.The pg driver adapter maintains its own
pg.Pool.prisma.$executeRawUnsafe('SET timezone = ...')will applySET(session-level) only to whichever pooled connection was checked out at that moment; the subsequentevent.create/event.findFirstcalls may acquire a different connection that is still running in UTC. In that case neither the read bug (#26786) nor the write bug (#28629) is actually exercised, and this regression test will pass even with the fixes reverted.Use an interactive
$transactionwithSET LOCALso the timezone change and the queries share a single connection:🔧 Proposed fix
- beforeEach(async () => { - // Force the session to a non-UTC timezone so both bugs manifest: - // - write bug: without '+00:00' the DB would store local time, not UTC - // - read bug: without proper conversion the returned string would shift by the offset - await prisma.$executeRawUnsafe(`SET timezone = 'America/New_York'`) - }) - test('round-trips a `@db.Timestamptz` value correctly with a non-UTC session', async () => { - const created = await prisma.event.create({ data: { happenedAt: utcDate } }) + const created = await prisma.$transaction(async (tx) => { + await tx.$executeRawUnsafe(`SET LOCAL timezone = 'America/New_York'`) + return tx.event.create({ data: { happenedAt: utcDate } }) + }) expect(created.happenedAt).toBeInstanceOf(Date) expect(created.happenedAt.toISOString()).toBe(UTC_ISO) }) test('reads back the correct UTC instant via findFirst', async () => { - await prisma.event.create({ data: { happenedAt: utcDate } }) - const found = await prisma.event.findFirst({ orderBy: { id: 'desc' } }) + const found = await prisma.$transaction(async (tx) => { + await tx.$executeRawUnsafe(`SET LOCAL timezone = 'America/New_York'`) + await tx.event.create({ data: { happenedAt: utcDate } }) + return tx.event.findFirst({ orderBy: { id: 'desc' } }) + }) expect(found?.happenedAt.toISOString()).toBe(UTC_ISO) })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/client/tests/functional/issues/26786-pg-timestamptz-non-utc/test.ts` around lines 16 - 35, The test sets the session timezone with prisma.$executeRawUnsafe('SET timezone ...') which may affect a different pooled connection than the subsequent queries; replace the ad-hoc SET with an interactive transaction that uses SET LOCAL so the timezone change and the tested queries run on the same connection. Concretely: remove or stop relying on beforeEach's global SET, and in each test wrap the operations in prisma.$transaction(async (tx) => { await tx.$executeRaw`SET LOCAL timezone = 'America/New_York'`; use tx.event.create(...) and tx.event.findFirst(...) (or tx.event.create then tx.event.findFirst in the same transaction) and assert on created/found.happenedAt using utcDate/UTC_ISO; use SET LOCAL instead of SET so the change is scoped to that transaction.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@packages/client/tests/functional/issues/26786-pg-timestamptz-non-utc/test.ts`:
- Around line 38-41: The optOut block in the test definition is redundant
because the test matrix in _matrix.ts already restricts runs to PostgreSQL
(Providers.POSTGRESQL); remove the optOut object from the test declaration in
test.ts (the optOut property listing sqlite/mysql/mongodb/cockroachdb/sqlserver)
so there’s a single source of truth, or alternatively remove the PostgreSQL-only
constraint in _matrix.ts—ensure you modify either the optOut property in the
test file or the matrix definition (Providers.POSTGRESQL) but not both.
- Around line 16-35: The test sets the session timezone with
prisma.$executeRawUnsafe('SET timezone ...') which may affect a different pooled
connection than the subsequent queries; replace the ad-hoc SET with an
interactive transaction that uses SET LOCAL so the timezone change and the
tested queries run on the same connection. Concretely: remove or stop relying on
beforeEach's global SET, and in each test wrap the operations in
prisma.$transaction(async (tx) => { await tx.$executeRaw`SET LOCAL timezone =
'America/New_York'`; use tx.event.create(...) and tx.event.findFirst(...) (or
tx.event.create then tx.event.findFirst in the same transaction) and assert on
created/found.happenedAt using utcDate/UTC_ISO; use SET LOCAL instead of SET so
the change is scoped to that transaction.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4042e52b-525f-4665-b07d-c488f822274e
📒 Files selected for processing (5)
packages/adapter-pg/src/__tests__/conversion.test.tspackages/adapter-pg/src/conversion.tspackages/client/tests/functional/issues/26786-pg-timestamptz-non-utc/_matrix.tspackages/client/tests/functional/issues/26786-pg-timestamptz-non-utc/prisma/_schema.tspackages/client/tests/functional/issues/26786-pg-timestamptz-non-utc/test.ts
matingathani
force-pushed
the
fix/adapter-pg-timestamptz-conversion
branch
from
8125a1a to
d0af4bd
Compare
…z; fix pool isolation in test normalize_timestamptz now preserves sub-millisecond (up to 6-digit) fractional seconds that JavaScript Date drops during UTC conversion. Fractional seconds are unaffected by timezone shifts, so we extract them, parse with ms precision, then reattach the full fractional string. The regression test now uses $transaction + SET LOCAL so the session timezone is guaranteed to apply to every query on the same connection, fixing a potential pool-isolation bug where SET timezone could affect a different connection than the subsequent Prisma queries. The redundant optOut block is also removed since _matrix.ts already restricts the suite to PostgreSQL.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
packages/adapter-pg/src/__tests__/conversion.test.ts (1)
45-73: 🧹 Nitpick | 🔵 TrivialSolid coverage for
normalize_timestamptz.UTC‑6, UTC+5:30, bare
+00, cross‑midnight sub‑second, and 6‑digit microsecond cases are all covered and align with the implementation. Two optional additions you may want (not blockers):
- An array‑path case through
customParsers[1185](TIMESTAMPTZ_ARRAY) to lock innormalize_array(normalize_timestamptz).- A 3‑digit fractional input (e.g.,
2024-01-01 09:26:34.887-06) to document that the fast path (no fracMatch) still emits.887+00:00— right now only the 6‑digit branch is exercised for fractional preservation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/adapter-pg/src/__tests__/conversion.test.ts` around lines 45 - 73, Add two small tests: one exercising the array path by calling customParsers[TIMESTAMPTZ_ARRAY] (or invoking normalize_array(normalize_timestamptz)) with an array string to ensure the array wrapper uses normalize_timestamptz, and one feeding a 3‑digit fractional timestamptz (e.g., '2024-01-01 09:26:34.887-06') into customParsers[TIMESTAMPTZ_OID] to assert it emits '.887+00:00' (verifying the fast path preserves 3ms precision); locate tests near the existing normalize_timestamptz suite and reference customParsers, TIMESTAMPTZ_ARRAY, TIMESTAMPTZ_OID, normalize_array, and normalize_timestamptz when adding assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/adapter-pg/src/conversion.ts`:
- Around line 312-327: normalize_timestamptz can pass an offset-less ISO-like
string to Date (which V8 will treat as local time); add a defensive fallback in
normalize_timestamptz to detect missing timezone designator (neither a +/-hh nor
trailing Z) and explicitly append '+00:00' before parsing so the conversion is
unambiguous, while preserving the existing fractional-second handling and the
reattachment of original digits; refer to the normalize_timestamptz function and
the withFullOffset/withSeparator/fracMatch variables when making the change.
In
`@packages/client/tests/functional/issues/26786-pg-timestamptz-non-utc/test.ts`:
- Around line 10-38: The second test's findFirst call relies on ordering to pick
the row but could pick a different row if DB state changes; modify the test
inside the transaction (the test named 'reads back the correct UTC instant via
findFirst') to capture the created row (the result of tx.event.create assigned
to e.g. created) and then call tx.event.findFirst with a where filter on that
created.id (replace the existing findFirst({ orderBy: { id: 'desc' } }) call) so
the lookup is deterministic and independent of other rows.
---
Duplicate comments:
In `@packages/adapter-pg/src/__tests__/conversion.test.ts`:
- Around line 45-73: Add two small tests: one exercising the array path by
calling customParsers[TIMESTAMPTZ_ARRAY] (or invoking
normalize_array(normalize_timestamptz)) with an array string to ensure the array
wrapper uses normalize_timestamptz, and one feeding a 3‑digit fractional
timestamptz (e.g., '2024-01-01 09:26:34.887-06') into
customParsers[TIMESTAMPTZ_OID] to assert it emits '.887+00:00' (verifying the
fast path preserves 3ms precision); locate tests near the existing
normalize_timestamptz suite and reference customParsers, TIMESTAMPTZ_ARRAY,
TIMESTAMPTZ_OID, normalize_array, and normalize_timestamptz when adding
assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1ab0a9aa-b120-411a-b14e-c0ed310292ac
📒 Files selected for processing (5)
packages/adapter-pg/src/__tests__/conversion.test.tspackages/adapter-pg/src/conversion.tspackages/client/tests/functional/issues/26786-pg-timestamptz-non-utc/_matrix.tspackages/client/tests/functional/issues/26786-pg-timestamptz-non-utc/prisma/_schema.tspackages/client/tests/functional/issues/26786-pg-timestamptz-non-utc/test.ts
Comment on lines
+10
to
+38
| testMatrix.setupTestSuite(() => { | ||
| // A fixed UTC instant used throughout this suite. | ||
| const UTC_ISO = '2025-11-24T15:26:34.887Z' | ||
| const utcDate = new Date(UTC_ISO) | ||
|
|
||
| test('round-trips a @db.Timestamptz value correctly with a non-UTC session', async () => { | ||
| // Wrap in a transaction so SET LOCAL affects every query on the same connection. | ||
| await prisma.$transaction(async (tx) => { | ||
| // Force a non-UTC session timezone so both bugs manifest: | ||
| // - write bug: without '+00:00' the DB would store local time, not UTC | ||
| // - read bug: without proper conversion the returned string would shift by the offset | ||
| await tx.$executeRawUnsafe(`SET LOCAL timezone = 'America/New_York'`) | ||
| const created = await tx.event.create({ data: { happenedAt: utcDate } }) | ||
|
|
||
| expect(created.happenedAt).toBeInstanceOf(Date) | ||
| expect(created.happenedAt.toISOString()).toBe(UTC_ISO) | ||
| }) | ||
| }) | ||
|
|
||
| test('reads back the correct UTC instant via findFirst', async () => { | ||
| await prisma.$transaction(async (tx) => { | ||
| await tx.$executeRawUnsafe(`SET LOCAL timezone = 'America/New_York'`) | ||
| await tx.event.create({ data: { happenedAt: utcDate } }) | ||
| const found = await tx.event.findFirst({ orderBy: { id: 'desc' } }) | ||
|
|
||
| expect(found?.happenedAt.toISOString()).toBe(UTC_ISO) | ||
| }) | ||
| }) | ||
| }) |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
LGTM — prior review feedback addressed.
Running SET LOCAL timezone inside prisma.$transaction ensures the timezone override and the subsequent create / findFirst all run on the same pooled connection, so both the write (#28629) and read (#26786) paths are actually exercised. The asserted instant 2025-11-24T15:26:34.887Z against a New York session would fail if either bug regressed.
Minor, non‑blocking: findFirst({ orderBy: { id: 'desc' } }) in the second test implicitly relies on no other rows with a larger id existing at the time of the query. Within the same transaction this is guaranteed, so fine in practice — but a narrower findFirst({ where: { id: created.id } }) (capturing the created row's id) would make the assertion independent of DB state and slightly more robust against future test additions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/client/tests/functional/issues/26786-pg-timestamptz-non-utc/test.ts`
around lines 10 - 38, The second test's findFirst call relies on ordering to
pick the row but could pick a different row if DB state changes; modify the test
inside the transaction (the test named 'reads back the correct UTC instant via
findFirst') to capture the created row (the result of tx.event.create assigned
to e.g. created) and then call tx.event.findFirst with a where filter on that
created.id (replace the existing findFirst({ orderBy: { id: 'desc' } }) call) so
the lookup is deterministic and independent of other rows.
…d array+skip tests - normalize_timestamptz: strip sub-minute seconds from historical LMT offsets (e.g. +05:53:20) before parsing so Date.parse does not fail. Also guard both code paths with isNaN() and fall back to the old label-swap behaviour on any input Date cannot parse, preventing RangeError from propagating to the caller. - Add TIMESTAMPTZ_ARRAY unit test: run the array parser (OID 1185, wired via normalize_array(normalize_timestamptz)) against a PostgreSQL array literal to lock in the array conversion path. - Add LMT-era offset unit test. - Functional test: add skipDriverAdapter for js_neon because adapter-neon still carries the same bugs; the fix is tracked in a separate PR.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/adapter-pg/src/__tests__/conversion.test.ts`:
- Around line 76-81: The test for parse in conversion.test.ts incorrectly
expects applying the full seconds offset; update the assertion so it matches the
implementation that drops the seconds component (see the
.replace(/([+-]\d{2}):(\d{2}):\d{2}$/, '$1:$2') behavior in conversion.ts).
Change the expected result for parse('1900-01-01 10:00:00+05:53:20') to reflect
10:00:00 minus +05:53 => '1900-01-01T04:07:00.000+00:00' so the test matches the
documented/implemented logic.
In `@packages/adapter-pg/src/conversion.ts`:
- Around line 340-344: The fallback that returns withSeparator.replace(...,
'+00:00') should emit a visible warning instead of silently returning a mutated
timestamp: update the fallback in conversion.ts (the code path that checks
Date.parse/isNaN and ends with return
withSeparator.replace(/[+-]\d{2}(:\d{2})?(:\d{2})?$/, '+00:00')) to call debug()
(or the project's logging facility) before returning, including the original
input and the computed withSeparator value so regressions are observable; ensure
the debug message clearly flags that Date.parse failed and this fallback was
used.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ef821930-1b36-4782-9a02-76c738c5d077
📒 Files selected for processing (3)
packages/adapter-pg/src/__tests__/conversion.test.tspackages/adapter-pg/src/conversion.tspackages/client/tests/functional/issues/26786-pg-timestamptz-non-utc/test.ts
Comment on lines
+76
to
+81
| it('normalises LMT-era offsets with seconds component (e.g. +05:53:20)', () => { | ||
| // Some historical timezone abbreviations include a seconds component that | ||
| // JS Date.parse cannot handle. The seconds component should be dropped and | ||
| // the remaining ±HH:MM offset used for the conversion. | ||
| expect(parse('1900-01-01 10:00:00+05:53:20')).toBe('1900-01-01T04:06:40.000+00:00') | ||
| }) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm vitest actually picks up this describe block and runs the LMT case.
fd -t f 'conversion.test.ts' packages/adapter-pg
rg -n "normalises LMT-era|1900-01-01 10:00:00" packages/adapter-pg
# Simulate the code path in isolation.
node -e '
const s = "1900-01-01 10:00:00+05:53:20";
const withFull = s.replace(" ", "T")
.replace(/([+-]\d{2}):(\d{2}):\d{2}$/, "$1:$2")
.replace(/([+-]\d{2})$/, "$1:00");
const d = new Date(withFull);
console.log(d.toISOString().replace(/Z$/, "+00:00"));
'Repository: prisma/prisma
Length of output: 422
🏁 Script executed:
fd -t f '*.ts' packages/adapter-pg/src -path '*test*' -prune -o -type f -name '*conversion*' -o -name '*parse*' | head -20Repository: prisma/prisma
Length of output: 225
🏁 Script executed:
# Find the parse function and normalize_timestamptz implementation
rg -n "normalize_timestamptz|function parse|const parse" packages/adapter-pg/src --type ts -A 5 | head -100Repository: prisma/prisma
Length of output: 3395
🏁 Script executed:
# Look at the actual conversion.test.ts to understand the parse function
cat packages/adapter-pg/src/__tests__/conversion.test.ts | head -100Repository: prisma/prisma
Length of output: 4422
🏁 Script executed:
# Get the full normalize_timestamptz function
sed -n '312,350p' packages/adapter-pg/src/conversion.tsRepository: prisma/prisma
Length of output: 1958
🏁 Script executed:
# Also check if there are any test skip markers or other clues
rg -n "skip|only|todo" packages/adapter-pg/src/__tests__/conversion.test.ts | head -20Repository: prisma/prisma
Length of output: 137
Test expectation 04:06:40.000 contradicts the implementation and its own comment.
The code explicitly drops the seconds component from the offset (line 318 of conversion.ts: .replace(/([+-]\d{2}):(\d{2}):\d{2}$/, '$1:$2')), and the function comment confirms: "The seconds component is dropped because JS Date does not support sub-minute offsets." When applied to the input '1900-01-01 10:00:00+05:53:20', the implementation computes 10:00:00 − 05:53 (seconds dropped) = 04:07:00, not 04:06:40.
The expected value 04:06:40 incorrectly assumes the full +05:53:20 offset is applied.
Update the expectation to match the actual behavior:
- expect(parse('1900-01-01 10:00:00+05:53:20')).toBe('1900-01-01T04:06:40.000+00:00')
+ expect(parse('1900-01-01 10:00:00+05:53:20')).toBe('1900-01-01T04:07:00.000+00:00')📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| it('normalises LMT-era offsets with seconds component (e.g. +05:53:20)', () => { | |
| // Some historical timezone abbreviations include a seconds component that | |
| // JS Date.parse cannot handle. The seconds component should be dropped and | |
| // the remaining ±HH:MM offset used for the conversion. | |
| expect(parse('1900-01-01 10:00:00+05:53:20')).toBe('1900-01-01T04:06:40.000+00:00') | |
| }) | |
| it('normalises LMT-era offsets with seconds component (e.g. +05:53:20)', () => { | |
| // Some historical timezone abbreviations include a seconds component that | |
| // JS Date.parse cannot handle. The seconds component should be dropped and | |
| // the remaining ±HH:MM offset used for the conversion. | |
| expect(parse('1900-01-01 10:00:00+05:53:20')).toBe('1900-01-01T04:07:00.000+00:00') | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/adapter-pg/src/__tests__/conversion.test.ts` around lines 76 - 81,
The test for parse in conversion.test.ts incorrectly expects applying the full
seconds offset; update the assertion so it matches the implementation that drops
the seconds component (see the .replace(/([+-]\d{2}):(\d{2}):\d{2}$/, '$1:$2')
behavior in conversion.ts). Change the expected result for parse('1900-01-01
10:00:00+05:53:20') to reflect 10:00:00 minus +05:53 =>
'1900-01-01T04:07:00.000+00:00' so the test matches the documented/implemented
logic.
Comment on lines
312
to
345
| function normalize_timestamptz(time: string): string { | ||
| return time.replace(' ', 'T').replace(/[+-]\d{2}(:\d{2})?$/, '+00:00') | ||
| // PostgreSQL returns TIMESTAMPTZ values in the session timezone (e.g. '2024-01-01 09:26:34-06'). | ||
| // We must convert to UTC rather than merely swapping the offset label. | ||
| const withSeparator = time.replace(' ', 'T') | ||
| // Normalise bare hour offsets (e.g. '-06') and hour-with-seconds offsets | ||
| // (e.g. '+05:53:20', emitted for LMT-era historical timezones) to ±HH:MM so | ||
| // that Date.parse can handle them. The seconds component is dropped because | ||
| // JS Date does not support sub-minute offsets and the fractional-second error | ||
| // is negligible for real-world timestamps. | ||
| const withFullOffset = withSeparator | ||
| .replace(/([+-]\d{2}):(\d{2}):\d{2}$/, '$1:$2') | ||
| .replace(/([+-]\d{2})$/, '$1:00') | ||
| // JavaScript Date only handles millisecond (3-digit) precision. Extract any extra fractional | ||
| // digits and reattach after conversion — fractional seconds are unchanged by a timezone shift. | ||
| const fracMatch = withFullOffset.match(/\.(\d{4,})/) | ||
| if (fracMatch) { | ||
| const fullFrac = fracMatch[1] | ||
| const trimmed = withFullOffset.replace(/\.(\d{4,})/, '.' + fullFrac.slice(0, 3)) | ||
| const d = new Date(trimmed) | ||
| if (!isNaN(d.getTime())) { | ||
| return d.toISOString().replace(/\.\d{3}Z$/, '.' + fullFrac + '+00:00') | ||
| } | ||
| } else { | ||
| const d = new Date(withFullOffset) | ||
| if (!isNaN(d.getTime())) { | ||
| return d.toISOString().replace(/Z$/, '+00:00') | ||
| } | ||
| } | ||
| // Fallback for any input that Date.parse cannot handle: preserve the | ||
| // original string with the separator normalised to 'T' and the offset | ||
| // label rewritten to '+00:00'. This matches the old behaviour and avoids | ||
| // a thrown RangeError propagating to the caller. | ||
| return withSeparator.replace(/[+-]\d{2}(:\d{2})?(:\d{2})?$/, '+00:00') | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Reproduce the exact runtime behaviour for the LMT test input.
node -e '
const s = "1900-01-01 10:00:00+05:53:20";
const withSep = s.replace(" ", "T");
const withFull = withSep
.replace(/([+-]\d{2}):(\d{2}):\d{2}$/, "$1:$2")
.replace(/([+-]\d{2})$/, "$1:00");
console.log("withFullOffset:", withFull);
const d = new Date(withFull);
console.log("isNaN:", isNaN(d.getTime()));
console.log("toISOString:", d.toISOString());
console.log("final:", d.toISOString().replace(/Z$/, "+00:00"));
'Repository: prisma/prisma
Length of output: 186
🏁 Script executed:
sed -n '75,85p' packages/adapter-pg/src/__tests__/conversion.test.tsRepository: prisma/prisma
Length of output: 594
LMT-offset seconds component is dropped but test expects preservation—implementation does not match test assertion.
The code strips the :20 seconds from offset +05:53:20 before calling Date.parse, producing 04:07:00. However, the test at packages/adapter-pg/src/__tests__/conversion.test.ts:80 asserts '1900-01-01T04:06:40.000+00:00'—the result that would occur if the full +05:53:20 offset were applied (10:00:00 − 05:53:20 = 04:06:40, not 04:07:00).
The test comment states "The seconds component should be dropped," yet the expectation contradicts this—it expects the UTC result as if seconds were NOT dropped. Either:
- Drop the seconds and update the test to assert
04:07:00, or - Manually compute UTC using the full
HH:MM:SSoffset to preserve sub-minute precision for historical timezones.
Comment on lines
+340
to
+344
| // Fallback for any input that Date.parse cannot handle: preserve the | ||
| // original string with the separator normalised to 'T' and the offset | ||
| // label rewritten to '+00:00'. This matches the old behaviour and avoids | ||
| // a thrown RangeError propagating to the caller. | ||
| return withSeparator.replace(/[+-]\d{2}(:\d{2})?(:\d{2})?$/, '+00:00') |
Contributor
There was a problem hiding this comment.
Fallback silently reintroduces the pre-fix bug.
When Date.parse fails, this fallback swaps the offset label to +00:00 without adjusting the time — the exact behavior that issue #26786 is fixing. If a future PG output shape trips the isNaN guard, callers will silently receive a wrong instant rather than a loud failure.
Consider at least logging a warning (e.g., via debug()) so regressions are observable in production, rather than silently returning corrupt data.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/adapter-pg/src/conversion.ts` around lines 340 - 344, The fallback
that returns withSeparator.replace(..., '+00:00') should emit a visible warning
instead of silently returning a mutated timestamp: update the fallback in
conversion.ts (the code path that checks Date.parse/isNaN and ends with return
withSeparator.replace(/[+-]\d{2}(:\d{2})?(:\d{2})?$/, '+00:00')) to call debug()
(or the project's logging facility) before returning, including the original
input and the computed withSeparator value so regressions are observable; ensure
the debug message clearly flags that Date.parse failed and this fallback was
used.
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.
Summary
Fixes two bugs in
packages/adapter-pg/src/conversion.tsthat cause@db.Timestamptzfields to return or store wrong values when the PostgreSQL session timezone is not UTC.Read bug — closes #26786
normalize_timestamptzswapped the offset label to+00:00without adjusting the time:Write bug — closes #28629
formatDateTimeemitted no timezone suffix, so PostgreSQL treated the value as being in the server's local timezone:For
TIMESTAMP WITHOUT TIME ZONEcolumns PostgreSQL silently ignores timezone suffixes, so the+00:00addition is a safe no-op for those columns.Test plan
packages/adapter-pg/src/__tests__/conversion.test.tsupdated and extended — cover UTC-6, UTC+5:30, UTC+0, and cross-midnight precision cases fornormalize_timestamptzpackages/client/tests/functional/issues/26786-pg-timestamptz-non-utc/— sets the PostgreSQL session toAmerica/New_Yorkand verifies the full write → read round-trip produces the original UTC instantdriver-adapter-unit-tests/client-functional-testsjobs will validate against a live database