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
35 changes: 35 additions & 0 deletions .changeset/20351-number-comparand-door.md
Original file line number Diff line number Diff line change
@@ -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)

<!-- adr-0087: not-required (no-migration-prescription) a refusal of a filter COMPARAND at the engine's query door: no authorable key, spelling or stored shape moves, `packages/spec` is untouched (the grammar, the verdict and the words shipped with the spec contract), and no stored row is read or rewritten. What is refused is a string that names no number, compared against a declared numeric field or a numeric aggregated column; which number the caller meant is not something a ledger entry can decide. The other categories are closed on facts: the package publishes (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. 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.
Original file line number Diff line number Diff line change
Expand Up @@ -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 }, []],
Expand All @@ -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).
Expand Down
7 changes: 5 additions & 2 deletions packages/objectql/src/engine-aggregate-positions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -242,8 +242,11 @@ describe('[#20334] having — a placeholder the resolver cannot resolve is refus
const REFUSED: ReadonlyArray<readonly [string, () => unknown, Record<string, unknown>, 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',
Expand Down
Loading
Loading