Skip to content

fix(driver-sql): keep a PostgreSQL date a calendar-day string, not a local-midnight Date - #11561

Merged
os-zhuang merged 2 commits into
mainfrom
claude/issue-11389-pg-date-tz-skew
Aug 24, 2026
Merged

fix(driver-sql): keep a PostgreSQL date a calendar-day string, not a local-midnight Date#11561
os-zhuang merged 2 commits into
mainfrom
claude/issue-11389-pg-date-tz-skew

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Fixes#11389

On PostgreSQL every Field.date read back through SqlDriver was one calendar day early whenever the Node process ran east of UTC. The stored value was always right; the read corrupted it, so the wrong day was already in the REST payload. Production-reported on an app container running TZ=Asia/Shanghai.

What I measured before choosing a route

The card offers two routes and prefers (b). Both were measured rather than assumed, on a live PostgreSQL 16 and MySQL 8.0 configured to CI parity (PG timezone='Asia/Shanghai', MySQL @@global.time_zone='+08:00'), with only the process TZ changed between runs.

1. The read path really is on the local clock.pg materialises OID 1082 with new Date(y, m - 1, d):

process TZpg materialised for stored 2026-08-24toDateOnly returned
UTC2026-08-24T00:00:00.000Z2026-08-24
America/New_York2026-08-24T04:00:00.000Z2026-08-24
Asia/Shanghai2026-08-23T16:00:00.000Z2026-08-23

2. The two-clock trap is real, and route (a) would break the write path.toDateOnly is shared by the read (formatOutput, presentReadValue), write (formatInput) and filter (coerceFilterValue) paths, and they do not hand it Dates from one source:

argument for calendar day 2026-08-24TZ=Asia/ShanghaiTZ=America/New_York
caller's new Date('2026-08-24') — write/filter…T00:00Z, UTC comp 24, local comp 24…T00:00Z, UTC comp 24, local comp 23
caller's new Date(2026, 7, 24) — write/filter…T16:00Z, UTC comp 23, local comp 24…T04:00Z, UTC comp 24, local comp 24
what pg handed the READ path…T16:00Z, UTC comp 23, local comp 24…T04:00Z, UTC comp 24, local comp 24

So no single clock is right for every Date that can arrive at that helper. Reading local components fixes row 3 and breaks row 1: a new Date('2026-08-24') comparand becomes 2026-08-23 west of UTC — the identical one-day error, moved onto the write and filter paths. Route (a) is measurably wrong, and the card's preference for (b) is confirmed rather than taken on trust.

