Skip to content

Commit 1da39f5

Browse files
baozhoutaoclaude
baozhoutao
and
claude
authored
fix(metadata): isMissingTableError no longer reads Postgres' write-path missing-COLUMN phrase as a missing TABLE (#6347) (#6613)
* fix(metadata): stop judging Postgres' write-path missing-COLUMN phrase as a missing TABLE (#6347) `MISSING_TABLE`'s message test demands table/relation next to `does not exist`, but Postgres' write-path phrasing — `column "label" of relation "sys_team" does not exist` — carries a complete legal missing-table phrase as a SUBSTRING, so it matched. The function's own docblock names 42703 as a failure that must stay loud. A message regex can never exclude a superstring, so the repair is a front-exclusion evaluated before any positive test: the column-level SQLSTATEs the docblock already names (42703 / 42704 / 3D000) and the `"x" of relation "y"` sub-object phrasing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KDU3qAuJyajAQm3GkUXdfA * docs(metadata): record the measured reverse-verification result + changeset (#6347) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KDU3qAuJyajAQm3GkUXdfA --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent a954634 commit 1da39f5

3 files changed

Lines changed: 328 additions & 3 deletions

File tree

‎.changeset/missing-table-column-of-relation.md‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
---
2+
"@objectstack/metadata": patch
3+
---
4+
5+
fix(metadata): `isMissingTableError` no longer reads Postgres' write-path missing-COLUMN message as a missing TABLE (#6347)
6+
7+
`isMissingTableError` is the single predicate that licenses a caller to treat a
8+
failed read as "the table is not provisioned yet, so there are genuinely no
9+
rows". Its own docblock names `column "x" does not exist` (SQLSTATE 42703) as a
10+
real failure that must stay loud — "a case where 'start numbering at 1' would be
11+
the wrong answer against a table that may be full of rows" — and the code did
12+
not honour that, in one direction only.
13+
14+
Postgres has **two** missing-column phrasings:
15+
16+
| path | message | judged |
17+
|:---|:---|:---|
18+
| read (`SELECT`) | `column "bogus" does not exist` | correctly NOT a missing table |
19+
| write (`INSERT`/`UPDATE`/`ALTER`) | `column "label" of relation "sys_team" does not exist` | **wrongly** a missing table |
20+
21+
The write-path phrase contains a complete, legal missing-table phrase —
22+
`relation "sys_team" does not exist` — as a substring, so the table-scoped
23+
message test matched it. The code channel did not rescue it either: the matcher
24+
is a sequential OR, so an error carrying `code: '42703'` falls past both code
25+
lines and is decided by its message. The same superstring covers every other
26+
sub-object of a relation Postgres phrases this way, e.g.
27+
`constraint "uq_x" of relation "sys_team" does not exist` (42704).
28+
29+
A message regex can never exclude a superstring, so the repair is a
30+
**front-exclusion** evaluated before any positive test: the column-level
31+
SQLSTATEs the docblock already names (`42703`, `42704`, `3D000`) and the
32+
`"x" of relation "y"` sub-object phrasing. Recognising one ends the question
33+
with `false` — it does not descend into `cause`, because an error that
34+
identifies as "a column of an existing relation" is that error whatever it
35+
wraps.
36+
37+
What changes for you: a driver error of that shape now propagates instead of
38+
being silenced. Every consumer of the predicate is affected the same way, and
39+
all of them get louder rather than quieter — `DatabaseLoader.nextEventSeq` and
40+
`SysMetadataRepository`'s history counters no longer restart `event_seq` at 1,
41+
`ObjectQLEngine`'s autonumber seed no longer reseeds from 0, and the metadata
42+
loaders no longer answer "nothing declared". The set of errors judged benign
43+
shrinks; nothing that was loud becomes quiet. Genuine missing-table detection is
44+
unchanged for PostgreSQL, MySQL/MariaDB and the SQLite family.

‎packages/metadata/src/utils/schema-sync-errors.test.ts‎

Lines changed: 189 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,3 +268,192 @@ describe('the two classifications are independent, not complementary', () => {
268268
expect(isMissingTableError(err)).toBe(false);
269269
});
270270
});
271+
272+
/**
273+
* #6347 — the phrase corpus.
274+
*
275+
* The defect this pins is a **substring** one, so it cannot be caught by
276+
* checking one phrase at a time: Postgres' write-path missing-COLUMN message,
277+
* `column "label" of relation "sys_team" does not exist`, contains a complete
278+
* legal missing-TABLE phrase (`relation "sys_team" does not exist`) inside it.
279+
* Measured on `origin/main` before the fix, straight against the signature's
280+
* message regex: write-path phrase => `true`, read-path phrase
281+
* (`column "bogus" does not exist`) => `false`. Only the second half was ever
282+
* pinned (see "rejects other 'does not exist' objects" above), which is why the
283+
* first half survived — the test suite asked about the phrasing the consumer's
284+
* own read path produces, and the hole is in the other one.
285+
*
286+
* So the corpus is driven as a table, both directions in one place:
287+
*
288+
* - `LOUD` — must NEVER be judged benign. A false benign here is not a wrong
289+
* log line: `DatabaseLoader.nextEventSeq` answers `return 1` on `true`, so a
290+
* misjudged column error restarts `event_seq` at 1 against a history table
291+
* full of rows, and the colliding insert SUCCEEDS.
292+
* - `BENIGN` — must STILL be judged benign, verbatim. An exclusion is a
293+
* subtraction, and the guarded surface may not shrink while it is applied.
294+
*
295+
* Reverse verification, direction predicted BEFORE running (2026-08-08):
296+
* deleting `MISSING_TABLE.excludes` turns the write-path / sub-object rows of
297+
* `LOUD` RED, while every `BENIGN` row and every read-path row stays green.
298+
* Measured: `8 failed | 48 passed` — the direction held, and the count came in
299+
* two ABOVE the six rows predicted by counting phrases, because two rows go red
300+
* through the code channel rather than the phrase: `42703 whose message is a
301+
* genuine missing-table phrase` and the `cause`-rescue case below. Both are
302+
* part of the same repair (a code is a fact, prose is a guess) and are recorded
303+
* here rather than trimmed to fit the prediction. The read-path rows are green
304+
* in both directions on purpose: they were never the hole, and a corpus that
305+
* could not tell the two apart would be reporting the wrong repair.
306+
*/
307+
describe('isMissingTableError — Postgres sub-object phrases are NOT missing tables (#6347)', () => {
308+
/** Errors that must stay LOUD — `isMissingTableError` must answer `false`. */
309+
const LOUD: ReadonlyArray<readonly [name: string, error: unknown]> = [
310+
// ── the reported hole: the write-path (INSERT/UPDATE/ALTER) phrasing ──
311+
[
312+
'PG write-path missing column, bare message',
313+
new Error('column "label" of relation "sys_team" does not exist'),
314+
],
315+
[
316+
'PG write-path missing column, with SQLSTATE 42703',
317+
Object.assign(new Error('column "label" of relation "sys_team" does not exist'), {
318+
code: '42703',
319+
}),
320+
],
321+
[
322+
'PG write-path missing column, thrown as a bare string',
323+
'column "label" of relation "sys_team" does not exist',
324+
],
325+
[
326+
'PG write-path missing column, wrapped as `cause`',
327+
Object.assign(new Error('insert into "sys_team" failed'), {
328+
cause: Object.assign(
329+
new Error('column "label" of relation "sys_team" does not exist'),
330+
{ code: '42703' },
331+
),
332+
}),
333+
],
334+
[
335+
'PG write-path missing column, single-quoted identifiers',
336+
new Error("column 'label' of relation 'sys_team' does not exist"),
337+
],
338+
// ── the same shape for other sub-objects of an EXISTING relation ──
339+
[
340+
'PG missing constraint of a relation (42704)',
341+
Object.assign(
342+
new Error('constraint "uq_sys_team_name" of relation "sys_team" does not exist'),
343+
{ code: '42704' },
344+
),
345+
],
346+
// ── the read-path phrasing: already excluded before #6347, kept pinned ──
347+
[
348+
'PG read-path missing column, bare message',
349+
new Error('column "bogus" does not exist'),
350+
],
351+
[
352+
'PG read-path missing column, with SQLSTATE 42703',
353+
Object.assign(new Error('column "bogus" does not exist'), { code: '42703' }),
354+
],
355+
// ── code-first: 42703 means not-a-table whatever the prose says ──
356+
[
357+
'42703 with an opaque message',
358+
Object.assign(new Error('db error'), { code: '42703' }),
359+
],
360+
[
361+
'42703 whose message is a genuine missing-table phrase (code wins, loudly)',
362+
Object.assign(new Error('relation "sys_team" does not exist'), { code: '42703' }),
363+
],
364+
// ── the docblock's other named neighbours, still excluded ──
365+
[
366+
'PG missing role (42704)',
367+
Object.assign(new Error('role "app_rw" does not exist'), { code: '42704' }),
368+
],
369+
[
370+
'PG missing database (3D000)',
371+
Object.assign(new Error('database "objectstack" does not exist'), { code: '3D000' }),
372+
],
373+
// ── other dialects' missing-column prose, for completeness ──
374+
[
375+
'MySQL unknown column (ER_BAD_FIELD_ERROR)',
376+
Object.assign(new Error("Unknown column 'label' in 'field list'"), {
377+
code: 'ER_BAD_FIELD_ERROR',
378+
errno: 1054,
379+
}),
380+
],
381+
[
382+
'SQLite unknown column on a read',
383+
Object.assign(new Error('no such column: bogus'), { code: 'SQLITE_ERROR' }),
384+
],
385+
[
386+
'SQLite unknown column on a write',
387+
Object.assign(new Error('table sys_team has no column named label'), {
388+
code: 'SQLITE_ERROR',
389+
}),
390+
],
391+
];
392+
393+
/** Errors that must STILL be benign — the guarded surface, verbatim. */
394+
const BENIGN: ReadonlyArray<readonly [name: string, error: unknown]> = [
395+
['SQLite / libsql message', new Error('no such table: sys_metadata_history')],
396+
['SQLite message with a schema prefix', new Error('no such table: main.sys_metadata_history')],
397+
[
398+
'libsql remote, driver prefix in front of the phrase',
399+
new Error('SQLITE_UNKNOWN: SQLite error: no such table: sys_metadata_history'),
400+
],
401+
['PG message', new Error('relation "sys_metadata_history" does not exist')],
402+
[
403+
'PG SQLSTATE 42P01 with an opaque message',
404+
Object.assign(new Error('db error'), { code: '42P01' }),
405+
],
406+
['MySQL message', new Error("Table 'app.sys_metadata_history' doesn't exist")],
407+
[
408+
'MySQL ER_NO_SUCH_TABLE',
409+
Object.assign(new Error('opaque'), { code: 'ER_NO_SUCH_TABLE' }),
410+
],
411+
['MySQL errno 1146', Object.assign(new Error('opaque'), { errno: 1146 })],
412+
['a bare thrown string', 'no such table: sys_metadata_history'],
413+
[
414+
'genuine missing table wrapped as `cause`',
415+
Object.assign(new Error('find failed'), {
416+
cause: Object.assign(new Error('relation "sys_metadata_history" does not exist'), {
417+
code: '42P01',
418+
}),
419+
}),
420+
],
421+
];
422+
423+
it.each(LOUD)('stays loud: %s', (_name, error) => {
424+
expect(isMissingTableError(error)).toBe(false);
425+
});
426+
427+
it.each(BENIGN)('still benign: %s', (_name, error) => {
428+
expect(isMissingTableError(error)).toBe(true);
429+
});
430+
431+
it('does not let a sub-object phrase be rescued by a missing-table `cause`', () => {
432+
// Recognising "a column of an existing relation" ENDS the question: the
433+
// exclusion returns false without descending. The alternative — keep
434+
// walking and let a nested 42P01 win — would put the corrupting verdict
435+
// back one level down, reachable by any driver that wraps.
436+
const err = Object.assign(
437+
new Error('column "label" of relation "sys_team" does not exist'),
438+
{
439+
code: '42703',
440+
cause: Object.assign(new Error('relation "sys_team" does not exist'), {
441+
code: '42P01',
442+
}),
443+
},
444+
);
445+
expect(isMissingTableError(err)).toBe(false);
446+
});
447+
448+
it('leaves the DDL predicate alone — `already exists` on a column still matches', () => {
449+
// The exclusion is scoped to MISSING_TABLE. `ALREADY_EXISTS` documents
450+
// that Postgres' own `column "x" of relation "y" already exists` matches
451+
// its test, and it is genuinely benign there: the column IS provisioned.
452+
const err = Object.assign(
453+
new Error('column "environment_id" of relation "sys_metadata" already exists'),
454+
{ code: '42701' },
455+
);
456+
expect(isSchemaAlreadyExistsError(err)).toBe(true);
457+
expect(isMissingTableError(err)).toBe(false);
458+
});
459+
});

