diff --git a/.changeset/20546-no-operator-object-on-scalar.md b/.changeset/20546-no-operator-object-on-scalar.md new file mode 100644 index 00000000000..f0bb380083d --- /dev/null +++ b/.changeset/20546-no-operator-object-on-scalar.md @@ -0,0 +1,29 @@ +--- +"@objectstack/objectql": minor +--- + +fix(objectql)!: a plain object with no `$` operator where a scalar field's value belongs is refused with `INVALID_FILTER` / 400 at `where`, a per-aggregation `filter` and `having`, on every driver + +Clause-②: no (narrowing) + + + +**BREAKING**: this narrows what a filter may put beneath a scalar field. A plain object with no `$`-operator key — `{ "amount": { "a": 1 } }`, `{}` included — where a value of a field that holds scalar values belongs is refused by the engine before any driver is asked, where the in-memory driver answered it with no records (every record under `$not`) and the SQL driver refused it in its own words. It ships as `minor` under the launch-window convention for accept-set narrowings. No export or published type changes. + +The judged fields are the spec's scalar-valued classes: every type in `SCALAR_FILTER_HEAD_TYPES` (text-like, numeric, boolean, date, datetime, time, single-option, `autonumber`, `summary`), with or without `multiple: true`, and the multi-option types (`multiselect`, `checkboxes`, `tags`). A `having` column is judged by the type it carries: a `count` / `sum` / `avg` is a number, a groupBy or `min` / `max` column the type of its field, a date bucket a date or text label. + +The refusal names the field, its declared type, the object's keys and the position (`where.amount`, `aggregations[1].filter.amount`, `having.total`). No mechanical rewrite exists, because which value or operator the caller meant is not in the object; the fix is one line by hand: compare the field with a value (`{ "amount": 12 }`) or an operator (`{ "amount": { "$gt": 12 } }`), and to filter by a related record, name a relation field. + +Measured through `engine.find` / `engine.aggregate` and `POST /data/:object/query`, three rows: + +| position | filter | before: memory · SQLite · PostgreSQL 16 | now, on all three | +|:--|:--|:--|:--| +| `where` | `{ amount: { a: 1 } }` (number), `{ title: { a: 1 } }` (text), and the select, boolean, date, autonumber, multi-select and `multiple: true` select twins | no records · the driver's 400 · the driver's 400 | `INVALID_FILTER` / 400, the engine's words | +| `where` | `{ $not: { amount: { a: 1 } } }` | every record · the driver's 400 · the driver's 400 | `INVALID_FILTER` / 400 | +| per-aggregation `filter` | `{ amount: { a: 1 } }`, `{ amount: {} }` | count 0 on all three | `INVALID_FILTER` / 400 | +| `having` | `{ total: { a: 1 } }` (a `sum`), `{ title: { a: 1 } }` (a groupBy) | no group on all three | `INVALID_FILTER` / 400 | +| `where` | control: `{ owner: { region: "NA" } }` on a `lookup`, `{ meta: { a: 1 } }` on a `json` field | no records / one record · the driver's 400 · the driver's 400 | unchanged: reaches the driver as written | + +**Who is affected.** A caller that sends a no-operator object beneath a scalar field to the in-memory driver — a test suite, a local or embedded deployment on `InMemoryDriver`, a flow or hook calling the engine in-process — and read the empty answer as a real one. On `SqlDriver` the same filter was already a 400, now in the engine's words; at the per-aggregation `filter` and `having` it was a silent count of 0 or an empty group set on every driver. No existing test in `@objectstack/objectql` or `@objectstack/rest` sent the shape: both suites pass with no fixture changed. + +**Unchanged.** A relation field (`lookup`, `master_detail`, `user`, `tree`, single or multiple) keeps its nested-relation form, and a structured-JSON field (`json`, `composite`, `address`, …) its object comparand; both reach the driver as written, which answers them as before. File and media fields, `formula` (refused one door earlier, `INVALID_FIELD`), undeclared keys, a `{ $field }` reference and every operator bag are not judged by this refusal. A `Map` or a class instance keeps the comparand-type door's refusal in its own words. diff --git a/packages/objectql/src/engine-no-operator-object-door.test.ts b/packages/objectql/src/engine-no-operator-object-door.test.ts new file mode 100644 index 00000000000..fe7f2d32e23 --- /dev/null +++ b/packages/objectql/src/engine-no-operator-object-door.test.ts @@ -0,0 +1,377 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#20546] A plain object with no `$`-operator key where a scalar column's + * value belongs — `{ amount: { a: 1 } }` over a declared `number` field — is + * refused `INVALID_FILTER` / 400, naming the field and the path, by the + * no-operator-object arm of the number-comparand door's walk, at every + * position the engine judges: `where` (object form and `FilterArray` sugar, on + * every verb and the judge), `aggregations[i].filter` and `having`. + * + * Measured on the base (`origin/main` `fbec216e2d`) through `engine.find` / + * `engine.aggregate` and `POST /api/v1/data/:object/query`, three rows: + * + * | position · filter | InMemoryDriver | SqlDriver, SQLite | SqlDriver, PostgreSQL 16 | + * |:--|:--|:--|:--| + * | `where` `{ amount: { a: 1 } }`, `{ title: { a: 1 } }` and the other scalar classes | 200, no rows | 400, the driver's words | 400, the driver's words | + * | `where` `{ $not: { amount: { a: 1 } } }` | 200, every row | 400 | 400 | + * | `aggregations[1].filter` `{ amount: { a: 1 } }` | count 0 | count 0 | count 0 | + * | `having` `{ total: { a: 1 } }` | no group | no group | no group | + * + * The InMemoryDriver cell is this suite's recording driver by construction: + * the arm answers before any driver is resolved, so no read runs. The SQL + * cells — SQLite always, PostgreSQL and MySQL where their URLs are set — and + * the two controls over a real driver live in `@objectstack/rest`'s + * `data-no-operator-object-door.test.ts`. + * + * The two controls triage named stay accepted: a relation field's nested + * relation filter and a JSON-typed field's object comparand reach the driver + * exactly as written. + */ + +import { describe, it, expect, beforeEach } from 'vitest'; +import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; +import { + FieldType, + FILE_REFERENCE_TYPES, + MULTI_OPTION_TYPES, + REFERENCE_VALUE_TYPES, + SCALAR_FILTER_HEAD_TYPES, + STRUCTURED_JSON_TYPES, + type EngineAggregateOptions, + type EngineQueryOptions, + type FilterCondition, +} from '@objectstack/spec/data'; +import { ObjectQL } from './engine.js'; +import { holdsScalarValues, isNoOperatorObject } from './no-operator-object-door.js'; + +const OBJECT = 'no_op_object_probe'; +const OWNER = 'no_op_object_owner'; + +const OPTIONS = [{ label: 'X', value: 'x' }, { label: 'Y', value: 'y' }]; + +const PROBE = { + name: OBJECT, + label: 'No-operator object probe', + fields: { + amount: { name: 'amount', type: 'number' }, + price: { name: 'price', type: 'currency' }, + title: { name: 'title', type: 'text' }, + kind: { name: 'kind', type: 'select', options: OPTIONS }, + kinds: { name: 'kinds', type: 'select', multiple: true, options: OPTIONS }, + labels: { name: 'labels', type: 'multiselect', options: OPTIONS }, + flag: { name: 'flag', type: 'boolean' }, + placed_on: { name: 'placed_on', type: 'date' }, + seen_at: { name: 'seen_at', type: 'datetime' }, + code: { name: 'code', type: 'autonumber' }, + // The accepted side. + owner: { name: 'owner', type: 'lookup', reference: OWNER }, + owners: { name: 'owners', type: 'lookup', reference: OWNER, multiple: true }, + boss: { name: 'boss', type: 'master_detail', reference: OWNER }, + meta: { name: 'meta', type: 'json' }, + ship_to: { name: 'ship_to', type: 'address' }, + photo: { name: 'photo', type: 'image' }, + }, +}; + +const OWNER_OBJECT = { name: OWNER, label: 'Owner', fields: { region: { name: 'region', type: 'text' } } }; + +/** name · a scalar-valued field · its declared type. */ +const JUDGED: ReadonlyArray = [ + ['amount', 'number'], + ['price', 'currency'], + ['title', 'text'], + ['kind', 'select'], + ['kinds', 'select'], + ['labels', 'multiselect'], + ['flag', 'boolean'], + ['placed_on', 'date'], + ['seen_at', 'datetime'], + ['code', 'autonumber'], +]; + +/** name · a field on the accepted side · the no-operator object it may carry. */ +const ACCEPTED: ReadonlyArray]> = [ + ['owner', { region: 'NA' }], + ['owners', { region: 'NA' }], + ['boss', { region: 'NA' }], + ['meta', { a: 1 }], + ['ship_to', { city: 'Paris' }], + ['photo', { url: 'x' }], +]; + +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 }; +} + +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 }); + +const ENVELOPE = { code: 'INVALID_FILTER', status: 400 }; + +const envelopeOf = (err: Thrown) => ({ code: err?.code, status: err?.status }); + +describe('[#20546] a no-operator object where a scalar column\'s value belongs, 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(OWNER_OBJECT as any, 'test'); + engine.registry.registerObject(PROBE as any, 'test'); + reads.length = 0; + writes.length = 0; + }); + + // ── where ──────────────────────────────────────────────────────────────── + + it('refuses it under every scalar-valued field with the ADR-0112 envelope, naming the field, its type and the path — no read', async () => { + for (const [field, type] of JUDGED) { + const err = await refusalOf(engine.find(OBJECT, { where: { [field]: { a: 1 } } as FilterCondition })); + expect(envelopeOf(err), field).toEqual(ENVELOPE); + expect(err!.httpStatus, field).toBe(400); + expect(err!.message, field).toMatch(/^find\('no_op_object_probe'\): /); + expect(err!.message, field).toContain(`filter on '${field}'`); + expect(err!.message, field).toContain(`at where.${field},`); + expect(err!.message, field).toContain(`the declared ${type} field '${field}'`); + expect(err!.message, field).toContain('keys "a"'); + expect(err!.message, field).toContain('NOT applied'); + } + expect(reads).toHaveLength(0); + }); + + it('refuses {} and a deeper object too, and names each by its keys, never its values', async () => { + const empty = await refusalOf(engine.find(OBJECT, { where: { amount: {} } as FilterCondition })); + expect(envelopeOf(empty)).toEqual(ENVELOPE); + expect(empty!.message).toContain('puts an empty object {} at where.amount,'); + const deep = await refusalOf(engine.find(OBJECT, { where: { title: { a: { secret: 'value' } } } as FilterCondition })); + expect(envelopeOf(deep)).toEqual(ENVELOPE); + expect(deep!.message).toContain('keys "a"'); + expect(deep!.message).not.toContain('value"'); + expect(reads).toHaveLength(0); + }); + + it('covers every engine verb that collects a filter — read and write sides — and the judge', async () => { + const where = { amount: { a: 1 } } as FilterCondition; + 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, { title: 'x' }, { where, multi: true }), + () => engine.delete(OBJECT, { where, multi: true }), + ]) { + const err = await refusalOf(call()); + expect(envelopeOf(err)).toEqual(ENVELOPE); + expect(err!.message).toContain('at where.amount,'); + } + const judged = engine.judgeFilter(OBJECT, where); + expect(judged).toMatchObject({ ok: false, ...ENVELOPE }); + expect((judged as { message: string }).message).toContain("filter on 'amount'"); + expect(reads).toHaveLength(0); + expect(writes).toHaveLength(0); + }); + + it('reaches inside $and / $or / $not, with the path each arm sits at — structure launders nothing', async () => { + const cases: ReadonlyArray = [ + [{ $and: [{ title: 'a' }, { amount: { a: 1 } }] }, 'where.$and[1].amount'], + [{ $or: [{ amount: { a: 1 } }, { amount: 30 }] }, 'where.$or[0].amount'], + // Memory answered EVERY row for this one on the base. + [{ $not: { amount: { a: 1 } } }, 'where.$not.amount'], + ]; + for (const [where, path] of cases) { + const err = await refusalOf(engine.find(OBJECT, { where })); + expect(envelopeOf(err), path).toEqual(ENVELOPE); + expect(err!.message, path).toContain(`at ${path},`); + } + expect(reads).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. + const err = await refusalOf( + engine.find(OBJECT, { where: [['amount', '=', { a: 1 }]] } as unknown as EngineQueryOptions), + ); + expect(envelopeOf(err)).toEqual(ENVELOPE); + expect(err!.message).toContain('at where.amount,'); + expect(reads).toHaveLength(0); + }); + + it('CONTROL the accepted side reaches the driver exactly as written: relation, JSON-bearing and file fields', async () => { + for (const [field, spec] of ACCEPTED) { + reads.length = 0; + const where = { [field]: spec } as FilterCondition; + await expect(engine.find(OBJECT, { where }), field).resolves.toBeDefined(); + expect(reads, field).toHaveLength(1); + expect(reads[0]?.ast?.where, field).toEqual(where); + } + expect(engine.judgeFilter(OBJECT, { owner: { region: 'NA' } })).toEqual({ ok: true }); + expect(engine.judgeFilter(OBJECT, { meta: { a: 1 } })).toEqual({ ok: true }); + }); + + it('CONTROL an operator bag, a { $field } reference and a scalar comparand are not this arm\'s', async () => { + for (const where of [ + { amount: { $gt: 5 } }, + { amount: 5, title: 'a' }, + { amount: { $gt: { $field: 'price' } } }, + { kinds: { $in: ['x'] } }, + { labels: 'x' }, + { placed_on: { $gte: '2026-01-01' } }, + ] as FilterCondition[]) { + reads.length = 0; + await expect(engine.find(OBJECT, { where }), JSON.stringify(where)).resolves.toBeDefined(); + expect(reads, JSON.stringify(where)).toHaveLength(1); + } + }); + + it('CONTROL a Map is a comparand, not structure — the comparand-type door refuses it in its own words', async () => { + const err = await refusalOf(engine.find(OBJECT, { where: { title: new Map() } as unknown as FilterCondition })); + expect(envelopeOf(err)).toEqual(ENVELOPE); + expect(err!.message).not.toContain('no operator key'); + }); + + it('GUARD an UNKNOWN field keeps the engine\'s registry-less tolerance — no second opinion about a name', async () => { + await expect(engine.find(OBJECT, { where: { not_a_field: { a: 1 } } as FilterCondition })).resolves.toBeDefined(); + expect(reads).toHaveLength(1); + }); + + // ── the per-aggregation `filter` and `having` ──────────────────────────── + + it('refuses it in ONE aggregation\'s own filter, rooted at that position — no read', async () => { + for (const [field, type] of [['amount', 'number'], ['title', 'text']] as const) { + reads.length = 0; + const err = await refusalOf(engine.aggregate(OBJECT, { + aggregations: [ + { function: 'count', alias: 'all' }, + { function: 'count', alias: 'bad', filter: { [field]: { a: 1 } } }, + ], + } as EngineAggregateOptions)); + expect(envelopeOf(err), field).toEqual(ENVELOPE); + expect(err!.message, field).toMatch(/^aggregate\('no_op_object_probe'\): /); + expect(err!.message, field).toContain(`at aggregations[1].filter.${field},`); + expect(err!.message, field).toContain(`the declared ${type} field '${field}'`); + expect(reads, field).toHaveLength(0); + } + }); + + it('refuses it in having over a column holding scalar values, naming the aggregated column — no read', async () => { + const cases: ReadonlyArray = [ + [{ groupBy: ['title'], aggregations: [{ function: 'sum', field: 'amount', alias: 'total' }], having: { total: { a: 1 } } }, 'total', 'number'], + [{ groupBy: ['title'], aggregations: [{ function: 'count', alias: 'n' }], having: { title: { a: 1 } } }, 'title', 'text'], + [{ groupBy: ['title'], aggregations: [{ function: 'max', field: 'placed_on', alias: 'last' }], having: { last: {} } }, 'last', 'date'], + [{ groupBy: [{ field: 'placed_on', dateGranularity: 'month', alias: 'month' }], aggregations: [{ function: 'count', alias: 'n' }], having: { month: { a: 1 } } }, 'month', 'text'], + ] as ReadonlyArray; + for (const [query, column, type] of cases) { + reads.length = 0; + const err = await refusalOf(engine.aggregate(OBJECT, query)); + expect(envelopeOf(err), column).toEqual(ENVELOPE); + expect(err!.message, column).toContain(`at having.${column},`); + expect(err!.message, column).toContain(`the aggregated column '${column}', which carries a ${type} value`); + expect(reads, column).toHaveLength(0); + } + }); + + it('CONTROL the accepted side stays accepted in the per-aggregation filter and in having', async () => { + await expect(engine.aggregate(OBJECT, { + aggregations: [ + { function: 'count', alias: 'all' }, + { function: 'count', alias: 'na', filter: { owner: { region: 'NA' } } }, + { function: 'count', alias: 'm', filter: { meta: { a: 1 } } }, + ], + } as EngineAggregateOptions)).resolves.toBeDefined(); + for (const [groupBy, having] of [ + ['meta', { meta: { a: 1 } }], + ['owner', { owner: { region: 'NA' } }], + ] as const) { + await expect(engine.aggregate(OBJECT, { + groupBy: [groupBy], + aggregations: [{ function: 'count', alias: 'n' }], + having, + } as EngineAggregateOptions), groupBy).resolves.toBeDefined(); + } + }); + + // ── 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: Record }> = [ + { door: 'where object', query: { where: { amount: { a: 1 } } } }, + { door: '$filter string', query: { $filter: JSON.stringify({ amount: { a: 1 } }) } }, + { door: 'filter AST', query: { filter: [['amount', '=', { a: 1 }]] } }, + ]; + + it.each(DOORS)('the $door door refuses it', async ({ query }) => { + const err = await refusalOf(protocol.findData({ object: OBJECT, query } as any)); + expect(envelopeOf(err)).toEqual(ENVELOPE); + expect(err!.message).toContain("filter on 'amount'"); + expect(reads).toHaveLength(0); + }); + }); + + // ── the classification ─────────────────────────────────────────────────── + + it('GUARD the judged columns are the spec\'s scalar-valued classes, and the accepted side is never judged', () => { + for (const type of FieldType.options) { + const expected = SCALAR_FILTER_HEAD_TYPES.has(type) || MULTI_OPTION_TYPES.has(type); + expect(holdsScalarValues(type), type).toBe(expected); + } + for (const type of [...REFERENCE_VALUE_TYPES, ...STRUCTURED_JSON_TYPES, ...FILE_REFERENCE_TYPES, 'formula']) { + expect(holdsScalarValues(type), type).toBe(false); + } + expect(holdsScalarValues('not_a_type')).toBe(false); + }); + + it('GUARD structure is a PLAIN object with no $ key — a Date, an array, a Map, a class instance and an operator bag are not', () => { + expect(isNoOperatorObject({ a: 1 })).toBe(true); + expect(isNoOperatorObject({})).toBe(true); + expect(isNoOperatorObject(Object.create(null))).toBe(true); + for (const value of [new Date(0), [1], new Map(), new (class Box {})(), { $gt: 1 }, { a: 1, $eq: 2 }, { $field: 'x' }, null, 'a', 1]) { + expect(isNoOperatorObject(value), String(value)).toBe(false); + } + }); +}); 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 index a0e31cb4eaa..06b3e1d68db 100644 --- a/packages/objectql/src/engine-number-comparand-declared-type-door.test.ts +++ b/packages/objectql/src/engine-number-comparand-declared-type-door.test.ts @@ -508,12 +508,15 @@ describe('[#20351] the number-comparand declared-type door at the engine collect 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]]); + // [#20546] The same reading's types, for the walk's no-operator-object arm + // (nothing here is an object without a `$` key, so it refuses nothing). + const types = new Map([['total', 'number'], ['label', 'text'], ['unknown', undefined]]); const having = { total: { $in: ['1', 2] }, label: { $eq: 'abc' }, unknown: { $eq: 'abc' } }; - const narrowed = narrowHavingNumberComparands(OBJECT, having, classes); + const narrowed = narrowHavingNumberComparands(OBJECT, having, classes, types); 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); + expect(narrowHavingNumberComparands(OBJECT, untouched, classes, types)).toBe(untouched); }); // ── the REST doors that reach findData ────────────────────────────────── diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 7f0ef2e5378..cf36d291b9a 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -269,6 +269,7 @@ import { applyHaving, aggregatedRowColumns, aggregatedRowColumnClasses, + aggregatedRowColumnTypes, assertAggregationFilterIsEvaluable, assertHavingIsEvaluable, assertHavingIsFilterCondition, @@ -1021,6 +1022,10 @@ function lowerWhereFilterArray( // 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. + // [#20546] Its walk carries a second arm, asked first at every field: a + // plain object with no `$` key where a scalar column's value belongs + // (`{ amount: { a: 1 } }`) is refused `INVALID_FILTER` / 400. Memory + // answered it with no rows (every row under `$not`), SQL with its own 400. 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 @@ -1106,7 +1111,9 @@ function lowerWhereFilterArray( // spelling. assertTemporalComparandsInterpretable(object, operation, schema, condition); // [#20351] Same door as the object branch, on the LOWERED condition — the - // array sugar (`[['amount','>','abc']]`) names numeric fields too. + // array sugar (`[['amount','>','abc']]`) names numeric fields too. [#20546] + // …and lowers `['amount', '=', { a: 1 }]` to the no-operator object its + // second arm refuses. lowered.where = narrowNumberComparands(object, operation, schema, condition); return lowered as T; } @@ -16358,7 +16365,9 @@ export class ObjectQL implements IObjectQLEngine { // 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. + // evaluator compares it. Rooted at this position. [#20546] Its + // walk's no-operator-object arm too: `{ amount: { a: 1 } }` here + // counted no row, silently, on every driver. const numeric = narrowNumberComparands( object, 'aggregate', this._registry.getObject(object), aggFilter, `aggregations[${i}].filter`, ); @@ -16492,7 +16501,14 @@ export class ObjectQL implements IObjectQLEngine { // `$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); + // [#20546] The same walk's no-operator-object arm judges every column + // whose TYPE holds scalar values (hence the types, beside the + // classes): `{ total: { a: 1 } }` kept no group, silently, on every + // driver, where its `where` twin answered two ways. + const numeric = narrowHavingNumberComparands( + object, having, havingColumnClasses, + aggregatedRowColumnTypes(query.groupBy, query.aggregations, declaredFields), + ); if (numeric !== query.having) query = { ...query, having: numeric }; } const driver = this.getDriver(object); diff --git a/packages/objectql/src/having-filter.ts b/packages/objectql/src/having-filter.ts index 02cfa699b24..f78ca55920e 100644 --- a/packages/objectql/src/having-filter.ts +++ b/packages/objectql/src/having-filter.ts @@ -664,18 +664,26 @@ const NUMERIC_RESULT_FUNCTIONS: ReadonlySet = new Set(['count', 'count_d const VALUE_RESULT_FUNCTIONS: ReadonlySet = new Set(['min', 'max']); /** - * A declared field's class, by the spec's value-class sets. `undefined` when - * the declaration cannot tell: no field map (a registry-less host), no such - * field, or a `formula`, whose type names no stored value class. + * A declared field's `type`. `undefined` when the declaration cannot tell: no + * field map (a registry-less host), no such field, or no readable type. */ -function declaredFieldClass( +function declaredFieldType( fields: Record | undefined, name: unknown, -): AggregatedColumnClass | undefined { +): string | undefined { if (!fields || typeof fields !== 'object' || typeof name !== 'string') return undefined; if (!Object.prototype.hasOwnProperty.call(fields, name)) return undefined; const type = (fields[name] as { type?: unknown } | undefined)?.type; - if (typeof type !== 'string' || type === 'formula') return undefined; + return typeof type === 'string' ? type : undefined; +} + +/** + * The class a declared type carries, by the spec's value-class sets. + * `undefined` when the type cannot tell: none, or a `formula`, whose type names + * no stored value class. + */ +function classOfDeclaredType(type: string | undefined): AggregatedColumnClass | undefined { + if (type === undefined || type === 'formula') return undefined; if (CALENDAR_DATE_TYPES.has(type)) return 'date'; if (INSTANT_TYPES.has(type)) return 'datetime'; if (CLOCK_TIME_TYPES.has(type)) return 'time'; @@ -685,6 +693,64 @@ function declaredFieldClass( return 'text'; } +/** + * A declared field's class, by the spec's value-class sets. `undefined` when + * the declaration cannot tell: no field map (a registry-less host), no such + * field, or a `formula`, whose type names no stored value class. + */ +function declaredFieldClass( + fields: Record | undefined, + name: unknown, +): AggregatedColumnClass | undefined { + return classOfDeclaredType(declaredFieldType(fields, name)); +} + +/** + * [#20546] Each aggregated column's TYPE, read statically off the query and + * the object's field declaration — the one reading {@link aggregatedRowColumnClasses} + * classes, kept whole for a rule that needs more than the class: + * + * - a groupBy projection carries its field's declared type; a `day` date + * bucket is a `date` (its label is `YYYY-MM-DD`), and a coarser bucket a + * `text` label; + * - `count` / `count_distinct` / `sum` / `avg` carry a `number`; + * - `min` / `max` carry the type of the field they read. + * + * `undefined` where the declaration cannot tell. The no-operator-object arm of + * the number-comparand door's walk reads it at `having`: the `text` class + * lumps a `json` or `lookup` groupBy in with a real text column, and only the + * type tells a column that holds scalar values from one that does not. + */ +export function aggregatedRowColumnTypes( + groupBy: unknown, + aggregations: unknown, + fields: Record | undefined, +): Map { + const types = new Map(); + for (const g of Array.isArray(groupBy) ? groupBy : []) { + if (typeof g === 'string') { + types.set(g, declaredFieldType(fields, g)); + continue; + } + const item = g as { alias?: unknown; field?: unknown; dateGranularity?: unknown } | null; + const name = item?.alias ?? item?.field; + if (typeof name !== 'string') continue; + const granularity = item?.dateGranularity; + types.set(name, granularity == null + ? declaredFieldType(fields, item?.field) + : granularity === 'day' ? 'date' : 'text'); + } + for (const a of Array.isArray(aggregations) ? aggregations : []) { + const agg = a as { alias?: unknown; function?: unknown; field?: unknown } | null; + if (typeof agg?.alias !== 'string') continue; + const fn = String(agg.function); + types.set(agg.alias, NUMERIC_RESULT_FUNCTIONS.has(fn) + ? 'number' + : VALUE_RESULT_FUNCTIONS.has(fn) ? declaredFieldType(fields, agg.field) : undefined); + } + return types; +} + /** * [#20127] Each aggregated column's class, read STATICALLY — off the query and * the object's field declaration, never off a row — so a verdict that needs it @@ -700,6 +766,9 @@ function declaredFieldClass( * A column whose class the declaration cannot tell maps to `undefined`, and a * rule reading this map does not judge it — the fail-open direction the * engine's other declared-type doors take for a registry-less host. + * + * [#20546] Derived from {@link aggregatedRowColumnTypes}, so the class and + * the type of a column are one reading of the query, never two. */ export function aggregatedRowColumnClasses( groupBy: unknown, @@ -707,26 +776,8 @@ export function aggregatedRowColumnClasses( fields: Record | undefined, ): Map { const classes = new Map(); - for (const g of Array.isArray(groupBy) ? groupBy : []) { - if (typeof g === 'string') { - classes.set(g, declaredFieldClass(fields, g)); - continue; - } - const item = g as { alias?: unknown; field?: unknown; dateGranularity?: unknown } | null; - const name = item?.alias ?? item?.field; - if (typeof name !== 'string') continue; - const granularity = item?.dateGranularity; - classes.set(name, granularity == null - ? declaredFieldClass(fields, item?.field) - : granularity === 'day' ? 'date' : 'text'); - } - for (const a of Array.isArray(aggregations) ? aggregations : []) { - const agg = a as { alias?: unknown; function?: unknown; field?: unknown } | null; - if (typeof agg?.alias !== 'string') continue; - const fn = String(agg.function); - classes.set(agg.alias, NUMERIC_RESULT_FUNCTIONS.has(fn) - ? 'numeric' - : VALUE_RESULT_FUNCTIONS.has(fn) ? declaredFieldClass(fields, agg.field) : undefined); + for (const [column, type] of aggregatedRowColumnTypes(groupBy, aggregations, fields)) { + classes.set(column, classOfDeclaredType(type)); } return classes; } diff --git a/packages/objectql/src/no-operator-object-door.ts b/packages/objectql/src/no-operator-object-door.ts new file mode 100644 index 00000000000..da382e41694 --- /dev/null +++ b/packages/objectql/src/no-operator-object-door.ts @@ -0,0 +1,145 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#20546] The NO-OPERATOR-OBJECT arm: a plain object with no `$`-operator key + * written where a scalar field's value belongs — `{ amount: { a: 1 } }` over a + * declared `number` field — is refused with `INVALID_FILTER` / 400, naming the + * field and the path, at every position the engine judges. + * + * ## ⛔ Not a door of its own: an ARM of the number-comparand door's walk + * + * This module holds the arm's three parts — which columns it judges, what it + * refuses, and the words — and nothing that walks a filter. The walk is + * `number-comparand-declared-type-door.ts`'s `walkCondition`, the one filter + * walk the engine runs at all three filter positions with the column's + * declaration in hand (`where` in both spellings, `aggregations[i].filter`, + * and `having`). Triage directed it there: "If the same walk is the natural + * site, it lands serially after that PR, in the same walk. ⛔ No second + * traversal of the filter." That walk already stood on the exact branch — a + * field spec with no `$` key — and stepped past it for the numeric fields it + * judges; it now asks this module about every declared field before the number + * arm runs. + * + * ## What ran before this arm, measured on `origin/main` `fbec216e2d` + * + * Three rows, through `engine.find` / `engine.aggregate` and + * `POST /api/v1/data/:object/query` (both doors answered alike): + * + * | position · filter | InMemoryDriver | SqlDriver, SQLite | SqlDriver, PostgreSQL 16 | + * |:--|:--|:--|:--| + * | `where` `{ amount: { a: 1 } }` (number), `{ title: { a: 1 } }` (text), and the select, boolean, date, autonumber, multi-select, `multiple: true` select twins | 200, no rows | 400 `INVALID_FILTER`, the driver's words | same as SQLite | + * | `where` `{ $not: { amount: { a: 1 } } }` | 200, **every row** | 400, the driver's words | same | + * | `aggregations[1].filter` `{ amount: { a: 1 } }` | 200, count 0 | 200, count 0 | 200, count 0 | + * | `having` `{ total: { a: 1 } }` (a `sum`), `{ title: { a: 1 } }` (a groupBy) | 200, no group | 200, no group | 200, no group | + * + * One mistake, two answers at `where`, decided by the driver; a silent empty + * at the two positions the engine evaluates itself. + * + * ## Which columns it judges — a closed definition, from the spec's classes + * + * A column is judged when its declared type stores SCALAR values: one scalar + * ({@link SCALAR_FILTER_HEAD_TYPES}, `@objectstack/spec/data`'s published + * "stores one scalar value" set, derived from the ADR-0104 value classes — the + * #8371 dotted-head verdict reads the same set), or a list of scalar members + * (the same set under `multiple: true`, and the inherently-multi option types, + * {@link MULTI_OPTION_TYPES}). An object can never equal such a value, nor any + * member of it, on any backend. + * + * - **Multi-value fields are judged, unlike the dotted-head verdict's + * carve-out.** That carve-out exists because a numeric-index DOTTED path + * (`'tags.0'`) reaches an array member on two backends. The nested-object + * spelling does not: `{ tags: { 0: 'x' } }` over a `multiple: true` select + * answered 200 with no rows on InMemoryDriver and 400 on both SQL dialects, + * the same split as the scalar case (measured on the base above). + * - **The accepted side, never judged:** a relation (`lookup`, + * `master_detail`, `user`, `tree`, single or multiple) — `{ owner: { region: + * 'NA' } }` is a nested-relation condition, the form `FilterCondition` + * declares; a structured-JSON type (`json`, `composite`, `address`, …) — an + * object comparand is a whole-value match; a file or media type (a legacy + * stored value is an inline metadata object); `formula` (refused one door + * earlier, `INVALID_FIELD`); and a type this module has not met. That is + * the fail-open direction every neighbour takes: a hole, not a false 400. + * - **A `{ $field }` reference is never this arm's**: it carries a `$` key. So + * does every operator bag, however malformed — the drivers and the + * comparand doors answer those. + * - **`{}` is judged too** under a judged column. It has no `$` key, and at the + * two engine-evaluated positions it counted no row and kept no group on every + * driver; at `where` the drivers already refused it, and now the engine does, + * first. + * + * ## Structure test + * + * A PLAIN object only — prototype `Object.prototype` or `null`. A `Map` or a + * class instance is a comparand the comparand-type door refuses in its own + * words one call later, and a `Date` or an array is a comparand too: none of + * them is filter structure. + * + * @see https://github.com/objectstack-ai/objectstack/issues/20546 + */ + +import { MULTI_OPTION_TYPES, SCALAR_FILTER_HEAD_TYPES } from '@objectstack/spec/data'; + +/** + * Does a column of this declared type hold scalar values — one, or a list of + * scalar members — so that an object beneath it can match nothing? The arm's + * one classification; see the module header for the closed definition and the + * accepted side. + */ +export function holdsScalarValues(type: string): boolean { + return SCALAR_FILTER_HEAD_TYPES.has(type) || MULTI_OPTION_TYPES.has(type); +} + +/** + * Is this field spec a PLAIN object with no `$`-operator key — filter + * structure where a value belongs? `{}` included; a `Map`, a class instance, a + * `Date` and an array are comparands, not structure (see the module header). + */ +export function isNoOperatorObject(spec: unknown): spec is Record { + if (typeof spec !== 'object' || spec === null || Array.isArray(spec)) return false; + const proto = Object.getPrototypeOf(spec); + if (proto !== Object.prototype && proto !== null) return false; + return !Object.keys(spec).some((key) => key.startsWith('$')); +} + +/** What the arm found: the column, its declaration, where it sits, and the object's keys. */ +export interface NoOperatorObjectRefusal { + /** The filter key, which names the column. */ + readonly field: string; + /** Its declared `FieldType` — for `having`, the type the aggregated column carries. */ + readonly declaredType: string; + /** The key path the object sits at (`where.amount`, `having.total`, …). */ + readonly path: string; + /** The object's own keys, in order — `[]` for `{}`. */ + readonly keys: readonly string[]; + /** `having`: the column is an aggregated row's, not a declared field of the object. */ + readonly aggregated: boolean; +} + +/** How the object is named in the words: its keys, never its values. */ +function describeObject(keys: readonly string[]): string { + if (keys.length === 0) return 'an empty object {}'; + const shown = keys.slice(0, 3).map((key) => JSON.stringify(key)).join(', '); + const more = keys.length > 3 ? `, and ${keys.length - 3} more` : ''; + return `an object with no operator key (keys ${shown}${more})`; +} + +/** + * The refusal's words. Every position reads the same, less the subject: a + * declared field at `where` and `aggregations[i].filter`, an aggregated column + * at `having`. No tracker id: the lesson is in the sentence. + */ +export function noOperatorObjectRefusalMessage(refusal: NoOperatorObjectRefusal, context: string): string { + const subject = refusal.aggregated + ? `the aggregated column '${refusal.field}', which carries a ${refusal.declaredType} value` + : `the declared ${refusal.declaredType} field '${refusal.field}'`; + return ( + `${context}: filter on '${refusal.field}' puts ${describeObject(refusal.keys)} at ${refusal.path}, ` + + `where a value of ${subject} belongs. An object with no "$" operator is filter structure, not ` + + 'a value: beneath a field it is a nested-relation condition, which only a relation field (lookup, ' + + 'master-detail, user, tree) can carry, or a whole-value match, which only a JSON-bearing field ' + + `can hold. A ${refusal.declaredType} column holds scalar values — one, or a list of them — so no ` + + 'record can match an object there, and an empty answer would read exactly like a real one. The ' + + `filter was NOT applied. Compare '${refusal.field}' with a value ({ "${refusal.field}": VALUE }) ` + + `or an operator ({ "${refusal.field}": { "$eq": VALUE } }).` + ); +} diff --git a/packages/objectql/src/number-comparand-declared-type-door.ts b/packages/objectql/src/number-comparand-declared-type-door.ts index 55db8b9df2d..16151b29f6c 100644 --- a/packages/objectql/src/number-comparand-declared-type-door.ts +++ b/packages/objectql/src/number-comparand-declared-type-door.ts @@ -126,6 +126,17 @@ * flags (`$null` / `$exists` / `$empty`), the text operators and a * `{ $field }` reference are not a value of the field and are left alone. * + * ## [#20546] The walk's second arm + * + * The walk below is the one filter walk the engine runs at all three + * positions with each column's declaration in hand, so it also carries the + * no-operator-object arm (`no-operator-object-door.ts`): a plain object with + * no `$` key where a value of a column holding scalar values belongs — any + * such column, not only a numeric one — is refused with `INVALID_FILTER` / + * 400, naming the field and the path. It is asked first at every field key; + * the number arm reads what it lets through. That module holds the arm's + * classification and words; ⛔ nothing there walks a filter. + * * @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) @@ -143,6 +154,12 @@ import { } from '@objectstack/spec/data'; import { invalidFilterError } from './filter-comparand-shape.js'; import type { AggregatedColumnClass } from './having-filter.js'; +import { + holdsScalarValues, + isNoOperatorObject, + noOperatorObjectRefusalMessage, + type NoOperatorObjectRefusal, +} from './no-operator-object-door.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); @@ -153,8 +170,22 @@ const LIST_OPERATORS: ReadonlySet = new Set(NUMBER_COMPARAND_DOOR_LIST_O /** 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; +/** + * What one filter position supplies to the walk about a KEY: the two facts its + * two arms read, or `null` when the key names no column the position knows. + */ +interface KeyFacts { + /** The number arm's field meta — `null` when that arm has nothing to judge here. */ + readonly number: NumberComparandDoorFieldMeta | null; + /** + * [#20546] The column's declared type when it holds scalar values + * (`holdsScalarValues`), so the no-operator-object arm judges it — else `null`. + */ + readonly scalarType: string | null; +} + +/** What one filter position supplies to the walk: the facts a KEY names, or `null`. */ +type FactsOf = (key: string) => KeyFacts | null; /** * [#20510] What the CALLER already knows about the position being walked — @@ -179,10 +210,15 @@ const AGGREGATION_FILTER_SITE: RefusalSiteContext = { aggregated: false, boundBy /** `having`: an aggregated-row column, evaluated by the engine, never bound. */ const HAVING_SITE: RefusalSiteContext = { aggregated: true, boundByDriver: false }; +/** The first refusal the walk met, and which arm raised it. */ +type Refusal = + | { readonly arm: 'number'; readonly site: NonNumericComparand } + | { readonly arm: 'no-operator-object'; readonly site: NoOperatorObjectRefusal }; + /** 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 }; + | { readonly ok: false; readonly refusal: Refusal }; const kept = (value: unknown): Outcome => ({ ok: true, value }); @@ -231,7 +267,7 @@ function judgeComparand( if (verdict.verdict !== 'door-refusal') return kept(comparand); return { ok: false, - refusal: { + refusal: { arm: 'number', site: { field, declaredType: meta.type, ...(meta.returnType === undefined ? {} : { returnType: meta.returnType }), @@ -243,7 +279,7 @@ function judgeComparand( // driver-bound), which is exactly what `WHERE_SITE` above says. ...(ctx.aggregated ? { aggregated: true as const } : {}), ...(ctx.boundByDriver ? {} : { boundByDriver: false as const }), - }, + } }, }; } @@ -259,7 +295,11 @@ function judgeFieldSpec( if (!isFilterNode(spec)) return judgeComparand(meta, field, spec, path, ctx); // 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. + // contract no backend agrees with. [#20546] Under a column that holds + // scalar values — every numeric type — the walk's no-operator-object arm + // refuses one before this function is called; what still reaches this line + // sits under a column that arm does not judge (a `formula`, refused a door + // earlier). const ops = Object.keys(spec); if (!ops.some((op) => op.startsWith('$'))) return kept(spec); if (isFieldReference(spec)) return kept(spec); @@ -288,7 +328,17 @@ function judgeFieldSpec( /** * The walk, shared by every position: the node structure is judged the same - * way wherever the condition sits; only {@link MetaOf} differs. + * way wherever the condition sits; only {@link FactsOf} differs. + * + * [#20546] It carries TWO arms, asked in order at every field key: the + * no-operator-object arm (`no-operator-object-door.ts` — a plain object with + * no `$` key where a scalar column's value belongs), then the number arm + * ({@link judgeFieldSpec}). One traversal, one set of boundaries (the depth + * bound, the combinators descended, the `$` and dotted keys skipped), two + * questions — the shape the spec's save-door walk takes for its own arms + * (`checkFilterConditionComparands`: "One walk, one set of boundaries, `n` + * arms"). A second walk would have to redraw every one of those boundaries, + * and the two copies would part the first time one moved. * * Structure is discarded the same three conservative ways the sibling gates * discard it: `$and` / `$or` / `$not` are descended, any OTHER `$` key at node @@ -296,7 +346,7 @@ function judgeFieldSpec( * 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, ctx: RefusalSiteContext): Outcome { +function walkCondition(factsOf: FactsOf, node: unknown, path: string, depth: number, ctx: RefusalSiteContext): Outcome { if (depth > 32 || !isFilterNode(node)) return kept(node); let out: Record | undefined; for (const [key, value] of Object.entries(node)) { @@ -306,16 +356,30 @@ function walkCondition(metaOf: MetaOf, node: unknown, path: string, depth: numbe 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, ctx); + const walked = walkCondition(factsOf, arm, `${here}[${index}]`, depth + 1, ctx); 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, ctx); + judged = walkCondition(factsOf, value, here, depth + 1, ctx); } else { if (key.startsWith('$') || key.includes('.')) continue; - const meta = metaOf(key); + const facts = factsOf(key); + if (!facts) continue; + // [#20546] The no-operator-object arm, first: filter structure where a + // scalar column's value belongs can match no record on any backend, so + // it is refused whichever arm would otherwise read the value. + if (facts.scalarType !== null && isNoOperatorObject(value)) { + return { + ok: false, + refusal: { + arm: 'no-operator-object', + site: { field: key, declaredType: facts.scalarType, path: here, keys: Object.keys(value), aggregated: ctx.aggregated }, + }, + }; + } + const meta = facts.number; // 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. @@ -329,12 +393,17 @@ function walkCondition(metaOf: MetaOf, node: unknown, path: string, depth: numbe } /** The judged fields of a `where` or a per-aggregation `filter`: the object's declared map. */ -function declaredMetaOf(schema: unknown): MetaOf | null { +function declaredFactsOf(schema: unknown): FactsOf | 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); + return (key) => { + if (!Object.prototype.hasOwnProperty.call(fields, key)) return null; + const meta = fieldMetaOf(fields[key]); + if (!meta) return null; + return { number: meta, scalarType: holdsScalarValues(meta.type) ? meta.type : null }; + }; } /** @@ -343,22 +412,29 @@ function declaredMetaOf(schema: unknown): MetaOf | 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. + * [#20546] The number arm's answer only: when the walk's first refusal is the + * no-operator-object arm's, this answers `null` — {@link narrowNumberComparands} + * is the call that raises either. */ export function findNonNumericComparand( schema: unknown, where: unknown, path = 'where', ): NonNumericComparand | null { - const metaOf = declaredMetaOf(schema); - if (!metaOf) return null; + const factsOf = declaredFactsOf(schema); + if (!factsOf) return null; // [#20510] Every caller of this walk (`where`, the per-aggregation `filter`) // reads a real declared field; only `where` itself ever binds to a driver. - const walked = walkCondition(metaOf, where, path, 0, path === 'where' ? WHERE_SITE : AGGREGATION_FILTER_SITE); - return walked.ok ? null : walked.refusal; + const walked = walkCondition(factsOf, where, path, 0, path === 'where' ? WHERE_SITE : AGGREGATION_FILTER_SITE); + return walked.ok || walked.refusal.arm !== 'number' ? null : walked.refusal.site; } -function refuse(context: string, refusal: NonNumericComparand): never { - throw invalidFilterError(numberComparandRefusalMessage(refusal, context)); +function refuse(context: string, refusal: Refusal): never { + throw invalidFilterError( + refusal.arm === 'number' + ? numberComparandRefusalMessage(refusal.site, context) + : noOperatorObjectRefusalMessage(refusal.site, context), + ); } /** @@ -385,9 +461,9 @@ export function narrowNumberComparands( where: W, path = 'where', ): W { - const metaOf = declaredMetaOf(schema); - if (!metaOf) return where; - const walked = walkCondition(metaOf, where, path, 0, path === 'where' ? WHERE_SITE : AGGREGATION_FILTER_SITE); + const factsOf = declaredFactsOf(schema); + if (!factsOf) return where; + const walked = walkCondition(factsOf, where, path, 0, path === 'where' ? WHERE_SITE : AGGREGATION_FILTER_SITE); if (!walked.ok) refuse(`${operation}('${object}')`, walked.refusal); return walked.value as W; } @@ -406,14 +482,26 @@ export function narrowNumberComparands( * ({@link HAVING_SITE}), and — like the per-aggregation `filter` — the engine * evaluates `having` itself, never the driver, so it carries no PostgreSQL * clause either. + * + * [#20546] `types` (`aggregatedRowColumnTypes`, from the same reading of the + * query as `classes`) is what the walk's no-operator-object arm reads here: + * the column's type, since the class lumps a `json` or `lookup` groupBy in + * with a text column. Its refusal names the aggregated column too. */ export function narrowHavingNumberComparands( object: string, having: H, classes: ReadonlyMap, + types: ReadonlyMap, ): H { const walked = walkCondition( - (key) => (classes.get(key) === 'numeric' ? { type: 'number' } : null), + (key) => { + const type = types.get(key); + return { + number: classes.get(key) === 'numeric' ? { type: 'number' } : null, + scalarType: type !== undefined && holdsScalarValues(type) ? type : null, + }; + }, having, 'having', 0, diff --git a/packages/rest/src/data-no-operator-object-door.test.ts b/packages/rest/src/data-no-operator-object-door.test.ts new file mode 100644 index 00000000000..04546d72cfa --- /dev/null +++ b/packages/rest/src/data-no-operator-object-door.test.ts @@ -0,0 +1,244 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#20546] A plain object with no `$`-operator key where a scalar field's + * value belongs is refused at the public door — `POST /api/v1/data/:object/query` + * and `engine.find` answer `400 INVALID_FILTER` in the engine's words, naming + * the field and the path, before any read — over a real `SqlDriver`, with the + * two controls triage named reaching the driver as written: a `lookup` field's + * nested relation filter and a `json` field's object comparand. + * + * Measured on the base (`origin/main` `fbec216e2d`) through this door and the + * engine, three rows: + * + * | `where` | InMemoryDriver | SQLite | PostgreSQL 16 | + * |:--|:--|:--|:--| + * | `{ amount: { a: 1 } }` (number), `{ title: { a: 1 } }` (text) | 200, no rows | 400 `INVALID_FILTER`, the driver's words | same as SQLite | + * | control `{ owner: { region: 'NA' } }` (lookup) | 200, no rows | 400, the driver's words | same | + * | control `{ meta: { a: 1 } }` (json) | 200, one row | 400, the driver's words | same | + * | `aggregations[1].filter` `{ amount: { a: 1 } }` / `having` `{ total: { a: 1 } }` | count 0 / no group | same | same | + * + * The arm sits in the engine, in front of every driver, so one verdict holds + * on each cell; InMemoryDriver's row is `@objectstack/objectql`'s + * `engine-no-operator-object-door.test.ts` by construction (the arm answers + * before a driver is resolved). The controls are this door's to let through, + * not to fix: what a driver answers for them afterwards is its own, and is + * pinned here only as "the driver was asked, and the words are not the arm's". + * + * ## 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 its tables, 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_no_op_object_20546'; +const OWNER = 'rest_no_op_owner_20546'; + +const OWNER_OBJECT = { + name: OWNER, + label: 'Owner 20546', + fields: { region: { name: 'region', type: 'text' as const } }, +}; + +const LEDGER = { + name: OBJECT, + label: 'Ledger 20546', + fields: { + title: { name: 'title', type: 'text' as const }, + amount: { name: 'amount', type: 'number' as const }, + owner: { name: 'owner', type: 'lookup' as const, reference: OWNER }, + meta: { name: 'meta', type: 'json' as const }, + }, +}; + +const OWNERS = [{ id: 'u1', region: 'NA' }, { id: 'u2', region: 'EU' }]; + +const ROWS = [ + { id: 'd1', title: 'a', amount: 5, owner: 'u1', meta: { a: 1 } }, + { id: 'd2', title: 'b', amount: 12, owner: 'u2', meta: { a: 2 } }, + { id: 'd3', title: 'c', amount: 30, owner: 'u1', meta: { b: 1 } }, +]; + +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 `where` — a no-operator object under a scalar field. */ +const REFUSED: ReadonlyArray = [ + ['a number field (the card)', { amount: { a: 1 } }, 'amount', 'where.amount'], + ['a text field', { title: { a: 1 } }, 'title', 'where.title'], + ['{} under a number field', { amount: {} }, 'amount', 'where.amount'], + ['inside $not (every row on memory, before)', { $not: { amount: { a: 1 } } }, 'amount', 'where.$not.amount'], +]; + +/** name · the `where` — the two controls triage named. */ +const CONTROLS: ReadonlyArray = [ + ["a lookup field's nested relation filter", { owner: { region: 'NA' } }], + ["a json field's object comparand", { meta: { a: 1 } }], +]; + +/** The arm's own words, in every refusal it raises — a control must never be answered in them. */ +const ARM_WORDS = 'is filter structure, not a value'; + +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 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)( + `[#20546] a no-operator object under a scalar 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 }>; + + const dropTables = async () => { + if (cell.id === 'sqlite') return; + await driver?.execute(`drop table if exists ${OBJECT}`).catch(() => {}); + await driver?.execute(`drop table if exists ${OWNER}`).catch(() => {}); + }; + + beforeAll(async () => { + driver = new SqlDriver(config as any); + await dropTables(); + engine = new ObjectQL(); + engine.registerDriver(driver, true); + await engine.init(); + engine.registry.registerObject(OWNER_OBJECT as any); + engine.registry.registerObject(LEDGER as any); + await engine.syncSchemas(); + for (const row of OWNERS) await engine.insert(OWNER, { ...row } as any); + 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 () => { + await dropTables(); + try { await engine?.destroy(); } catch { /* noop */ } + }); + + it('where: 400 INVALID_FILTER in the engine\'s words, naming the field and the path, through REST and engine.find — no read', async () => { + const before = reads.n; + for (const [name, where, field, path] of REFUSED) { + 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 '${field}'`); + expect(res.body.error, `REST, ${name}`).toContain(`at ${path},`); + 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(err?.message, `engine.find, ${name}`).toContain(`at ${path},`); + expect(err?.message, `engine.find, ${name}`).toContain(ARM_WORDS); + } + 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; + const filter = await query({ + aggregations: [{ function: 'count', alias: 'n' }, { function: 'count', alias: 'm', filter: { amount: { a: 1 } } }], + } satisfies EngineAggregateOptions as Record); + expect(filter.status, JSON.stringify(filter.body)).toBe(400); + expect(filter.body.code).toBe('INVALID_FILTER'); + expect(filter.body.error).toContain('at aggregations[1].filter.amount,'); + const having = await query({ + groupBy: ['title'], + aggregations: [{ function: 'sum', field: 'amount', alias: 'total' }], + having: { total: { a: 1 } }, + } satisfies EngineAggregateOptions as Record); + expect(having.status, JSON.stringify(having.body)).toBe(400); + expect(having.body.code).toBe('INVALID_FILTER'); + expect(having.body.error).toContain('at having.total,'); + expect(reads.n - before, 'no read of the object — every refusal precedes the driver').toBe(0); + }); + + it('CONTROL the lookup nested relation filter and the json object comparand reach the driver, never the arm\'s refusal', async () => { + for (const [name, where] of CONTROLS) { + const before = reads.n; + const res = await query({ where }); + expect(reads.n - before, `REST, ${name}: the driver was asked`).toBe(1); + expect(JSON.stringify(res.body), `REST, ${name}`).not.toContain(ARM_WORDS); + const read = reads.n; + const err = await refusalOf(engine.find(OBJECT, { where })); + expect(reads.n - read, `engine.find, ${name}: the driver was asked`).toBe(1); + expect(err?.message ?? '', `engine.find, ${name}`).not.toContain(ARM_WORDS); + } + }); + + it('CONTROL a scalar comparand and an operator are answered as before', async () => { + const plain = await query({ where: { amount: 12 } }); + expect(plain.status, JSON.stringify(plain.body)).toBe(200); + expect(plain.body.records.map((r: any) => r.id)).toEqual(['d2']); + const op = await query({ where: { amount: { $gt: 10 } } }); + expect(op.status, JSON.stringify(op.body)).toBe(200); + expect(op.body.records.map((r: any) => r.id).sort()).toEqual(['d2', 'd3']); + }); + }, + ); +}