Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions .changeset/20502-number-comparand-non-string.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
---
"@objectstack/spec": minor
"@objectstack/objectql": minor
---

fix(spec)!: a boolean, a `Date` or an array compared against a number field is refused with `INVALID_FILTER` / 400 at `where`, a per-aggregation `filter` and `having`, the same as a non-numeric string

Clause-②: no (narrowing)

<!-- adr-0087: not-required (no-migration-prescription) a refusal of a filter COMPARAND value at the engine's query door, decided by the published number-comparand verdict: no authorable key, spelling, export or stored shape moves (the verdict's function, its case table and its words keep their names; three exports are added), and no stored row is read or rewritten. What is refused is a boolean, a Date or an array compared against a declared numeric field or a numeric aggregated column; which number the caller meant is not something a ledger entry can decide (reading true as 1 is exactly the silent coercion this refuses). The other categories are closed on facts: both packages publish (not `unpublished`); no ADR-0087 id covers a filter comparand (not `registered` / `already-registered`); and the change is runtime behaviour, not a declaration (not `runtime-interface-only` / `type-surface-only`). -->

**BREAKING**: this narrows what a filter may compare a number field with. `numberComparandDoorVerdict`, the published verdict the engine's number-comparand door consumes, judged strings only; it now also answers `door-refusal` for a boolean, a `Date` and an array, so the engine refuses them before any read, on every driver. It ships as `minor` under the launch-window convention for accept-set narrowings. The string rule is unchanged, and so are both packages' root exports except three additions to `@objectstack/spec/data`: `NON_NUMERIC_VALUE_FORMS` and the types `NonNumericValueForm` and `NonNumericComparandForm` (the refusal's `form` and the refusal site's `value` widen to carry a non-string).

FROM `true` / `false`, a `Date`, or an array where one value belongs (a scalar operator's comparand, or a member of `$in` / `$nin` / `$between`), 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`) → 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 the filter means (`12`, `-3.5`, `1e3`), or compare a `Date` with a date or datetime field.

Measured through `engine.find` / `engine.aggregate` and `POST /data/:object/query`, three rows (5, 12, 30) of a `number` field:

| position | comparand | before: memory · SQLite · PostgreSQL 16 | now, on all three |
|:--|:--|:--|:--|
| `where` | `$gt true` | no rows · every row · `DATABASE_ERROR` (500) | `INVALID_FILTER` / 400 |
| `where` | `$ne true` | every row · every row · 500 | `INVALID_FILTER` / 400 |
| `where` | `$between [true, 20]` | no rows · two rows · 500 | `INVALID_FILTER` / 400 |
| `where` | `$gt` a `Date` (in-process callers) | no rows · no rows · 500 | `INVALID_FILTER` / 400 |
| `where` | `$gt [1]` | a driver's own 400, in each driver's words | `INVALID_FILTER` / 400, in one set of words |
| `where` | a `$in` member `[1]` | no rows · a driver 400 · a driver 400 | `INVALID_FILTER` / 400 |
| per-aggregation `filter` | `$gt true` / `$gt [1]` (`$gt` a `Date`) | count 3 (count 0), on all three | `INVALID_FILTER` / 400 |
| `having` on `sum(amount)` | `$gt true` / `$gt [1]` (`$gt` a `Date`) | every group (no group), on all three | `INVALID_FILTER` / 400 |
| all three positions | `$gt 10` (the numeric control) | 2 rows / count 2 / both groups | the same |

**Who is affected.** A caller that compares a number field with a boolean or an array through any door that reaches the engine (a `where`, `$filter` or `filter` body of `POST /data/:object/query`, or an in-process engine call), or with a `Date` in-process (a flow, a hook, server code). No example app, platform object or other in-repo producer compares a number field that way.

**Unchanged.** A number, a `bigint` (narrowed or refused by the comparand-type door, as before) and `null` (the null test) are answered as before. A value outside the accepted comparand types (`undefined`, a plain object, a `Map`) keeps the comparand-type door's own refusal and words. An array at an equality slot (implicit, `$eq`, `$ne`) keeps the comparand-shape door's refusal. A boolean or a `Date` compared against a boolean, date or datetime field is not this door's subject. Driver-direct callers that never pass through the engine keep each driver's native binding.
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,9 @@ import { describe, it, expect, beforeEach } from 'vitest';
import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol';
import {
NON_NUMERIC_STRING_FORMS,
NON_NUMERIC_VALUE_FORMS,
NUMBER_COMPARAND_DOOR_CASES,
normalizeFilterComparandTypes,
NUMBER_COMPARAND_DOOR_FIXTURE,
NUMBER_COMPARAND_DOOR_FIXTURE_OBJECT,
NUMBER_COMPARAND_DOOR_LIST_OPERATORS,
Expand Down Expand Up @@ -157,8 +159,9 @@ describe('[#20351] the number-comparand declared-type door at the engine collect
// 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 refused form the grammar names is driven, not just the card's "abc" —
// and [#20502] every non-string form (a boolean, a Date, an array) beside them.
expect(new Set(REFUSALS.map((c) => c.form))).toEqual(new Set([...NON_NUMERIC_STRING_FORMS, ...NON_NUMERIC_VALUE_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\]$/, '')));
Expand Down Expand Up @@ -383,6 +386,110 @@ describe('[#20351] the number-comparand declared-type door at the engine collect
expect(await groups({ id: { $ne: 'abc' } })).toEqual(['r1', 'r2', 'r3']);
});

// ── [#20502] a boolean, a Date, an array: one refusal at every position ──

/** The non-string comparands the widened verdict refuses — each a value the card measured three ways. */
const NON_STRING: ReadonlyArray<readonly [string, () => unknown, string]> = [
['true', () => true, 'boolean'],
['false', () => false, 'boolean'],
['a Date', () => new Date(Date.UTC(2026, 0, 1)), 'date'],
['an array', () => [10], 'array'],
];

it('[#20502] refuses a boolean, a Date or an array in ONE aggregation\'s own filter, rooted at that position — no read', async () => {
for (const [name, value, form] of NON_STRING) {
for (const op of ['$gt', '$lte'] 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]: value() } } },
],
} as EngineAggregateOptions));
expect(err, `${name} ${op}`).not.toBeNull();
expect({ code: err!.code, status: err!.status }, `${name} ${op}`).toEqual({ code: 'INVALID_FILTER', status: 400 });
expect(err!.message, `${name} ${op}`).toContain(`aggregations[1].filter.f_number.${op}`);
expect(err!.message, `${name} ${op}`).toMatch(/^aggregate\('number_door_probe'\): filter on 'f_number' compares a declared number field/);
expect(findNonNumericComparand(engine.registry.getObject(OBJECT), { f_number: { [op]: value() } }), name)
.toMatchObject({ field: 'f_number', form });
expect(reads, `${name} ${op}`).toHaveLength(0);
}
}
});

