TML-3224: encode timestamptz parameters as UTC ISO strings - #30093
TML-3224: encode timestamptz parameters as UTC ISO strings#30093wmadden-electric wants to merge 1 commit into
Conversation
…3224) pg/timestamptz@1 handed the Date to the pg driver, whose dateToString writes the local wall clock beside a whole-minute offset suffix. Zones whose historical rule is local mean time carry a seconds component, so the stored instant drifted by that remainder (Europe/Berlin, year 120: +28s). Serializing the instant ourselves as a Z-suffixed ISO string has no offset to lose, mirroring the JSON lane and the pg/date@1 fix. Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io>
|
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe PostgreSQL timestamptz codec now encodes ChangesTimestamptz encoding
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This PR makes timestamptz parameters serialize as UTC ISO strings, preventing writer-timezone-dependent historical timestamp errors. No actionable merge-blocking risk remains after normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
size-limit report 📦
|
@prisma/orm-extension-arktype-json
@prisma/orm-extension-middleware-cache
@prisma/orm-extension-paradedb
@prisma/orm-extension-pgvector
@prisma/orm-extension-postgis
@prisma/orm-extension-supabase
@prisma/orm-family-mongo
@prisma/orm-family-sql
@prisma/orm-framework
@prisma/orm-mongo
@prisma/orm-postgres
@prisma/orm-sqlite
@prisma/orm-target-mongo
@prisma/orm-target-postgres
@prisma/orm-target-sqlite
@prisma/orm-toolchain
commit: |
| async encode(value: Date, _ctx: CodecCallContext): Promise<string> { | ||
| return pgTimestamptzEncode(value); | ||
| } | ||
| async decode(wire: Date, _ctx: CodecCallContext): Promise<Date> { |
There was a problem hiding this comment.
This is technically unsound if typescript accepts this, our types say that TWire is now Date | string without distinguishing between the directions in which the value travels. The first argument of this method is supposed to accept TWire. So in principle this should now be
| async decode(wire: Date, _ctx: CodecCallContext): Promise<Date> { | |
| async decode(wire: Date | string, _ctx: CodecCallContext): Promise<Date> { |
with the corresponding branch for handling string values, based on the types.
Whether this is possible at run time is a separate issue. I'd prefer the implementation to match the types, otherwise this can blow up later when composing the abstractions in a different way, even if this branch is not going to be reachable at run time right now. The codec might be used for something other than decoding the values that come from the pg driver, and having it declare that it can parse string values but not actually parse them is not ideal.
3 participants
Writing
new Date('0120-01-01T00:00:00Z')to atimestamptzcolumn from aTZ=Europe/Berlinprocess stored0120-01-01T00:00:28Z— 28 seconds late. Same code,TZ=UTC: exact. This PR makes thetimestamptzcodec serialize the instant itself, so what gets stored no longer depends on the writer's timezone.Linear: TML-3224
The bug
It is on the write side, in how a JavaScript
Datebecomes a query parameter — not in the session and not in decoding (a directselectunder both timezones shows the same sessionTimeZone, identical wire text, and a correct parse).pg/timestamptz@1.encodehanded theDatestraight to thepgdriver, whosedateToStringwrites the local wall clock (getHours/getMinutes/getSeconds) next to an offset suffix built fromgetTimezoneOffset(), which is whole minutes. For year 120, Europe/Berlin's rule is local mean time,+00:53:28— an offset with seconds. So the instant was serialized as:Postgres reads that literally: 00:53:28 at +00:53 is
00:00:28Z. Any zone whose historical rule carries seconds reproduces it; UTC — and therefore CI — cannot, which is why this only surfaced as a "local-only" red inissues-28192-pg-historical-dates.The fix
encodenow returnsvalue.toISOString()— aZ-suffixed UTC string with no offset to lose — through a newpgTimestamptzEncodehelper whose doc comment records the mechanism. This mirrors the codec's existing JSON-lane encoder and the fixpg/date@1already received for the samedateToStringcause. The codec's wire type widens toDate | string; decode is unchanged.Verification
@internal/target-postgres: 1412 tests, lint, typecheck, build green.@internal/adapter-postgresand@internal/sql-orm-clienttests and typecheck green;integration-teststypecheck green.issues-28192-pg-historical-datesintegration file run from aTZ=Europe/Berlinshell against the rebuilt package — 5 passed, 4 expected-fail (the pre-existing 2-digit-yeardatecases), 0 failed. No timezone pinning anywhere.Deliberately not in this PR
pg/timestamp@1codec's two lanes disagree on which wall clock aDatemaps to (JSON lane: UTC components; parameter lane: local, viadateToString). Naive timestamps must not be implicitly forced to UTC; which wall clock they should use is a design decision, tracked in TML-3224's follow-up section.🤖 Generated with Claude Code
Summary by CodeRabbit
Datevalues and date strings.Datevalues, preserving existing behavior.