diff --git a/.changeset/20351-number-comparand-door.md b/.changeset/20351-number-comparand-door.md new file mode 100644 index 00000000000..fbbd71619a9 --- /dev/null +++ b/.changeset/20351-number-comparand-door.md @@ -0,0 +1,35 @@ +--- +"@objectstack/objectql": minor +--- + +fix(objectql)!: a string compared against a number field must be a number: a non-numeric one is refused with `INVALID_FILTER` / 400 on `where`, a per-aggregation `filter` and `having`, and a numeric one is narrowed to its number (#20351) + +Clause-②: no (narrowing) + + + +**BREAKING**: this narrows what a filter may compare a number field with. A string the platform's numeric grammar does not read as a number used to answer 200 with no rows (every row under `$ne`) on memory and SQLite and a 500 on PostgreSQL; it now answers 400, before any read, on every driver. It ships as `minor` under the launch-window convention for accept-set narrowings. `@objectstack/objectql`'s root exports are unchanged. + +FROM a string that is not a JSON number spelling of a finite number (`"abc"`, `""`, `" 12 "`, `"0x10"`, `"1,000"`, `"+5"`, `"007"`, `"Infinity"`, a `{placeholder}`), compared against a `number`, `currency`, `percent`, `rating`, `slider`, `progress` or `summary` field (or a `count` / `sum` / `avg`, or a numeric `min` / `max` / groupBy column in `having`) at the implicit comparand, `$eq` / `$ne` / `$gt` / `$gte` / `$lt` / `$lte`, or a member of `$in` / `$nin` / `$between` → TO `INVALID_FILTER` / 400, naming the field, its declared type, the comparand, its position and what is wrong with it. The fix is one line: send the number (`12`, `-3.5`, `1e3`) or a string of exactly that spelling (`"12"`). + +Measured through `engine.find` / `engine.aggregate` and `POST /data/:object/query` (the two doors agree), three rows (5, 12, 30): + +| position | comparand on a `number` field | before: memory · SQLite · PostgreSQL 16 | now, on all three | +|:--|:--|:--|:--| +| `where` | `$gt` / `$eq` / implicit / a `$in` member `"abc"`; `$eq ""` | no rows · no rows · `DATABASE_ERROR` (500) | `INVALID_FILTER` / 400 | +| `where` | `$ne "abc"` | every row · every row · 500 | `INVALID_FILTER` / 400 | +| `where`, over REST | `$gt "{current_user_id}"` (resolved to the user's id) | no rows · no rows · 500 | `INVALID_FILTER` / 400 | +| per-aggregation `filter` | `$gt "abc"` (`$ne "abc"`) | count 0 (3), on all three | `INVALID_FILTER` / 400 | +| `having` on `sum(amount)` | `$gt "abc"` (`$ne "abc"`) | no group (every group), on all three | `INVALID_FILTER` / 400 | +| `where` | `$gt "12"` / `$eq "12"` | **no rows** · 1 row · 1 row | 1 row, the number's answer | + +What changes: + +- A new door at the engine's single filter collection point, after the temporal-comparand door. It reads `@objectstack/spec/data`'s published contract (`numberComparandDoorVerdict` over `NUMERIC_VALUE_TYPES`, the numeric grammar, and `numberComparandRefusalMessage` for the words); the engine carries no numeric grammar of its own. +- It runs on `where` in both spellings (the filter object and the `FilterArray` sugar) on `find`, `findOne`, `count`, `aggregate`, `update` and `delete`, and on `IObjectQLEngine.judgeFilter`; on each per-aggregation `filter`, against the object's declared fields; and on `having`, over the columns the engine classes numeric. +- A numeric string is rewritten to its number, copy-on-write, before any driver or in-memory evaluator reads it. InMemoryDriver used to compare `"12"` as a string and match nothing; it now matches what `12` matches, as SQLite and PostgreSQL already did. +- A `{placeholder}` compared against a number field is refused unresolved: every filter token resolves to an id or a date, never a number. + +**Who is affected.** A caller that compares a number field with a string that is not a plain number, through any door that reaches the engine: a REST query parameter (`?amount=abc`), a `where` / `$filter` / `filter` body of `POST /data/:object/query`, or an in-process engine call. A caller that sends a number, or a numeric string such as `"12"` or `"1e3"`, is unaffected, except that memory now answers it as the other drivers do. + +**Unchanged.** A numeric comparand: the `$gt 10` controls at `where`, the per-aggregation `filter` and `having` answered identically before and after on memory, SQLite and PostgreSQL, through the engine and REST. Not judged by this door, so the filter reaches the driver as written (pinned per case in the engine suite): a string compared against a non-numeric field; `$null` / `$exists` / `$empty`; the text operators (a text operator over a number field keeps its own refusal); a `{ $field }` reference; a dotted key; a key that names no declared field. A `formula` field is still refused one door earlier with `INVALID_FIELD`. RLS, sharing and tenant predicates the security layer composes onto a query are not judged by this door; a policy predicate is judged at authoring where the host hands the rule the engine's `judgeFilter`. A boolean or a `Date` compared against a number field is outside this contract (it judges strings) and keeps its old answer, measured: `$gt true` no rows on memory, every row on SQLite and a 500 on PostgreSQL; a `Date` no rows on memory and SQLite and a 500 on PostgreSQL. diff --git a/packages/objectql/src/engine-aggregate-having-temporal-door.test.ts b/packages/objectql/src/engine-aggregate-having-temporal-door.test.ts index 22d06373b54..6ffa642721b 100644 --- a/packages/objectql/src/engine-aggregate-having-temporal-door.test.ts +++ b/packages/objectql/src/engine-aggregate-having-temporal-door.test.ts @@ -331,9 +331,6 @@ describe('[#20263] having — what the door leaves alone answers exactly as befo ['the first instant of year 1 on min(datetime) — inside the range', { first_opened: { $gt: '0001-01-01T00:00:00.000Z' } }, ['c1', 'c2', 'c3', 'c4']], ['a wall clock on max(time)', { last_slot: { $gte: '12:00' } }, ['c2', 'c3', 'c4']], ['the number for 10000-01-01 on max(time) — not judged on time', { last_slot: { $gt: Y10000 } }, []], - ['a string on sum — not temporal', { total: { $gt: 'not-a-date' } }, []], - ['a string on count — not temporal', { n: { $gt: 'not-a-date' } }, []], - ['a string on avg — not temporal', { mean: { $gt: 'not-a-date' } }, []], ['a {placeholder} is stepped around, as on where', { last_placed: { $lte: '{today}' } }, ['c1', 'c2', 'c3', 'c4']], ['the empty string (its own card)', { last_placed: { $gt: '' } }, ['c1', 'c2', 'c3', 'c4']], ['null in the equality slot', { last_placed: null }, []], @@ -353,6 +350,22 @@ describe('[#20263] having — what the door leaves alone answers exactly as befo }); } + // [#20351] A string on a NUMERIC column is not this door's either, and it is + // no longer compared as written: the number-comparand door, which runs after + // this one, refuses it in its own words, before any read. (It kept no group, + // with a 200, before that door existed.) + it('a string on sum, count or avg is not this door\'s: the number-comparand door refuses it, before any read', async () => { + for (const column of ['total', 'n', 'mean']) { + for (const path of ['native', 'rows'] as const) { + const { engine, reads } = await makeEngine(path, ROWS); + const { err } = await outcome(() => engine.aggregate(OBJECT, query(path, { [column]: { $gt: 'not-a-date' } }))); + expect({ code: err?.code, status: err?.status }, `${column} ${path}`).toEqual({ code: 'INVALID_FILTER', status: 400 }); + expect(err?.message, `${column} ${path}`).toContain(`compares a declared number field against "not-a-date" at having.${column}.$gt`); + expect(reads, `${column} ${path}`).toEqual({ aggregate: 0, find: 0 }); + } + } + }); + // [#20334] An unknown one is stepped around by this door too, and is then // refused one layer down by the token resolver, in its own code, as on // `where` (it kept no group with a 200 before `having` resolved tokens). diff --git a/packages/objectql/src/engine-aggregate-positions.test.ts b/packages/objectql/src/engine-aggregate-positions.test.ts index ffae736e1bd..63271265bc9 100644 --- a/packages/objectql/src/engine-aggregate-positions.test.ts +++ b/packages/objectql/src/engine-aggregate-positions.test.ts @@ -242,8 +242,11 @@ describe('[#20334] having — a placeholder the resolver cannot resolve is refus const REFUSED: ReadonlyArray unknown, Record, string]> = [ ["the card's row: an unknown token on max(date), which kept no group", () => ({ last_placed: { $gte: '{not_a_token}' } }), { placed_on: { $gte: '{not_a_token}' } }, 'FILTER_TOKEN_UNKNOWN'], - ['an unknown token on count, a column no temporal door judges', - () => ({ n: { $gte: '{not_a_token}' } }), { amount: { $gte: '{not_a_token}' } }, 'FILTER_TOKEN_UNKNOWN'], + // [#20351] On a text column: a NUMERIC column (`n`, a count) is now the + // number-comparand door's, which refuses a placeholder unresolved — no + // filter token resolves to a number. + ['an unknown token on a text groupBy column, which neither the temporal nor the number door judges', + () => ({ customer_id: { $gte: '{not_a_token}' } }), { customer_id: { $gte: '{not_a_token}' } }, 'FILTER_TOKEN_UNKNOWN'], ['a near-miss spelling ({TODAY})', () => ({ last_placed: { $gte: '{TODAY}' } }), { placed_on: { $gte: '{TODAY}' } }, 'FILTER_TOKEN_UNKNOWN'], ['an unknown token under $and, beside an arm that holds', diff --git a/packages/objectql/src/engine-number-comparand-declared-type-door.test.ts b/packages/objectql/src/engine-number-comparand-declared-type-door.test.ts new file mode 100644 index 00000000000..40b4807b763 --- /dev/null +++ b/packages/objectql/src/engine-number-comparand-declared-type-door.test.ts @@ -0,0 +1,467 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#20351] The NUMBER-comparand declared-type door at the engine's filter + * collection point — lane (2) of the two-lane route #20336 took on #15661's + * precedent. + * + * The door's definition lives in `@objectstack/spec/data` + * (`filter-number-comparand-declared-type.ts`, lane 1): the grammar, the + * verdict, the words, the fixture and the derived case table. This file is its + * CONSUMER, driven the way that module's header prescribes — register + * {@link NUMBER_COMPARAND_DOOR_FIXTURE} against a recording driver and run the + * cases through `find`: + * + * - `door-refusal`: rejects with `code` AND `status` (the ADR-0112 envelope — + * `toThrow()` alone is not a pin), the message carries every `mustMention` + * substring, and NO driver read ran. + * - `narrows`: the driver read ran and received `c.expectedFilter()`. + * - `passes` / `deferred`: the driver read ran and received the filter as + * written. + * + * ## Two partitions, each measured rather than dropped + * + * - **`formula`** — refused one door EARLIER, by the #8296 materializable door, + * with `INVALID_FIELD`, whatever its return type: no driver materialises a + * formula column. Pinned in the direction it answers (the contract's module + * header says the suite partitions these rows out), and the door's own walk + * is pinned to judge the class correctly for the day that neighbour opens. + * - **The staged `$empty` row** — `$empty` is declared but staged out of + * `FILTER_OPERATORS` until its engine arm lands (#20311's lane cards), so an + * end-to-end drive can answer it for a reason that is not this door's. The + * row is pinned at the DOOR ALONE (the walk and the narrowing return it + * untouched), and partitioned out of the engine drive. + * + * The three-driver and REST cells (InMemoryDriver's answer is this suite's + * recording driver's by construction — the door runs before any driver is + * resolved; SqlDriver on SQLite, PostgreSQL and MySQL) live in + * `@objectstack/rest`'s `data-number-comparand-door.test.ts`. + * + * @see https://github.com/objectstack-ai/objectstack/issues/20351 (this door) + * @see https://github.com/objectstack-ai/objectstack/issues/20336 (the contract) + */ + +import { describe, it, expect, beforeEach } from 'vitest'; +import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; +import { + NON_NUMERIC_STRING_FORMS, + NUMBER_COMPARAND_DOOR_CASES, + NUMBER_COMPARAND_DOOR_FIXTURE, + NUMBER_COMPARAND_DOOR_FIXTURE_OBJECT, + NUMBER_COMPARAND_DOOR_LIST_OPERATORS, + NUMBER_COMPARAND_DOOR_SCALAR_OPERATORS, + type EngineAggregateOptions, + type EngineQueryOptions, + type FilterCondition, + type NumberComparandDoorCase, + type NumberComparandDoorNarrowsCase, + type NumberComparandDoorRefusalCase, +} from '@objectstack/spec/data'; +import { ObjectQL } from './engine.js'; +import { + findNonNumericComparand, + narrowHavingNumberComparands, + narrowNumberComparands, +} from './number-comparand-declared-type-door.js'; + +const OBJECT = NUMBER_COMPARAND_DOOR_FIXTURE_OBJECT; + +interface SeenRead { ast: any } + +/** Minimal recording driver — the same witness shape as the sibling door suites. */ +function makeRecordingDriver() { + const rows = new Map>(); + const reads: SeenRead[] = []; + const writes: SeenRead[] = []; + const run = (_ast: any) => [...rows.values()]; + const driver: any = { + name: 'recording', version: '0.0.0', supports: {}, + async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; }, + async find(_o: string, ast: any) { reads.push({ ast }); return run(ast); }, + async findOne(_o: string, ast: any) { reads.push({ ast }); return run(ast)[0] ?? null; }, + async count(_o: string, ast: any) { reads.push({ ast }); return run(ast).length; }, + async create(_o: string, data: Record) { + const id = (data.id as string) ?? `r_${rows.size + 1}`; + const row = { ...data, id }; rows.set(id, row); return row; + }, + async update(_o: string, id: string, data: Record) { + const cur = rows.get(id) ?? {}; + const up = { ...cur, ...data, id }; rows.set(id, up); return up; + }, + async updateMany(_o: string, ast: any) { writes.push({ ast }); return 0; }, + async delete(_o: string, id: string) { return rows.delete(id); }, + async deleteMany(_o: string, ast: any) { writes.push({ ast }); return 0; }, + async bulkCreate(o: string, batch: Record[]) { + return Promise.all(batch.map((r) => this.create(o, r))); + }, + async beginTransaction() { return { commit: async () => {}, rollback: async () => {} }; }, + async commit() {}, async rollback() {}, + }; + return { driver, reads, writes, rows }; +} + +type Thrown = (Error & { code?: string; status?: number; httpStatus?: number }) | null; + +const refusalOf = async (p: Promise): Promise => + p.then(() => null, (e: any) => e as Error & { code?: string; status?: number }); + +/** A `formula` case — judged one door earlier, see the header. */ +const isFormulaCase = (c: NumberComparandDoorCase): boolean => c.declaredType === 'formula'; +/** The staged `$empty` row — pinned at the door alone, see the header. */ +const isStagedCase = (c: NumberComparandDoorCase): boolean => c.position.endsWith('.$empty'); +const engineDriven = (c: NumberComparandDoorCase): boolean => !isFormulaCase(c) && !isStagedCase(c); + +const SEEDED = [ + { id: 'r1', f_number: 5 }, + { id: 'r2', f_number: 12 }, + { id: 'r3', f_number: 30 }, +]; + +describe('[#20351] the number-comparand declared-type door at the engine collection point', () => { + let engine: ObjectQL; + let reads: SeenRead[]; + let writes: SeenRead[]; + + beforeEach(async () => { + const rec = makeRecordingDriver(); + reads = rec.reads; + writes = rec.writes; + engine = new ObjectQL(); + engine.registerDriver(rec.driver, true); + await engine.init(); + engine.registry.registerObject(NUMBER_COMPARAND_DOOR_FIXTURE as any, 'test'); + for (const row of SEEDED) await engine.insert(OBJECT, { ...row }); + reads.length = 0; + writes.length = 0; + }); + + // ── the derived case table, driven end to end ──────────────────────────── + + const REFUSALS = NUMBER_COMPARAND_DOOR_CASES.filter( + (c): c is NumberComparandDoorRefusalCase => c.verdict === 'door-refusal' && engineDriven(c)); + const NARROWS = NUMBER_COMPARAND_DOOR_CASES.filter( + (c): c is NumberComparandDoorNarrowsCase => c.verdict === 'narrows' && engineDriven(c)); + const PASSES = NUMBER_COMPARAND_DOOR_CASES.filter((c) => c.verdict === 'passes' && engineDriven(c)); + const DEFERRED = NUMBER_COMPARAND_DOOR_CASES.filter((c) => c.verdict === 'deferred' && engineDriven(c)); + const FORMULA = NUMBER_COMPARAND_DOOR_CASES.filter(isFormulaCase); + const STAGED = NUMBER_COMPARAND_DOOR_CASES.filter(isStagedCase); + + it('GUARD the case table is partitioned exactly, and every partition that carries a verdict is non-empty', () => { + expect(NUMBER_COMPARAND_DOOR_CASES.length).toBe( + REFUSALS.length + NARROWS.length + PASSES.length + DEFERRED.length + FORMULA.length + STAGED.length, + ); + expect(REFUSALS.length).toBeGreaterThan(0); + expect(NARROWS.length).toBeGreaterThan(0); + expect(PASSES.length).toBeGreaterThan(0); + expect(FORMULA.length).toBeGreaterThan(0); + // The untyped formula is the table's only deferred row, and it is judged one door earlier. + expect(DEFERRED).toHaveLength(0); + expect(STAGED.map((c) => c.verdict)).toEqual(['passes']); + // Every refused form the grammar names is driven, not just the card's "abc". + expect(new Set(REFUSALS.map((c) => c.form))).toEqual(new Set(NON_NUMERIC_STRING_FORMS)); + // Every judged position is driven both ways. + const positions = (cs: readonly NumberComparandDoorCase[]) => + new Set(cs.filter((c) => c.key === 'f_number').map((c) => c.position.replace(/\[\d\]$/, ''))); + const judged = ['f_number', ...NUMBER_COMPARAND_DOOR_SCALAR_OPERATORS.map((op) => `f_number.${op}`), + ...NUMBER_COMPARAND_DOOR_LIST_OPERATORS.map((op) => `f_number.${op}`)]; + for (const p of judged) { + expect(positions(REFUSALS).has(p), `refused at ${p}`).toBe(true); + expect(positions(NARROWS).has(p), `narrowed at ${p}`).toBe(true); + } + }); + + it('refuses every door-refusal case with the ADR-0112 envelope, in the contract\'s words, and NO driver read runs', async () => { + for (const c of REFUSALS) { + reads.length = 0; + const err = await refusalOf(engine.find(OBJECT, { where: c.filter() })); + expect(err, `${c.name}: expected a refusal`).not.toBeNull(); + expect({ code: err!.code, status: err!.status }, c.name).toEqual({ code: c.code, status: c.status }); + expect(err!.httpStatus, c.name).toBe(400); + for (const substring of c.mustMention) { + expect(err!.message, `${c.name}: message must mention ${substring}`).toContain(substring); + } + expect(err!.message, c.name).toMatch(/^find\('number_door_probe'\): /); + expect(err!.message, c.name).toMatch(/NOT applied/); + expect(reads, `${c.name}: the driver must not have been read`).toHaveLength(0); + } + }); + + it('narrows every numeric string to its number — the driver receives the expected filter, the caller\'s is untouched', async () => { + for (const c of NARROWS) { + reads.length = 0; + const filter = c.filter(); + const asWritten = JSON.stringify(filter); + await expect(engine.find(OBJECT, { where: filter }), c.name).resolves.toBeDefined(); + expect(reads, `${c.name}: the driver must have been read`).toHaveLength(1); + expect(reads[0]?.ast?.where, `${c.name}: the driver must receive the number`).toEqual(c.expectedFilter()); + // Copy-on-write: the filter belongs to the caller (view metadata, flow config). + expect(JSON.stringify(filter), `${c.name}: the caller's filter must not be edited`).toBe(asWritten); + } + }); + + it('lets every passing case through UNCHANGED — not a numeric field, or not a string', async () => { + for (const c of PASSES) { + reads.length = 0; + const filter = c.filter(); + await expect(engine.find(OBJECT, { where: filter }), c.name).resolves.toBeDefined(); + expect(reads, `${c.name}: the driver must have been read`).toHaveLength(1); + expect(reads[0]?.ast?.where, `${c.name}: the filter must reach the driver unchanged`).toEqual(filter); + } + }); + + it('NAMED DIVERGENCE — every formula case is refused one door EARLIER, by #8296, with INVALID_FIELD', async () => { + for (const c of FORMULA) { + reads.length = 0; + const err = await refusalOf(engine.find(OBJECT, { where: c.filter() })); + expect(err, `${c.name}: expected the #8296 refusal`).not.toBeNull(); + expect({ code: err!.code, status: err!.status }, c.name).toEqual({ code: 'INVALID_FIELD', status: 400 }); + expect(reads, c.name).toHaveLength(0); + } + // …and the door's own walk judges the class by its return type, so the day + // that neighbour opens, this door already answers. + const schema = engine.registry.getObject(OBJECT); + expect(findNonNumericComparand(schema, { f_formula_number: { $gt: 'abc' } })) + .toMatchObject({ field: 'f_formula_number', declaredType: 'formula', returnType: 'number', form: 'not-a-number' }); + expect(findNonNumericComparand(schema, { f_formula_text: { $gt: 'abc' } })).toBeNull(); + expect(findNonNumericComparand(schema, { f_formula_untyped: { $gt: 'abc' } })).toBeNull(); + }); + + it('the staged $empty row is pinned at the DOOR ALONE — the walk neither refuses nor rewrites it', () => { + const schema = engine.registry.getObject(OBJECT); + for (const c of STAGED) { + const filter = c.filter(); + expect(findNonNumericComparand(schema, filter), c.name).toBeNull(); + expect(narrowNumberComparands(OBJECT, 'find', schema, filter), c.name).toBe(filter); + } + }); + + // ── the door's reach: every verb, both filter forms, nested structure ──── + + it('covers every engine verb that collects a filter — read and write sides', async () => { + const where = { f_number: { $gt: 'abc' } }; + for (const call of [ + () => engine.find(OBJECT, { where }), + () => engine.findOne(OBJECT, { where }), + () => engine.count(OBJECT, { where }), + () => engine.aggregate(OBJECT, { where, aggregations: [{ function: 'count', alias: 'n' }] } as EngineAggregateOptions), + () => engine.update(OBJECT, { f_text: 'x' }, { where, multi: true }), + () => engine.delete(OBJECT, { where, multi: true }), + ]) { + const err = await refusalOf(call()); + expect(err).not.toBeNull(); + expect({ code: err!.code, status: err!.status }).toEqual({ code: 'INVALID_FILTER', status: 400 }); + expect(err!.message).toContain("'f_number'"); + } + expect(reads).toHaveLength(0); + expect(writes).toHaveLength(0); + }); + + it('answers the same mistake arriving as FilterArray sugar — one answer per mistake, not per spelling', async () => { + // The cast names the contract being bypassed: `FilterArray` is INPUT-ONLY + // sugar `EngineQueryOptions.where` deliberately excludes (#5285). + const err = await refusalOf( + engine.find(OBJECT, { where: [['f_number', '>', 'abc']] } as unknown as EngineQueryOptions), + ); + expect({ code: err!.code, status: err!.status }).toEqual({ code: 'INVALID_FILTER', status: 400 }); + expect(err!.message).toContain('where.f_number.$gt'); + expect(reads).toHaveLength(0); + + await engine.find(OBJECT, { where: [['f_number', '>', '12']] } as unknown as EngineQueryOptions); + expect(reads).toHaveLength(1); + expect(reads[0]?.ast?.where).toEqual({ f_number: { $gt: 12 } }); + }); + + it('reaches inside $and / $or / $not — structure does not launder the comparand, and narrowing reaches there too', async () => { + for (const where of [ + { $and: [{ f_text: 'a' }, { f_number: { $gt: 'abc' } }] }, + { $or: [{ f_text: 'a' }, { f_currency: { $in: [1, 'x'] } }] }, + { $not: { f_percent: { $between: ['', 10] } } }, + ]) { + reads.length = 0; + const err = await refusalOf(engine.find(OBJECT, { where: where as FilterCondition })); + expect(err, JSON.stringify(where)).not.toBeNull(); + expect({ code: err!.code, status: err!.status }).toEqual({ code: 'INVALID_FILTER', status: 400 }); + expect(reads).toHaveLength(0); + } + await engine.find(OBJECT, { where: { $or: [{ f_text: 'a' }, { $not: { f_number: { $in: ['12', 5] } } }] } }); + expect(reads[0]?.ast?.where).toEqual({ $or: [{ f_text: 'a' }, { $not: { f_number: { $in: [12, 5] } } }] }); + }); + + it('refuses a {placeholder} against a number field UNRESOLVED — before the token resolver, in the door\'s words', async () => { + const err = await refusalOf(engine.find( + OBJECT, + { where: { f_number: { $gt: '{current_user_id}' } }, context: { userId: 'u1' } } as EngineQueryOptions, + )); + expect({ code: err!.code, status: err!.status }).toEqual({ code: 'INVALID_FILTER', status: 400 }); + expect(err!.message).toContain('{placeholder}'); + expect(reads).toHaveLength(0); + }); + + it('the judge-only judgeFilter gives the verdict execution gives, without a read', () => { + const refused = engine.judgeFilter(OBJECT, { f_number: { $gt: 'abc' } }); + expect(refused).toMatchObject({ ok: false, code: 'INVALID_FILTER', status: 400 }); + expect((refused as { message: string }).message).toContain("'f_number'"); + expect(engine.judgeFilter(OBJECT, { f_number: { $gt: '12' } })).toEqual({ ok: true }); + expect(reads).toHaveLength(0); + }); + + // ── the per-aggregation `filter` and `having` ──────────────────────────── + + it('refuses a non-numeric string in ONE aggregation\'s own filter, rooted at that position — no read', async () => { + for (const op of ['$gt', '$ne', '$eq'] as const) { + reads.length = 0; + const err = await refusalOf(engine.aggregate(OBJECT, { + aggregations: [ + { function: 'count', alias: 'all' }, + { function: 'count', alias: 'bad', filter: { f_number: { [op]: 'abc' } } }, + ], + } as EngineAggregateOptions)); + expect(err, op).not.toBeNull(); + expect({ code: err!.code, status: err!.status }, op).toEqual({ code: 'INVALID_FILTER', status: 400 }); + expect(err!.message, op).toContain(`aggregations[1].filter.f_number.${op}`); + expect(err!.message, op).toMatch(/^aggregate\('number_door_probe'\): /); + expect(reads, op).toHaveLength(0); + } + }); + + it('a numeric string in a per-aggregation filter counts what its number counts — the control', async () => { + const count = async (filter: FilterCondition) => { + const rows = await engine.aggregate(OBJECT, { + aggregations: [{ function: 'count', alias: 'all' }, { function: 'count', alias: 'm', filter }], + } as EngineAggregateOptions); + return Number((rows[0] as Record).m); + }; + for (const [asString, asNumber] of [ + [{ f_number: { $eq: '12' } }, { f_number: { $eq: 12 } }], + [{ f_number: { $gt: '10' } }, { f_number: { $gt: 10 } }], + [{ f_number: { $in: ['5', '30'] } }, { f_number: { $in: [5, 30] } }], + ] as const) { + expect(await count(asString as FilterCondition), JSON.stringify(asString)).toBe(await count(asNumber as FilterCondition)); + } + expect(await count({ f_number: { $eq: '12' } })).toBe(1); + }); + + it('refuses a non-numeric string against a NUMERIC `having` column — count, sum and a numeric min alike', async () => { + for (const [fn, field] of [['count', undefined], ['sum', 'f_number'], ['min', 'f_currency']] as const) { + for (const op of ['$gt', '$ne'] as const) { + reads.length = 0; + const err = await refusalOf(engine.aggregate(OBJECT, { + groupBy: ['f_text'], + aggregations: [{ function: fn, ...(field ? { field } : {}), alias: 'total' }], + having: { total: { [op]: 'abc' } }, + } as EngineAggregateOptions)); + expect(err, `${fn} ${op}`).not.toBeNull(); + expect({ code: err!.code, status: err!.status }, `${fn} ${op}`).toEqual({ code: 'INVALID_FILTER', status: 400 }); + expect(err!.message, `${fn} ${op}`).toContain(`having.total.${op}`); + expect(reads, `${fn} ${op}`).toHaveLength(0); + } + } + }); + + it('a {placeholder} against a numeric `having` column is refused unresolved, as on `where` — before the resolver', async () => { + const err = await refusalOf(engine.aggregate(OBJECT, { + groupBy: ['f_text'], + aggregations: [{ function: 'count', alias: 'n' }], + having: { n: { $gte: '{not_a_token}' } }, + } as EngineAggregateOptions)); + expect({ code: err!.code, status: err!.status }).toEqual({ code: 'INVALID_FILTER', status: 400 }); + expect(err!.message).toContain('having.n.$gte'); + expect(err!.message).toContain('{placeholder}'); + expect(reads).toHaveLength(0); + }); + + it('a numeric string in `having` keeps the groups its number keeps; a text column is not this door\'s', async () => { + const groups = async (having: FilterCondition) => + (await engine.aggregate(OBJECT, { + groupBy: ['id'], + aggregations: [{ function: 'sum', field: 'f_number', alias: 'total' }], + having, + } as EngineAggregateOptions)).map((r) => (r as Record).id).sort(); + expect(await groups({ total: { $gt: '10' } })).toEqual(await groups({ total: { $gt: 10 } })); + expect(await groups({ total: { $eq: '12' } })).toEqual(['r2']); + // `id` is a text column of the aggregated row — the door has no opinion there. + expect(await groups({ id: { $ne: 'abc' } })).toEqual(['r1', 'r2', 'r3']); + }); + + it('GUARD the having walk narrows copy-on-write and judges only columns classed numeric', () => { + const classes = new Map([['total', 'numeric' as const], ['label', 'text' as const], ['unknown', undefined]]); + const having = { total: { $in: ['1', 2] }, label: { $eq: 'abc' }, unknown: { $eq: 'abc' } }; + const narrowed = narrowHavingNumberComparands(OBJECT, having, classes); + expect(narrowed).toEqual({ total: { $in: [1, 2] }, label: { $eq: 'abc' }, unknown: { $eq: 'abc' } }); + expect(having.total.$in).toEqual(['1', 2]); + const untouched = { total: { $gt: 5 } }; + expect(narrowHavingNumberComparands(OBJECT, untouched, classes)).toBe(untouched); + }); + + // ── the REST doors that reach findData ────────────────────────────────── + + describe('the REST doors — one answer however the query arrived', () => { + let protocol: ObjectStackProtocolImplementation; + + beforeEach(() => { + protocol = new ObjectStackProtocolImplementation(engine); + }); + + const DOORS: ReadonlyArray<{ door: string; query: (comparand: string) => Record }> = [ + // `where` object — `POST /data/:object/query` body. + { door: 'where object', query: (v) => ({ where: { f_number: { $gt: v } } }) }, + // `$filter` string — the OData spelling, JSON nested in a querystring value. + { door: '$filter string', query: (v) => ({ $filter: JSON.stringify({ f_number: { $gt: v } }) }) }, + // Filter AST — the sugar the ObjectUI client and FilterBuilder emit. + { door: 'filter AST', query: (v) => ({ filter: [['f_number', '>', v]] }) }, + // An implicit query parameter — `GET /data/:object?f_number=…`, always a string. + { door: 'implicit query parameter', query: (v) => ({ f_number: v }) }, + ]; + + it.each(DOORS)('the $door door refuses a non-numeric string against a declared number field', async ({ query }) => { + const err = await refusalOf(protocol.findData({ object: OBJECT, query: query('abc') } as any)); + expect(err).not.toBeNull(); + expect({ code: err!.code, status: err!.status }).toEqual({ code: 'INVALID_FILTER', status: 400 }); + expect(err!.message).toContain("'f_number'"); + expect(reads).toHaveLength(0); + }); + + it.each(DOORS)('the $door door hands the driver the number a numeric string names', async ({ query }) => { + await expect(protocol.findData({ object: OBJECT, query: query('12') } as any)).resolves.toBeDefined(); + expect(reads).toHaveLength(1); + expect(JSON.stringify(reads[0]?.ast?.where)).toContain('12'); + expect(JSON.stringify(reads[0]?.ast?.where)).not.toContain('"12"'); + }); + }); + + // ── where the door deliberately has NO opinion ─────────────────────────── + + it('GUARD a registry-less host gets no verdict — a door that cannot see the field map invents none', () => { + expect(findNonNumericComparand(undefined, { f_number: { $gt: 'abc' } })).toBeNull(); + expect(findNonNumericComparand({}, { f_number: { $gt: 'abc' } })).toBeNull(); + expect(findNonNumericComparand({ fields: {} }, { f_number: { $gt: 'abc' } })).toBeNull(); + const where = { f_number: { $gt: '12' } }; + expect(narrowNumberComparands(OBJECT, 'find', undefined, where)).toBe(where); + }); + + it('GUARD an UNKNOWN filter field keeps the engine\'s registry-less tolerance — no second opinion about a name', async () => { + await expect(engine.find(OBJECT, { where: { not_a_field: { $gt: 'abc' } } })).resolves.toBeDefined(); + expect(reads).toHaveLength(1); + }); + + it('GUARD a filter with nothing to narrow is returned BY REFERENCE — the common path allocates nothing', () => { + const schema = engine.registry.getObject(OBJECT); + for (const where of [ + { f_number: { $gt: 5 } }, + { f_number: 5, f_text: 'abc' }, + { f_text: { $gt: 'abc' } }, + { f_number: { $null: true } }, + { f_number: { $gt: { $field: 'f_currency' } } }, + { 'f_number.x': { $gt: 'abc' } }, + ]) { + expect(narrowNumberComparands(OBJECT, 'find', schema, where), JSON.stringify(where)).toBe(where); + } + }); + + it('GUARD an unrecognised $ combinator leaves the fields beneath it ungated — a hole, never a false 400', () => { + expect(findNonNumericComparand( + { fields: { f_number: { type: 'number' } } }, + { $nor: [{ f_number: { $gt: 'abc' } }] }, + )).toBeNull(); + }); +}); diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 0924074a338..8a52f7630a3 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -48,6 +48,7 @@ import { assertTemporalComparandsInterpretable, } from './temporal-comparand-door.js'; import { assertTextOperatorTargetsAreStringCapable } from './text-operator-declared-type-door.js'; +import { narrowHavingNumberComparands, narrowNumberComparands } from './number-comparand-declared-type-door.js'; // Seek pagination for the walks that must read EVERY row — the autonumber seed // scan is one (#6249). Shared with `summary-backfill` rather than re-rolled: // the cursor merge is the part that is easy to get subtly wrong. @@ -1007,6 +1008,14 @@ function lowerWhereFilterArray( // hence the door steps around `{placeholder}` strings rather than judging // them; the token resolver refuses the unknown ones a moment later, loudly. assertTemporalComparandsInterpretable(object, operation, schema, where); + // [#20351] The NUMBER-comparand declared-type door, fifth on the same seam + // and fifth question about the same predicate: is this string a number the + // declared numeric column can be compared with. It refuses a non-numeric + // string (`INVALID_FILTER` / 400) and narrows a numeric one to its number, + // copy-on-write, by `@objectstack/spec/data`'s grammar and verdict. Before + // the token resolver, like the temporal door: a `{placeholder}` resolves to + // an id or a date, never a number, so it is refused here unresolved. + const numeric = narrowNumberComparands(object, operation, schema, where); // [#7872] The comparand-type door, on the OBJECT form. `parseFilterAST` // runs the same walk on everything it lowers or passes through, but // NEITHER door routes an object-form filter through it — Door 1 gates on @@ -1016,7 +1025,7 @@ function lowerWhereFilterArray( // its pinned wording for the list-operator shapes), type door second; // the walk is copy-on-write, so the common path allocates nothing and a // narrowed bigint replaces the bag rather than editing the caller's. - const normalized = normalizeFilterComparandTypes(where, `${operation}('${object}')`); + const normalized = normalizeFilterComparandTypes(numeric, `${operation}('${object}')`); if (normalized !== where) { return { ...(bag as Record), where: normalized } as T; } @@ -1090,7 +1099,9 @@ function lowerWhereFilterArray( // a gate on one branch would answer one mistake two ways depending on the // spelling. assertTemporalComparandsInterpretable(object, operation, schema, condition); - lowered.where = condition; + // [#20351] Same door as the object branch, on the LOWERED condition — the + // array sugar (`[['amount','>','abc']]`) names numeric fields too. + lowered.where = narrowNumberComparands(object, operation, schema, condition); return lowered as T; } @@ -1147,9 +1158,10 @@ function admissionRefusalOf( * 1. {@link lowerWhereFilterArray}: the shape gate, then (object form) * `assertListComparandShapes` → `assertFilterIsMaterializable` (the dotted, * then the virtual-field verdict) → `assertTextOperatorTargetsAreStringCapable` - * → `assertTemporalComparandsInterpretable` → `normalizeFilterComparandTypes`, - * or (array form) `isFilterAST` → `parseFilterAST` → the same three - * field-map doors on the lowered condition. + * → `assertTemporalComparandsInterpretable` → `narrowNumberComparands` + * (#20351) → `normalizeFilterComparandTypes`, or (array form) `isFilterAST` + * → `parseFilterAST` → the same four field-map doors on the lowered + * condition. * 2. {@link resolveWhereFilterTokens}: the placeholder resolver. * * What differs by verb sits BETWEEN or AROUND those stages and judges @@ -16293,6 +16305,15 @@ export class ObjectQL implements IObjectQLEngine { assertTemporalComparandsInterpretable( object, 'aggregate', this._registry.getObject(object), aggFilter, `aggregations[${i}].filter`, ); + // [#20351] …and the NUMBER-comparand door, fifth here as it is + // fifth on `where`'s seam: a non-numeric string against a declared + // numeric field counted no row (every row under `$ne`) where its + // `where` twin was a 500 on PostgreSQL, and a numeric string is + // narrowed to its number, copy-on-write, before the in-memory + // evaluator compares it. Rooted at this position. + const numeric = narrowNumberComparands( + object, 'aggregate', this._registry.getObject(object), aggFilter, `aggregations[${i}].filter`, + ); // [#20122] …and the two doors `having` took at its own entry // (#20099), so a refusal here is the FILTER's, never the data's: // 1. the comparand-TYPE door `where` takes in @@ -16315,7 +16336,7 @@ export class ObjectQL implements IObjectQLEngine { // refused. Refused in `where`'s words for that comparison, which // withhold the fields, the operator and the reason; the // withheld half goes to this log, as the driver writes its own. - const typed = normalizeFilterComparandTypes(aggFilter, `aggregate('${object}')`, `aggregations[${i}].filter`); + const typed = normalizeFilterComparandTypes(numeric, `aggregate('${object}')`, `aggregations[${i}].filter`); assertAggregationFilterIsEvaluable(typed, i, { object, fields: (this._registry.getObject(object) as { fields?: unknown } | undefined)?.fields, @@ -16417,7 +16438,14 @@ export class ObjectQL implements IObjectQLEngine { // that refusal's words; on the caller's own clause, before the // bigint narrowing, which is what `where`'s object form judges. assertHavingTemporalComparandsInterpretable(object, query.having, havingColumnClasses, query); - if (having !== query.having) query = { ...query, having }; + // [#20351] …then the NUMBER-comparand door `where` and the + // per-aggregation `filter` take, over the columns #20127 classes + // `numeric`: a non-numeric string kept no group (every group under + // `$ne`) on both `applyHaving` doors, and a numeric string is narrowed + // to its number. On the bigint-narrowed clause, so the two + // narrowings compose. + const numeric = narrowHavingNumberComparands(object, having, havingColumnClasses); + if (numeric !== query.having) query = { ...query, having: numeric }; } const driver = this.getDriver(object); this.logger.debug(`Aggregate on ${object} using ${driver.name}`, query); diff --git a/packages/objectql/src/number-comparand-declared-type-door.ts b/packages/objectql/src/number-comparand-declared-type-door.ts new file mode 100644 index 00000000000..aa4a3247903 --- /dev/null +++ b/packages/objectql/src/number-comparand-declared-type-door.ts @@ -0,0 +1,363 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#20351] The NUMBER-comparand declared-type door, at the engine's single + * filter collection point: the fifth gate on the seam that already carries + * the #5869 comparand-shape gate, the #8296 unmaterializable-field gate, the + * #15661 text-operator declared-type gate and the #8690 temporal-comparand + * gate. It answers a fifth question about the same predicate: *is this string + * a number the column can be compared with.* + * + * ## The direction this implements (triage, recorded on #20336) + * + * > **Direction, decided here:** the door refuses a non-numeric string against + * > a number field with `INVALID_FILTER` / 400, naming the field, on every + * > driver and position, before any bind. That is the loud answer the charter + * > prefers, and the one the temporal door already gives. ⛔ Not a driver-side + * > catch that turns PostgreSQL's 500 into a 200. + * + * Routed on #15661's two-lane precedent. Lane (1), the CONTRACT, is + * `@objectstack/spec/data`'s `filter-number-comparand-declared-type.ts` + * (#20336): the platform's one numeric grammar, the pure verdict + * ({@link numberComparandDoorVerdict}), the refusal words + * ({@link numberComparandRefusalMessage}), the fixture and the case table. + * This file is lane (2), the door that consults it. ⛔ Nothing here reads a + * string as a number: the verdict does, so a form the grammar admits or + * refuses tomorrow is admitted or refused here with no change in this package. + * + * ## What ran before this door, measured on `origin/main` 3062e5001 + * + * `where { amount: { $gt: "abc" } }` over a declared `number` field, three rows + * (5, 12, 30), through `engine.find` / `engine.aggregate` and + * `POST /api/v1/data/:object/query`: + * + * | position | InMemoryDriver | SqlDriver, SQLite | SqlDriver, PostgreSQL 16 | + * |:--|:--|:--|:--| + * | `where`: `$gt` / `$eq` / implicit / `$in` member `"abc"` | 200, no rows | 200, no rows | 500 `DATABASE_ERROR` | + * | `where`: `$eq ""`, and `$gt "{current_user_id}"` over REST | 200, no rows | 200, no rows | 500 `DATABASE_ERROR` | + * | `where`: `$gt "12"` / `$eq "12"` (a numeric string) | 200, **no rows** | 200, 1 row | 200, 1 row | + * | per-aggregation `filter`: `$gt "abc"` | 200, count 0 | 200, count 0 | 200, count 0 | + * | `having` on `sum(amount)`: `$gt "abc"` | 200, no group | 200, no group | 200, no group | + * + * One client mistake, three answers, one of them a server fault; and a numeric + * string read two ways (the memory matcher compares `12 > "12"` without + * coercing it, the SQL backends bind it with numeric affinity or input). + * + * ## The door's two answers + * + * - **Refuse** a string the grammar does not read as a number: `INVALID_FILTER` + * / 400, the existing filter envelope, in the contract's words, before any + * driver is resolved. A `{placeholder}` is refused too, unresolved: every + * filter token resolves to an id or a date, never a number (the contract + * argues it), and this door runs before `resolveWhereTokens`, as its + * temporal neighbour records it must. + * - **Narrow** a string the grammar does read as a number to that number, + * copy-on-write, so every backend receives the one value the string names: + * the caller's filter is never edited, and a filter with nothing to narrow + * is returned by reference (the common path allocates nothing). + * + * ## Where it sits in the ladder, and why exactly there + * + * After the temporal door and before the comparand-TYPE door + * (`normalizeFilterComparandTypes`) on `where`'s object form, and after the + * temporal door on the lowered array form, so the order of the field-aware + * doors is the same on both spellings: + * + * 1. `assertListComparandShapes` — can this comparand run at all (#5869). + * 2. `assertFilterIsMaterializable` — is there a column (#8296). + * 3. `assertTextOperatorTargetsAreStringCapable` — can the column ever hold a + * string (#15661). A text operator over a number field is refused there, + * and its comparand is a substring, never a number: this door does not + * judge the text operators. + * 4. `assertTemporalComparandsInterpretable` — can the column's storage rule + * read the value (#8690). The two doors judge disjoint field classes. + * 5. **this door** — is the string a number (#20351). + * + * `formula` is judged one door EARLIER and never reaches this one: + * `assertFilterIsMaterializable` refuses every filter over a formula field with + * `INVALID_FIELD` / 400. The verdict is still handed a formula's `returnType`, + * so the day that class becomes filterable this door already answers it. + * + * ## The positions, and the one that is not this seam's + * + * - **`where`**, both spellings — the object form every protocol door hands + * over, and the `FilterArray` sugar after `parseFilterAST` lowers it — on + * every verb that calls `lowerWhereFilterArray` (`find` / `findOne` / + * `count` / `aggregate` / `update` / `delete`), and on the judge-only + * `judgeFilter` (`judgeWhereAdmission` calls the same function). + * - **The per-aggregation `filter`** ({@link narrowNumberComparands} with its + * path rooted at `aggregations[i].filter`), against the object's declared + * fields, since that filter narrows the object's raw rows. + * - **`having`** ({@link narrowHavingNumberComparands}). The engine evaluates + * it over the aggregated rows, so the column is the aggregated one: judged + * when #20127's `aggregatedRowColumnClasses` classes it `numeric` (a `count` + * / `sum` / `avg`, and a groupBy or `min` / `max` of a numeric field). Such + * a column has no declared `FieldType` of its own; the verdict is handed + * `number`, the member of the numeric class it holds. + * - **Not here: RLS / sharing / tenant predicates.** Like every door on this + * seam it runs on the CALLER's filter, before the middleware chain composes + * those predicates onto the AST: an injected read filter is the platform's + * own, not a declaration the caller can fix. A policy predicate reaches this + * door at AUTHORING instead, where `validateRlsPredicateEnforceability` asks + * the engine's `judgeFilter` — which runs this door — when the host hands + * it a judge. + * + * ## Scope — the same boundaries as the neighbours + * + * - **UNDOTTED keys naming a declared field.** A dotted key is + * `filter-dotted-head`'s subject, and an undeclared key keeps the engine's + * registry-less tolerance (#7534): no second opinion about a name. + * - **A registry-less host gets no verdict** — a door that cannot see the field + * map invents none. + * - **Only the value comparisons**: the implicit comparand, the scalar + * operators and every member of the list operators the contract names. The + * flags (`$null` / `$exists` / `$empty`), the text operators and a + * `{ $field }` reference are not a value of the field and are left alone. + * + * @see numberComparandDoorVerdict — the pure verdict (lane 1, `@objectstack/spec`). + * @see https://github.com/objectstack-ai/objectstack/issues/20336 (the contract) + * @see https://github.com/objectstack-ai/objectstack/issues/20351 (this door) + */ + +import { + NUMBER_COMPARAND_DOOR_LIST_OPERATORS, + NUMBER_COMPARAND_DOOR_SCALAR_OPERATORS, + numberComparandDoorVerdict, + numberComparandFieldVerdict, + numberComparandRefusalMessage, + type NumberComparandDoorFieldMeta, + type NumberComparandRefusalSite, +} from '@objectstack/spec/data'; +import { invalidFilterError } from './filter-comparand-shape.js'; +import type { AggregatedColumnClass } from './having-filter.js'; + +/** The operators whose one comparand is judged — the contract's list, never a re-listing. */ +const SCALAR_OPERATORS: ReadonlySet = new Set(NUMBER_COMPARAND_DOOR_SCALAR_OPERATORS); + +/** The operators each of whose MEMBERS is judged as a comparand in its own right. */ +const LIST_OPERATORS: ReadonlySet = new Set(NUMBER_COMPARAND_DOOR_LIST_OPERATORS); + +/** A comparand the door refuses: the site the contract's words are written from. */ +export type NonNumericComparand = NumberComparandRefusalSite; + +/** What one filter position supplies to the walk: the field meta a KEY names, or `null`. */ +type MetaOf = (key: string) => NumberComparandDoorFieldMeta | null; + +/** The walk's answer: the (possibly narrowed) node, or the first refusal. */ +type Outcome = + | { readonly ok: true; readonly value: unknown } + | { readonly ok: false; readonly refusal: NonNumericComparand }; + +const kept = (value: unknown): Outcome => ({ ok: true, value }); + +/** + * A plain object — filter STRUCTURE rather than a comparand. The same + * classification the sibling gates make: a `Date` is a comparand even though + * `typeof` calls it an object. + */ +function isFilterNode(value: unknown): value is Record { + return ( + typeof value === 'object' + && value !== null + && !Array.isArray(value) + && !(value instanceof Date) + ); +} + +/** A `{ $field: 'other_column' }` reference is not a literal — never judged. */ +function isFieldReference(value: unknown): boolean { + return isFilterNode(value) && typeof (value as { $field?: unknown }).$field === 'string'; +} + +/** The slice of a field declaration the verdict reads. */ +function fieldMetaOf(def: unknown): NumberComparandDoorFieldMeta | null { + if (!isFilterNode(def)) return null; + const type = (def as { type?: unknown }).type; + if (typeof type !== 'string') return null; + const returnType = (def as { returnType?: unknown }).returnType; + return typeof returnType === 'string' ? { type, returnType } : { type }; +} + +/** One comparand at a judged position: the spec's verdict, routed. */ +function judgeComparand( + meta: NumberComparandDoorFieldMeta, + field: string, + comparand: unknown, + path: string, +): Outcome { + const verdict = numberComparandDoorVerdict(meta, comparand); + if (verdict.verdict === 'narrows') return kept(verdict.value); + if (verdict.verdict !== 'door-refusal') return kept(comparand); + return { + ok: false, + refusal: { + field, + declaredType: meta.type, + ...(meta.returnType === undefined ? {} : { returnType: meta.returnType }), + path, + // `door-refusal` is answered for a string comparand only. + value: comparand as string, + form: verdict.form, + }, + }; +} + +/** One judged field's constraint: `{ amount: }`. */ +function judgeFieldSpec( + meta: NumberComparandDoorFieldMeta, + field: string, + spec: unknown, + path: string, +): Outcome { + // Not filter structure → an implicit-equality comparand, judged at this path. + if (!isFilterNode(spec)) return judgeComparand(meta, field, spec, path); + // A field spec with no `$` key is a deep-equality / nested-relation + // condition; the #5869 gate records why descending into one would invent a + // contract no backend agrees with. + const ops = Object.keys(spec); + if (!ops.some((op) => op.startsWith('$'))) return kept(spec); + if (isFieldReference(spec)) return kept(spec); + let out: Record | undefined; + for (const op of ops) { + const comparand = spec[op]; + if (SCALAR_OPERATORS.has(op)) { + const judged = judgeComparand(meta, field, comparand, `${path}.${op}`); + if (!judged.ok) return judged; + if (judged.value !== comparand) (out ??= { ...spec })[op] = judged.value; + continue; + } + // A list operator whose comparand is not a list is the shape gate's + // refusal, one door earlier; nothing is left here to judge. + if (!LIST_OPERATORS.has(op) || !Array.isArray(comparand)) continue; + let members: unknown[] | undefined; + for (const [index, member] of comparand.entries()) { + const judged = judgeComparand(meta, field, member, `${path}.${op}[${index}]`); + if (!judged.ok) return judged; + if (judged.value !== member) (members ??= [...comparand])[index] = judged.value; + } + if (members) (out ??= { ...spec })[op] = members; + } + return kept(out ?? spec); +} + +/** + * The walk, shared by every position: the node structure is judged the same + * way wherever the condition sits; only {@link MetaOf} differs. + * + * Structure is discarded the same three conservative ways the sibling gates + * discard it: `$and` / `$or` / `$not` are descended, any OTHER `$` key at node + * level is skipped WITHOUT descending (an unrecognised combinator leaves the + * fields beneath it ungated — a hole, not a false 400), and a dotted key names + * a path this door does not judge. Copy-on-write throughout. + */ +function walkCondition(metaOf: MetaOf, node: unknown, path: string, depth: number): Outcome { + if (depth > 32 || !isFilterNode(node)) return kept(node); + let out: Record | undefined; + for (const [key, value] of Object.entries(node)) { + const here = `${path}.${key}`; + let judged: Outcome; + if (key === '$and' || key === '$or') { + if (!Array.isArray(value)) continue; + let arms: unknown[] | undefined; + for (const [index, arm] of value.entries()) { + const walked = walkCondition(metaOf, arm, `${here}[${index}]`, depth + 1); + if (!walked.ok) return walked; + if (walked.value !== arm) (arms ??= [...value])[index] = walked.value; + } + judged = kept(arms ?? value); + } else if (key === '$not') { + judged = walkCondition(metaOf, value, here, depth + 1); + } else { + if (key.startsWith('$') || key.includes('.')) continue; + const meta = metaOf(key); + // Only a judged field can refuse or narrow a comparand; a `formula` + // whose return type is unreadable is `deferred`, and everything else is + // `not-judged` — the spec's verdict, never a list here. + if (!meta || numberComparandFieldVerdict(meta) !== 'judged') continue; + judged = judgeFieldSpec(meta, key, value, here); + } + if (!judged.ok) return judged; + if (judged.value !== value) (out ??= { ...node })[key] = judged.value; + } + return kept(out ?? node); +} + +/** The judged fields of a `where` or a per-aggregation `filter`: the object's declared map. */ +function declaredMetaOf(schema: unknown): MetaOf | null { + // A registry-less host must not invent a verdict about a field map it cannot + // see — the same early return every neighbour makes. + const fields = (schema as { fields?: Record } | undefined)?.fields; + if (!fields || typeof fields !== 'object') return null; + return (key) => (Object.prototype.hasOwnProperty.call(fields, key) ? fieldMetaOf(fields[key]) : null); +} + +/** + * Walk one `FilterCondition` and return the FIRST string a declared numeric + * field cannot be compared with, or `null`. + * + * Exported for the same reason the sibling walks are: a consumer that needs to + * ask "would the engine door refuse this?" without provoking the refusal. + */ +export function findNonNumericComparand( + schema: unknown, + where: unknown, + path = 'where', +): NonNumericComparand | null { + const metaOf = declaredMetaOf(schema); + if (!metaOf) return null; + const walked = walkCondition(metaOf, where, path, 0); + return walked.ok ? null : walked.refusal; +} + +function refuse(context: string, refusal: NonNumericComparand): never { + throw invalidFilterError(numberComparandRefusalMessage(refusal, context)); +} + +/** + * Refuse every string a declared numeric field cannot be compared with, and + * narrow every numeric string to its number — `INVALID_FILTER` / 400, this + * package's existing filter envelope, in the contract's words. No code is + * minted. + * + * Returns `where` BY REFERENCE when nothing was narrowed, otherwise a copy: + * the filter belongs to the caller and may be reused (view metadata, flow + * node config). + * + * `path` roots the refusal at the position the filter sits in: `where` by + * default, `aggregations[i].filter` for a per-aggregation filter. + */ +export function narrowNumberComparands( + object: string, + operation: string, + schema: unknown, + where: W, + path = 'where', +): W { + const metaOf = declaredMetaOf(schema); + if (!metaOf) return where; + const walked = walkCondition(metaOf, where, path, 0); + if (!walked.ok) refuse(`${operation}('${object}')`, walked.refusal); + return walked.value as W; +} + +/** + * The `having` position: the same walk, the same verdict and the same words, + * over the aggregated row's columns — judged when `classes` (#20127's + * `aggregatedRowColumnClasses`, handed in rather than derived again) classes a + * column `numeric`. Refuses or narrows exactly as {@link narrowNumberComparands} + * does, rooted at `having`. + */ +export function narrowHavingNumberComparands( + object: string, + having: H, + classes: ReadonlyMap, +): H { + const walked = walkCondition( + (key) => (classes.get(key) === 'numeric' ? { type: 'number' } : null), + having, + 'having', + 0, + ); + if (!walked.ok) refuse(`aggregate('${object}')`, walked.refusal); + return walked.value as H; +} diff --git a/packages/rest/src/data-number-comparand-door.test.ts b/packages/rest/src/data-number-comparand-door.test.ts new file mode 100644 index 00000000000..20e627b2073 --- /dev/null +++ b/packages/rest/src/data-number-comparand-door.test.ts @@ -0,0 +1,255 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#20351] A non-numeric string compared against a declared number field is + * refused at the public door — `POST /api/v1/data/:object/query` and + * `engine.find` / `engine.aggregate` answer `400 INVALID_FILTER` at `where`, + * the per-aggregation `filter` and `having`, before any read — over a real + * `SqlDriver`, with a numeric comparand as the control and a numeric string + * shown to count what its number counts. + * + * Measured on the base (`3062e5001`) through this door and the engine, three + * rows (5, 12, 30): + * + * | position | InMemoryDriver | SQLite | PostgreSQL 16 | + * |:--|:--|:--|:--| + * | `where`: `$gt` / `$eq` / `$ne` / implicit / `$in` member `"abc"`, `$eq ""` | 200, no rows (`$ne`: every row) | same | 500 `DATABASE_ERROR` | + * | `where`: `$gt "12"` / `$eq "12"` | 200, no rows | 200, 1 row | 200, 1 row | + * | per-aggregation `filter` `$gt "abc"` (`$ne "abc"`) | count 0 (3) | count 0 (3) | count 0 (3) | + * | `having` on `sum(amount)` `$gt "abc"` (`$ne "abc"`) | no group (every group) | same | same | + * + * The door sits in the engine, in front of every driver, so one verdict holds + * on each cell; the engine-level pin that drives the contract's whole case + * table through a recording driver is `@objectstack/objectql`'s + * `engine-number-comparand-declared-type-door.test.ts`. InMemoryDriver's row + * is that pin's by construction: the door answers before a driver is resolved. + * + * ## The dialect axis of THIS file + * + * The SQLite cell always runs. The PostgreSQL and MySQL cells run where + * `OS_TEST_POSTGRES_URL` / `OS_TEST_MYSQL_URL` are set and are a named skip + * otherwise. ⚠️ No CI job provisions those variables for this package (the + * `Temporal Conformance (live PG + MySQL)` job runs `driver-sql`, + * `metadata-protocol` and one `runtime` file), so the live cells are + * red-capable and un-run in CI; the PR that landed this file carries their + * local PostgreSQL run. Each live cell owns one table, dropped before and + * after. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import type { EngineAggregateOptions, FilterCondition } from '@objectstack/spec/data'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; +import { RestServer } from './rest-server'; + +const OBJECT = 'rest_number_door_20351'; + +const LEDGER = { + name: OBJECT, + label: 'Ledger 20351', + fields: { + customer_id: { name: 'customer_id', type: 'text' as const }, + amount: { name: 'amount', type: 'number' as const }, + price: { name: 'price', type: 'currency' as const }, + }, +}; + +const ROWS = [ + { id: 'd1', customer_id: 'c1', amount: 5, price: 50 }, + { id: 'd2', customer_id: 'c1', amount: 12, price: 120 }, + { id: 'd3', customer_id: 'c2', amount: 30, price: 300 }, +]; + +interface Cell { + id: 'sqlite' | 'pg' | 'mysql'; + label: string; + env: string | null; + config: () => Record | null; +} + +const CELLS: readonly Cell[] = [ + { id: 'sqlite', label: 'sqlite', env: null, config: () => ({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }) }, + { + id: 'pg', + label: 'live postgres', + env: 'OS_TEST_POSTGRES_URL', + config: () => (process.env.OS_TEST_POSTGRES_URL ? { client: 'pg', connection: process.env.OS_TEST_POSTGRES_URL } : null), + }, + { + id: 'mysql', + label: 'live mysql', + env: 'OS_TEST_MYSQL_URL', + config: () => (process.env.OS_TEST_MYSQL_URL ? { client: 'mysql2', connection: process.env.OS_TEST_MYSQL_URL } : null), + }, +]; + +/** name · the constraint on `amount` — each a non-numeric string at a judged position. */ +const REFUSED: ReadonlyArray = [ + ['$gt "abc" (the card)', { $gt: 'abc' }], + ['$eq "abc"', { $eq: 'abc' }], + ['$ne "abc"', { $ne: 'abc' }], + ['implicit "abc"', 'abc'], + ['a $in member "abc"', { $in: [10, 'abc'] }], + ['a $between bound "abc"', { $between: ['abc', 20] }], + ['$eq "" (blank)', { $eq: '' }], + ['$gt "1,000" (a locale spelling)', { $gt: '1,000' }], + ['$gt "{current_user_id}" (a placeholder)', { $gt: '{current_user_id}' }], +]; + +/** name · the constraint as a numeric string · the same as a number · `where` count. */ +const NARROWED: ReadonlyArray = [ + ['$gt "12"', { $gt: '12' }, { $gt: 12 }, 1], + ['$eq "12"', { $eq: '12' }, { $eq: 12 }, 1], + ['implicit "5"', '5', 5, 1], + ['$in ["5", "30"]', { $in: ['5', '30'] }, { $in: [5, 30] }, 2], + ['$between ["1e1", "3e1"]', { $between: ['1e1', '3e1'] }, { $between: [10, 30] }, 2], + ['$ne "12"', { $ne: '12' }, { $ne: 12 }, 2], +]; + +function createMockServer() { + const noop = () => {}; + return { get: noop, post: noop, put: noop, delete: noop, patch: noop, use: noop, listen: async () => {}, close: async () => {} }; +} + +function makeRes() { + const res: any = { + write: () => true, end: () => {}, + header: () => res, + status: (code: number) => { res._status = code; return res; }, + json: (body: any) => { res._json = body; return res; }, + }; + return res; +} + +const perAggregation = (filter: FilterCondition): EngineAggregateOptions => ({ + aggregations: [{ function: 'count', alias: 'n' }, { function: 'count', alias: 'm', filter }], +}); + +/** + * `native`: SqlDriver aggregates and the engine applies `having` to its + * answer. `rows`: a filtered aggregation sends the engine to the rows path, + * where it aggregates itself and then applies `having`. + */ +const grouped = (path: 'native' | 'rows', having: FilterCondition): EngineAggregateOptions => ({ + groupBy: ['customer_id'], + aggregations: [ + { function: 'sum', field: 'amount', alias: 'total' }, + { function: 'max', field: 'price', alias: 'top' }, + ...(path === 'rows' ? [{ function: 'count' as const, alias: 'fb', filter: { customer_id: { $ne: '' } } }] : []), + ], + having, +}); + +const refusalOf = async (p: Promise) => + p.then(() => null, (e: any) => e as Error & { code?: string; status?: number }); + +for (const cell of CELLS) { + const config = cell.config(); + describe.skipIf(!config)( + `[#20351] a non-numeric string against a number field at the public door — ${cell.label}${config ? '' : ` (skipped: set ${cell.env} to run this cell)`}`, + () => { + let engine: ObjectQL; + let driver: any; + const reads = { n: 0 }; + let query: (body: Record) => Promise<{ status: number; body: any }>; + + beforeAll(async () => { + driver = new SqlDriver(config as any); + if (cell.id !== 'sqlite') await driver.execute(`drop table if exists ${OBJECT}`).catch(() => {}); + engine = new ObjectQL(); + engine.registerDriver(driver, true); + await engine.init(); + engine.registry.registerObject(LEDGER as any); + await engine.syncSchemas(); + for (const row of ROWS) await engine.insert(OBJECT, { ...row } as any); + + // Reads of THIS object — the protocol's own metadata traffic is not the question. + for (const verb of ['find', 'findOne', 'count', 'aggregate'] as const) { + const real = driver[verb].bind(driver); + driver[verb] = (o: string, ...rest: unknown[]) => { if (o === OBJECT) reads.n += 1; return real(o, ...rest); }; + } + + const protocol = new ObjectStackProtocolImplementation(engine as any); + const rest = new RestServer(createMockServer() as any, protocol as any, { api: { requireAuth: false } } as any); + (rest as any).resolveExecCtx = async () => ({ userId: 'test-user' }); + rest.registerRoutes(); + const route = rest.getRoutes().find((r: any) => r.method === 'POST' && r.path === '/api/v1/data/:object/query'); + expect(route).toBeDefined(); + query = async (body) => { + const res = makeRes(); + // What the wire carries: JSON. + await route!.handler({ params: { object: OBJECT }, body: JSON.parse(JSON.stringify(body)), query: {}, headers: {} } as any, res); + return { status: res._status ?? 200, body: res._json }; + }; + }); + + afterAll(async () => { + if (cell.id !== 'sqlite') await driver?.execute(`drop table if exists ${OBJECT}`).catch(() => {}); + try { await engine?.destroy(); } catch { /* noop */ } + }); + + it('where: 400 INVALID_FILTER naming the field, through REST and engine.find — no read', async () => { + const before = reads.n; + for (const [name, spec] of REFUSED) { + const where = { amount: spec } as FilterCondition; + const res = await query({ where }); + expect(res.status, `REST, ${name}: ${JSON.stringify(res.body)}`).toBe(400); + expect(res.body.code, `REST, ${name}`).toBe('INVALID_FILTER'); + expect(res.body.error, `REST, ${name}`).toContain("filter on 'amount'"); + const err = await refusalOf(engine.find(OBJECT, { where })); + expect({ code: err?.code, status: err?.status }, `engine.find, ${name}`).toEqual({ code: 'INVALID_FILTER', status: 400 }); + } + expect(reads.n - before, 'no read of the object — every refusal precedes the driver').toBe(0); + }); + + it('the per-aggregation filter and having: 400 INVALID_FILTER at their own positions — no read', async () => { + const before = reads.n; + for (const op of ['$gt', '$ne'] as const) { + const filter = await query(perAggregation({ amount: { [op]: 'abc' } } as FilterCondition) as Record); + expect(filter.status, `filter ${op}: ${JSON.stringify(filter.body)}`).toBe(400); + expect(filter.body.code, `filter ${op}`).toBe('INVALID_FILTER'); + expect(filter.body.error, `filter ${op}`).toContain(`aggregations[1].filter.amount.${op}`); + const engineFilter = await refusalOf(engine.aggregate(OBJECT, perAggregation({ amount: { [op]: 'abc' } } as FilterCondition))); + expect({ code: engineFilter?.code, status: engineFilter?.status }, `engine filter ${op}`).toEqual({ code: 'INVALID_FILTER', status: 400 }); + for (const path of ['native', 'rows'] as const) { + for (const column of ['total', 'top'] as const) { + const having = await query(grouped(path, { [column]: { [op]: 'abc' } } as FilterCondition) as Record); + expect(having.status, `having ${path} ${column} ${op}: ${JSON.stringify(having.body)}`).toBe(400); + expect(having.body.code, `having ${path} ${column} ${op}`).toBe('INVALID_FILTER'); + expect(having.body.error, `having ${path} ${column} ${op}`).toContain(`having.${column}.${op}`); + } + } + } + expect(reads.n - before, 'no read of the object — every refusal precedes the driver').toBe(0); + }); + + it('the control: a number is answered, and a numeric string counts what its number counts at every position', async () => { + for (const [name, asString, asNumber, count] of NARROWED) { + const s = await query({ where: { amount: asString } }); + const n = await query({ where: { amount: asNumber } }); + expect(s.status, `where, ${name}: ${JSON.stringify(s.body)}`).toBe(200); + expect(n.status, `where control, ${name}`).toBe(200); + expect(s.body.records.length, `where, ${name}`).toBe(count); + expect(s.body.records.map((r: any) => r.id).sort(), `where, ${name}`).toEqual(n.body.records.map((r: any) => r.id).sort()); + expect((await engine.find(OBJECT, { where: { amount: asString } as FilterCondition })).length, `engine.find, ${name}`).toBe(count); + const fs = await query(perAggregation({ amount: asString } as FilterCondition) as Record); + const fn = await query(perAggregation({ amount: asNumber } as FilterCondition) as Record); + expect(fs.status, `filter, ${name}: ${JSON.stringify(fs.body)}`).toBe(200); + expect(Number(fs.body.records[0]?.m), `filter, ${name}`).toBe(count); + expect(Number(fs.body.records[0]?.m), `filter, ${name}`).toBe(Number(fn.body.records[0]?.m)); + } + for (const path of ['native', 'rows'] as const) { + const groupsOf = async (having: FilterCondition) => { + const res = await query(grouped(path, having) as Record); + expect(res.status, `having ${path} ${JSON.stringify(having)}: ${JSON.stringify(res.body)}`).toBe(200); + return res.body.records.map((r: any) => r.customer_id).sort(); + }; + expect(await groupsOf({ total: { $gt: '20' } }), `having ${path}`).toEqual(['c2']); + expect(await groupsOf({ total: { $gt: '20' } }), `having ${path}`).toEqual(await groupsOf({ total: { $gt: 20 } })); + expect(await groupsOf({ top: { $eq: '120' } }), `having ${path}`).toEqual(['c1']); + } + }); + }, + ); +} diff --git a/packages/rest/src/data-query-having-temporal-door.test.ts b/packages/rest/src/data-query-having-temporal-door.test.ts index e1c0b3dfb4d..57a00504bc1 100644 --- a/packages/rest/src/data-query-having-temporal-door.test.ts +++ b/packages/rest/src/data-query-having-temporal-door.test.ts @@ -184,7 +184,7 @@ describe('[#20263] having — the 2026 control and the non-temporal columns answ ['a 2026 day $lt on max(date)', { last_placed: { $lt: '2026-02-01' } }, ['c1', 'c3']], ['a 2026 instant $gt on min(datetime)', { first_opened: { $gt: '2026-02-01T00:00:00.000Z' } }, ['c2', 'c3', 'c4']], ['a wall clock $lt on max(time)', { last_slot: { $lt: '12:00' } }, ['c1']], - ['a string on sum — not temporal, not judged', { total: { $gt: 'not-a-date' } }, []], + ['a number on sum — not temporal, not judged', { total: { $gt: 500 } }, ['c2']], ]; for (const [name, having, kept] of KEPT) { it(`${name}: keeps ${kept.join(', ') || 'no group'}, engine and REST, both paths`, async () => { @@ -197,4 +197,20 @@ describe('[#20263] having — the 2026 control and the non-temporal columns answ } }); } + + // [#20351] A string on sum is not this door's, and it no longer keeps no + // group with a 200: the number-comparand door refuses it, before any read. + it('a string on sum — not temporal: refused by the number-comparand door, engine and REST, both paths, no read', async () => { + const { engine, post, reads } = await boot(); + const having = { total: { $gt: 'not-a-date' } }; + for (const path of ['native', 'rows'] as const) { + const err = await refusalOf(engine.aggregate(OBJECT, grouped(path, having))); + expect({ code: err?.code, status: err?.status }, `engine, ${path}`).toEqual({ code: 'INVALID_FILTER', status: 400 }); + const res = await post(grouped(path, having) as Record); + expect(res._status, JSON.stringify(res._json)).toBe(400); + expect(res._json.code, `REST, ${path}`).toBe('INVALID_FILTER'); + expect(res._json.error, `REST, ${path}`).toContain('at having.total.$gt'); + } + expect(reads.n).toBe(0); + }); }); diff --git a/packages/rest/src/rest-aggregate-numeric-having.test.ts b/packages/rest/src/rest-aggregate-numeric-having.test.ts index 267a0c1d155..d6af006929b 100644 --- a/packages/rest/src/rest-aggregate-numeric-having.test.ts +++ b/packages/rest/src/rest-aggregate-numeric-having.test.ts @@ -16,6 +16,9 @@ * | `{ n: { $lt: 'not-a-date' } }` | no group | c1–c4 | no group | * | `{ total: { $lt: 'not-a-date' } }` | no group | c1–c4 | c1–c4 | * + * [#20351] The two string rows are refused now, `INVALID_FILTER` / 400 before + * any read, by the engine's number-comparand door (the `REFUSED` table below). + * * The native path handed the SQL client's strings through (`"n": "2"`, * `"total": "500.000000000000000000000000000000"`); `SqlDriver.aggregate` now * presents them as numbers (`sql-driver-20335-aggregate-numeric-presentation.test.ts` @@ -123,9 +126,16 @@ const KEPT: ReadonlyArray, string[]]> ['sum $eq', { total: { $eq: 1200 } }, ['c2']], ['avg $in', { mean: { $in: [250, 600] } }, ['c1', 'c2']], ['avg $eq', { mean: { $eq: 50 } }, ['c3']], - ['a string $lt on count — no group', { n: { $lt: 'not-a-date' } }, []], - ['a string $lt on sum — no group', { total: { $lt: 'not-a-date' } }, []], - ['an extended-year ISO $gt on avg — no group', { mean: { $gt: '+010000-01-01T00:00:00.000Z' } }, []], +]; + +// [#20351] having · the refused key path — a string that names no number, on a +// numeric column. These kept no group (c1–c4 on PostgreSQL's native path) +// before the number-comparand door; they are refused now, before any read, on +// every path, door and dialect. +const REFUSED: ReadonlyArray, string]> = [ + ['a string $lt on count', { n: { $lt: 'not-a-date' } }, 'having.n.$lt'], + ['a string $lt on sum', { total: { $lt: 'not-a-date' } }, 'having.total.$lt'], + ['an extended-year ISO $gt on avg', { mean: { $gt: '+010000-01-01T00:00:00.000Z' } }, 'having.mean.$gt'], ]; for (const cell of CELLS) { @@ -185,6 +195,19 @@ for (const cell of CELLS) { expect(answers.native).toStrictEqual(answers.rows); }); + for (const [name, having, at] of REFUSED) { + it(`${name}: INVALID_FILTER / 400 at ${at} — engine and REST, native and rows`, async () => { + for (const path of ['native', 'rows'] as const) { + const err = await engine.aggregate(OBJECT, grouped(path, having)).then(() => null, (e: any) => e); + expect({ code: err?.code, status: err?.status }, `engine, ${path}`).toEqual({ code: 'INVALID_FILTER', status: 400 }); + const res = await post(grouped(path, having) as Record); + expect(res._status, JSON.stringify(res._json)).toBe(400); + expect(res._json.code, `REST, ${path}`).toBe('INVALID_FILTER'); + expect(res._json.error, `REST, ${path}`).toContain(at); + } + }); + } + for (const [name, having, kept] of KEPT) { it(`${name}: keeps ${kept.join(', ') || 'no group'} — engine and REST, native and rows`, async () => { for (const path of ['native', 'rows'] as const) {