diff --git a/.changeset/driver-sql-unresolvable-where-column-refused.md b/.changeset/driver-sql-unresolvable-where-column-refused.md new file mode 100644 index 0000000000..13af483db8 --- /dev/null +++ b/.changeset/driver-sql-unresolvable-where-column-refused.md @@ -0,0 +1,91 @@ +--- +"@objectstack/driver-sql": minor +"@objectstack/spec": minor +--- + +fix(driver-sql): one unresolvable WHERE column, one answer — `find()` and `count()` both refuse with `INVALID_FILTER` / 400 naming the column (#8790) + +**BREAKING** accept-set narrowing on a GA public data API, shipped as `minor` +under the lockstep launch-window convention. The migration prescription is +registered under protocol major 18, where `os migrate meta` users will look. + + + +## The defect + +One predicate had two answers. `SqlDriver.findRows()` carries the #3821 +unknown-column recovery ladder, and every rung of it is built from +`buildBase()`, which **always re-applies `query.where`**. So the ladder can drop +a projection and can drop an ORDER BY, but it can never drop the clause that +actually failed when the unresolvable column is in the WHERE — both rungs raise +the same error and the method fell to `return []`. `SqlDriver.count()` runs a +separate statement and has no ladder at all, so the identical predicate threw. + +Measured on a real `SqlDriver` over better-sqlite3, one table, one seeded row: + +``` +where { 'title.x': 'y' } + find() -> 0 rows, NO ERROR + count() -> THREW code=SQLITE_ERROR status=undefined + select count(*) as `count` from `task` where `title`.`x` = 'y' + - no such column: title.x + +CONTROL where { title: 'Design' } + find() -> 1 row + count() -> 1 +``` + +A list view calls both halves, so one query produced an empty page from the rows +half and a 500-shaped failure from the total half. A caller reading only the rows +got a silent empty page that says "no records exist" for what was really "your +predicate never ran" — the single most AI-legible failure to get wrong, since an +agent reads "no matching records" and writes its next query on that belief. + +The thrown half was no better: the dialect's own `code`, no `status` (so an +unclassified 5xx at the REST boundary rather than a caller mistake), and the +statement's **bound literals inlined in the message** — the same predicate-text +disclosure shape #7929 redacted elsewhere. + +## The fix + +Ruled 2026-08-15 on #8790: **refuse both halves** with `INVALID_FILTER` / 400, +naming the column. That envelope is not minted here — it is what every sibling +refusal on this path already answers, required on both SQL drivers by +`cross-field-conformance-cases.ts` and pinned by +`sql-driver-boolean-identity.test.ts` and +`sql-driver-cross-field-conformance.test.ts`. What closes is a +declared-vs-enforced gap, not a new posture. + +The caller-visible message names the column and the object and nothing else. The +dialect's own message — the compiled statement, bound literals and all — goes to +the **server log** instead, so the operator keeps the debugging aid that +`count()`'s raw throw used to provide without it reaching the caller. + +**The #3821 ladder keeps both of its recoveries.** Only the WHERE-failure +terminal `return []` became a refusal, and the asymmetry is the ruling rather +than an oversight: "rows matter more than their order" is an argument about how +rows are *presented*, and it does not transfer to a predicate. A dropped sort is +a correct answer in an unhelpful order; a dropped WHERE is records the caller +explicitly excluded. Recover-both was rejected for exactly that reason. + +## Reach, stated rather than assumed + +The refusal fires on the wordings the ladder has always recognised — SQLite +(`no such column: x`) and Postgres (`column "x" does not exist`). MySQL spells +the condition `Unknown column 'x' in 'where clause'`, which neither arm matches, +so on MySQL an unresolvable column still travels out as the raw dialect error. +That gap is pinned as a fact in the new suite and filed separately: widening the +predicate would also hand MySQL the #3821 projection and ORDER-BY recoveries it +has never had, which is an accept-set change in the opposite direction from this +one. + +## Who is affected + +Callers that reach the driver with a filter key the table has no column for. The +ingress doors already refuse this where they can judge — `assertFilterFieldsExist` +(`@objectstack/metadata-protocol`) answers `INVALID_FIELD` / 400 for everything +reaching `findData`, with the sentence this refusal now echoes verbatim: *a +filter on a field that does not exist can only match zero records, so the query +was refused instead of answered with an empty list*. What changes is the +backstop underneath them: a registry the door could not read, and a dotted key +judged on its head segment only. diff --git a/packages/drivers/driver-sql/src/sql-driver-unresolvable-where-column-refusal.test.ts b/packages/drivers/driver-sql/src/sql-driver-unresolvable-where-column-refusal.test.ts new file mode 100644 index 0000000000..f1c6cc815e --- /dev/null +++ b/packages/drivers/driver-sql/src/sql-driver-unresolvable-where-column-refusal.test.ts @@ -0,0 +1,415 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectstack#8790 — one unresolvable WHERE column, ONE answer. + * + * Measured before the fix, real `SqlDriver` on better-sqlite3, one seeded row: + * + * ``` + * where { 'title.x': 'y' } + * find() -> 0 rows, NO ERROR + * count() -> THREW code=SQLITE_ERROR status=undefined + * select count(*) as `count` from `task` where `title`.`x` = 'y' + * - no such column: title.x + * ``` + * + * Two answers to one predicate: a list view calls both halves, so the user got + * an empty page from the rows half and a 500-shaped failure from the total + * half — or, reading only the rows, a silent empty page that says "no records + * exist" for what was really "your predicate never ran". + * + * Maintainer ruling 2026-08-15 (issue comment 5302931807): **refuse both**, + * with the ADR-0112 envelope this path already declares — `INVALID_FILTER` / + * 400, naming the column. Recover-both was excluded by the card's own argument + * (dropping a WHERE returns rows the caller excluded), and the #3821 ladder + * KEEPS its projection and ORDER-BY recoveries: only the WHERE-failure terminal + * `return []` became a refusal. + * + * ## What each half of this suite is for + * + * A refusal pin alone cannot show the refusal is SELECTIVE rather than blanket, + * and a blanket refusal here would break every query on the platform. So every + * refusal case is stated next to a CONTROL on the same shape with a resolvable + * column, asserting the rows/count still come back unchanged. The third block + * pins what was deliberately NOT changed — the two recoveries — because those + * are what an over-broad fix destroys silently. + * + * ## Scope: a plain unresolvable COLUMN, not a dotted PATH + * + * The card's headline repro was a dotted key, and measuring it across live + * backends showed why that cannot be this suite's assertion: the three dialects + * do not agree on what a dotted key IS. Postgres classifies it as an undefined + * TABLE (`42P01`) and always did, so #8790 changed nothing there; SQLite + * classifies it as an undefined COLUMN and cannot be told apart from a column + * literally named `title.x`. The FILTER-axis verdict on dotted paths is #8371, + * still open. So the ruled scope — a plain column the table lacks — carries the + * refusal pins, and the dotted key is RECORDED per dialect in + * `DOTTED_STATUS_QUO` with #8371 named as the owner of the verdict. + * + * ## The DIALECT axis + * + * `INVALID_FILTER` / 400 is required on both SQL drivers by + * `cross-field-conformance-cases.ts`, so a fix pinned on one dialect proves + * half the contract. The end-to-end sweep runs the `DIALECT_CELLS` cells whose + * wording the driver recognises (SQLite always, live Postgres when the runner + * provisions it), and the MESSAGE-SHAPE sweep at the bottom covers all three + * dialects' real error texts unconditionally — that parsing is the only + * dialect-sensitive part of the fix, and it must not go unmeasured on a runner + * without a live server. + * + * ⚠️ MySQL is NOT in the end-to-end sweep, and that omission is a measurement, + * not an oversight: MySQL spells this condition `Unknown column 'x' in 'where + * clause'`, which `isUnresolvableColumnError` matches with neither arm — so on + * MySQL the raw dialect error has always travelled out and still does. The + * message-shape sweep pins exactly that, so the gap is visible and goes red the + * day someone widens the predicate. It is filed separately rather than closed + * here because widening would also hand MySQL the #3821 recoveries it has never + * had, which is an accept-set change in the opposite direction from this one. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import type { DriverQuery } from '@objectstack/spec/contracts'; +import { SqlDriver, isUnresolvableColumnError, unresolvableColumnNameOf } from './sql-driver.js'; +import { DIALECT_CELLS, declareDialectCell, type DialectCell } from './live-dialect-matrix.testkit.js'; + +const TABLE = 'unresolvable_where_task'; + +/** + * The bound literal the caller's filter carries. Distinctive on purpose: the + * pre-fix `count()` message inlined it verbatim (`… where \`title\`.\`x\` = + * 'y' …`), so asserting its ABSENCE from every caller-visible message is what + * pins the redaction rather than merely the status code. + */ +const SECRET_LITERAL = 'zz-bound-literal-must-not-leak'; + +/** Cells whose unresolvable-column wording this driver recognises. */ +const RECOGNISED_CELLS = DIALECT_CELLS.filter((c) => c.id === 'sqlite' || c.id === 'pg'); + +/** + * The RULED scope: a plain column the table does not have. + * + * ⛔ A dotted key is deliberately NOT in this list, and the reason is measured + * rather than stylistic — see `DOTTED_STATUS_QUO` below. #8790's ruling is + * about a column the backend cannot resolve; the FILTER-axis verdict on dotted + * PATHS is #8371's, still open, and this suite must not decide it by assertion. + * + * `DriverQuery` rather than `as any`: its own docblock records that a direct + * caller holding only a `where` used to reach for a blanket cast and lose + * `where`'s type with it, which is the erasure `check:query-options-erasure` + * ratchets. A refusal suite that names columns for a living should be the last + * place to throw the type away. + */ +const UNRESOLVABLE: ReadonlyArray<{ label: string; where: NonNullable; named: string }> = [ + { label: 'a plain unknown column', where: { nosuchcol: SECRET_LITERAL }, named: 'nosuchcol' }, +]; + +/** + * [#8790 → #8371] What a DOTTED key does per dialect, measured, and why this + * suite records it instead of asserting the refusal on it. + * + * The three backends do not agree on what a dotted key even IS, because knex + * compiles `{'title.x': v}` to the qualified reference `"title"."x"`: + * + * | dialect | classification | error | + * |----------|-------------------|----------------------------------------------| + * | sqlite | undefined COLUMN | `no such column: title.x` | + * | postgres | undefined TABLE | `42P01 missing FROM-clause entry for table` | + * | mysql | undefined COLUMN | `ER_BAD_FIELD_ERROR Unknown column 'title.x'` | + * + * Measured live on PG 16.13 and MySQL 8.0.46, `origin/main` vs this branch. + * Postgres reads `title` as a TABLE, so its error is `undefined_table`, not + * `undefined_column` — a shape {@link isUnresolvableColumnError} does not match + * and never has. The consequence, and the reason this is a recording rather + * than a regression: on Postgres a dotted key raised a raw `42P01` on BOTH + * halves **before this card and after it, byte for byte**. #8790 changed + * nothing there. The card's headline repro (`find()` [] / `count()` throws) is + * SQLite-specific; on Postgres the two halves already agreed. + * + * On SQLite the dotted key DOES now refuse, because SQLite hands the driver a + * message indistinguishable from a plain missing column — the driver cannot + * tell `{'title.x': v}` (a path) from a column literally NAMED `title.x` + * without inspecting the key for a `.`, which is judging dotted-ness and is + * #8371's call. So this table pins the STATUS QUO, per dialect, and names the + * owner of the verdict. ⛔ Do not "fix" a cell here by teaching the classifier + * `42P01`: that mints a dotted-path verdict at the driver and pre-empts #8371. + * MySQL's cell is owned by #8926 (its wording matches neither arm at all). + */ +const DOTTED_STATUS_QUO: Readonly> = { + sqlite: { code: 'INVALID_FILTER', enveloped: true }, + pg: { code: '42P01', enveloped: false }, +}; + +async function caught(run: () => Promise): Promise { + try { + await run(); + } catch (err) { + return err; + } + return expect.fail('expected the query to be refused, but it resolved'); +} + +function declareSweep(cell: DialectCell): void { +describe(`[#8790] driver-sql — unresolvable WHERE column refuses on BOTH halves (${cell.label})`, () => { + let driver: SqlDriver; + + beforeAll(async () => { + driver = new SqlDriver(cell.config()); + // Live cells reuse one database, so the sweep starts from a dropped table. + await driver.execute(`drop table if exists ${TABLE}`).catch(() => {}); + await driver.initObjects([ + { name: TABLE, fields: { title: { type: 'string' }, rank: { type: 'integer' } } }, + ]); + await driver.create(TABLE, { id: 't1', title: 'Design', rank: 1 }, { bypassTenantAudit: true }); + await driver.create(TABLE, { id: 't2', title: 'Build', rank: 2 }, { bypassTenantAudit: true }); + }); + + afterAll(async () => { + await driver.execute(`drop table if exists ${TABLE}`).catch(() => {}); + await driver.disconnect(); + }); + + // ─────────────────────────────────────────────────────────────── + // THE RULING — both halves refuse, with the declared envelope + // ─────────────────────────────────────────────────────────────── + + for (const { label, where, named } of UNRESOLVABLE) { + it(`find() refuses ${label} with INVALID_FILTER / 400 naming the column`, async () => { + const err = await caught(() => driver.find(TABLE, { where })); + expect(err.code).toBe('INVALID_FILTER'); + expect(err.status).toBe(400); + expect(err.message).toContain(named); + }); + + it(`count() refuses ${label} with INVALID_FILTER / 400 naming the column`, async () => { + const err = await caught(() => driver.count(TABLE, { where })); + expect(err.code).toBe('INVALID_FILTER'); + expect(err.status).toBe(400); + expect(err.message).toContain(named); + }); + + // The two halves must not merely both fail — they must fail the SAME way. + // Answering one predicate two different ways is the whole defect. + it(`find() and count() answer ${label} with the same envelope`, async () => { + const onFind = await caught(() => driver.find(TABLE, { where })); + const onCount = await caught(() => driver.count(TABLE, { where })); + expect({ code: onCount.code, status: onCount.status, message: onCount.message }) + .toEqual({ code: onFind.code, status: onFind.status, message: onFind.message }); + }); + } + + // ─────────────────────────────────────────────────────────────── + // DOTTED KEY — recorded, NOT adjudicated (verdict owned by #8371) + // ─────────────────────────────────────────────────────────────── + + // Both halves still have to AGREE — that is #8790's actual subject, and it + // holds on every dialect regardless of which verdict #8371 lands. What this + // deliberately does NOT assert is that the agreed answer is a refusal. + it('a dotted key answers find() and count() the same way, whatever that way is', async () => { + const onFind = await caught(() => driver.find(TABLE, { where: { 'title.x': SECRET_LITERAL } })); + const onCount = await caught(() => driver.count(TABLE, { where: { 'title.x': SECRET_LITERAL } })); + expect(onCount.code).toBe(onFind.code); + expect(onCount.status).toBe(onFind.status); + }); + + it('a dotted key is at this dialect\'s recorded status quo (see DOTTED_STATUS_QUO)', async () => { + const expected = DOTTED_STATUS_QUO[cell.id]; + expect(expected, `no recorded status quo for cell '${cell.id}'`).toBeDefined(); + const err = await caught(() => driver.find(TABLE, { where: { 'title.x': SECRET_LITERAL } })); + expect(err.code).toBe(expected.code); + expect(err.status).toBe(expected.enveloped ? 400 : undefined); + }); + + // ⭐ The invariant half of the ruling — true under every branch it could have + // taken. `count()` used to answer the dialect's own error with the bound + // literal inlined: a declared-vs-enforced gap against ADR-0112 AND the + // predicate-text disclosure shape #7929 redacted elsewhere. + it('neither half puts the bound literal or the compiled statement on the wire', async () => { + for (const half of [ + () => driver.find(TABLE, { where: { nosuchcol: SECRET_LITERAL } }), + () => driver.count(TABLE, { where: { nosuchcol: SECRET_LITERAL } }), + ]) { + const err = await caught(half); + expect(err.message).not.toContain(SECRET_LITERAL); + expect(err.message).not.toContain('select '); + // The dialect's own code must not survive either — it is what made this + // an unclassified 5xx at the REST boundary rather than a caller mistake. + expect(err.code).not.toBe('SQLITE_ERROR'); + expect(err.status).toBe(400); + } + }); + + // ─────────────────────────────────────────────────────────────── + // CONTROLS — the refusal is SELECTIVE, not blanket + // ─────────────────────────────────────────────────────────────── + + it('CONTROL a resolvable column on the same shape still returns its rows and its count', async () => { + const rows = await driver.find(TABLE, { where: { title: 'Design' } }); + expect(rows.map((r: any) => r.id)).toEqual(['t1']); + expect(await driver.count(TABLE, { where: { title: 'Design' } })).toBe(1); + }); + + it('CONTROL an unfiltered read is untouched', async () => { + const rows = await driver.find(TABLE, {}); + expect(rows.map((r: any) => r.id).sort()).toEqual(['t1', 't2']); + expect(await driver.count(TABLE)).toBe(2); + }); + + it('CONTROL a resolvable predicate that genuinely matches nothing is still an honest empty list', async () => { + // The one empty answer that must never become a 400: the predicate ran. + expect(await driver.find(TABLE, { where: { title: 'no-such-title' } })).toEqual([]); + expect(await driver.count(TABLE, { where: { title: 'no-such-title' } })).toBe(0); + }); + + it('CONTROL an error that is not about an unresolvable column still propagates unchanged', async () => { + const err = await caught(() => driver.find('no_such_table_at_all', {})); + expect(err.code).not.toBe('INVALID_FILTER'); + }); + + // ─────────────────────────────────────────────────────────────── + // WHAT WAS DELIBERATELY NOT CHANGED — the #3821 ladder's recoveries + // ─────────────────────────────────────────────────────────────── + + it('KEEPS the #3821 projection recovery — and honours the WHERE while recovering', async () => { + const rows = await driver.find(TABLE, { + fields: ['title', 'nosuchfield'], + where: { rank: { $gte: 1 } }, + }); + expect(rows.map((r: any) => r.title).sort()).toEqual(['Build', 'Design']); + }); + + it('KEEPS the #3821 ORDER-BY recovery — and honours the WHERE while recovering', async () => { + const rows = await driver.find(TABLE, { + orderBy: [{ field: 'nosuchfield', order: 'asc' }], + where: { rank: { $gte: 2 } }, + }); + expect(rows.map((r: any) => r.id)).toEqual(['t2']); + }); + + it('KEEPS both recoveries together, with a resolvable WHERE', async () => { + const rows = await driver.find(TABLE, { + fields: ['title', 'nosuchfield'], + orderBy: [{ field: 'alsomissing', order: 'asc' }], + where: { rank: { $gte: 1 } }, + }); + expect(rows).toHaveLength(2); + }); + + // The discriminator between "the ladder recovered" and "the ladder refused": + // an unresolvable PROJECTION plus an unresolvable WHERE is not recoverable, + // because no rung may drop the predicate. It must refuse — and it must name + // the WHERE's column, not the projection's, since the projection is the half + // the ladder actually fixed. + it('refuses when the WHERE is unresolvable even though the projection was recoverable', async () => { + const err = await caught(() => + driver.find(TABLE, { fields: ['title', 'nosuchfield'], where: { alsomissing: 'x' } }), + ); + expect(err.code).toBe('INVALID_FILTER'); + expect(err.status).toBe(400); + expect(err.message).toContain('alsomissing'); + expect(err.message).not.toContain('nosuchfield'); + }); +}); +} + +// A matrix that silently finds zero cells reports OK — assert the axis is real +// before iterating it. +describe('[#8790] the dialect axis this suite runs', () => { + it('covers the cells whose wording the driver recognises', () => { + expect(RECOGNISED_CELLS.map((c) => c.id)).toEqual(['sqlite', 'pg']); + }); +}); + +for (const cell of RECOGNISED_CELLS) { + declareDialectCell(cell, 'unresolvable WHERE column refusal', declareSweep); +} + +// ───────────────────────────────────────────────────────────────── +// MESSAGE SHAPE — all three dialects, no live server required +// ───────────────────────────────────────────────────────────────── + +/** + * Real error texts, as knex surfaces them (statement, ` - `, then the driver's + * own words). These are what the fix actually reads, so they are pinned as + * data: a runner without `OS_TEST_POSTGRES_URL` still measures that Postgres' + * wording yields its column name, which is the half of the both-dialect + * contract an unprovisioned cell would otherwise leave unproven. + */ +const DIALECT_MESSAGES: ReadonlyArray<{ + dialect: string; + message: string; + recognised: boolean; + column: string | null; +}> = [ + { + dialect: 'sqlite (better-sqlite3)', + message: "select * from `task` where `nosuchcol` = 'y' - no such column: nosuchcol", + recognised: true, + column: 'nosuchcol', + }, + { + dialect: 'sqlite, dotted key', + message: "select count(*) as `count` from `task` where `title`.`x` = 'y' - no such column: title.x", + recognised: true, + column: 'title.x', + }, + { + dialect: 'postgres, quoted', + message: 'select * from "task" where "nosuchcol" = $1 - column "nosuchcol" does not exist', + recognised: true, + column: 'nosuchcol', + }, + { + dialect: 'postgres, table-qualified and unquoted', + message: 'select * from "task" - column task.nosuchcol does not exist', + recognised: true, + column: 'task.nosuchcol', + }, + // ⚠️ The measured reason the DOTTED case is recorded rather than refused. + // Postgres reads `title.x` as table `title`, column `x`, so it raises + // undefined_TABLE (42P01), not undefined_column (42703). Neither arm of the + // predicate matches it — before this card or after. Teaching the classifier + // this shape would mint a dotted-path verdict the driver has no business + // making; #8371 owns that. See `DOTTED_STATUS_QUO`. + { + dialect: 'postgres, DOTTED key — undefined_table, NOT recognised', + message: 'select * from "task" where "title"."x" = $1 - missing FROM-clause entry for table "title"', + recognised: false, + column: null, + }, + // ⚠️ The measured gap. MySQL's wording matches neither arm of the predicate, + // so this condition still travels out as the raw dialect error on MySQL. + // Pinned so the reach is a stated fact rather than an assumption, and so + // widening the predicate goes red here first. + { + dialect: 'mysql (ER_BAD_FIELD_ERROR) — NOT recognised, see the head note', + message: "select * from `task` where `nosuchcol` = 'y' - Unknown column 'nosuchcol' in 'where clause'", + recognised: false, + column: null, + }, +]; + +describe('[#8790] dialect wording — what the refusal recognises and what it names', () => { + for (const { dialect, message, recognised, column } of DIALECT_MESSAGES) { + it(`${dialect}: recognised=${recognised}, column=${column ?? 'null'}`, () => { + const err = Object.assign(new Error(message), { code: 'DIALECT' }); + expect(isUnresolvableColumnError(err)).toBe(recognised); + expect(unresolvableColumnNameOf(err)).toBe(column); + }); + } + + it('an unrecognised wording still refuses, just without a name', () => { + // `null` from the extractor must never be read as "not an unresolvable + // column after all" — that would restore the silent `[]` for every wording + // the parser has not been taught. + const err = new Error('no such column: '); + expect(isUnresolvableColumnError(err)).toBe(true); + expect(unresolvableColumnNameOf(err)).toBe(null); + }); + + it('a non-error value is not mistaken for a column failure', () => { + expect(isUnresolvableColumnError(null)).toBe(false); + expect(isUnresolvableColumnError(undefined)).toBe(false); + expect(isUnresolvableColumnError('no such column: x')).toBe(false); + expect(unresolvableColumnNameOf(null)).toBe(null); + }); +}); diff --git a/packages/drivers/driver-sql/src/sql-driver.ts b/packages/drivers/driver-sql/src/sql-driver.ts index 4fda1867da..c8b8ab59e0 100644 --- a/packages/drivers/driver-sql/src/sql-driver.ts +++ b/packages/drivers/driver-sql/src/sql-driver.ts @@ -591,6 +591,128 @@ function unsupportedFilterError(message: string): Error { return err; } +/** + * [#8790] Does this dialect error say the statement named a column the backend + * could not resolve? + * + * ONE spelling of the question, read by both halves that have to answer it — + * {@link SqlDriver.findRows}' recovery ladder and {@link SqlDriver.count}. It + * used to be an inline predicate inside `findRows` alone, which is exactly how + * one condition ended up with two behaviours: `find()` swallowed it into `[]` + * and `count()`, having no copy of the test, threw the dialect's own error. + * + * ⚠️ REACH, deliberately UNCHANGED from the inline predicate this replaces — + * SQLite (`no such column: x`) and Postgres (`column "x" does not exist`). + * MySQL spells the same condition `Unknown column 'x' in 'where clause'` + * (`ER_BAD_FIELD_ERROR`) and is matched by NEITHER arm, so on MySQL an + * unresolvable column has always travelled out as the raw dialect error and + * still does. That gap is real and is filed separately rather than closed + * here: widening this predicate would ALSO hand MySQL the #3821 projection and + * ORDER-BY recoveries it has never had, which is an accept-set change on a GA + * read path in the opposite direction from the one #8790 was ruled on — not a + * free extension of it. Widen it in a card that rules on that, not in passing. + */ +export function isUnresolvableColumnError(error: unknown): boolean { + const message = (error as { message?: unknown } | null | undefined)?.message; + if (typeof message !== 'string') return false; + return ( + message.includes('no such column') || + (message.includes('column') && message.includes('does not exist')) + ); +} + +/** + * [#8790] The column name the dialect named, or `null` when its wording did not + * yield one. + * + * The ruling requires the refusal to NAME the column, and the dialect's message + * is the only place that name exists: the driver reaches this point precisely + * when no local field map could answer the question (the registry-backstop case + * the ladder below exists for), so there is nothing else to ask. + * + * ⛔ The name is ALL that may be taken from that message. The rest of it is the + * compiled statement with the caller's bound literals inlined + * (`… where \`title\`.\`x\` = 'y' - no such column: title.x`), which is the + * predicate-text disclosure #7929 redacted elsewhere and the reason `count()`'s + * old raw throw was a defect rather than merely an unhelpful envelope. The full + * text goes to the server log — see + * {@link SqlDriver.refuseUnresolvableFilterColumn} — never to the caller. + * + * `null` is a real answer, not a failure to handle: an unrecognised wording + * still refuses, with the same code and status, just without the name. Reading + * a `null` as "not an unresolvable column after all" would restore the silent + * `[]` for every dialect message this function has not been taught. + */ +export function unresolvableColumnNameOf(error: unknown): string | null { + const message = (error as { message?: unknown } | null | undefined)?.message; + if (typeof message !== 'string') return null; + // SQLite: `… - no such column: title.x` + const sqlite = /no such column:\s*([^\s,;)]+)/.exec(message); + if (sqlite) return sqlite[1]; + // Postgres, quoted: `… - column "nosuchcol" does not exist` + const pgQuoted = /column\s+"([^"]+)"\s+does not exist/.exec(message); + if (pgQuoted) return pgQuoted[1]; + // Postgres, unquoted and possibly table-qualified: `column task.nosuchcol does not exist` + const pgBare = /column\s+([A-Za-z0-9_$.]+)\s+does not exist/.exec(message); + if (pgBare) return pgBare[1]; + return null; +} + +/** + * [#8790, maintainer ruling 2026-08-15] A WHERE the backend could not compile + * because it names a column that does not resolve — refused, on BOTH read + * halves, with the ADR-0112 envelope this path already declares. + * + * # What this replaces, and why neither old answer survived + * + * One predicate used to get two answers. `find()` fell to the #3821 ladder's + * terminal `return []` — a caller reading "no records exist" for what was + * really "your predicate never ran", the single most AI-legible failure to get + * wrong, since an agent writes its next query on that belief. `count()`, which + * has no ladder, threw the dialect's own error: `code: 'SQLITE_ERROR'`, no + * `status`, and the statement's bound literals inlined in the message. A list + * view calls both, so one query produced an empty page and a 500-shaped total. + * + * `INVALID_FILTER` / 400 is not minted here — it is the envelope every sibling + * refusal on this path already answers with ({@link unsupportedFilterError}), + * required on both SQL drivers by `cross-field-conformance-cases.ts` and pinned + * by `sql-driver-boolean-identity.test.ts` and + * `sql-driver-cross-field-conformance.test.ts`. What #8790 closes is a + * declared-vs-enforced gap, not a new posture. + * + * # Why refusal rather than recovery, on this clause only + * + * The ladder around it KEEPS recovering a projection and an ORDER BY, and that + * asymmetry is the ruling, not an oversight: "rows matter more than their + * order" is an argument about how rows are *presented*, and it does not + * transfer to a predicate. Dropping a WHERE would answer with records the + * caller explicitly excluded — a wrong answer, where a dropped sort is a + * correct answer in an unhelpful order. Recover-both was rejected for exactly + * that reason. + * + * # The wording + * + * The middle sentence is the ingress door's, verbatim + * (`assertFilterFieldsExist`, `@objectstack/metadata-protocol`): one condition + * refused at two layers must not be explained two different ways. This one adds + * what only the driver knows — that the column is missing from the TABLE, which + * a caller whose field map declares it can only fix by syncing the schema. + */ +function unresolvableFilterColumnError(object: string, column: string | null): Error { + return unsupportedFilterError( + (column === null + ? `A filter on object '${object}' names a column the database could not resolve` + : `Filter on '${column}' names a column that object '${object}' has no column for`) + + ', so the predicate never ran. A filter on a field that does not exist can only match ' + + 'zero records, so the query was refused instead of answered with an empty list. Check ' + + "the name against the object's fields; if the field was declared recently, run schema " + + 'sync so the column exists before filtering on it.' + + (column === null + ? ' The name the database reported is in the server log.' + : ''), + ); +} + /** * [#7929] The full, operand-naming text of a refusal whose caller-visible * message was redacted — carried on the Error under a SYMBOL key. @@ -4299,11 +4421,7 @@ export class SqlDriver implements IDataDriver { try { results = await builder; } catch (error: any) { - const isUnknownColumn = - error.message && - (error.message.includes('no such column') || - (error.message.includes('column') && error.message.includes('does not exist'))); - if (isUnknownColumn) { + if (isUnresolvableColumnError(error)) { // A `$select` projection naming a column the table lacks (e.g. a // generic list view auto-requesting `status`/`due_date`/`image` on an // object without them) makes the WHOLE query fail. Swallowing that @@ -4324,6 +4442,18 @@ export class SqlDriver implements IDataDriver { // drop the sort and return them unordered rather than nothing. Ladder: // projection first (it is the likelier culprit and the cheaper thing // to lose), then the sort, then give up. + // + // [#8790, maintainer ruling 2026-08-15] Where the ladder STOPS is the + // other half of the same argument. Every rung is built from + // `buildBase()`, which always re-applies `query.where` — so the ladder + // can drop a projection and can drop a sort, but it can never drop the + // clause that failed when the unresolvable column is in the WHERE. + // Both rungs then raise the same error and the method used to fall to + // `return []`. That terminal is now a refusal: rows matter more than + // their order, but nothing matters more than the predicate, because + // answering without it returns records the caller excluded. See + // {@link unresolvableFilterColumnError}. The two recoveries above are + // deliberately untouched — the ruling narrows the terminal only. const retries: Array<() => any> = []; if (query.fields) retries.push(() => buildBase().select('*')); if (orderKeys.length > 0) { @@ -4331,16 +4461,22 @@ export class SqlDriver implements IDataDriver { } results = []; let recovered = false; + // The error to NAME the column from. The last rung is the one that has + // dropped everything droppable, so a column still unresolved there is + // the caller's WHERE — naming it from the ORIGINAL error would name the + // projection column on a query whose projection the ladder just fixed. + let lastError: unknown = error; for (const retry of retries) { try { results = await retry(); recovered = true; break; - } catch { + } catch (retryError) { // Try the next, broader fallback. + lastError = retryError; } } - if (!recovered) return []; + if (!recovered) throw this.unresolvableFilterColumnRefusal(object, lastError); } else { throw error; } @@ -5890,6 +6026,34 @@ export class SqlDriver implements IDataDriver { return null; } + /** + * [#8790] Compose the refusal for a WHERE column the backend could not + * resolve, writing the dialect's own message to the SERVER LOG on the way. + * + * Logging is the half that keeps this a redaction rather than a deletion. The + * dialect message is genuinely useful — it carries the compiled statement — + * and `count()`'s old raw throw was the only place an operator ever saw it. + * It also carries the caller's bound literals inlined, which is why it may + * not travel to the caller (#7929's line, applied to the one refusal on this + * path that is raised from a dialect error rather than composed from the + * filter AST). So: statement to the log, column name to the caller. + * + * Returns the error rather than throwing it, the same shape + * {@link SqlDriver.resolveWithheldFilterRefusal} uses, so each call site + * spells its own `throw` and no reader has to know whether this returns. + */ + protected unresolvableFilterColumnRefusal(object: string, error: unknown): Error { + const column = unresolvableColumnNameOf(error); + const detail = (error as { message?: unknown } | null | undefined)?.message; + this.logger.warn( + `[sql-driver] INVALID_FILTER — a WHERE column could not be resolved on '${object}'` + + (column === null ? '' : ` ('${column}')`) + + '. The dialect message below is kept server-side because it inlines the statement ' + + `bound literals (#7929, #8790): ${typeof detail === 'string' ? detail : String(error)}`, + ); + return unresolvableFilterColumnError(object, column); + } + async count(object: string, query?: DriverQuery, options?: DriverOptions): Promise { const builder = this.getBuilder(object, options); this.applyTenantScope(builder, object, options); @@ -5898,7 +6062,23 @@ export class SqlDriver implements IDataDriver { this.applyFilters(builder, query.where); } - const result = await builder.count<{ count: number }[]>('* as count'); + // [#8790] `count()` has no recovery ladder and needs none — it carries no + // projection and no ORDER BY, so the only clause an unresolvable column can + // be in is the WHERE, which is the one clause {@link SqlDriver.findRows}' + // ladder may not drop either. What it needs is the ladder's *terminal*: the + // same `INVALID_FILTER` / 400, naming the same column, so the two halves of + // one list view stop answering one predicate two different ways. Before + // this, the dialect error travelled out untouched — `SQLITE_ERROR`, no + // `status`, bound literals inlined in the message. + let result: { count: number }[]; + try { + result = await builder.count<{ count: number }[]>('* as count'); + } catch (error) { + if (isUnresolvableColumnError(error)) { + throw this.unresolvableFilterColumnRefusal(object, error); + } + throw error; + } if (result && result.length > 0) { const row: any = result[0]; return Number(row.count ?? row['count(*)'] ?? 0); diff --git a/packages/spec/src/migrations/entries/semantic/18.driver-sql-unresolvable-where-column-refused.ts b/packages/spec/src/migrations/entries/semantic/18.driver-sql-unresolvable-where-column-refused.ts new file mode 100644 index 0000000000..53fa88bd05 --- /dev/null +++ b/packages/spec/src/migrations/entries/semantic/18.driver-sql-unresolvable-where-column-refused.ts @@ -0,0 +1,68 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import type { SemanticMigration } from '../../types.js'; + +export const entry: SemanticMigration = { + id: 'driver-sql-unresolvable-where-column-refused', + surface: + 'a `where` naming a column the table does not have, on `driver-sql` (and its ' + + '`TursoDriver` / `SqliteWasmDriver` subclasses) — `find()` / `findOne()` answered ' + + '`[]` and `count()` threw the dialect\'s own error; both now refuse with ' + + '`INVALID_FILTER` / 400', + replacement: + 'name a column the object actually has, or run schema sync so a recently declared ' + + 'field exists as a column before filtering on it. A caller that legitimately wants ' + + '"no rows unless this matches" gets that from a predicate over a real column; there ' + + 'is no spelling of an unresolvable column that means "match nothing", which is ' + + 'exactly what the old empty list was mistaken for', + reason: + 'One predicate had two answers. `SqlDriver.findRows()` carries the #3821 unknown-' + + 'column recovery ladder, whose rungs are all built from `buildBase()` — and ' + + '`buildBase()` always re-applies `query.where`. So the ladder can drop a projection ' + + 'and can drop an ORDER BY, but it can never drop the clause that failed when the ' + + 'unresolvable column is in the WHERE: both rungs raise the same error and the method ' + + 'fell to `return []`. `SqlDriver.count()` runs a separate statement with no ladder at ' + + 'all, so the identical predicate threw. Measured on better-sqlite3, one seeded row: ' + + "`where { 'title.x': 'y' }` gave `find()` 0 rows and NO error, while `count()` threw " + + "`code: 'SQLITE_ERROR'`, `status: undefined`, message `select count(*) as \\`count\\` " + + "from \\`task\\` where \\`title\\`.\\`x\\` = 'y' - no such column: title.x`.\n\n" + + 'A list view calls both halves, so one query produced an empty page from the rows ' + + 'half and a 500-shaped failure from the total half — and a caller reading only the ' + + 'rows got a silent empty page saying "no records exist" for what was really "your ' + + 'predicate never ran". That is the single most AI-legible failure to get wrong: an ' + + 'agent reads "no matching records" and writes its next query on that belief. The ' + + "thrown half was no better — the dialect's own `code`, no `status` (an unclassified " + + '5xx at the REST boundary rather than a caller mistake), and the statement\'s bound ' + + 'literals inlined in the message, the same predicate-text disclosure shape #7929 ' + + 'redacted elsewhere.\n\n' + + 'Ruled 2026-08-15 on #8790: refuse BOTH halves with `INVALID_FILTER` / 400, naming ' + + 'the column. The envelope is not minted here — it is what every sibling refusal on ' + + 'this path already answers, required on both SQL drivers by ' + + '`cross-field-conformance-cases.ts` and pinned by `sql-driver-boolean-identity.test.ts` ' + + 'and `sql-driver-cross-field-conformance.test.ts` — so what closes is a declared-vs-' + + 'enforced gap, not a new posture. Recover-both was excluded by the card\'s own ' + + 'argument: dropping a WHERE returns rows the caller explicitly excluded, and #3821\'s ' + + '"rows matter more than their order" is an argument about how rows are PRESENTED, ' + + 'which does not transfer to a predicate. The ladder KEEPS both of its recoveries — ' + + 'only the WHERE-failure terminal became a refusal.\n\n' + + 'Reach, stated rather than assumed: the refusal fires on the wordings the ladder has ' + + 'always recognised — SQLite (`no such column: x`) and Postgres (`column "x" does not ' + + "exist`). MySQL spells it `Unknown column 'x' in 'where clause'`, which neither arm " + + 'matches, so on MySQL this condition still travels out as the raw dialect error; ' + + 'widening that predicate would also hand MySQL the #3821 recoveries it has never had, ' + + 'which is an accept-set change in the opposite direction and is filed separately.\n\n' + + 'This is a CODE-path API, not stored metadata, so — like ' + + '`engine-dotted-projection-refused` and `engine-find-formula-filter-refused` — there ' + + 'is no `sys_metadata` row for the D2 chain to rewrite and this entry is the ' + + 'notification channel. No mechanical rewrite exists: the platform cannot know which ' + + 'real column a mistyped filter key meant, and guessing one would answer with rows the ' + + 'caller never asked for. #8790, #3821, #7929, #8371, ADR-0112.', + acceptanceCriteria: + 'No saved report `query.filter`, flow condition, sharing/permission rule or hook ' + + 'filters on a name the queried object has no column for. Reads and counts complete ' + + 'with no `INVALID_FILTER` whose message says "names a column that object" or "names a ' + + 'column the database could not resolve". Where a filter key was a relationship ' + + 'traversal spelled as a dotted path, rewrite it against a column on the queried ' + + 'object — the driver never resolved such a path and answered `[]`, so any list that ' + + 'looked correct under one was already showing nothing.', +}; diff --git a/packages/spec/src/migrations/registry.ts b/packages/spec/src/migrations/registry.ts index ed9e62d81a..7a4ceb39ba 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -4986,6 +4986,70 @@ const step18: MigrationStep = { 'the connection form) and still connects; no URL-embedded credential remains in any ' + 'stored `sys_metadata` row or authored source.', }, + { + id: 'driver-sql-unresolvable-where-column-refused', + surface: + 'a `where` naming a column the table does not have, on `driver-sql` (and its ' + + '`TursoDriver` / `SqliteWasmDriver` subclasses) — `find()` / `findOne()` answered ' + + '`[]` and `count()` threw the dialect\'s own error; both now refuse with ' + + '`INVALID_FILTER` / 400', + replacement: + 'name a column the object actually has, or run schema sync so a recently declared ' + + 'field exists as a column before filtering on it. A caller that legitimately wants ' + + '"no rows unless this matches" gets that from a predicate over a real column; there ' + + 'is no spelling of an unresolvable column that means "match nothing", which is ' + + 'exactly what the old empty list was mistaken for', + reason: + 'One predicate had two answers. `SqlDriver.findRows()` carries the #3821 unknown-' + + 'column recovery ladder, whose rungs are all built from `buildBase()` — and ' + + '`buildBase()` always re-applies `query.where`. So the ladder can drop a projection ' + + 'and can drop an ORDER BY, but it can never drop the clause that failed when the ' + + 'unresolvable column is in the WHERE: both rungs raise the same error and the method ' + + 'fell to `return []`. `SqlDriver.count()` runs a separate statement with no ladder at ' + + 'all, so the identical predicate threw. Measured on better-sqlite3, one seeded row: ' + + "`where { 'title.x': 'y' }` gave `find()` 0 rows and NO error, while `count()` threw " + + "`code: 'SQLITE_ERROR'`, `status: undefined`, message `select count(*) as \\`count\\` " + + "from \\`task\\` where \\`title\\`.\\`x\\` = 'y' - no such column: title.x`.\n\n" + + 'A list view calls both halves, so one query produced an empty page from the rows ' + + 'half and a 500-shaped failure from the total half — and a caller reading only the ' + + 'rows got a silent empty page saying "no records exist" for what was really "your ' + + 'predicate never ran". That is the single most AI-legible failure to get wrong: an ' + + 'agent reads "no matching records" and writes its next query on that belief. The ' + + "thrown half was no better — the dialect's own `code`, no `status` (an unclassified " + + '5xx at the REST boundary rather than a caller mistake), and the statement\'s bound ' + + 'literals inlined in the message, the same predicate-text disclosure shape #7929 ' + + 'redacted elsewhere.\n\n' + + 'Ruled 2026-08-15 on #8790: refuse BOTH halves with `INVALID_FILTER` / 400, naming ' + + 'the column. The envelope is not minted here — it is what every sibling refusal on ' + + 'this path already answers, required on both SQL drivers by ' + + '`cross-field-conformance-cases.ts` and pinned by `sql-driver-boolean-identity.test.ts` ' + + 'and `sql-driver-cross-field-conformance.test.ts` — so what closes is a declared-vs-' + + 'enforced gap, not a new posture. Recover-both was excluded by the card\'s own ' + + 'argument: dropping a WHERE returns rows the caller explicitly excluded, and #3821\'s ' + + '"rows matter more than their order" is an argument about how rows are PRESENTED, ' + + 'which does not transfer to a predicate. The ladder KEEPS both of its recoveries — ' + + 'only the WHERE-failure terminal became a refusal.\n\n' + + 'Reach, stated rather than assumed: the refusal fires on the wordings the ladder has ' + + 'always recognised — SQLite (`no such column: x`) and Postgres (`column "x" does not ' + + "exist`). MySQL spells it `Unknown column 'x' in 'where clause'`, which neither arm " + + 'matches, so on MySQL this condition still travels out as the raw dialect error; ' + + 'widening that predicate would also hand MySQL the #3821 recoveries it has never had, ' + + 'which is an accept-set change in the opposite direction and is filed separately.\n\n' + + 'This is a CODE-path API, not stored metadata, so — like ' + + '`engine-dotted-projection-refused` and `engine-find-formula-filter-refused` — there ' + + 'is no `sys_metadata` row for the D2 chain to rewrite and this entry is the ' + + 'notification channel. No mechanical rewrite exists: the platform cannot know which ' + + 'real column a mistyped filter key meant, and guessing one would answer with rows the ' + + 'caller never asked for. #8790, #3821, #7929, #8371, ADR-0112.', + acceptanceCriteria: + 'No saved report `query.filter`, flow condition, sharing/permission rule or hook ' + + 'filters on a name the queried object has no column for. Reads and counts complete ' + + 'with no `INVALID_FILTER` whose message says "names a column that object" or "names a ' + + 'column the database could not resolve". Where a filter key was a relationship ' + + 'traversal spelled as a dotted path, rewrite it against a column on the queried ' + + 'object — the driver never resolved such a path and answered `[]`, so any list that ' + + 'looked correct under one was already showing nothing.', + }, { id: 'field-scale-precision-integer-refused', surface: 'object field `scale` / `precision` declarations (`Field.number` and friends) — '