it('[#20502] refuses a boolean, a Date or an array against a NUMERIC `having` column — count, sum and a numeric min alike', async () => {
for (const [name, value] of NON_STRING) {
for (const [fn, field] of [['count', undefined], ['sum', 'f_number'], ['min', 'f_currency']] as const) {
reads.length = 0;
const err = await refusalOf(engine.aggregate(OBJECT, {
groupBy: ['f_text'],
aggregations: [{ function: fn, ...(field ? { field } : {}), alias: 'total' }],
having: { total: { $gt: value() } },
} as EngineAggregateOptions));
expect(err, `${name} ${fn}`).not.toBeNull();
expect({ code: err!.code, status: err!.status }, `${name} ${fn}`).toEqual({ code: 'INVALID_FILTER', status: 400 });
expect(err!.message, `${name} ${fn}`).toContain('having.total.$gt');
expect(err!.message, `${name} ${fn}`).toContain("filter on 'total' compares a declared number field");
expect(reads, `${name} ${fn}`).toHaveLength(0);
}
}
});

it('[#20502] the numeric control at all three positions: a number reaches the driver and the evaluator as written', async () => {
reads.length = 0;
await engine.find(OBJECT, { where: { f_number: { $gt: 10 } } });
expect(reads[0]?.ast?.where).toEqual({ f_number: { $gt: 10 } });
const counted = await engine.aggregate(OBJECT, {
aggregations: [{ function: 'count', alias: 'all' }, { function: 'count', alias: 'm', filter: { f_number: { $gt: 10 } } }],
} as EngineAggregateOptions);
expect(Number((counted[0] as Record<string, unknown>).m)).toBe(2);
const groups = (await engine.aggregate(OBJECT, {
groupBy: ['id'],
aggregations: [{ function: 'sum', field: 'f_number', alias: 'total' }],
having: { total: { $gt: 10 } },
} as EngineAggregateOptions)).map((r) => (r as Record<string, unknown>).id).sort();
expect(groups).toEqual(['r2', 'r3']);
});

