Skip to content

fix(driver-sql): stamp updated_at when the driver never ran DDL (#11067) - #11177

Merged
os-zhuang merged 1 commit into
mainfrom
claude/issue-11067-timestamps-without-ddl
Aug 23, 2026
Merged

fix(driver-sql): stamp updated_at when the driver never ran DDL (#11067)#11177
os-zhuang merged 1 commit into
mainfrom
claude/issue-11067-timestamps-without-ddl

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Fixes#11067

SqlDriver.update() refreshed updated_at only for tables in tablesWithTimestamps, and every one of that set's fill sites is downstream of DDL. A skipSchemaSync / OS_SKIP_SCHEMA_SYNC=1 deployment — documented behaviour, not a misconfiguration — therefore booted with the set empty and never stamped, so updated_at recorded the row's creation time forever.

The fill sites: there are four, not three

Re-derived at the lines on 927ccbb23 rather than taken from the card. The card names three; the rotation branch inside initObjects is a fourth:

#SiteWhy it is DDL-only
1initObjects, createTable branchwe just built the table
2initObjects, "existing table already has updated_at" branchdecided from a physical columnInfo()
3initObjects, rotation branch (immediately before ensureRotation)the one the card missed
4aliasShardBookkeepingthe rotation-shard copy of (3)

I touched none of them. They are the observations, and they stay exactly as they are — a table this driver's DDL built is never in the new speculative state at all, so the DDL path is unchanged. The fix adds a parallel updatedAtColumnState map for tables the driver knows about without having run DDL against them.

1 — Registration-time inference, at zero round trips

registerManagedObjectMetadata() is the one place a managed object reaches the driver on every boot posture: initObjects calls it first and then issues DDL, and a skipSchemaSync boot (#10995's registerObjectMetadata()) calls it and stops. Every table this driver's own DDL creates gets created_at/updated_at unconditionally, so registration now records updatedAtColumnState[table] = 'presumed'. In-memory, no probe — the currency skipSchemaSync exists to save.

Kept out of tablesWithTimestamps deliberately: that set means observed, and conflating an inference with an observation is what would make the next reader trust it too far. Registration never downgrades an answer already resolved from the database, so re-driving it on reload is idempotent.

2 — How the lazy fallback distinguishes the missing-column failure

It asks the database, never the error text. The first stamped UPDATE to a presumed table is speculative; if it fails, resolveUpdatedAtColumnPresence() runs one columnInfo() — the same call initObjects already uses to decide the identical fact on the DDL path — and only a definitive absent triggers recovery. Anything else rethrows the original error untouched.

That matters because the three dialects spell the same failure three different ways, measured in the ablation below:

sqlite SqliteError: … - no such column: updated_at
pg error: … - column "updated_at" of relation "os11067_nocol" does not exist
mysql Error: … - Unknown column 'updated_at' in 'field list'

A fallback keyed to those strings would look correct and silently stop recovering on the next server upgrade. Three further properties:

  • An empty column set answers null, not "absent". No columns means the table is not visible to us at all — the caller rethrows the write's own error, which names the real problem.
  • Retrying is safe. A single UPDATE is atomic on all three dialects: one that fails to compile or plan changed no rows, so re-issuing it without the stamp cannot double-apply. The retry is the caller's own payload minus updated_at — byte-identical to what main sends today.
  • It happens at most once per table. A successful stamped UPDATE proves the column exists (a column named in a SET list that is not there is a parse/plan error on every dialect here, whatever the row count), so success settles the table as present; a resolved absence settles it as absent and is never re-probed. §3 of the pin measures this as a round-trip budget rather than claiming it.

The caller's transaction survives it

On Postgres any statement error aborts the whole transaction (25P02), so a bare try/catch whose recovery issues SQL on the same transaction can never run there — the recovery statement is the one that raises the error you observe. When options.transaction is set, the speculative write is fenced in a knex nested transaction (a SAVEPOINT) through the existing attemptWithoutPoisoning, #8269's mechanism applied to the second speculative write in this driver. Outside a caller transaction no fence is used: knex runs the statement in its own implicit transaction, so a failure is already isolated, and a savepoint there would be a cost the flag exists to avoid.

Two deliberate narrowings

  • The insert path is untouched.stampInsertTimestamps also writes created_at, and none of the evidence above says anything about created_at — an UPDATE's success proves only that updated_at exists. It keeps reading tablesWithTimestamps exactly as before, so no new insert-path rejection is possible.
  • Federated/external objects are untouched.registerExternalObject does not route through managed registration, so a remote table is never presumed to carry audit columns.

rotatedUpdateById shares the one decision helper but threads no fallback, and that is a property of the path rather than an omission: rotationShardsOf returns shards only once ensureRotation has run, and site 3 records the stronger fact on the line immediately before that call — so the presumption can never be the source there.

Before/after measurement

The card's repro sketch, as a real pin (sql-driver-timestamps-without-ddl.test.ts): out-of-band create table, a SqlDriver that never runs initObjects, create() a row, backdate it to 2020-01-01T00:00:00.000Z, update() through the driver, read updated_at back.

Backdating rather than sleeping is how "let time pass" is modelled: knex.fn.now() compiles to MySQL's CURRENT_TIMESTAMP, which carries no fractional digits, so an insert default of current_timestamp(3) and an update a few hundred ms later can legitimately land on the same or an earlier stored value. The sentinel removes that race without weakening the assertion — the stamp either moved to ~now or it did not move at all, and those are six years apart.

Baseline, on unmodified 927ccbb23 — red on all three dialects, one identical failure:

FAIL #11067 — updated_at without DDL (sqlite) > §1 …
FAIL #11067 — updated_at without DDL (live postgres) > §1 …
FAIL #11067 — updated_at without DDL (live mysql) > §1 …
AssertionError: expected 1577836800000 to be greater than 1577836800000
Tests 3 failed | 18 passed (21)

1577836800000 is the backdated sentinel: updated_at had not moved at all.

After:Test Files 1 passed (1) · Tests 21 passed (21).

Dialects measured on

Not SQLite-only — updated_at is dialect-dependent in the code under test (this.isSqlite ? new Date().toISOString() : this.knex.fn.now()). Every leg runs across all three cells through declareDialectCell, so an unprovisioned dialect is reported rather than omitted:

cellserververified
sqlitebetter-sqlite3, :memory:
live postgresPostgreSQL 16.13
live mysqlMySQL 8.0.46

The whole package was then run under CI's exact posture for the Temporal Conformance (live PG + MySQL) required check — PG server Asia/Shanghai, MySQL server +08:00, process TZ=America/New_York, OS_EXPECT_LIVE_DIALECT_MATRIX=1 (the three-way zone skew the matrix's non-vacuity guard demands):

Test Files 114 passed (114)
Tests 2384 passed (2384)

Ablation — the fallback leg is not vacuous

The leg that decides the tier is §2, so it was mutated rather than trusted. updatedAtStampIsPresumed() was forced to false (registration-time inference left in place, fallback disabled), the mutation confirmed on disk by anchored grep counts (anchor 1→0, marker 0→1) before the run, and restored from a pristine copy by an EXIT INT TERM trap. No rebuild is involved: the pin imports ../src/index.js by relative path, so dist/ is not on the resolution path.

Predicted before running — §2 and §5 red, §1/§4/§6 green. Observed:

× §2 still updates a hand-migrated table that has NO `updated_at` column [×3 cells]
× §5 leaves the caller's transaction usable when the fallback fires inside it [×3 cells]
× §3 … [×3 cells] ← cascade: §3 asserts NO_COL is already settled, which §2 does
Tests 9 failed | 12 passed (21)

Those §2 failures are the new rejection the pair exists to prevent — the three dialect errors quoted above are exactly what a user's working update() would have started returning had option 1 shipped bare. Restore verified after the run (marker 0, anchor 1).

Gates

Union re-run after the final commit, at 0cb8abfc6:

  • pnpm lint (repo-wide eslint . --no-inline-config, no narrowing) · check:type-check-debt --re-measure (33 ledger entries, none above its recorded number)
  • check:adr-anchors (added by hand — the ADR-0120 roster on sql-driver.ts is not path-derivable) · check:driver-conformance · check:slot-lookup · check:test-source-alias · check:type-source-resolution · check:type-check-coverage
  • check:query-options-erasure · check:engine-double-contract · check:where-matcher · check:nul-bytes (the four convention-triggered by a new test file; the ratchets report "baseline key set verified against 927ccbb: no files added" — no ceiling was raised)
  • check:changeset-gate-self-tests · check:objectui-changeset · check-adr-0087-registration · check-changeset-no-major · check-empty-changeset · check-ci-filter-parity · check-plugin-teardown-shape · check-affected-docs
  • ✅ 41 downstream consumer packages (pnpm --filter '...@objectstack/driver-sql' typecheck — the prefix form, dependents)

Out of scope

Option 3 from the card — having the out-of-band migration path declare the fact — is a deployment-contract change and is not opened here.

Two unrelated updated_at gaps were found while measuring this one and are filed as #11176, not fixed here: updateMany() never stamps, and upsert()'s merge branch does not advance it on Postgres/MySQL (measured on live PG 16.13, on the fully DDL-managed path).


Generated by Claude Code

…1067)
`update()` refreshed `updated_at` only for tables in `tablesWithTimestamps`,
and all FOUR of that set's fill sites are downstream of DDL (the card said
three; `initObjects`' rotation branch is the fourth). A `skipSchemaSync` /
`OS_SKIP_SCHEMA_SYNC=1` boot — documented behaviour, not a misconfiguration —
therefore served every UPDATE with the set empty and never stamped, so
`updated_at` recorded the row's creation time forever.
Ships the pair:
1. `registerObjectMetadata()` records the declared-shape expectation in a new
`updatedAtColumnState` map, at zero round trips. Kept apart from
`tablesWithTimestamps`, which means "observed", not "inferred".
2. The first stamped UPDATE to such a table is speculative. On failure the
driver asks the database (`columnInfo()`) whether `updated_at` is really
absent — never the dialect's error text — and only then re-issues the
caller's own statement unstamped. Any other failure rethrows the original
error. Without (2), a hand-migrated table lacking the column would turn a
working `update()` into a new rejection.
A success proves the column exists, so one round settles the table; an absence
is cached and never re-probed. When a caller transaction is open the
speculative write is fenced in a SAVEPOINT via `attemptWithoutPoisoning`,
because Postgres aborts the whole transaction on any statement error (#8269).
The insert path (`stampInsertTimestamps`, which also writes `created_at`) and
federated objects (`registerExternalObject`) are deliberately untouched.
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 10 documentable anchor(s).

16 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json 10485009a693b07cf90c98e81a0d8b824c62be95.

3 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • 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 10485009a693b07cf90c98e81a0d8b824c62be95packageMentionDocs.

Which tree this was computed on

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

⚠️ 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 10485009a693b07cf90c98e81a0d8b824c62be95 → 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-zhuang
os-zhuang marked this pull request as ready for review August 23, 2026 00:52
@os-zhuang
os-zhuang added this pull request to the merge queueAug 23, 2026
Merged via the queue into main with commit 479fba5Aug 23, 2026
32 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-11067-timestamps-without-ddl branch August 23, 2026 01:10
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

Development

Successfully merging this pull request may close these issues.

driver-sql: updated_at is never refreshed on a deployment that skips boot schema sync — tablesWithTimestamps is also only filled by DDL

2 participants

@os-zhuang@claude