Skip to content

Commit 7993e67

Browse files
hotlongclaude
andauthored
fix(service-analytics): postgres's missing-COLUMN wording stays a hard failure (#6035) (#6346)
`isMissingSourceError`'s docblock promises the dataset degradation path is scoped to missing SOURCE and that "column/syntax errors stay hard failures so real query bugs still surface". One postgres wording broke that by construction: column "label" of relation "acct" does not exist (SQLSTATE 42703) carries `relation "acct" does not exist` inside it verbatim. #5717 anchored the postgres limb to postgres's real missing-table wording and this string matched anyway -- it had to, because it literally contains that wording. No tightening of "does this say a relation is missing" can exclude it; only asking the more specific question first can. The fix is therefore an ORDERING, not a better regex: subtract the column phrase, then classify. Both consequences were wrong, and which one fired was an accident of whether the named relation happened to be the dataset's own object: a joined name produced a loud but FALSE cross-datasource topology refusal, while the dataset's own name degraded the widget to an empty grid with the mistyped column mentioned to nobody. Both halves are pinned. The subtraction is `rest-server.ts`'s `mapDataError` regex verbatim (its `unknownColumn` probe has extracted this same phrase ahead of the unknown-object branch since #5352, so the REST face answers 400 INVALID_FIELD rather than 404) -- the same pattern rather than a second dialect of it, so the two faces cannot disagree about what counts as postgres saying "column". `missingSourceRelation` subtracts it too: measured on origin/main it answered `sys_team` for this wording, so guarding only the sniffer would leave "is something missing" and "what is missing" contradicting each other -- the exact disagreement #5717 closed on this limb. This aligns a predicate with its own documentation rather than repairing an incident: analytics is a read face and postgres spells an unknown column in a SELECT as `column "bogus" does not exist`, with no `relation` in it. The value is that the disagreement no longer depends on that dormancy holding. #5717's 13 measured in-repo wordings are re-pinned as `queryDataset` OUTCOMES (empty grid / topology refusal / propagated) rather than as private-predicate booleans, so the pair is exercised jointly through the public API: exactly ONE verdict moves, the column phrase; the other 12 are unchanged. Claude-Session: https://claude.ai/code/session_015a5qkLzpGXhLL2F5gvJ7dD Co-authored-by: Claude <noreply@anthropic.com>
1 parent 9765a4d commit 7993e67

3 files changed

Lines changed: 364 additions & 6 deletions

