Uh oh!
There was an error while loading. Please reload this page.
fix(driver-sql): advance updated_at on updateMany() and on upsert()'s merge branch - #11240
Conversation
… merge branch (#11176) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RfyXxZ2WPjcjhuXpiQQc3y
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RfyXxZ2WPjcjhuXpiQQc3y
📓 Docs Drift CheckThis PR changes 1 package(s): 11 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 3 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 9 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 12af2830372c802bc8b2dd6d458b75b1e68dbf08 && git checkout 12af2830372c802bc8b2dd6d458b75b1e68dbf08
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 38cf397ea9b1b5aea338805e7559f70576b88fb1 653ab3058ba24284698da43a5086c03fefad42eb && git checkout -B drift-repro 38cf397ea9b1b5aea338805e7559f70576b88fb1 && git merge --no-ff 653ab3058ba24284698da43a5086c03fefad42eb
node scripts/docs-audit/affected-docs.mjs --json 38cf397ea9b1b5aea338805e7559f70576b88fb1
|
Uh oh!
There was an error while loading. Please reload this page.
Fixes#11176
Two write doors in
packages/drivers/driver-sql/src/sql-driver.tsleft "last modified" reading the row's previous value, on every deployment — DDL-managed or not. That is what separates them from #11067, whose defect neededskipSchemaSync; #11067's fix (merged as #11177) is in the base here and neither introduces nor absorbs these.The two defects
1.
updateMany()stamped nothing. NostampsUpdatedAtconsultation, noupdated_at.update(),bulkUpdate()androtatedUpdateById()all stamp; this door did not. A bulk edit therefore left every row it touched reading its creation time.2.
upsert()'s merge branch did not advance it on Postgres and MySQL. The merge set is derived from the KEYS of the formatted payload, so a column absent from the payload is absent fromON CONFLICT ... DO UPDATE.stampInsertTimestampsis the only thing that putupdated_atthere, and it returns early on any non-SQLite dialect (if (!this.isSqlite ...) return) because the column DEFAULT already stores a zone-aware instant on insert. A DEFAULT does not re-fire on the conflict path. SQLite was accidentally correct; Postgres and MySQL were not — the merge site's own comment claimed otherwise ("Everything else (incl.updated_at) merges as before"), and that comment is corrected in place.Nothing errored either way, which is why it went unnoticed: list-view sorts, delta/incremental sync, cache invalidation and audit answers simply read a stale
updated_at. A bulk status change, and a sync/import that upserts, are exactly the operations most likely to be feeding a downstream delta consumer.Measured, live, before and after
PostgreSQL 16.13 (
timezone='Asia/Shanghai') and MySQL 8.0.46 (time_zone='+08:00'), processTZ=America/New_York,OS_EXPECT_LIVE_DIALECT_MATRIX=1, tables built by the driver's owninitObjectssotablesWithTimestampsis correctly populated. Baseline taken with the driver source restored tomain(git restore --source=origin/main, absence of the new symbols proved on disk by grep before the run) and the new test file present:The direction was predicted before running and is recorded in the test file's head note. The asymmetry is the point: a test that only ran on SQLite would have been green before and after.
What the fix does
updateManyreuses driver-sql:updated_atis never refreshed on a deployment that skips boot schema sync —tablesWithTimestampsis also only filled by DDL #11067's decision whole rather than re-deriving it:stampsUpdatedAtdecides,keepSuppliedUpdatedAthonours 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 opt-in historical import, andupdateWithPresumedTimestampcarries the speculative case so a hand-migrated table genuinely lacking the column keeps working. The statement is defined once as anissueclosure so the speculative attempt, the fenced retry and the plain path cannot drift in WHERE or tenant scope — the same shapeupdate()uses. When nothing is stamped the payload object is passed through untouched, so a timestamp-less object's statement is byte-identical to whatmainemits.upsertgainsstampUpsertUpdatedAt, which fillsupdated_aton every dialect. It fills only an EMPTY slot, matchingstampInsertTimestamps, so a caller-supplied value is preserved and the SQL emitted for SQLite is unchanged.Two deliberate narrowings, both stated because they are decisions
a. The upsert stamp reads OBSERVED presence, never #11067's
presumedstate (observedUpdatedAtColumn, notstampsUpdatedAt).presumedexists so an UPDATE can speculate and then RECOVER; the upsert door has no such recovery, and a wrong presumption there would name a missing column in an INSERT column list — turning a call that works today into a hard failure. Net effect: on askipSchemaSyncdeployment the upsert door stamps only after some stamped UPDATE has settled the table aspresent. That fixes fewer cases; it breaks none. Section 6 of the test pins the property that would break first if this were ever widened without a recovery.b. The upsert stamp uses
upsertUpdatedAtStamp(), notupdatedAtStamp(), and the difference is one digit of precision on MySQL. The value lands in the INSERT payload too, andupdatedAtStamp()'s bareknex.fn.now()compiles to an unqualifiedCURRENT_TIMESTAMPthat MySQL truncates to whole seconds — against aDATETIME(3)column whose DEFAULT isnow(3). Using it here would make a freshly INSERTED row'supdated_atread up to 999 ms EARLIER than itscreated_at: a new defect on a branch that had none. Section 4 measures this. The UPDATE door's own truncation is pre-existing, is NOT changed here, and is filed as #11224.On-hold neighbours: neither restart condition fired
upsert's empty-merge-set fallback is merge-ALL, which re-admits every insert-only column — currently unreachable, but it is the wrong shape for "nothing to merge" #8740 (upsert's empty-merge-set fallback is merge-ALL) — restart isinsertOnlyUpsertColumnsgaining a member, a fourth SQL dialect, or a conflict-target column gaining a non-null DEFAULT. This adds a MERGEABLE column to the payload; the insert-only set is untouched. Not triggered, not repaired. One consequence worth recording, and the drivers(sql): an upsert that merges on a non-PK conflict key silently REWRITES the existing row's primary key — measured on SQLite and MySQL alike #8622 note in the code says it: a timestamped object no longer reaches that fallback on Postgres or MySQL either (as it already did not on SQLite). The branch stays reachable, and stays drivers(sql):upsert's empty-merge-set fallback is merge-ALL, which re-admits every insert-only column — currently unreachable, but it is the wrong shape for "nothing to merge" #8740's to decide, for an object with no observedupdated_atcolumn.sqliteCanonicalDatetimeSqlorbackfillCanonicalDatetimes. Neither symbol is in this diff. Not triggered.The card's second observation: measured, reported, deliberately not fixed
The card asked whether
updateManypassingdatathrough withoutformatInput/applyWriteColumnMapis deliberate or a second defect. It is a second defect — three of them — and it is filed as #11223, not fixed here: it changes which VALUES a caller may write, not whetherupdated_atadvances. Measured live:22P02 invalid input syntax for type json) and on SQLite ("SQLite3 can only bind numbers, strings, bigints, buffers, and null"), whereupdate()writes them correctly.columnMapobject's bulk update names a column that does not exist — the WHERE is mapped and the SET is not, in one statement:update `legacy_p` set `name` = 'Bulk' where `full_name` = 'Renamed'givesno such column: name.update()writes"2026-03-04T05:06:07.000Z",updateMany()writes"2026-05-06 07:08:09"raw. That is the pre-[17.0.0-rc.0] SQLite datetime window filters return empty: filter comparands coerced to epoch-ms while writes store ISO TEXT #3912 zone-naive formneedsLegacyDatetimeRepairexists to repair on read, being newly written today, and it contradictscanonicalDatetimeFields' "proven canonical" claim.It is not load-bearing for this change: the stamp is written as the literal post-map column name (the spelling
created_atcarries everywhere in this file) and is applied after the payload is assembled, so it is correct with or without that fix. The probe output shows it landing correctly in all three statements.Verification
All measurements at
653ab3058(this branch's head,mainmerged in atdd84ddd79; branch delta vsmainre-asserted as exactly the three intended files after the merge). Every exit code captured by redirect-then-capture, never across a pipe.node scripts/pm/dispatch-gates.mjs(no paths — it derived its own 3-path change set from merge base9cc1940a1) named 13 path-derived families and 6 convention-triggered ones; all were run, pluscheck:adr-anchors(which that derivation does not select) andcheck:nul-bytes. Each gate's own verdict line, not a bare exit code:No ratchet ceiling was raised, no test skipped, disabled or quarantined.
Declared narrowing on downstream coverage.
driver-sqlhas 48 downstream dependents; rather than run that whole closure locally, the DOWNSTREAM direction (--filternaming each dependent explicitly) was run for the six selected by grepping every downstream test file forupdateManyor.upsert(together withupdated_at, plus the twoSqlDriversubclasses:The remaining dependents are CI's; the merge queue additionally rebuilds this PR as merged onto the current
mainand re-runs the required workflows on that generation.Generated by Claude Code