From 0cb8abfc625e1e0f6017bf8817a4496247c1bad7 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 23 Aug 2026 00:18:13 +0000 Subject: [PATCH] fix(driver-sql): stamp `updated_at` when the driver never ran DDL (#11067) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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 Claude-Session: https://claude.ai/code/session_01RfyXxZ2WPjcjhuXpiQQc3y --- .../driver-sql-updated-at-without-ddl.md | 77 ++++ .../sql-driver-timestamps-without-ddl.test.ts | 354 ++++++++++++++++++ packages/drivers/driver-sql/src/sql-driver.ts | 285 +++++++++++++- 3 files changed, 700 insertions(+), 16 deletions(-) create mode 100644 .changeset/driver-sql-updated-at-without-ddl.md create mode 100644 packages/drivers/driver-sql/src/sql-driver-timestamps-without-ddl.test.ts diff --git a/.changeset/driver-sql-updated-at-without-ddl.md b/.changeset/driver-sql-updated-at-without-ddl.md new file mode 100644 index 0000000000..41e212c903 --- /dev/null +++ b/.changeset/driver-sql-updated-at-without-ddl.md @@ -0,0 +1,77 @@ +--- +"@objectstack/driver-sql": patch +--- + +fix(driver-sql): `updated_at` is stamped on a deployment that never runs the driver's DDL (#11067) + +`SqlDriver.update()` refreshed `updated_at` only for tables in +`tablesWithTimestamps`, and every one of that set's **four** fill sites is +downstream of DDL: `initObjects`' `createTable` branch, its "the existing table +already has an `updated_at` column" branch (decided from a physical +`columnInfo()`), its rotation branch, and `aliasShardBookkeeping`'s +rotation-shard copy. (The card reported three; the rotation branch inside +`initObjects` is the fourth.) + +So a deployment that manages DDL out-of-band — `skipSchemaSync` / +`OS_SKIP_SCHEMA_SYNC=1`, documented in +`content/docs/deployment/environment-variables.mdx` as "skip the implicit +`db:sync` on boot; use after running migrations manually" — booted with that set +empty and never stamped. The column carries only an INSERT-time `DEFAULT now()`, +with no `ON UPDATE` clause or trigger on any dialect, so `updated_at` recorded +the row's **creation** time forever. Nothing errored: list-view sorts, delta and +incremental sync, cache invalidation and audit answers were simply wrong. +Measured before the fix on SQLite, live Postgres 16.13 and live MySQL 8.0.46 — +a row backdated to `2020-01-01T00:00:00Z` and then updated through the driver +came back still reading `2020-01-01T00:00:00Z` on all three. + +The fix is a pair, and the second half is what keeps it a bug fix rather than a +contract change. + +1. **Inferred from the declared shape, at registration time.** + `registerObjectMetadata()` — the DDL-free entry point a `skipSchemaSync` boot + already calls — now records that a managed object's table is *expected* to + carry `updated_at`, because every table this driver's own DDL creates gets + `created_at`/`updated_at` unconditionally. That costs **zero round trips**, + which is the currency `skipSchemaSync` exists to save. It is kept in a new + `updatedAtColumnState` map rather than in `tablesWithTimestamps`, because it + is an inference and that set means "observed". + +2. **A lazy, one-shot fallback for the table where the inference is wrong.** On + a hand-migrated table that genuinely lacks the column, (1) alone would turn an + `update()` that succeeds today into a loud failure — a *new rejection for a + call that works*. Instead, the first stamped UPDATE to such a table is + speculative: if it fails, the driver asks the database (`columnInfo()`) + whether `updated_at` is really absent, and only then re-issues the caller's + own statement without the stamp, logging a warning naming the divergence. Any + other failure rethrows the **original** error untouched. Deliberately not + keyed to the dialect's error text: the three dialects spell it three ways + (`42703`, `ER_BAD_FIELD_ERROR`, `no such column`), and those strings are + version-dependent. + +Steady state is free in both directions. 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 one success settles the +table permanently; a resolved absence is cached and never re-probed. Tables the +driver's DDL built were already in `tablesWithTimestamps` and never enter the +speculative state at all, so the DDL path is byte-for-byte unchanged. + +When the caller has a transaction open, the speculative write is fenced in a +knex nested transaction (a `SAVEPOINT`) via the existing +`attemptWithoutPoisoning` — on Postgres any statement error aborts the whole +transaction (`25P02`), so an unfenced `try/catch` whose recovery issues SQL on +that transaction could never run there (#8269). + +Two narrowings, both deliberate: + +- **The insert path is untouched.** `stampInsertTimestamps` writes `created_at` + as well, and none of the evidence above says anything about `created_at`, so + it keeps reading `tablesWithTimestamps` exactly as before. +- **Federated/external objects are untouched.** `registerExternalObject` does + not route through managed registration, so a remote table is never presumed to + carry audit columns. + +Pinned by `sql-driver-timestamps-without-ddl.test.ts`, which runs the card's +repro sketch plus the missing-column leg, the round-trip budget, the +caller-transaction leg and an unrelated-failure leg across SQLite **and** live +Postgres / MySQL through `declareDialectCell`, so an unprovisioned dialect is +reported rather than omitted. diff --git a/packages/drivers/driver-sql/src/sql-driver-timestamps-without-ddl.test.ts b/packages/drivers/driver-sql/src/sql-driver-timestamps-without-ddl.test.ts new file mode 100644 index 0000000000..93b2a4b1d2 --- /dev/null +++ b/packages/drivers/driver-sql/src/sql-driver-timestamps-without-ddl.test.ts @@ -0,0 +1,354 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#11067] `updated_at` must advance on a deployment that never runs the + * driver's DDL — and must keep working on a hand-migrated table that genuinely + * has no `updated_at` column. + * + * ## The defect, re-derived at the lines rather than taken from the card + * + * `SqlDriver.update()` stamps `updated_at` only for tables in + * `tablesWithTimestamps`. That set is filled in FOUR places (the card says + * three), and every one of them is downstream of DDL: + * + * 1. `initObjects`' `createTable` branch — the table we just built; + * 2. `initObjects`' "the existing table already has an `updated_at` column" + * branch, decided from a physical `columnInfo()`; + * 3. `initObjects`' rotation branch, just before `ensureRotation`; + * 4. `aliasShardBookkeeping`, the rotation-shard copy of (3). + * + * So a deployment that manages DDL out-of-band — `skipSchemaSync` / + * `OS_SKIP_SCHEMA_SYNC=1`, documented in + * `content/docs/deployment/environment-variables.mdx` as "skip the implicit + * `db:sync` on boot; use after running migrations manually" — boots with the + * set EMPTY and never stamps. The column carries only an INSERT-time + * `DEFAULT now()`, with no `ON UPDATE` clause or trigger on any dialect, so + * `updated_at` records the row's CREATION time forever. Consumers are wrong + * without being unavailable: list-view sorts, delta/incremental sync, cache + * invalidation, audit answers. + * + * ## What is pinned here, and why each leg exists + * + * §1 The card's repro sketch as a measurement: an out-of-band table, a driver + * that never runs `initObjects`, `create()` → backdate → `update()` → the + * stamp moved. Backdating to a sentinel instant is how "let time pass" is + * modelled: it is deterministic on every dialect, where a real sleep races + * MySQL's `CURRENT_TIMESTAMP` second granularity. + * + * §2 The other half of the pair, and the leg that decides the tier. On a + * hand-migrated table genuinely LACKING the column, inferring from the + * declared shape alone would turn an `update()` that succeeds today into a + * loud failure — a NEW rejection for a call that works. The lazy fallback is + * what keeps that from happening, so it is pinned directly: the update must + * still succeed, still write the caller's data, and resolve the question + * ONCE. + * + * §3 The steady-state cost. §1's updates must issue ZERO introspection + * round-trips — that is the currency `skipSchemaSync` exists to save, and a + * fix that probes every table on first write spends it. §2's first update + * may probe exactly once; every later one must not. + * + * §4 An object the driver was never told about at all stays untouched — the + * inference is keyed to registration, not to "any table name that reaches + * `update()`". + * + * §5 The caller's transaction survives the fallback. On Postgres ANY statement + * error aborts the whole transaction (`25P02`), so a `try/catch` whose + * recovery issues SQL on the same transaction can never run there — the + * hazard `attemptWithoutPoisoning` was built for (#8269). Pinned on live PG, + * where it is real, and run on the other cells too. + * + * Every cell runs on SQLite AND on live Postgres / MySQL through + * `declareDialectCell`, so an unprovisioned dialect is REPORTED rather than + * omitted. `updated_at` is dialect-dependent in the code under test + * (`this.isSqlite ? new Date().toISOString() : this.knex.fn.now()`), which is + * why this is not a SQLite-only pin. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { SqlDriver } from '../src/index.js'; +import { DIALECT_CELLS, declareDialectCell, type DialectCell } from './live-dialect-matrix.testkit.js'; + +/** Driver options every write here uses — these fixtures are not tenant-scoped. */ +const OPTS = { bypassTenantAudit: true }; + +/** + * The instant a row is backdated to before the `update()` under test. + * + * A sentinel far in the past rather than a sleep: `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. Backdating + * removes the race without weakening what is asserted: the stamp either moved + * to ~now or it did not move at all, and those are six years apart. + */ +const BACKDATED_MS = Date.parse('2020-01-01T00:00:00.000Z'); + +/** The object as an out-of-band migration would have declared it. */ +function prefObject(name: string) { + return { + name, + fields: { + id: { type: 'text' }, + key: { type: 'text' }, + value: { type: 'json' }, + }, + } as any; +} + +/** + * `create table t (id text primary key, key text, value jsonb, created_at + * timestamptz default now(), updated_at timestamptz default now())` — the + * card's sketch, spelled in each dialect's own types, exactly as a hand-written + * migration would. `withUpdatedAt: false` is the hand-migrated table that + * genuinely lacks the column. + * + * The audit columns take the SAME physical types `createAuditTimestampColumn` + * would have produced (`DATETIME(3)` on MySQL — #3942 — and the canonical + * ISO-8601 `strftime` default on SQLite), so this fixture models a migration + * that got the schema right, not one that diverged from the driver. + */ +function outOfBandDdl(cell: DialectCell, table: string, withUpdatedAt: boolean): string { + const audit = (name: string) => { + switch (cell.id) { + case 'mysql': + return `\`${name}\` datetime(3) default current_timestamp(3)`; + case 'pg': + return `"${name}" timestamptz default now()`; + default: + return `"${name}" text default (strftime('%Y-%m-%dT%H:%M:%fZ','now'))`; + } + }; + const cols = [audit('created_at')]; + if (withUpdatedAt) cols.push(audit('updated_at')); + if (cell.id === 'mysql') { + return ( + `create table \`${table}\` (id varchar(64) primary key, \`key\` varchar(255), ` + + `value json, ${cols.join(', ')})` + ); + } + const jsonType = cell.id === 'pg' ? 'jsonb' : 'text'; + return `create table "${table}" (id text primary key, "key" text, value ${jsonType}, ${cols.join(', ')})`; +} + +/** Whatever the dialect handed back for an audit column, as epoch ms. */ +function asInstant(value: unknown): number { + if (value instanceof Date) return value.getTime(); + if (typeof value === 'number') return value; + const text = String(value); + // SQLite stores TEXT. The canonical form already carries `Z`; a zone-naive + // legacy form is read as UTC here rather than as host-local, matching what + // `repairNaiveUtcAuditTimestamp` does on the read path. + const parsed = Date.parse(/[zZ]|[+-]\d{2}:?\d{2}$/.test(text) ? text : `${text.replace(' ', 'T')}Z`); + return parsed; +} + +/** Read the audit columns straight out of storage, past every read-side coercion. */ +async function readAudit(driver: SqlDriver, table: string, id: string) { + const row: any = await (driver as any).knex(table).where('id', id).first(); + return { + createdAt: asInstant(row.created_at), + updatedAt: row.updated_at === undefined ? undefined : asInstant(row.updated_at), + row, + }; +} + +/** Force `updated_at` (or `created_at`) back to the sentinel, bypassing the driver. */ +async function backdate(driver: SqlDriver, table: string, id: string, columns: string[]): Promise { + const patch: Record = {}; + const iso = new Date(BACKDATED_MS).toISOString(); + for (const c of columns) { + patch[c] = (driver as any).isSqlite ? iso : new Date(BACKDATED_MS); + } + await (driver as any).knex(table).where('id', id).update(patch); +} + +/** + * Count the SCHEMA-INTROSPECTION statements a block issues. + * + * `columnInfo()` compiles to `information_schema.columns` on Postgres and + * MySQL and to `PRAGMA table_info` on SQLite, so one predicate covers all three + * cells. This is what makes §3 a measurement of the round-trip budget rather + * than a claim about it. + */ +async function countIntrospections(driver: SqlDriver, run: () => Promise): Promise { + const knex = (driver as any).knex; + let seen = 0; + const onQuery = (q: any) => { + if (/information_schema|pragma\s+table_info/i.test(String(q?.sql ?? ''))) seen += 1; + }; + knex.on('query', onQuery); + try { + await run(); + } finally { + knex.removeListener('query', onQuery); + } + return seen; +} + +function measure(cell: DialectCell): void { + describe(`#11067 — updated_at without DDL (${cell.label})`, () => { + let driver: SqlDriver; + // Short, dialect-safe table names: MySQL's identifier rules are the tightest. + const WITH_COL = 'os11067_with'; + const NO_COL = 'os11067_nocol'; + const UNREGISTERED = 'os11067_unreg'; + const TX_TABLE = 'os11067_tx'; + + beforeAll(async () => { + driver = new SqlDriver(cell.config()); + const knex = (driver as any).knex; + for (const [table, withUpdatedAt] of [ + [WITH_COL, true], + [NO_COL, false], + [UNREGISTERED, true], + [TX_TABLE, false], + ] as const) { + await knex.raw(outOfBandDdl(cell, table, withUpdatedAt)); + } + // THE LINE UNDER TEST — the whole of what a `skipSchemaSync` boot does for + // these objects. No CREATE TABLE, no ALTER TABLE, no round-trip. + // `UNREGISTERED` is deliberately absent from this list (§4). + driver.registerObjectMetadata([prefObject(WITH_COL), prefObject(NO_COL), prefObject(TX_TABLE)]); + }); + + afterAll(async () => { + await driver?.disconnect(); + }); + + // ── §1 The card's repro sketch, measured ──────────────────────────────── + + it('§1 advances `updated_at` on a table the driver never ran DDL against', async () => { + const id = 'w1'; + await driver.create(WITH_COL, { id, key: 'ui.recent', value: { a: 1 } }, OPTS); + await backdate(driver, WITH_COL, id, ['created_at', 'updated_at']); + const before = await readAudit(driver, WITH_COL, id); + expect(before.updatedAt).toBe(BACKDATED_MS); + + await driver.update(WITH_COL, id, { key: 'ui.pinned' }, OPTS); + + const after = await readAudit(driver, WITH_COL, id); + // The defect: today this equals BACKDATED_MS — the row's creation time, + // frozen forever. + expect(after.updatedAt).toBeGreaterThan(BACKDATED_MS); + expect(after.updatedAt).toBeGreaterThan(Date.now() - 10 * 60_000); + // `created_at` is the row's birth instant and the update must not disturb it. + expect(after.createdAt).toBe(BACKDATED_MS); + // …and the caller's own patch still landed. + expect(after.row.key).toBe('ui.pinned'); + }); + + it('§1b keeps an explicit `updated_at` under `preserveAudit` (historical import)', async () => { + // #3493: the opt-in historical import is the one caller allowed to pin the + // value. The inference must not force-advance past it. + const id = 'w2'; + await driver.create(WITH_COL, { id, key: 'k', value: null }, OPTS); + const supplied = new Date(BACKDATED_MS); + await driver.update( + WITH_COL, + id, + { key: 'k2', updated_at: (driver as any).isSqlite ? supplied.toISOString() : supplied }, + { ...OPTS, preserveAudit: true } as any, + ); + const after = await readAudit(driver, WITH_COL, id); + expect(after.updatedAt).toBe(BACKDATED_MS); + }); + + // ── §2 The pair's second half: the column genuinely is not there ──────── + + it('§2 still updates a hand-migrated table that has NO `updated_at` column', async () => { + const id = 'n1'; + await driver.create(NO_COL, { id, key: 'ui.recent', value: { a: 1 } }, OPTS); + + // The call that must NOT become a new rejection. + const returned = await driver.update(NO_COL, id, { key: 'ui.pinned' }, OPTS); + expect(returned).toBeTruthy(); + + const after = await readAudit(driver, NO_COL, id); + expect(after.updatedAt).toBeUndefined(); + expect(after.row.key).toBe('ui.pinned'); + + // And it keeps working — the negative answer is remembered, not re-derived + // into a second failure. + await driver.update(NO_COL, id, { key: 'ui.third' }, OPTS); + expect((await readAudit(driver, NO_COL, id)).row.key).toBe('ui.third'); + }); + + // ── §3 The round-trip budget, measured rather than claimed ────────────── + + it('§3 spends ZERO introspection round-trips on the healthy table, and at most one on the other', async () => { + const healthy = await countIntrospections(driver, async () => { + await driver.create(WITH_COL, { id: 'w3', key: 'k', value: null }, OPTS); + await driver.update(WITH_COL, 'w3', { key: 'k2' }, OPTS); + await driver.update(WITH_COL, 'w3', { key: 'k3' }, OPTS); + }); + // The currency `skipSchemaSync` exists to save. A fix that probes every + // table on first write spends it on every table. + expect(healthy).toBe(0); + + // §2 already resolved NO_COL, so by now it is settled and free. + const settled = await countIntrospections(driver, async () => { + await driver.update(NO_COL, 'n1', { key: 'k4' }, OPTS); + await driver.update(NO_COL, 'n1', { key: 'k5' }, OPTS); + }); + expect(settled).toBe(0); + }); + + // ── §4 Registration is what arms the inference ────────────────────────── + + it('§4 leaves an object the driver was never told about exactly as it was', async () => { + const id = 'u1'; + await (driver as any).knex(UNREGISTERED).insert({ id, key: 'k', value: null }); + await backdate(driver, UNREGISTERED, id, ['created_at', 'updated_at']); + + await driver.update(UNREGISTERED, id, { key: 'k2' }, OPTS); + + const after = await readAudit(driver, UNREGISTERED, id); + // Unchanged behaviour: nothing told this driver the object exists, so it + // makes no claim about the physical shape and stamps nothing. + expect(after.updatedAt).toBe(BACKDATED_MS); + expect(after.row.key).toBe('k2'); + }); + + // ── §5 The fallback must not poison a caller's transaction ────────────── + + it("§5 leaves the caller's transaction usable when the fallback fires inside it", async () => { + // Postgres aborts the WHOLE transaction on any statement error (`25P02`), + // so this is the leg that decides whether the recovery is safe at all. + const id = 't1'; + const trx = await driver.beginTransaction(); + try { + await driver.create(TX_TABLE, { id, key: 'k', value: null }, { ...OPTS, transaction: trx } as any); + await driver.update(TX_TABLE, id, { key: 'k2' }, { ...OPTS, transaction: trx } as any); + // The transaction is still usable AFTER the speculative write failed and + // was recovered — this statement is the proof. + await driver.update(TX_TABLE, id, { key: 'k3' }, { ...OPTS, transaction: trx } as any); + await driver.commit(trx); + } catch (error) { + await driver.rollback(trx).catch(() => {}); + throw error; + } + const after = await readAudit(driver, TX_TABLE, id); + expect(after.row.key).toBe('k3'); + expect(after.updatedAt).toBeUndefined(); + }); + + // ── §6 A real dialect fault is still a real dialect fault ─────────────── + + it('§6 does not swallow a write failure that has nothing to do with `updated_at`', async () => { + // The fallback asks the database whether `updated_at` exists rather than + // reading the error text, and rethrows the ORIGINAL error when it does. + // Without this leg the recovery could turn any failing update into a + // silent unstamped one. + const id = 'w4'; + await driver.create(WITH_COL, { id, key: 'k', value: null }, OPTS); + await expect( + driver.update(WITH_COL, id, { no_such_column_11067: 'x' } as any, OPTS), + ).rejects.toBeTruthy(); + }); + }); +} + +for (const cell of DIALECT_CELLS) { + declareDialectCell(cell, 'timestamps without DDL (#11067)', measure); +} diff --git a/packages/drivers/driver-sql/src/sql-driver.ts b/packages/drivers/driver-sql/src/sql-driver.ts index 5244d6eb37..e0e03aa885 100644 --- a/packages/drivers/driver-sql/src/sql-driver.ts +++ b/packages/drivers/driver-sql/src/sql-driver.ts @@ -3966,6 +3966,48 @@ export class SqlDriver implements IDataDriver { /** External columnMap inverse: physical remote column -> logical field (for read output remap). */ protected columnFieldByObject: Record> = {}; protected tablesWithTimestamps: Set = new Set(); + /** + * [#11067] What is known about `updated_at` on a table this driver was told + * about WITHOUT running DDL against it. + * + * ## Why {@link tablesWithTimestamps} could not answer this + * + * That set means "OBSERVED to carry the audit columns", and every one of its + * four fill sites is downstream of DDL: `initObjects`' `createTable` branch + * (we built the table, so we know), its "the existing table already has an + * `updated_at` column" branch (decided from a physical `columnInfo()`), its + * rotation branch, and `aliasShardBookkeeping`'s rotation-shard copy. A + * deployment that manages DDL out-of-band — `skipSchemaSync` / + * `OS_SKIP_SCHEMA_SYNC=1`, the documented posture after running migrations + * manually — reaches none of them, so it served every UPDATE with the set + * empty and never stamped. `updated_at` then records the row's CREATION time + * forever: the column carries an INSERT-time `DEFAULT now()` and no dialect + * here gives it an `ON UPDATE` clause or a trigger. Nothing errors; list-view + * sorts, delta sync, cache invalidation and audit answers are just wrong. + * + * ## The three states, and what may move between them + * + * - `presumed` — {@link registerManagedObjectMetadata} put it here from the + * DECLARED shape alone, at zero round-trips: every table this driver's own + * DDL creates gets `created_at`/`updated_at` unconditionally, so a managed + * object registered with us is expected to have them. That is an inference, + * not an observation, which is the whole reason this is a separate map + * rather than more entries in `tablesWithTimestamps`. + * - `present` — a stamped UPDATE SUCCEEDED. A column named in a SET list that + * does not exist is a parse/plan error on every dialect here, independent + * of how many rows matched, so the success IS the proof. From then on the + * table is settled and costs nothing. + * - `absent` — {@link resolveUpdatedAtColumnPresence} asked the database and + * it is genuinely not there (a hand-migrated table that diverged). Never + * stamped again, and never re-probed. + * + * ⛔ Deliberately NOT read by {@link stampInsertTimestamps}. That helper writes + * `created_at` too, and none of the evidence above says anything about + * `created_at` — promoting on an UPDATE's success would let it write a column + * this driver has no reason to believe exists. The insert path keeps reading + * `tablesWithTimestamps` exactly as before. + */ + protected updatedAtColumnState: Map = new Map(); /** Tables this driver created since connect — see `getSchemaSyncStats`. */ protected tablesCreatedHere: Set = new Set(); /** Tables that were already present when this driver first touched them. */ @@ -5812,27 +5854,218 @@ export class SqlDriver implements IDataDriver { return options?.preserveAudit === true && formatted.updated_at != null; } + /** + * The canonical instant an UPDATE stamps into `updated_at` on this dialect. + * + * On SQLite (no native timestamp type) full ISO-8601 WITH an explicit `Z` — + * matching the insert paths ({@link stampInsertTimestamps}) so create and + * update agree on one zone-explicit format. The previous + * `…replace('T',' ').replace('Z','')` wrote a zone-NAIVE, space-separated + * string that `Date.parse` reads as LOCAL time, silently shifting the instant + * by the host offset on a non-UTC runtime (the objectos freshness-probe + * miss). Postgres/MySQL keep native `now()` — a real zone-aware TIMESTAMP + * that never had the issue. + */ + protected updatedAtStamp(): string | Knex.Raw { + return this.isSqlite ? new Date().toISOString() : this.knex.fn.now(); + } + + /** + * [#11067] Should an UPDATE to `object` refresh `updated_at`? + * + * `true` on the DDL-observed tables exactly as before, and now also on a + * table whose declared shape says it has the column — see + * {@link updatedAtColumnState} for why the second answer is kept apart from + * the first. `absent` (the database was asked and said no) and "never heard + * of this object" both stay `false`. + */ + protected stampsUpdatedAt(object: string): boolean { + if (this.tablesWithTimestamps.has(object)) return true; + const state = this.updatedAtColumnState.get(object); + return state === 'presumed' || state === 'present'; + } + + /** + * [#11067] Is that stamp still a PRESUMPTION — i.e. must this write carry the + * fallback? + * + * Only for a table in the `presumed` state and outside `tablesWithTimestamps`. + * Once a stamped UPDATE has succeeded the state is `present` and this is + * `false` forever after, which is what keeps the steady state free. + */ + protected updatedAtStampIsPresumed(object: string): boolean { + return !this.tablesWithTimestamps.has(object) && this.updatedAtColumnState.get(object) === 'presumed'; + } + + /** + * [#11067] Ask the DATABASE whether `object`'s physical table carries + * `updated_at`. `null` when the question could not be answered. + * + * ⛔ This is deliberately not a read of the failure's message. Every dialect + * spells the missing column differently (`42703` on Postgres, + * `ER_BAD_FIELD_ERROR` on MySQL, `SQLITE_ERROR: no such column` on SQLite), + * those strings are localizable and version-dependent, and a fallback keyed + * to them would silently stop recovering on the next server upgrade while + * looking correct. `columnInfo()` answers the actual question, in the + * dialect's own terms, and is the same call `initObjects` already uses to + * decide the identical fact on the DDL path. + * + * An empty column set means the table is not visible to us at all (it does + * not exist, or we cannot see it) — NOT that `updated_at` is missing from a + * table that is otherwise fine. Answering `null` there sends the caller back + * to rethrowing the original error, which names the real problem. + * + * Runs on the caller's transaction when there is one, so it observes the same + * snapshot — and, on Postgres, so it runs on a transaction that is still + * usable (see {@link updateWithPresumedTimestamp}). + */ + protected async resolveUpdatedAtColumnPresence( + object: string, + trx?: Knex.Transaction, + ): Promise { + const physical = this.physicalTableByObject[object] ?? object; + const runner = trx ?? this.knex; + let builder = runner(physical); + const remoteSchema = this.physicalSchemaByObject[object]; + if (remoteSchema) builder = builder.withSchema(remoteSchema); + let info: Record; + try { + info = (await builder.columnInfo()) as Record; + } catch { + // The probe itself failed — we learned nothing, so we may not claim the + // column is absent. The caller rethrows the write's own error. + return null; + } + const columns = Object.keys(info ?? {}); + if (columns.length === 0) return null; + return columns.includes('updated_at'); + } + + /** + * [#11067] Issue an UPDATE whose `updated_at` stamp is a PRESUMPTION, and + * recover if the column turns out not to be there. + * + * ## Why this exists rather than option 1 on its own + * + * Inferring from the declared shape is free and right for every table this + * driver's DDL would have built. On a hand-migrated table that genuinely + * lacks `updated_at` it is wrong, and wrong LOUDLY: an `update()` that + * succeeds today (without stamping) would start failing, because the driver + * would name a column that does not exist. That is a NEW REJECTION for a call + * that works — not something a bug fix may ship. This method is what keeps + * the net observable change to "`updated_at` is now correctly stamped on any + * physical shape". + * + * ## Why the recovery is safe to retry + * + * A single UPDATE is atomic on all three dialects: one that fails to compile + * or plan has changed no rows, so re-issuing it without the stamp cannot + * double-apply anything. The recovery re-issues the caller's own payload + * minus `updated_at` — the exact statement `main` sends today. + * + * ## Why the speculative write is fenced when a caller transaction is open + * + * On Postgres ANY statement error aborts the WHOLE transaction: every + * subsequent statement returns `25P02 current transaction is aborted` until + * rollback. So a bare `try { … } catch { …recover… }` whose recovery issues + * SQL on the same transaction can never run there — the recovery statement is + * the one that raises the error you observe. {@link attemptWithoutPoisoning} + * wraps the attempt in a knex nested transaction (a `SAVEPOINT`, released on + * success and rolled back to on failure), which leaves the outer transaction + * usable on every dialect. This is #8269's mechanism applied to the second + * speculative write in this driver. + * + * Outside a caller transaction no fence is needed: knex runs the statement in + * its own implicit transaction, so a failure is already isolated — and paying + * for a savepoint on the ordinary write path would be a cost the flag exists + * to avoid. + * + * ## Why 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`. Either way the table leaves the + * speculative state and every later write goes straight down the plain path, + * with no probe and no fence. + * + * @param issue re-issues the UPDATE for a given payload and options — the + * caller owns the WHERE and the tenant scope, so this method never rebuilds + * them and cannot get them wrong. + */ + protected async updateWithPresumedTimestamp( + object: string, + formatted: Record, + options: DriverOptions | undefined, + issue: (payload: Record, options?: DriverOptions) => Promise, + ): Promise { + const unstamped = (): Record => { + const { updated_at: _dropped, ...rest } = formatted; + return rest; + }; + const settleAbsent = (): void => { + this.updatedAtColumnState.set(object, 'absent'); + this.logger.warn( + `[sql-driver] '${object}' has no physical 'updated_at' column, so this driver will not ` + + 'stamp one on update (#11067). The object is registered as managed, whose schema this ' + + "driver's own DDL would give `created_at`/`updated_at` — a table migrated out-of-band " + + 'without them will keep a stale "last modified" for every consumer that reads it ' + + '(list-view sorts, delta sync, cache invalidation, audit). Add the column, or accept ' + + 'that it is not tracked here.', + ); + }; + + const parentTrx = options?.transaction as Knex.Transaction | undefined; + if (parentTrx) { + const attempt = await this.attemptWithoutPoisoning(parentTrx, (scoped) => + issue(formatted, { ...options, transaction: scoped }), + ); + if (attempt.ok) { + this.updatedAtColumnState.set(object, 'present'); + return attempt.value; + } + const present = await this.resolveUpdatedAtColumnPresence(object, parentTrx); + if (present !== false) throw attempt.error; + settleAbsent(); + return issue(unstamped(), options); + } + + try { + const affected = await issue(formatted, options); + this.updatedAtColumnState.set(object, 'present'); + return affected; + } catch (error) { + const present = await this.resolveUpdatedAtColumnPresence(object); + if (present !== false) throw error; + settleAbsent(); + return issue(unstamped(), options); + } + } + async update(object: string, id: string | number, data: Record, options?: DriverOptions): Promise { this.auditMissingTenant(object, 'update', options); const rotationShards = this.rotationShardsOf(object); if (rotationShards) return this.rotatedUpdateById(object, rotationShards, id, data, options); - const builder = this.getBuilder(object, options).where('id', id); - this.applyTenantScope(builder, object, options); const formatted = this.applyWriteColumnMap(object, this.formatInput(object, data)); - if (this.tablesWithTimestamps.has(object) && !this.keepSuppliedUpdatedAt(formatted, options)) { - // Canonical instant format. On SQLite (no native timestamp type) stamp - // full ISO-8601 WITH an explicit `Z` — matching the insert paths - // (`stampInsertTimestamps`) so create and update agree on one - // zone-explicit format. The previous `…replace('T',' ').replace('Z','')` - // wrote a zone-NAIVE, space-separated string that `Date.parse` reads as - // LOCAL time, silently shifting the instant by the host offset on a - // non-UTC runtime (the objectos freshness-probe miss). Postgres/MySQL keep - // native `now()` — a real zone-aware TIMESTAMP that never had the issue. - formatted.updated_at = this.isSqlite ? new Date().toISOString() : this.knex.fn.now(); - } + // One definition of the statement, so the speculative attempt, the fenced + // retry and the plain path cannot drift in WHERE or tenant scope. + const issue = (payload: Record, issueOptions?: DriverOptions): Promise => { + const builder = this.getBuilder(object, issueOptions).where('id', id); + this.applyTenantScope(builder, object, issueOptions); + return builder.update(payload) as unknown as Promise; + }; - await builder.update(formatted); + if (this.stampsUpdatedAt(object) && !this.keepSuppliedUpdatedAt(formatted, options)) { + formatted.updated_at = this.updatedAtStamp(); + if (this.updatedAtStampIsPresumed(object)) { + await this.updateWithPresumedTimestamp(object, formatted, options, issue); + } else { + await issue(formatted, options); + } + } else { + await issue(formatted, options); + } const readback = this.getBuilder(object, options).where('id', id); this.applyTenantScope(readback, object, options); @@ -6739,8 +6972,15 @@ export class SqlDriver implements IDataDriver { options?: DriverOptions, ): Promise { const formatted = this.applyWriteColumnMap(object, this.formatInput(object, data)); - if (this.tablesWithTimestamps.has(object) && !this.keepSuppliedUpdatedAt(formatted, options)) { - formatted.updated_at = this.isSqlite ? new Date().toISOString() : this.knex.fn.now(); + // [#11067] One definition of the decision, shared with {@link update}. No + // fallback is threaded here, and that is a property of the path rather than + // an omission: `rotationShardsOf` returns shards only once `ensureRotation` + // has run, and `initObjects` records the stronger `tablesWithTimestamps` + // fact on the line immediately before that call — so on this path the + // answer can never come from the declared-shape presumption, and there is + // nothing speculative to recover from. + if (this.stampsUpdatedAt(object) && !this.keepSuppliedUpdatedAt(formatted, options)) { + formatted.updated_at = this.updatedAtStamp(); } for (const shard of shards) { const builder = this.getBuilder(shard, options).where('id', id); @@ -7889,6 +8129,19 @@ export class SqlDriver implements IDataDriver { this.numericFields[tableName] = numericCols; this.autoNumberFields[tableName] = autoNumberCols; this.tenantFieldByTable[tableName] = tenantField; + // [#11067] The declared shape's answer to "does this table carry + // `updated_at`?", installed here because here is the one place a managed + // object reaches the driver on EVERY boot posture — `initObjects` calls + // this first, and a `skipSchemaSync` boot calls it and stops. Presumed + // rather than asserted (see {@link updatedAtColumnState}); DDL that + // OBSERVES the column still records the stronger fact in + // `tablesWithTimestamps`, so a table this driver built is never in the + // speculative state at all. Never downgrades an answer already resolved + // from the database — re-registration is idempotent metadata assignment + // and must not throw away a `columnInfo()` result. + if (!this.updatedAtColumnState.has(tableName)) { + this.updatedAtColumnState.set(tableName, 'presumed'); + } return { tableName, tenantField }; }