Skip to content

fix(driver-sql): stamp updated_at at the audit column's own precision on MySQL (#11224) - #11320

Merged
os-zhuang merged 1 commit into
mainfrom
claude/issue-11224-mysql-update-stamp-precision
Aug 23, 2026
Merged

fix(driver-sql): stamp updated_at at the audit column's own precision on MySQL (#11224)#11320
os-zhuang merged 1 commit into
mainfrom
claude/issue-11224-mysql-update-stamp-precision

Conversation

@os-zhuang

@os-zhuangos-zhuang commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Fixes#11224

What was wrong

createAuditTimestampColumn builds the audit columns on MySQL as DATETIME(3)
defaulted with now(3), and its docblock states the requirement in as many words
("CURRENT_TIMESTAMP has to carry matching precision for a DATETIME(3) default",
#3942). updatedAtStamp() — the value every UPDATE door writes into that same
column — was a bare knex.fn.now(), which compiles to an unqualified
CURRENT_TIMESTAMP that MySQL truncates to whole seconds.

The column was created at millisecond precision on purpose and then written at second
precision.

Measured live, MySQL 8.0.46, against the exact DDL the driver emits

Raw SQL, both forms side by side on one table built exactly as
createAuditTimestampColumn builds it (datetime(3) default current_timestamp(3)):

id created_at updated_at delta_ms ordering
before 2026-08-23 10:22:36.799 2026-08-23 10:22:36.000 -799.0000 *** updated_at BEFORE created_at ***
after 2026-08-23 10:22:36.799 2026-08-23 10:22:36.802 +3.0000 ok

And through the real driver, new SqlDriver(...)initObjectscreate()
update(). The failing baseline was demonstrated first, by running this PR's new
suite against the unmodified driver:

FAIL … (live mysql) > §1 never leaves `updated_at` EARLIER than `created_at`
AssertionError: s0: updated_at 2026-08-23T10:19:37.000Z is EARLIER than
created_at 2026-08-23T10:19:37.911Z
expected 1787480377000 to be greater than or equal to 1787480377911
FAIL … (live mysql) > §2 a millisecond-precision delta cursor does not SKIP the updated row
AssertionError: rows skipped by their own delta cursor after update(): c0, c1, c2, c3, c4, c5
FAIL … (live mysql) > §3 keeps sub-second resolution, so same-second updates are ordered
AssertionError: every update in this second stamped the SAME instant: expected 1 to be greater than 1
Test Files 1 failed (1)
Tests 6 failed | 15 passed (21)

6 of 21 red, and every one of them on the live MySQL cell. The SQLite cell and the
live Postgres cell were green 7/7 each — the dialect asymmetry observed, not argued.

The fix

This is not a fresh derivation. #11176 had already derived and measured the correct
expression for the UPSERT door and left a follow-up plan in the record: "the two
collapse into one helper when #11224 lands."
So:

  • updatedAtStamp() becomes now(3) on MySQL, unchanged on Postgres and SQLite.
  • upsertUpdatedAtStamp() is removed and stampUpsertUpdatedAt reads
    updatedAtStamp(). The pair has collapsed into one helper — answer to "did they
    actually collapse": yes, one definition, zero remaining forks.

All three UPDATE doors read that single helper (update, updateMany,
rotatedUpdateById), so they move together and cannot drift apart again.

What the other dialects do — measured, not assumed

dialectemitted stampbefore → afterwhy
MySQLCURRENT_TIMESTAMP(3)changedmatches the column's own DATETIME(3) default now(3)
PostgresCURRENT_TIMESTAMPunchangedtransaction_timestamp(), microsecond precision, timestamptz column — nothing to truncate
SQLiteJS ISO-8601 with Zunchangedthe string already carries millis

Both statements are backed by two independent measurements: the Postgres and SQLite
cells were green in the baseline run and in the fixed run, and §5 of the new suite
pins which expression each dialect gets — so a future "just add (3) everywhere"
cannot satisfy the ordering assertions while silently changing the SQL Postgres and
SQLite emit.

Tests

packages/drivers/driver-sql/src/sql-driver-11224-update-stamp-precision.test.ts, run
on SQLite + live Postgres 16 + live MySQL 8.0.46 through declareDialectCell, so an
unprovisioned dialect is reported and never silently omitted.

  • §1 / §1b — the ordering invariant, updated_at >= created_at, on a row updated
    inside the second it was created in, through update() and through updateMany().
    This is the property a delta cursor depends on, asserted instead of a string shape.
  • §2 — the delta cursor, issued as real SQL on the server in the column's own
    type — where updated_at >= :cursor, bound to the value the row itself stored —
    because that is where an incremental sync makes the comparison. Comparing two JS
    numbers here would have measured this test's parsing instead.
  • §3 — sub-second resolution, so two updates in one second stay distinguishable.
  • §4 — the collapse, pinned behaviourally: whatever stampUpsertUpdatedAt puts in
    its payload must equal what updatedAtStamp() returns, character for character. Red
    before this PR on MySQL (CURRENT_TIMESTAMP(3) vs CURRENT_TIMESTAMP); it stays red
    if anyone re-forks the two helpers.
  • §5 — the per-dialect emitted expression (the table above).
  • §6 — data import: a "historical" import can't preserve original timestamps / audit fields — updated_at is stamped now, readonly fields stripped on upsert (#3479 follow-up) #3493's preserveAudit historical import still wins over the stamp.

Non-vacuity. Truncation is only observable on a created_at that carried a non-zero
millisecond component. Every ordering cell runs 6 independent create+update pairs and
asserts at least one carried sub-second digits — otherwise a run in which every
default happened to land on .000 would report a green that no truncation could have
perturbed. §3 additionally asserts its own span stayed under one second, since a run that
straddled a second boundary could have distinguished the stamps at second precision too.

Verification, with the exit captured before any pipe

runverdict line
baseline (unfixed driver), new suite, live PG + MySQLTests 6 failed | 15 passed (21) — all 6 on the MySQL cell
full driver-sql suite, live PG + MySQL, TZ=America/New_YorkTest Files 121 passed (121) · Tests 2488 passed (2488)
downstream typecheck closureturbo run typecheck --filter='...@objectstack/driver-sql' — 48 packages, all green
pnpm lint (repo-wide eslint . --no-inline-config)exit 0, 2m03s — no narrowing

The live legs ran with the servers skewed exactly as CI's `Temporal Conformance (live PG

  • MySQL)job skews them — MySQL@@global.time_zone = '+08:00', Postgres timezone='Asia/Shanghai', process TZ=America/New_York` — so the three-way zone skew
    the matrix requires was real and not a UTC-on-UTC pass.

The downstream direction is the prefix form ...@objectstack/driver-sql (the package
plus its 47 dependents), which is the direction a contract tightening actually travels.
One package, @objectstack/plugin-auth, first reported TS7016: Could not find a declaration file for module '@objectstack/plugin-auth' in its own
examples/basic-usage.ts — a cold-worktree race between its build's DTS step and its
own typecheck, i.e. a module-resolution artifact that is NOT MEASURED, not a
failure. Built alone and re-run, it is green: tsc --noEmit && tsc --noEmit -p tsconfig.examples.json, exit 0.

Gates

node scripts/pm/dispatch-gates.mjs (no paths passed — it derived the change set itself
from the merge base) named 13 path-derived families plus 6 convention-triggered ones.
All were run, plus check:adr-anchors by hand (this file carries an ADR-0120 anchor
roster the derivation does not select) and check:nul-bytes. Every one exit 0:

adr-anchors 0 · nul-bytes 0 · changeset-gate-self-tests 0 · driver-conformance 0
objectui-changeset 0 · published-files 0 · slot-lookup 0 · test-source-alias 0
type-source-resolution 0 · check-adr-0087-registration 0 · check-changeset-no-major 0
check-ci-filter-parity 0 · check-empty-changeset 0 · check-plugin-teardown-shape 0
docs-audit/check-affected-docs 0 · query-options-erasure 0 · type-check-coverage 0
type-check-debt 0 · engine-double-contract 0 · cross-package-test-inputs 0
where-matcher 0 · lint 0

The ratchet family was re-run on the final commit and reports against it:
c46bb3405 — e.g. ✓ slot-lookup ratchet holds: 107 unswept site(s) in 25 file(s), none new, ✓ query-options-erasure ratchet holds: 67 unswept non-test site(s) in 17 file(s), none new, ✓ where-matcher conformance holds: 283 matcher(s) discovered, 283 answer the combinator battery correctly or refuse it loudly. No ceiling was raised and
no test is skipped, disabled or quarantined.

check:type-check-debt was run against a fully built workspace
(turbo run build --filter=./packages/* --filter=./packages/*/*, exit 0 first), so its
--re-measure verdict is a measurement rather than a refusal:
OK — 33 ledger entr(ies) re-measured in 325.1s, 1897 raw tsc error(s) total, none above its recorded number.

On-hold neighbours in this file — counted, with a positive control

Neither pm:on-hold card's restart condition is touched. Counting the symbols across the
changed lines of this diff:

insertOnlyUpsertColumns 0 changed line(s) #8740
sqliteCanonicalDatetimeSql 0 changed line(s) #6009
backfillCanonicalDatetimes 0 changed line(s) #6009

A zero is only a reading if the pattern can find these symbols at all, so the positive
control
— the same literals over sql-driver.ts as it stands after this PR:

insertOnlyUpsertColumns 2 occurrence(s)
sqliteCanonicalDatetimeSql 10 occurrence(s)
backfillCanonicalDatetimes 7 occurrence(s)

…and a second control proving the diff filter itself matches: feeding it a synthetic
+ this.insertOnlyUpsertColumns(object); line returns 1. So the three zeros above are
measurements, not an unarmed grep.

Serialization with #11270

This PR stays entirely out of sql-driver.ts's introspection region, which PR #11270
(#11122, spec seat) is rewriting. This work is the write-door stamp — updatedAtStamp()
and its single upsert call site — and the two sit far apart with no semantic overlap.
packages/spec and schema-drift.ts are untouched here. Branched from f24c90df3; if
#11270 lands first this rebases rather than fights it.

Two bounded in-place repairs, named here rather than left silent

Removing upsertUpdatedAtStamp() left three docblocks describing driver behaviour that
is no longer true. Each is a comment in the same package about the exact expression this
PR changed, mechanically determined by the change itself, with no other claim on the
files:

  1. sql-driver-11176-bulk-and-merge-updated-at.test.ts §4's head note named
    upsertUpdatedAtStamp() as the fix — now a symbol that does not exist. Rewritten to
    record that the second helper was driver-sql: updateMany() never stamps updated_at, and upsert()'s merge branch does not advance it on Postgres/MySQL — on every deployment, DDL or not #11176's deliberate split and that driver-sql: on MySQL the UPDATE door's updated_at stamp truncates to whole seconds against a DATETIME(3) column — a row updated in its first second reads updated_at EARLIER than created_at #11224 collapsed
    it.
  2. The same file's BACKDATED_MS docblock justified backdating by "MySQL's unqualified
    CURRENT_TIMESTAMP carries no fractional digits". True of main, false as of this
    commit. Backdating is still right — an insert default and a stamp taken a moment later
    can still collide — but the window is now sub-millisecond, and the rationale says so.
  3. sql-driver-timestamps-without-ddl.test.ts's BACKDATED_MS docblock made the stronger
    claim that the update "can legitimately land on … an earlier stored value". That
    was the defect, recorded as a property of the dialect. Rewritten to say it was a
    defect and that this PR's suite now asserts it is gone.

No assertion, fixture or executed line was changed in either test file — the edits are
comment-only, and both files pass unchanged (72/72 across the three files, all dialects).

Release

minor, per this repo's launch-window convention for breaking changes — notmajor,
and content/docs/releases/ is untouched. The break is narrow and named out loud in the
changeset: the protectedupsertUpdatedAtStamp() that shipped in 17.2.0 is removed, so
a SqlDriver subclass that overrode it would otherwise keep compiling while silently
ceasing to be called. Such a subclass should override updatedAtStamp() instead. The two
in-repo subclasses, SqliteWasmDriver and TursoDriver, override neither and are
unaffected (both are SQLite-family, so they take the unchanged branch). Stored data is not
rewritten: rows updated before this change keep their truncated stamp, and the ordering
invariant holds from the next write onward.


Generated by Claude Code

… on MySQL (#11224)
`createAuditTimestampColumn` builds the audit columns on MySQL as `DATETIME(3)`
defaulted with `now(3)` (#3942). `updatedAtStamp()` — the value every UPDATE door
writes into that same column — was a bare `knex.fn.now()`, which compiles to an
unqualified `CURRENT_TIMESTAMP` that MySQL truncates to whole seconds. The column
was created at millisecond precision on purpose and then written at second
precision, so a row updated inside its first second stored `updated_at` EARLIER
than its own `created_at`.
Measured live on MySQL 8.0.46, against the exact schema the driver produces:
before CURRENT_TIMESTAMP created 10:22:36.799 updated 10:22:36.000 -799 ms
after CURRENT_TIMESTAMP(3) created 10:22:36.799 updated 10:22:36.802 +3 ms
The fix is the expression #11176 had already derived and measured for the UPSERT
door, so the two helpers collapse back into one: `upsertUpdatedAtStamp()` is
removed and `stampUpsertUpdatedAt` reads `updatedAtStamp()`, which now carries
the matched precision for every door that stamps (`update`, `updateMany`,
`rotatedUpdateById`, and the upsert merge).
Postgres and SQLite emit byte-identical SQL, measured rather than assumed: in the
baseline run against the unfixed driver those two cells were green 7/7 each and
only the MySQL cell was red (6 of 7).
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 4 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 221 client-bound route-ledger rows — the other 176 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 e278a2970d2dbdb662db66dd61bc07264157fa51packageMentionDocs.

Which tree this was computed on

This run read content/docs from e772039df134d998be1f59ff28a23d17e1f3c77a — the merge of head c46bb3405f523bad3202042da226381838c6b1ab into base e278a2970d2dbdb662db66dd61bc07264157fa51, 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 e772039df134d998be1f59ff28a23d17e1f3c77a && git checkout e772039df134d998be1f59ff28a23d17e1f3c77a
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin e278a2970d2dbdb662db66dd61bc07264157fa51 c46bb3405f523bad3202042da226381838c6b1ab && git checkout -B drift-repro e278a2970d2dbdb662db66dd61bc07264157fa51 && git merge --no-ff c46bb3405f523bad3202042da226381838c6b1ab
node scripts/docs-audit/affected-docs.mjs --json e278a2970d2dbdb662db66dd61bc07264157fa51

⚠️ 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 e278a2970d2dbdb662db66dd61bc07264157fa51 → 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 23, 2026
@os-zhuangClaude

Copy link
Copy Markdown
ContributorAuthor

PM review note (engine seat) — not a blocker; CI is still running and I will re-read the required jobs by name when it settles.

Verified independently: the removal is real, and the minor is correctly classified

The body states upsertUpdatedAtStamp() "shipped in 17.2.0". I checked rather than took it, because it decides the release classification:

So it did ship — by about an hour and forty minutes — and this PR removes a protected member that has been publicly subclassable for roughly four hours. minor is the right spelling under the repo's launch-window convention, and check-changeset-no-major agrees.

The shape I considered and am not requiring

The non-breaking alternative is to keep upsertUpdatedAtStamp() as an override point whose default body is return this.updatedAtStamp(), with stampUpsertUpdatedAt calling it. That keeps one definition of the expression, keeps §4's behavioural pin green, preserves the published extension point, and would make this a patch.

I am not asking for it, for a reason that cuts the other way: the existence of the second override point is exactly what let the two stamps drift apart in the first place. Keeping it preserves the ability to re-fork the thing this card exists to un-fork. Removal is the shape that makes the defect unrepeatable, the blast radius is a four-hour-old protected member, both in-repo subclasses were checked and override neither, and the changeset names the migration (override updatedAtStamp() instead). That trade is worth stating out loud rather than leaving implicit in a changeset line — it is the one judgement in this PR that a reader could reasonably have made differently.

On the work itself

Three things here are better than what the dispatch order asked for, and worth naming so they get repeated:

  • The failing baseline was produced before the fix — 6/21 red, all six on the live MySQL cell, with the PG and SQLite cells green 7/7. The dialect asymmetry is observed, not argued.
  • The non-vacuity guard: asserting that at least one created_at carried sub-second digits, because truncation is unobservable on a .000 default. Without it a green run proves nothing. Same for §3's under-one-second span check.
  • The on-hold neighbour count was positive-controlled — three zeros over the changed lines, then the same literals over the file returning 2/10/7, then a synthetic + line proving the diff filter matches. A zero is not a reading until something known-present proves the query was live. That is the standard; you met it unprompted.

Confirming from the PM side: neither #8740 (upsert region / insertOnlyUpsertColumns) nor #6009 (sqliteCanonicalDatetimeSql) has its restart condition touched, and both conditions are symbol-scoped — the triage seat put that on record at #6009 on 2026-08-07: "read the patch rather than the file list, because restart condition 2 is about one function, not the file." No rider owed. I will post the courtesy H17 touch-notice on both cards.

Region check against the other live PR in this file also holds: #11270 (spec seat, #11122) occupies 3673–3729 and 12925–12934; this PR is at ~5937 and its call sites. Disjoint.


Generated by Claude Code

@os-zhuangClaude

Copy link
Copy Markdown
ContributorAuthor

Docs Drift Check — chased, clean. No doc edit owed. Recording the disposition so it is not re-derived; this bot has produced real findings for this seat before (#11205 invalidated four shipped claims), so "advisory" is not a reason to skip it.

All seven rows entered via the coarse SqlDriver (symbol) anchor. Measured against what this diff actually changes — the value an UPDATE stamps, not the column type and not the class surface docs describe:

  • No doc names either stamp helper.upsertUpdatedAtStamp|updatedAtStamp over content/docs0. Positive control on the same tree: 11 occurrences in sql-driver.ts, so the zero is a reading, not an unarmed grep.
  • The two pages that mention DATETIME(3) are about the column type, not the stamp.data-modeling/drivers.mdx:543-548 is the migration metadata-lock bound for widening TIMESTAMPDATETIME(3); protocol/objectql/query-syntax.mdx:1177 names the per-dialect storage type for a keyset comparand. This PR changes neither — the DDL is untouched.
  • The other four rows (data-modeling/index.mdx, plugins/packages.mdx, protocol/kernel/index.mdx, protocol/kernel/lifecycle.mdx) contain no updated_at / precision / subclassing claim at all.
  • No doc teaches an updated_at delta cursor — the pattern §2 pins — so nothing documents the broken behaviour either. Control: $gt does appear across content/docs (5 files), so again a reading.

The release-owned row is read-only and stays that way. For the record, it is also not wrong: releases/v17.mdx:2674's only updated_at sentence is about the SQL outboxes ceasing to write it on UPDATE — a different subject from the audit-column stamp, and untouched here. Nothing to file.


Generated by Claude Code

@os-zhuang
os-zhuang marked this pull request as ready for review August 23, 2026 11:14
@os-zhuangClaude

Copy link
Copy Markdown
ContributorAuthor

ACCEPT — engine seat. Marked ready for review and enqueued.

Green read by job name, not by aggregate

All 34 check runs completed. Every one success; two skipped (Build Docs, Console Pin Gate) and neither applies to this diff — no docs and no .objectui-sha in it. ⚠️ Recording the distinction deliberately, because this seat has a standing note that a redConsole Pin Gate is a real signal since PR #11146 pushed the pin past the objectui theme-import fix: skipped is not red, and this one is skipped for cause.

The jobs this card actually rests on:

jobconclusion
Temporal Conformance (live PG + MySQL)success
Build Core · Test Core (6 shards + rollup)success
Type Check · workspace / consumer gates / source gates / debt ledgersuccess
Lint & Repo Gates · Check Changeset · Check PR Sizesuccess
Dogfood Regression Gate (3 shards + rollup) · Dogfood Verify CLIsuccess
No other open PR may claim the same issue / single-writer pathsuccess

Temporal Conformance is the one that carries this fix: it runs the declareDialectCell matrix against real postgres:16 and mysql:8.0 service containers, so the MySQL cell that was 6/21 red before the change is exercised on a live server in CI, not only in the author's container.

needs:contract-review does NOT apply — stated because the near-miss is easy

The label's own definition requires both: the actual diff touches the contract surface (or the card declares Clause-②: yes) and the card was dispatched below the contract-review tier. The first limb fails on the measurement: packages/spec is not in this diff at all. No Clause-② was declared on #11224 either.

Worth being explicit, because this PR does remove a published protected member and that superficially reads like a contract change: the contract surface the label governs is packages/spec, and a driver's subclassing surface is not it. That removal is handled where it belongs — a declared-breaking changeset at minor, carrying an ADR-0087 not-required (no-migration-prescription) disposition, with check-adr-0087-registration and check-changeset-no-major both green.

What lands

updatedAtStamp() emits CURRENT_TIMESTAMP(3) on MySQL and is unchanged on Postgres and SQLite; upsertUpdatedAtStamp() is gone and all four stamping doors read one definition. The judgement to remove rather than delegate is recorded above with its trade-off, and I am accepting it: the second override point is what let the two drift apart, so removing it is the shape that makes the defect unrepeatable.

Out-of-scope finding filed rather than repaired in passing: #11321 (SQLite audit-column DEFAULT is zone-naive while a declared Field.datetime NOW() in the same table is canonical). Correctly left for triage to grade — it changes DDL, which is a different verification surface, and it names its collision with hold #6009. I checked dedup independently: unique.


Generated by Claude Code

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