it('[#20502] the same mistake as FilterArray sugar and inside $and / $or / $not — one answer per mistake', async () => {
const sugar = await refusalOf(
engine.find(OBJECT, { where: [['f_number', '>', true]] } as unknown as EngineQueryOptions),
);
expect({ code: sugar!.code, status: sugar!.status }).toEqual({ code: 'INVALID_FILTER', status: 400 });
expect(sugar!.message).toContain("filter on 'f_number' compares a declared number field against true at where.f_number.$gt");
for (const where of [
{ $and: [{ f_text: 'a' }, { f_number: { $gt: true } }] },
{ $or: [{ f_text: 'a' }, { f_currency: { $in: [1, new Date(0)] } }] },
{ $not: { f_percent: { $between: [[1], 10] } } },
]) {
const err = await refusalOf(engine.find(OBJECT, { where: where as FilterCondition }));
expect({ code: err?.code, status: err?.status }, JSON.stringify(where)).toEqual({ code: 'INVALID_FILTER', status: 400 });
}
expect(reads).toHaveLength(0);
expect(engine.judgeFilter(OBJECT, { f_number: { $gt: new Date(0) } })).toMatchObject({ ok: false, code: 'INVALID_FILTER', status: 400 });
});

it('[#20502] a value OUTSIDE the accepted comparand types is the comparand-TYPE door\'s refusal, in that door\'s words, on both spellings', async () => {
const context = `find('${OBJECT}')`;
for (const [name, value] of [['a plain object', { a: 1 }], ['undefined', undefined], ['a Map', new Map()]] as const) {
const where = { f_number: { $gt: value } };
// The verdict passes it, so THIS door has no second opinion …
expect(findNonNumericComparand(engine.registry.getObject(OBJECT), where), name).toBeNull();
// … and the engine answers exactly what the comparand-type door answers.
let expected: Error | undefined;
try { normalizeFilterComparandTypes(where, context); } catch (e) { expected = e as Error; }
expect(expected, name).toBeDefined();
const err = await refusalOf(engine.find(OBJECT, { where: where as FilterCondition }));
expect({ code: err?.code, status: err?.status }, name).toEqual({ code: 'INVALID_FILTER', status: 400 });
expect(err!.message, name).toBe(expected!.message);
}
const sugar = await refusalOf(engine.find(OBJECT, { where: [['f_number', '>', { a: 1 }]] } as unknown as EngineQueryOptions));
let expected: Error | undefined;
try { normalizeFilterComparandTypes({ f_number: { $gt: { a: 1 } } }, context); } catch (e) { expected = e as Error; }
expect(sugar!.message).toBe(expected!.message);
expect(reads).toHaveLength(0);
});

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' } };
Expand Down
29 changes: 19 additions & 10 deletions packages/objectql/src/number-comparand-declared-type-door.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,11 @@
* 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.*
* gate. It answers a fifth question about the same predicate: *is this
* comparand a number the column can be compared with.* [#20502] widened the
* question from strings to every comparand, in the spec's verdict alone: a
* boolean, a `Date` and an array are refused beside a non-numeric string,
* and this file changed only to carry a refused value that is not a string.
*
* ## The direction this implements (triage, recorded on #20336)
*
Expand Down Expand Up @@ -45,9 +48,12 @@
*
* ## 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
* - **Refuse** a string the grammar does not read as a number, and a boolean,
* a `Date` or an array (#20502): `INVALID_FILTER` / 400, the existing filter
* envelope, in the contract's words, before any driver is resolved. A value
* outside the comparand-type door's accepted set (`undefined`, a plain
* object, a `Map`) passes the verdict and is refused by that door, one call
* later, in its own words. 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.
Expand Down Expand Up @@ -178,7 +184,11 @@ function fieldMetaOf(def: unknown): NumberComparandDoorFieldMeta | null {
return typeof returnType === 'string' ? { type, returnType } : { type };
}

/** One comparand at a judged position: the spec's verdict, routed. */
/**
* One comparand at a judged position: the spec's verdict, routed. Whatever
* the comparand is — a string, a boolean, a `Date`, an array (#20502) — the
* verdict alone decides; this function only turns its answer into an outcome.
*/
function judgeComparand(
meta: NumberComparandDoorFieldMeta,
field: string,
Expand All @@ -195,8 +205,7 @@ function judgeComparand(
declaredType: meta.type,
...(meta.returnType === undefined ? {} : { returnType: meta.returnType }),
path,
// `door-refusal` is answered for a string comparand only.
value: comparand as string,
value: comparand,
form: verdict.form,
},
};
Expand Down Expand Up @@ -292,7 +301,7 @@ function declaredMetaOf(schema: unknown): MetaOf | null {
}

/**
* Walk one `FilterCondition` and return the FIRST string a declared numeric
* Walk one `FilterCondition` and return the FIRST comparand a declared numeric
* field cannot be compared with, or `null`.
*
* Exported for the same reason the sibling walks are: a consumer that needs to
Expand All @@ -314,7 +323,7 @@ function refuse(context: string, refusal: NonNumericComparand): never {
}

/**
* Refuse every string a declared numeric field cannot be compared with, and
* Refuse every comparand 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.
Expand Down
Loading
Loading