3. A correction to the card: MySQL is NOT protected by "SQLite round-trips as TEXT". The card says the bug is Postgres-only because SQLite keeps dates as TEXT. That is true of SQLite but says nothing about MySQL, and mysql2 materialises a DATE at local midnight too — measured identically to pg. MySQL is correct today for a different reason the card never names: withUtcSession (#3942) already pins connection.timezone: 'Z' on every mysql2 connection, and with that pin the same column arrives at UTC midnight:

opts={} Date iso=2026-08-23T16:00:00.000Z UTCcomp=2026-08-23
opts={"timezone":"Z"} Date iso=2026-08-24T00:00:00.000Z UTCcomp=2026-08-24
opts={"dateStrings":["DATE"]} string "2026-08-24"

That asymmetry is the actual root-cause shape: MySQL had a connection-level pin and PostgreSQL had none. The fix gives PostgreSQL its counterpart, and the MySQL cell is in the new matrix so that losing the mysql2 pin would reproduce this issue one dialect over.

The fix — route (b)

SqlDriver.withPostgresCalendarDayAsText, chained beside the existing withUtcSession in withConnectBound, installs a pool.afterCreate hook that registers connection-scoped parsers:

  • OID 1082 (date) returns the wire text unchanged — the wire form already isYYYY-MM-DD.
  • OID 1182 (date[]) reuses the connection's own text[] parser (OID 1009), which is pg's array-literal splitter with an identity element transform. No hand-rolled array parsing: NULL elements survive as null, {} yields [], and multi-dimensional arrays parse correctly.
  • timestamptz is untouched — an instant is exactly what a Date is for, and Field.datetime depends on it.

Three properties worth naming:

  • No process-wide mutation.pg.types.setTypeParser mutates the pg-types registry for every pg client in the process, including a host application's own. Registering on the connection scopes it to the pools this driver opened; verified with a second, plain pg.Client in the same process still receiving a Date.
  • No import of pg.pg is an optional peer dependency, and driver-sql builds through the shared root tsup.config.ts, which does not set shims: true — so createRequire(import.meta.url) would emit import.meta verbatim into the CJS bundle and break require('@objectstack/driver-sql') at load (the measured failure packages/metadata-protocol/tsup.config.ts documents). setTypeParser / getTypeParser are read off the pg.Client knex hands the hook instead, and the hook no-ops on a connection that does not expose them.
  • One unrelated coupling removed.withConnectBound used to return early when a client had no entry in DIALECT_CONNECT_TIMEOUT, which also skipped the session pins below it. Those answer a different question (what a value means on this connection, not how long a connect may take) and the lists differ: redshift speaks the pg wire protocol but has no timeout entry, so it silently opted out of a fix it needs. The early return is now a guarded block; sqlite's config is byte-identical either way (both transforms return their input unchanged for non-matching clients).

toDateOnly keeps its UTC-component reading and now documents that as its contract, with the measurement above inline and an explicit warning against "repairing" a date skew by switching to the local getters.

Reverse verification — the pin was seen red

Ran the new suite against the pre-fixsql-driver.ts (restored with git restore --source=origin/main; mutation confirmed on disk by grep -c withPostgresCalendarDayAsText returning 0 while getUTCFullYear still returned 1, and a lone unstaged M in git status --porcelain). No rebuild leg applies: driver-sql's vitest resolves the subject through a relative source import, not through dist/. 10 of 38 failed, in the predicted direction:

FAIL #11389 — Field.date is process-zone invariant on live postgres > reads 2026-08-24 and 2026-01-01 back unchanged under TZ=Asia/Shanghai
AssertionError: live postgres read the wrong calendar day under TZ=Asia/Shanghai: expected '2026-08-23' to be '2026-08-24'
FAIL #11389 — Field.date is process-zone invariant on live postgres > reads 2026-08-24 and 2026-01-01 back unchanged under TZ=Asia/Kolkata
AssertionError: live postgres read the wrong calendar day under TZ=Asia/Kolkata: expected '2026-08-23' to be '2026-08-24'
FAIL #11389 — what a live postgres date column materialises as > hands back strings for `date` and `date[]`, and keeps `timestamptz` an instant
AssertionError: a pg `date` must not arrive as a JS Date: expected 'object' to be 'string'

The UTC and America/New_York cells passed on pre-fix source, and so did every MySQL cell. That is the point of the matrix: the existing Temporal Conformance (live PG + MySQL) job pins TZ=America/New_York, which is west of UTC, where the pre-fix read names the right day. A TZ matrix that never runs east of UTC cannot fail this, so the suite asserts its own zone list contains an east-of-UTC cell before believing any of it. The fix was then restored with git checkout HEAD -- and re-proved byte-identical (git status --porcelain empty, git diff HEAD clean).

Tests

packages/drivers/driver-sql/src/sql-driver-11389-date-tz-skew.test.ts — 38 cases in three layers:

  1. Mechanism, serverless. The real pool.afterCreate is driven out of the real knex config with a recording connection: which OIDs get a parser and nothing else, that date comes back byte-for-byte, that date[] is bound to the connection's own text[] parser by identity, that every pg-wire client name is covered, that a host afterCreate is chained and its error surfaced, that a non-pg.Client degrades to a no-op, and that sqlite and mysql2 are untouched.
  2. The two-clock pin, serverless. Four caller-Date shapes across four process zones, on SQLite, covering both the write and filter paths. These pass on pre-fix source by design — they guard the rejected fix, not the bug.
  3. The live matrix. One connection, one row set, the process zone swept underneath it — the card's own end-to-end table, executed on live PG and live MySQL through find, distinct (the presentReadValue consumer) and a filter, plus a raw wire-form read asserting date[] and an untouched timestamptz.

Not fixed here, by nature

Rows already written through the old skew — an afterUpdate hook copying a date it had just read — still hold the wrong day. Nothing in a driver can identify them: a date written correctly and a date written through a skewed read are byte-identical on disk. That is a per-deployment data fix, and the changeset says so.

pgnative (knex's pg-native client) is deliberately outside the covered client set and was not measured — no path in this repo exercises it, and it parses on the C side where the JS parser registry may not apply.

Verification

Against live PostgreSQL 16 (timezone='Asia/Shanghai') and MySQL 8.0 (@@global.time_zone='+08:00'), process at TZ=America/New_York, OS_EXPECT_LIVE_DIALECT_MATRIX=1 — CI parity. Both servers were found down in the container (pg_lsclusters reported 16 main 5432 down plus a stale pid file; service mysql status reported stopped) and were started and configured for this run.

All of the following ran on 70c6c931ea, the head of this branch:

  • pnpm --filter @objectstack/driver-sql exec vitest run --maxWorkers=2Test Files 126 passed (126) / Tests 2549 passed (2549)
  • pnpm --filter @objectstack/driver-sql typecheck — clean
  • eslint . --no-inline-config over the whole repo, not a narrowed subset — exit 0, and eslint's own --format json accounting reports 5031 files, 0 errors, 0 warnings; both files this PR changes appear in that population with e=0 w=0
  • the path-derived gate families from node scripts/pm/dispatch-gates.mjs (which reads the change set from the merge base itself), all green: check:changeset-gate-self-tests, check:objectui-changeset, check:doc-anchors, check:doc-authoring, check:doc-formula-expressions, check:doc-security-posture, check:docs-audit-scope, check:docs-redirects, check:role-word, check:published-readme-links, check:published-files, check:slot-lookup, check:test-source-alias, check:type-source-resolution, check:cross-package-test-inputs, check:driver-conformance, check:empty-state, check:liveness, check:strictness-ledger, check:variant-docs, check-adr-0087-registration, check-changeset-no-major, check-empty-changeset, check-ci-filter-parity, check-doc-frontmatter, check-plugin-teardown-shape, check-section-landing-index, docs-audit/check-affected-docs
  • the convention-triggered families a new test file moves: check:query-options-erasure, check:engine-double-contract, check:where-matcher, check:type-check-coverage, check:nul-bytes — all green

Filed, not fixed here


Generated by Claude Code

…local-midnight Date
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RfyXxZ2WPjcjhuXpiQQc3y
… process timezone
Plus the changeset for the driver-sql calendar-day fix.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RfyXxZ2WPjcjhuXpiQQc3y
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/driver-sql, touching 7 documentable anchor(s).

6 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/data-modeling/drivers.mdx(via SqlDriver (symbol))
  • content/docs/data-modeling/index.mdx(via SqlDriver (symbol))
  • content/docs/plugins/packages.mdx(via SqlDriver (symbol))
  • content/docs/protocol/kernel/index.mdx(via SqlDriver (symbol))
  • content/docs/protocol/kernel/lifecycle.mdx(via SqlDriver (symbol))
  • content/docs/protocol/objectql/query-syntax.mdx(via SqlDriver (symbol))

1 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v17.mdx(via SqlDriver (symbol))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • the SDK route bridge reached 45 of 222 client-bound route-ledger rows — the other 177 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run: node scripts/docs-audit/affected-docs.mjs --bridge-coverage

Coarse fallback — 9 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json ba8420b58d36077fe79ca8ce0f201bb31f8be2bcpackageMentionDocs.

Which tree this was computed on

This run read content/docs from a5c5d83644d5c52062395a5c6136bcbdbdf57f9b — the merge of head 70c6c931ea0f53c2ac5cbe675f8c53e1c786ea09 into base ba8420b58d36077fe79ca8ce0f201bb31f8be2bc, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin a5c5d83644d5c52062395a5c6136bcbdbdf57f9b && git checkout a5c5d83644d5c52062395a5c6136bcbdbdf57f9b
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin ba8420b58d36077fe79ca8ce0f201bb31f8be2bc 70c6c931ea0f53c2ac5cbe675f8c53e1c786ea09 && git checkout -B drift-repro ba8420b58d36077fe79ca8ce0f201bb31f8be2bc && git merge --no-ff 70c6c931ea0f53c2ac5cbe675f8c53e1c786ea09
node scripts/docs-audit/affected-docs.mjs --json ba8420b58d36077fe79ca8ce0f201bb31f8be2bc

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs ba8420b58d36077fe79ca8ce0f201bb31f8be2bc → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Aug 24, 2026
@os-zhuang
os-zhuang marked this pull request as ready for review August 24, 2026 03:48
@os-zhuang
os-zhuang added this pull request to the merge queueAug 24, 2026
Merged via the queue into main with commit c05b40bAug 24, 2026
33 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-11389-pg-date-tz-skew branch August 24, 2026 04:04
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

2 participants

@os-zhuang@claude