Skip to content

Commit 479fba5

Browse files
os-zhuangclaude
andauthored
fix(driver-sql): stamp updated_at when the driver never ran DDL (#11067) (#11177)
`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. Claude-Session: https://claude.ai/code/session_01RfyXxZ2WPjcjhuXpiQQc3y Co-authored-by: Claude <noreply@anthropic.com>
1 parent ff9da91 commit 479fba5

3 files changed

Lines changed: 700 additions & 16 deletions

File tree

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
---
2+
"@objectstack/driver-sql": patch
3+
---
4+
5+
fix(driver-sql): `updated_at` is stamped on a deployment that never runs the driver's DDL (#11067)
6+
7+
`SqlDriver.update()` refreshed `updated_at` only for tables in
8+
`tablesWithTimestamps`, and every one of that set's **four** fill sites is
9+
downstream of DDL: `initObjects`' `createTable` branch, its "the existing table
10+
already has an `updated_at` column" branch (decided from a physical
11+
`columnInfo()`), its rotation branch, and `aliasShardBookkeeping`'s
12+
rotation-shard copy. (The card reported three; the rotation branch inside
13+
`initObjects` is the fourth.)
14+
15+
So a deployment that manages DDL out-of-band — `skipSchemaSync` /
16+
`OS_SKIP_SCHEMA_SYNC=1`, documented in
17+
`content/docs/deployment/environment-variables.mdx` as "skip the implicit
18+
`db:sync` on boot; use after running migrations manually" — booted with that set
19+
empty and never stamped. The column carries only an INSERT-time `DEFAULT now()`,
20+
with no `ON UPDATE` clause or trigger on any dialect, so `updated_at` recorded
21+
the row's **creation** time forever. Nothing errored: list-view sorts, delta and
22+
incremental sync, cache invalidation and audit answers were simply wrong.
23+
Measured before the fix on SQLite, live Postgres 16.13 and live MySQL 8.0.46 —
24+
a row backdated to `2020-01-01T00:00:00Z` and then updated through the driver
25+
came back still reading `2020-01-01T00:00:00Z` on all three.
26+
27+
The fix is a pair, and the second half is what keeps it a bug fix rather than a
28+
contract change.
29+
30+
1. **Inferred from the declared shape, at registration time.**
31+
`registerObjectMetadata()` — the DDL-free entry point a `skipSchemaSync` boot
32+
already calls — now records that a managed object's table is *expected* to
33+
carry `updated_at`, because every table this driver's own DDL creates gets
34+
`created_at`/`updated_at` unconditionally. That costs **zero round trips**,
35+
which is the currency `skipSchemaSync` exists to save. It is kept in a new
36+
`updatedAtColumnState` map rather than in `tablesWithTimestamps`, because it
37+
is an inference and that set means "observed".
38+
39+
2. **A lazy, one-shot fallback for the table where the inference is wrong.** On
40+
a hand-migrated table that genuinely lacks the column, (1) alone would turn an
41+
`update()` that succeeds today into a loud failure — a *new rejection for a
42+
call that works*. Instead, the first stamped UPDATE to such a table is
43+
speculative: if it fails, the driver asks the database (`columnInfo()`)
44+
whether `updated_at` is really absent, and only then re-issues the caller's
45+
own statement without the stamp, logging a warning naming the divergence. Any
46+
other failure rethrows the **original** error untouched. Deliberately not
47+
keyed to the dialect's error text: the three dialects spell it three ways
48+
(`42703`, `ER_BAD_FIELD_ERROR`, `no such column`), and those strings are
49+
version-dependent.
50+
51+
Steady state is free in both directions. A successful stamped UPDATE proves the
52+
column exists — a column named in a `SET` list that is not there is a parse/plan
53+
error on every dialect here, whatever the row count — so one success settles the
54+
table permanently; a resolved absence is cached and never re-probed. Tables the
55+
driver's DDL built were already in `tablesWithTimestamps` and never enter the
56+
speculative state at all, so the DDL path is byte-for-byte unchanged.
57+
58+
When the caller has a transaction open, the speculative write is fenced in a
59+
knex nested transaction (a `SAVEPOINT`) via the existing
60+
`attemptWithoutPoisoning` — on Postgres any statement error aborts the whole
61+
transaction (`25P02`), so an unfenced `try/catch` whose recovery issues SQL on
62+
that transaction could never run there (#8269).
63+
64+
Two narrowings, both deliberate:
65+
66+
- **The insert path is untouched.** `stampInsertTimestamps` writes `created_at`
67+
as well, and none of the evidence above says anything about `created_at`, so
68+
it keeps reading `tablesWithTimestamps` exactly as before.
69+
- **Federated/external objects are untouched.** `registerExternalObject` does
70+
not route through managed registration, so a remote table is never presumed to
71+
carry audit columns.
72+
73+
Pinned by `sql-driver-timestamps-without-ddl.test.ts`, which runs the card's
74+
repro sketch plus the missing-column leg, the round-trip budget, the
75+
caller-transaction leg and an unrelated-failure leg across SQLite **and** live
76+
Postgres / MySQL through `declareDialectCell`, so an unprovisioned dialect is
77+
reported rather than omitted.

0 commit comments

Comments
 (0)