File tree

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
---
2+
"@objectstack/service-analytics": patch
3+
---
4+
5+
fix(service-analytics): postgres 的「缺列」措辞不再被判为「缺源」(#6035)
6+
7+
数据集查询的降级路径靠驱动措辞判断「后端表没挂载」,从而把控件渲染成空网格而不是 500。
8+
它的判据 `isMissingSourceError` 自己的文档写明范围**只含缺表/缺对象,不含列/语法错误——
9+
后者要保持硬失败,好让真正的查询 bug 浮上来**。有一条 postgres 措辞按构造违反了这条承诺:
10+
11+
```
12+
column "label" of relation "acct" does not exist (SQLSTATE 42703)
13+
```
14+
15+
它内部**逐字包含**一整段合法的缺表措辞 `relation "acct" does not exist`#5717 把 postgres
16+
那一支从「同时含两个词的任意句子」收紧为锚定真实缺表措辞后,这条依然命中——它必然命中,因为它
17+
字面上**就是**那段措辞。所以任何对「这句话是不是在说某个 relation 不存在」的收紧都排除不掉它,
18+
只有**先问更具体的问题**才可以:修法是一个**判定顺序**(先摘掉缺列措辞,再做缺源判定),而不是
19+
一个更好的正则。
20+
21+
两种后果都是错的,而具体触发哪一种只取决于措辞里那个关系名是否恰好是数据集自己的对象:
22+
23+
- 名字是**被 JOIN 的表** → 报出一条响亮但**虚假**的跨数据源拓扑错误,把一个拼写错误说成数据源
24+
布局问题;
25+
- 名字是**数据集自己的对象** → 控件降级成空网格,只留一条 warn,拼错的列名不会告诉任何人。
26+
27+
两半现在都作为回归钉住。判定顺序抄 `rest-server.ts``mapDataError`#5352 起就在用的先例
28+
(它同样先摘出这条措辞,于是 REST 面回答 `400 INVALID_FIELD` 而不是 `404`),用的是同一条正则
29+
而不是它的第二种方言——两个面不该对「postgres 什么时候在说 column」给出不同答案。兄弟函数
30+
`missingSourceRelation` 做同样的前置摘除:实测在修改前它对这条措辞回答 `sys_team`,只修其一会让
31+
「是不是缺了什么」与「缺的是什么」相互矛盾,而那正是 #5717 在这一支上刚消除的分歧。
32+
33+
**这不修线上事故,而是让判据与它自己的文档一致。** analytics 是只读面,而 postgres 在 SELECT
34+
下的未知列措辞是 `column "bogus" does not exist`(不含 `relation`,本来就不命中);
35+
`column … of relation …` 是 INSERT/UPDATE/ALTER 措辞。价值在于:这条分歧不再依赖「读路径不产生该
36+
措辞」这个假设活着——哪天有任何写形状语句、驱动改措辞、或多包一层 `cause` 把它送到这个 catch
37+
面前,它会被正确分类,而不是被静默吞掉。
38+
39+
#5717 量过的 13 条仓内真实措辞全部重新钉住,并且是**按调用方可观测的结果**(空网格 / 拓扑拒收 /
40+
原样上抛)钉的,而不是按私有判据的布尔值——实测 **13 条里只有 1 条改判**,就是缺列那条,其余 12
41+
条(三个驱动家族的措辞、框架的 not-registered 信号、本包自己的拒收)逐条不变。
Lines changed: 270 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,270 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#6035] Postgres's missing-COLUMN wording stays a hard failure, because a
5+
* mistyped column name is a query bug and not an absent table.
6+
*
7+
* ## What was wrong
8+
*
9+
* `isMissingSourceError`'s own docblock promises the degradation path is
10+
* "Deliberately scoped to MISSING SOURCE (table/object/relation) — not
11+
* column/syntax errors, which stay hard failures so real query bugs still
12+
* surface". One driver wording broke that promise by construction:
13+
*
14+
* `column "label" of relation "acct" does not exist` (SQLSTATE 42703)
15+
*
16+
* carries `relation "acct" does not exist` inside it VERBATIM. #5717 anchored
17+
* the postgres limb to postgres's real missing-table wording, and this string
18+
* matched anyway — it had to, because it literally contains that wording. No
19+
* tightening of "does this say a relation is missing" can exclude it; only
20+
* asking the more specific question FIRST can, which is why the fix is an
21+
* ORDERING (subtract the column phrase, then classify) and not a better regex.
22+
*
23+
* Both consequences were wrong, and which one fired depended only on whether
24+
* the named relation happened to be the dataset's own object:
25+
*
26+
* - a JOINED table's name → a loud but FALSE cross-datasource topology
27+
* error, blaming the datasource layout for a typo;
28+
* - the dataset's OWN name → the widget degrades to an empty grid, one
29+
* `warn`, and the mistyped column is never mentioned to anyone.
30+
*
31+
* Both are pinned below, because "which half fires" is an accident of the
32+
* fixture and a fix that only closed one would look green from either side.
33+
*
34+
* ## Why this was dormant, stated as a limit rather than a boast
35+
*
36+
* Analytics is a READ face, and postgres does not use this wording on reads: a
37+
* SELECT naming an unknown column says `column "bogus" does not exist`, with no
38+
* `relation` in it (row 06 of the corpus below — a miss before and after).
39+
* `column … of relation …` is INSERT/UPDATE/ALTER phrasing. So this repairs a
40+
* predicate that disagreed with its own documentation, and does NOT repair a
41+
* production incident. The value is that the disagreement cannot outlive the
42+
* assumption that keeps it harmless — the day any write-shaped statement, any
43+
* driver re-wording, or any newly wrapped `cause` puts that phrase in front of
44+
* this catch, it is classified correctly instead of silently.
45+
*
46+
* ## The corpus, and why it is asserted through the public API
47+
*
48+
* PR #6045 (#5717) measured 13 real in-repo wordings and moved exactly one
49+
* verdict. This file re-pins all 13 — but as `queryDataset` OUTCOMES rather
50+
* than as booleans out of a private predicate, so what is guarded is the
51+
* behaviour a caller can actually observe (empty grid / topology refusal /
52+
* propagated verbatim), and so the pair `isMissingSourceError` +
53+
* `missingSourceRelation` is exercised JOINTLY. Each wording is thrown BARE, on
54+
* purpose: several of these producers carry an ADR-0112 envelope in real life
55+
* and would propagate via #5717's defence B no matter what the sniffer said,
56+
* which would make them vacuous as pins of the sniffer (#5046's lesson — a case
57+
* that passes because nothing is produced). Stripping the envelope is the same
58+
* isolation `dataset-degradation-envelope.test.ts` uses for its defence-C case.
59+
*
60+
* ## Reverse verification — direction predicted BEFORE running
61+
*
62+
* Ordinary direction (red), and deliberately NARROW: this change subtracts one
63+
* phrase, so restoring the deleted guard must turn red exactly the cases about
64+
* that phrase and nothing else. Predicted, with the guard removed from both
65+
* functions:
66+
*
67+
* - `column … of relation "sys_team" …` (a joined name) → RED, becomes the
68+
* false topology refusal;
69+
* - `column … of relation "opportunity" …` (own object) → RED, becomes an
70+
* empty grid;
71+
* - the 13-row corpus table → RED on row 05 ONLY;
72+
* - the other 12 rows, and every case in
73+
* `dataset-degradation-envelope.test.ts` → GREEN throughout.
74+
*
75+
* MEASURED, both guards disabled: **3 red / 15 green** in this file — the three
76+
* predicted cases and no others, row 05 the only corpus row to move, and
77+
* `dataset-degradation-envelope.test.ts` green at 11/11 in both states, which is
78+
* what makes "narrowing, not a behaviour change" a measurement rather than a
79+
* claim. (The green count is corrected from the 12 first written here — 18
80+
* cases, not 15; the RED SET was predicted exactly and is the part that carries
81+
* the argument, so the miscount is fixed in place rather than quietly.)
82+
*
83+
* The two reverted failures are worth quoting, because they are the defect
84+
* itself rather than a red mark: the joined-name case came back as
85+
* `[Analytics] dataset "sales" cannot be executed as one statement: table
86+
* "sys_team" is not on the default datasource …` — a datasource-topology story
87+
* told about a typo — and the own-object case failed as `expected undefined to
88+
* be an instance of Error`, i.e. nothing was thrown at all and the widget got
89+
* its empty grid.
90+
*/
91+
92+
import { describe, it, expect, vi } from 'vitest';
93+
import { DatasetSchema } from '@objectstack/spec/ui';
94+
import type { ExecutionContext } from '@objectstack/spec/kernel';
95+
import { AnalyticsService } from '../analytics-service.js';
96+
97+
const EMPTY = { rows: [], fields: [], totals: [] };
98+
99+
const dataset = DatasetSchema.parse({
100+
name: 'sales',
101+
label: 'Sales',
102+
object: 'opportunity',
103+
include: ['account'],
104+
dimensions: [{ name: 'region', field: 'account.region', type: 'string' }],
105+
measures: [{ name: 'revenue', aggregate: 'sum', field: 'amount' }],
106+
});
107+
108+
const SELECTION = { dimensions: ['region'], measures: ['revenue'] };
109+
const CTX = { tenantId: 'org_A' } as ExecutionContext;
110+
111+
function logger() {
112+
return { info: vi.fn(), debug: vi.fn(), warn: vi.fn(), error: vi.fn(), child: vi.fn() } as any;
113+
}
114+
115+
/** A service whose execution throws `thrown` — the only way into the catch under test. */
116+
function serviceThatThrows(thrown: unknown, log = logger()) {
117+
return new AnalyticsService({
118+
queryCapabilities: () => ({ nativeSql: true, objectqlAggregate: false, inMemory: false }),
119+
executeRawSql: async () => { throw thrown; },
120+
isRegisteredObject: () => true,
121+
logger: log,
122+
});
123+
}
124+
125+
/**
126+
* The three outcomes `queryDataset`'s catch can produce, named so a corpus row
127+
* reads as a behaviour rather than as an internal boolean.
128+
*/
129+
type Outcome = 'empty' | 'topology' | 'propagated';
130+
131+
async function outcomeOf(wording: string, log = logger()): Promise<Outcome> {
132+
try {
133+
const result = await serviceThatThrows(new Error(wording), log).queryDataset(dataset, SELECTION, CTX);
134+
expect(result).toEqual(EMPTY);
135+
return 'empty';
136+
} catch (e) {
137+
const message = String((e as Error)?.message);
138+
// The #5033 cross-datasource refusal is this layer's OWN wording; anything
139+
// else reaching the caller is the driver's error, untouched.
140+
return /cannot be executed as one statement/.test(message) ? 'topology' : 'propagated';
141+
}
142+
}
143+
144+
// ── the corpus PR #6045 measured, transcribed from its real producers ─────────
145+
146+
/** Postgres's missing-COLUMN wording, matching `rest.test.ts`'s pinned case. */
147+
const MISSING_COLUMN_JOINED = 'column "label" of relation "sys_team" does not exist';
148+
149+
const CORPUS: Array<[row: string, wording: string, outcome: Outcome]> = [
150+
['01 sqlite/libsql bare', 'no such table: opportunity', 'empty'],
151+
['02 sqlite via knex (sql-prefixed)', 'SELECT COUNT(*) FROM "opportunity" - no such table: opportunity', 'empty'],
152+
['03 postgres missing table', 'select "region" from "opportunity" - relation "opportunity" does not exist', 'empty'],
153+
['04 postgres schema-qualified', 'relation "public.crm_account" does not exist', 'topology'],
154+
// The one verdict this change moves. Before: `topology` — a false
155+
// cross-datasource refusal for a mistyped column.
156+
['05 postgres MISSING COLUMN (write path)', MISSING_COLUMN_JOINED, 'propagated'],
157+
['06 postgres missing column (read path)', 'column "bogus_dim" does not exist', 'propagated'],
158+
['07 mysql', "Table 'app.opportunity' doesn't exist", 'empty'],
159+
[
160+
'08 objectql datasource',
161+
"[ObjectQL] Datasource 'warehouse' configured for object 'crm_account' is not registered.",
162+
'topology',
163+
],
164+
['09 rest unknown object', "Object 'ghost' is not registered", 'topology'],
165+
[
166+
'10 analytics CUBE_NOT_FOUND (#3867)',
167+
"Cube 'ghost' not found: no cube is registered under that name, and it is not a " +
168+
'registered object either (a cube can only be auto-inferred from a registered object). ' +
169+
"Define a Cube in your stack, or check the object name.",
170+
'empty',
171+
],
172+
[
173+
'11 dataset-compiler relationship refusal',
174+
'[dataset-compiler] dataset "sales" includes relationship "bogus" which does not exist on object "opportunity".',
175+
'propagated',
176+
],
177+
[
178+
'12 read-scope nested/relation value',
179+
'[read-scope-sql] "owner" has a nested/relation value which is not supported in a read scope (fail-closed).',
180+
'propagated',
181+
],
182+
[
183+
'13 analytics measure gate (#4437)',
184+
"Measure 'ghost_sum' on cube 'opportunity' aggregates field 'ghost', which object " +
185+
"'opportunity' does not have. Valid measures: (none).",
186+
'propagated',
187+
],
188+
];
189+
190+
// ─────────────────────────────────────────────────────────────────────────────
191+
192+
describe('[#6035] postgres’s missing-COLUMN wording is a hard failure, not a missing source', () => {
193+
it('the wording is the one this repo already pins on the REST face', () => {
194+
// Guards the fixture against drifting away from the phrase the precedent
195+
// handles: `rest-server.ts`'s `mapDataError` extracts this same string to
196+
// answer `400 INVALID_FIELD` instead of a `404`, pinned in `rest.test.ts`.
197+
// If these two stop being the same sentence, the two faces have started
198+
// disagreeing about what postgres says — which is what this fix removed.
199+
expect(MISSING_COLUMN_JOINED).toMatch(
200+
/column\s+["'`]([a-z0-9_]+)["'`]\s+of relation\s+\S+\s+does not exist/i,
201+
);
202+
// …and it does contain a well-formed missing-TABLE wording, which is the
203+
// whole reason a subtraction is needed rather than a tighter anchor.
204+
expect(MISSING_COLUMN_JOINED).toMatch(/relation\s+["'`]?[A-Za-z0-9_$.]+["'`]?\s+does not exist/i);
205+
});
206+
207+
it('naming a JOINED table: propagates verbatim instead of a false topology refusal', async () => {
208+
const log = logger();
209+
let caught: Error | undefined;
210+
try {
211+
await serviceThatThrows(new Error(MISSING_COLUMN_JOINED), log).queryDataset(dataset, SELECTION, CTX);
212+
} catch (e) {
213+
caught = e as Error;
214+
}
215+
expect(caught, 'a mistyped column was swallowed by the degradation path').toBeInstanceOf(Error);
216+
// The driver's own sentence, unedited — the caller can read the column name.
217+
expect(String(caught?.message)).toBe(MISSING_COLUMN_JOINED);
218+
// Specifically NOT the #5033 cross-datasource story, which blames the
219+
// datasource layout for what is a spelling mistake.
220+
expect(String(caught?.message)).not.toMatch(/cannot be executed as one statement/);
221+
expect(String(caught?.message)).not.toMatch(/A dataset JOIN cannot cross datasources/);
222+
// Bare in ⇒ bare out; this layer must not invent an envelope for it.
223+
expect((caught as { code?: unknown })?.code).toBeUndefined();
224+
expect((caught as { status?: unknown })?.status).toBeUndefined();
225+
});
226+
227+
it('naming the dataset’s OWN object: propagates instead of degrading to an empty grid', async () => {
228+
// The other half of the same defect, and the quieter one: here the named
229+
// relation IS `opportunity`, so the old code took the degradation branch —
230+
// `{rows: []}` plus one warn, with the mistyped column mentioned to nobody.
231+
const own = 'column "reginn" of relation "opportunity" does not exist';
232+
const log = logger();
233+
let caught: Error | undefined;
234+
try {
235+
await serviceThatThrows(new Error(own), log).queryDataset(dataset, SELECTION, CTX);
236+
} catch (e) {
237+
caught = e as Error;
238+
}
239+
expect(caught, 'a mistyped column became a confident empty chart').toBeInstanceOf(Error);
240+
expect(String(caught?.message)).toBe(own);
241+
expect(log.warn).not.toHaveBeenCalledWith(expect.stringContaining('returning an empty result'));
242+
expect(log.warn).not.toHaveBeenCalledWith(expect.stringContaining('is unavailable'));
243+
});
244+
245+
it('an unquoted look-alike is left alone — the subtraction does not widen past postgres', async () => {
246+
// Direction-of-error guard. Postgres always quotes both names here, so the
247+
// subtraction requires quotes; a sentence that merely talks ABOUT a relation
248+
// must keep whatever verdict it had rather than newly becoming a hard
249+
// failure, since over-matching would regress #5033's deliberate leniency.
250+
expect(await outcomeOf('relation "opportunity" does not exist')).toBe('empty');
251+
});
252+
});
253+
254+
describe('[#6035] the other 12 in-repo wordings keep the verdict #5717 measured', () => {
255+
it.each(CORPUS)('%s → %s', async (_row, wording, expected) => {
256+
expect(await outcomeOf(wording)).toBe(expected);
257+
});
258+
259+
it('exactly one corpus row is a hard failure for the column reason', () => {
260+
// Counts the shape rather than restating the table: the corpus must keep
261+
// containing the moved row and its read-path neighbour, so a future edit
262+
// that quietly drops row 05 cannot leave this file passing.
263+
const columnRows = CORPUS.filter(([, wording]) => /column\s+["'`]/i.test(wording));
264+
expect(columnRows.map(([row]) => row)).toEqual([
265+
'05 postgres MISSING COLUMN (write path)',
266+
'06 postgres missing column (read path)',
267+
]);
268+
expect(columnRows.every(([, , outcome]) => outcome === 'propagated')).toBe(true);
269+
});
270+
});

0 commit comments

Comments
 (0)