‎packages/metadata/src/utils/schema-sync-errors.ts‎

Lines changed: 95 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,21 @@ interface DriverErrorSignature {
9191
readonly errnos: ReadonlySet<number>;
9292
/** `error.message` — the only signal SQLite-family drivers give. */
9393
readonly message: RegExp;
94+
/**
95+
* Optional **front-exclusion**, evaluated before any positive test (#6347).
96+
*
97+
* A message test can never exclude a *superstring*: once a legal phrase for
98+
* X appears inside a longer phrase that means NOT-X, no amount of widening
99+
* the X regex removes the match — the phrase really is in there. The only
100+
* repair is to recognise the not-X shape first and stop. So this is a
101+
* separate channel rather than another alternation in {@link message}.
102+
*/
103+
readonly excludes?: {
104+
/** SQLSTATEs / driver codes that positively mean "**not** this case". */
105+
readonly codes: ReadonlySet<string>;
106+
/** Message shapes that carry a legal match for this case as a substring. */
107+
readonly message: RegExp;
108+
};
94109
}
95110

96111
/**
@@ -132,6 +147,27 @@ const ALREADY_EXISTS: DriverErrorSignature = {
132147
* 1" would be the wrong answer against a table that may be full of rows. So the
133148
* message test demands the word table/relation next to the phrase rather than
134149
* the phrase alone, and the code set carries only the table-scoped SQLSTATEs.
150+
*
151+
* That was not enough on its own, and #6347 is why. Postgres has **two**
152+
* missing-column phrasings, one per direction:
153+
*
154+
* | path | phrase | SQLSTATE | matched the message test? |
155+
* |:---|:---|:---|:---|
156+
* | read (`SELECT`) | `column "bogus" does not exist` | 42703 | no |
157+
* | write (`INSERT`/`UPDATE`/`ALTER`) | `column "label" of relation "sys_team" does not exist` | 42703 | **yes** |
158+
*
159+
* The write-path phrase contains a complete, legal missing-table phrase —
160+
* `relation "sys_team" does not exist` — as a substring, so the table-scoped
161+
* test above matched it and answered *benign* about an error the docblock two
162+
* paragraphs up already named as one that must stay loud. The same holds for
163+
* every other sub-object of a relation Postgres phrases this way, e.g.
164+
* `constraint "uq_x" of relation "sys_team" does not exist` (42704). And
165+
* code-first does not rescue it: {@link matchesDriverError} is a sequential OR,
166+
* so a `code: '42703'` error simply falls past the two code lines and is
167+
* decided by the message.
168+
*
169+
* Hence {@link DriverErrorSignature.excludes}: the not-a-table shapes are
170+
* recognised FIRST, and recognition ends the question with `false`.
135171
*/
136172
const MISSING_TABLE: DriverErrorSignature = {
137173
codes: new Set([
@@ -146,18 +182,61 @@ const MISSING_TABLE: DriverErrorSignature = {
146182
*/
147183
message:
148184
/no such table|relation ["'`][^"'`]+["'`] does not exist|table ["'`][^"'`]+["'`] doesn'?t exist|unknown table/i,
185+
excludes: {
186+
/**
187+
* Exactly the three SQLSTATEs the docblock above already names as
188+
* must-stay-loud neighbours of `does not exist`. They are listed here
189+
* rather than merely trusted to miss the message test, because two of
190+
* them (42703 columns, 42704 constraints/triggers) have a phrasing that
191+
* *does* hit it, and because a code is a fact where prose is a guess.
192+
*
193+
* Postgres-shaped on purpose: measured, neither MySQL
194+
* (`Unknown column 'label' in 'field list'`) nor SQLite
195+
* (`no such column: bogus`, `table t has no column named label`)
196+
* phrases a sub-object failure so that a missing-table phrase falls out
197+
* of it, so there is nothing there to exclude. Adding their codes would
198+
* be surface with no defect behind it.
199+
*/
200+
codes: new Set([
201+
'42703', // undefined_column
202+
'42704', // undefined_object — constraint, trigger, role, type, …
203+
'3D000', // invalid_catalog_name — `database "x" does not exist`
204+
]),
205+
/**
206+
* `«sub-object» "x" of relation "y" …` — Postgres' phrasing for a
207+
* failure about something *inside* a relation, which therefore says the
208+
* relation itself is present. The two in-repo siblings that already
209+
* carry this phrase are `mapDataError` (`packages/rest`, #5352) and
210+
* `MISSING_COLUMN_OF_RELATION` (`service-analytics`, #6035/PR #6346);
211+
* this is a deliberate one-line copy rather than a cross-package import,
212+
* and deliberately **wider** than theirs. Both of those *extract* the
213+
* column name to phrase a better error, so a miss costs a vaguer
214+
* message; this one *excludes*, so a miss restores the corruption. It
215+
* therefore drops their `column`/`[a-z0-9_]+`/`does not exist` anchors:
216+
* any sub-object, any quoted identifier, any verdict. Over-matching here
217+
* only ever converts a benign verdict into a loud one, which is the
218+
* direction this whole module already errs in.
219+
*/
220+
message: /["'`][^"'`]+["'`]\s+of relation\s/i,
221+
},
149222
};
150223

151224
/** How far to follow an `error.cause` chain — drivers wrap, but not deeply. */
152225
const MAX_CAUSE_DEPTH = 4;
153226

154227
/**
155-
* The single matcher both predicates run on: code, then errno, then message,
156-
* then one step down the `cause` chain.
228+
* The single matcher both predicates run on: exclusions, then code, then errno,
229+
* then message, then one step down the `cause` chain.
157230
*
158231
* Unrecognised is always `false` — a benign verdict must be *earned*, never
159232
* defaulted to, because a false "benign" corrupts data while a false "real"
160233
* costs one error line.
234+
*
235+
* The exclusion runs at every node and, when it fires, returns `false` **without
236+
* descending into `cause`** (#6347). Two reasons, both the conservative
237+
* direction: an error that positively identifies as "a column of an existing
238+
* relation" *is* that error, whatever it wraps; and stopping can only ever
239+
* subtract benign verdicts, never add one.
161240
*/
162241
function matchesDriverError(
163242
error: unknown,
@@ -166,7 +245,10 @@ function matchesDriverError(
166245
): boolean {
167246
if (error === null || error === undefined || depth > MAX_CAUSE_DEPTH) return false;
168247

169-
if (typeof error === 'string') return signature.message.test(error);
248+
if (typeof error === 'string') {
249+
if (signature.excludes?.message.test(error)) return false;
250+
return signature.message.test(error);
251+
}
170252
if (typeof error !== 'object') return false;
171253

172254
const err = error as {
@@ -176,6 +258,12 @@ function matchesDriverError(
176258
cause?: unknown;
177259
};
178260

261+
const excludes = signature.excludes;
262+
if (excludes) {
263+
if (typeof err.code === 'string' && excludes.codes.has(err.code)) return false;
264+
if (typeof err.message === 'string' && excludes.message.test(err.message)) return false;
265+
}
266+
179267
if (typeof err.code === 'string' && signature.codes.has(err.code)) return true;
180268
if (typeof err.errno === 'number' && signature.errnos.has(err.errno)) return true;
181269
if (typeof err.message === 'string' && signature.message.test(err.message)) return true;
@@ -208,6 +296,10 @@ export function isSchemaAlreadyExistsError(error: unknown, depth = 0): boolean {
208296
* caller must report the consequence and give up rather than compute an answer
209297
* from data it never read (#4825).
210298
*
299+
* A failure about a **column** of a relation is never this case, in either of
300+
* Postgres' two phrasings — the relation is right there in the message because
301+
* it exists (#6347). See {@link MISSING_TABLE}'s `excludes`.
302+
*
211303
* @param error - The value thrown by a driver/engine read (`find`, `findOne`, …).
212304
* @param depth - Internal `cause`-chain recursion counter; callers pass nothing.
213305
* @returns `true` only when the error positively identifies as

0 commit comments

Comments
 (0)