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
19 changes: 19 additions & 0 deletions .changeset/driver-memory-array-comparand-refusal-date-equality.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
---
"@objectstack/driver-memory": minor
---

fix(driver-memory)!: an array comparand is refused instead of compared by reference, and a `Date` comparand is compared by time value

<!-- adr-0087: not-required (no-migration-prescription) An accept-set narrowing and an evaluation repair inside one driver's filter gate: no metadata key, spec symbol, Zod schema, object definition or stored representation is added, removed or renamed. `packages/spec`'s half of this diff is a docblock sentence and one test comment — `filter-comparand-type.ts`'s list of the cases its door deliberately does not rule, corrected to state what the drivers now do — and no schema member, accepted key or exported symbol moves with it, so `check:api-surface` and `check:authorable-surface` see an unchanged surface. What moves is which FILTERS `@objectstack/driver-memory` accepts, which is authored at a call site or in a stored `where`, never a key `objectstack migrate meta` could visit: there is no old spelling to rewrite into a new one, because the refused shape has no replacement spelling that means the same thing — an array comparand never had agreed semantics to preserve, which is the whole reason the cell is refused rather than converged. Measured on this tree, the authored corpus that could carry the shape holds none of it: a structural scan of every tracked JSON and YAML file with a `where` / `filters` / `filter` object (10 JSON candidates, 1 YAML candidate — the latter a GitHub Actions job named `filter`, not a filter) finds ZERO bare-array comparands, and across the repository's own suites exactly one case authored the shape (`{ code: [] }`, in this package's face-agreement table), moved in this diff to assert the refusal. Whether an out-of-repo host authors one is NOT measured and is not claimed to be zero. -->

`checkCondition` routed both `Date` and `Array` into `value == condition`, under a comment two lines above calling that "exact match" — and a comment seven lines below giving `==`'s reasons, all three of which are about primitives ("undefined/null mismatch", "string/number coercion", "convenient in weakly typed queries"). Between two objects `==` does no conversion at all: it compares REFERENCES. So the arm was neither exact nor a match, and the two adjacent comments described something the code could not do.

**BREAKING** accept-set narrowing on a published driver, shipped as `minor` under the repo's launch-window convention for breaking changes (`scripts/check-changeset-no-major.mjs`): a purely additive widening takes at least `minor`, and a narrowing is not the additive bucket, so `patch` is refused here — the public surface really does move. The two halves take DIFFERENT dispositions, and the difference is the contract's rather than this driver's:

**An ARRAY comparand is refused.** `@objectstack/spec`'s comparand door names an array outside `$in`/`$nin`/`$between` as a position it deliberately does not rule, leaving it "to the layers that already answer it"; `ACCEPTED_FILTER_COMPARAND_TYPES` has no array member, and `driver-sql` refuses one with its own message. This driver now answers the same way, in the ADR-0112 envelope its every other filter refusal speaks (`code: 'INVALID_FILTER'`, `status: 400`), with a message naming the field, the received shape, the accepted set and the operators that DO take a list. The refusal is raised from `assertFilterConditionShape` — the one gate every face of this package runs before it evaluates anything — so the live query path, the reference matcher and the analytics face cannot disagree about it.

Why refused rather than made to work, measured on one row `{ tags: ['a','b'] }` with the filter `{ tags: ['a','b'] }` before this change: the live query path returned the row (mingo deep-equals arrays) and the reference matcher returned none (`==` compared references). One filter, one package, two answers, neither reported — the same shape as this package's `{ field: {} }`, malformed-`$between` and non-boolean-`$null` refusals. Converging UP to deep equality was available and is not what landed: the contract declines to rule the cell, and every sibling that has answered it declines the semantics.

**A `Date` comparand is EVALUATED, not refused** — the opposite disposition, because the contract does rule this one. `Date` is a member of `ACCEPTED_FILTER_COMPARAND_TYPES`, and `FILTER_COMPARAND_TYPE_CASES` requires that a Date comparand "must pass the door and execute everywhere". It is now compared by time value, arm for arm with `@objectstack/formula`'s `looseEq` — the sibling record-at-a-time matcher this face's conformance suites are held against — which also settles the case that actually reaches a stored row: a declared `datetime` is canonicalised to ISO TEXT on write (ADR-0053 D-B1), so a `Date` comparand meets a STRING, and `==` stringified the Date to `"Wed Jan 01 2026 …"`, which no ISO value equals. `$eq` / `$ne` take the same equality, so one predicate no longer answers two ways depending on which spelling the author used. An Invalid Date has no time value and so equals nothing, itself included.

**What is deliberately NOT changed**, and is now pinned so a later edit cannot take it away by accident: a SCALAR comparand against a stored ARRAY. `{ tags: 'a' }` against a row storing `['a','b']` still answers false, and `{ tags: 'a,b' }` against that row still answers true, because `==` stringifies the stored array to `"a,b"`. That is a third bad direction of the same operator, but it is on the VALUE side, and the comparand door judges comparands. It is recorded, not repaired. The list operators (`$in` / `$nin` / `$between`), for which an array is the declared comparand, are untouched; so is the text family, whose comparand disposition this package's refusal gate had already recorded as deliberately fail-closed.
9 changes: 9 additions & 0 deletions .changeset/spec-comparand-door-array-parenthetical.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
"@objectstack/spec": minor
---

docs(spec): the comparand door's array parenthetical states what the drivers do today

`filter-comparand-type.ts`'s list of the cases its door deliberately does not rule described the array cell as "`driver-sql` refuses it with its own message; the document stores give it array-equality semantics". Measured on this tree, that second clause is no longer a true statement about `@objectstack/driver-memory`: its reference matcher compared an array comparand by REFERENCE (matching nothing), while its live query path deep-equalled it (matching the row) — one package, two answers, neither of them a stable "array-equality semantics" a reader could build on. With that driver's cell now refused, the sentence names the two dispositions that exist: `driver-sql` and `driver-memory` refuse it, each with its own message, and `driver-mongodb` hands it to MongoDB and inherits that engine's array semantics.

The paragraph's point is unchanged and deliberately kept: the door does not rule this position, the matrix did not measure it, and it is left to the layers that already answer it. Only the description of what those layers do is corrected — a stale factual clause is how the next reader re-derives "the spec promises array-equality" from a passage that explicitly promises nothing. The neighbouring pass-through pin's comment carried the same stale characterisation and is corrected the same way; its assertion (that `parseFilterAST` leaves `{ tags: ['a','b'] }` untouched) is unchanged and still pins the door not judging.
97 changes: 97 additions & 0 deletions packages/drivers/driver-memory/src/filter-refusal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@
*/

import { FILTER_OPERATORS, LOGICAL_OPERATORS, RETIRED_FILTER_OPERATORS } from '@objectstack/spec/data';
// [#16810] The accepted comparand-type set's own sentence, quoted rather than
// hand-copied — the same single-sourcing `driver-sql`'s refusals use, so the
// two backends cannot describe the accepted set differently.
import { ACCEPTED_FILTER_COMPARAND_TYPES_SENTENCE } from '@objectstack/spec/data';
// [#7536] The `$like` pattern language's shared gate, so this driver refuses
// the same malformed patterns as every other face.
import { hasDanglingLikeEscape } from '@objectstack/spec/data';
Expand Down Expand Up @@ -553,6 +557,85 @@ export function retiredFilterOperatorError(
);
}

/**
* [#16810] The comparison operators whose comparand is a SINGLE value.
*
* The list operators (`$in` / `$nin` / `$between`) are deliberately absent: an
* array is their DECLARED comparand. The text family (`$contains`,
* `$startsWith`, `$icontains`, `$like`, …) is absent too, and that absence is a
* decision this gate already recorded — "a stringified comparand for the LIKE
* family" is on its own "deliberately NOT refused" list, fail-closed and left
* alone. `driver-sql` refuses an array there as well (it cannot bind one); this
* driver keeps its recorded disposition rather than widening a refusal past the
* cell that was ruled.
*/
const SINGLE_VALUE_COMPARISON_OPERATORS: ReadonlySet<string> = new Set([
'$eq', '$ne', '$gt', '$gte', '$lt', '$lte',
]);

/**
* [#16810] An ARRAY where a comparison expects one comparable value —
* `{ tags: ['a','b'] }` and `{ tags: { $eq: ['a','b'] } }`.
*
* ## The cell, and who already answers it
*
* `@objectstack/spec`'s comparand door names this position and steps around it:
* an array outside `$in`/`$nin`/`$between` "is answered per driver today … the
* ruling does not name it, so the door leaves it to the layers that already
* answer it". `ACCEPTED_FILTER_COMPARAND_TYPES` — the six types the door DOES
* rule — has no array member, and `driver-sql` refuses one with
* `unbindableComparandError`. This is that same answer, in this package's
* envelope.
*
* ## Why refused rather than deep-equalled — the two answers, measured
*
* This package's faces did not agree, and the disagreement is the whole reason
* a refusal beats either silent answer. On one row `{ tags: ['a','b'] }`, with
* the filter `{ tags: ['a','b'] }`:
*
* | face | answer |
* |---|---|
* | the live query path (`InMemoryDriver.find` → mingo) | the row — mingo deep-equals arrays |
* | the reference matcher (`memory-matcher.ts`) | NO row |
*
* The matcher's arm routed an array into `value == condition`, and `==` between
* two objects compares REFERENCES: a deep-equal array is never that reference,
* so the predicate could not match any row it had not been handed literally.
* Fail-closed and silent — fewer rows, no error, no warning — which is the
* `#5240` / `#5328` / `#5347` shape exactly: one filter, one package, two
* answers, and `if (!rows.length)` cannot tell "genuinely none" from "the
* predicate never ran".
*
* Converging them UP (deep equality on both faces) was available and is not
* what landed: the spec's door does not rule the cell, and every sibling that
* has ruled it declines the semantics — `driver-sql` refuses it,
* `@objectstack/formula`'s record-at-a-time matcher answers `false` by an
* explicit "a bare array value is not a valid field spec" arm, and objectui's
* `ValueDataSource` was resolved the same way. A driver that calls itself a
* Reference Implementation inventing array-equality alone, on a cell the
* contract declined to rule, is how backends drift apart.
*
* ⚠️ The refusal is about the COMPARAND, never the stored value:
* `{ tags: 'a' }` against a row storing `['a','b']` is a scalar comparand and
* is untouched by this rule — the value side is not a position this door
* judges.
*/
export function arrayComparandError(field: string, value: unknown, path: string, op?: string): Error {
const position = op
? `Operator "${op}" on field "${field}"`
: `The implicit-equality comparand on field "${field}"`;
return unsupportedFilterError(
`${position} requires a single comparable value, but received an array ` +
`(${safeShapePreview(value)}) at ${path}. Use ${ACCEPTED_FILTER_COMPARAND_TYPES_SENTENCE}; ` +
`for a list use $in/$nin, and for a range use $between. It is refused rather than compared ` +
`because this package's two faces answered it differently and neither said so — the live ` +
`query path deep-equalled the array and returned the row, while the reference matcher ` +
`compared it by REFERENCE (== between two objects is a reference test) and returned none. ` +
`@objectstack/spec's comparand door leaves this position to the driver, and driver-sql ` +
`refuses it too.`,
);
}

/** [#5324] `$and`/`$or` take a list of nodes; anything else is refused. */
export function filterNodeListExpectedError(key: string, value: unknown, path: string): Error {
return unsupportedFilterError(
Expand Down Expand Up @@ -669,6 +752,11 @@ function assertFieldConstraintShape(
path: string,
capabilities: FilterFaceCapabilities,
): void {
// [#16810] The IMPLICIT-equality position, checked before the plain-object
// test below because an array is not a filter node and would otherwise leave
// this walk unjudged — which is how it reached the matcher's `==` arm and the
// live path's deep equality with nobody reconciling the two.
if (Array.isArray(spec)) throw arrayComparandError(field, spec, path);
if (!isFilterNode(spec)) return;
// [#5240] The zero-operator constraint keeps its own predicate rather than an
// inlined `keys.length === 0`, so the reasoning for what does and does not
Expand Down Expand Up @@ -702,6 +790,15 @@ function assertFieldConstraintShape(
if (op === '$null' && typeof spec[op] !== 'boolean') {
throw nonBooleanNullComparandError(field, spec[op], `${path}.$null`);
}
// [#16810] An ARRAY comparand on a single-value comparison — the operator
// spelling of the implicit-equality position refused at the top of this
// function, and the same cell `@objectstack/spec`'s comparand door leaves
// to the driver. The fourth comparand-shape rule this gate makes, for the
// reason the three above it give: a shape the operator cannot evaluate was
// being answered silently, and differently, by each face.
if (SINGLE_VALUE_COMPARISON_OPERATORS.has(op) && Array.isArray(spec[op])) {
throw arrayComparandError(field, spec[op], `${path}.${op}`, op);
}
// [#6520] `$icontains`' comparand is a NON-EMPTY string, the third
// comparand-shape rule and the twin of `driver-sql`'s
// `icontainsComparandError` — deliberately the same two rejections in one
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -568,7 +568,11 @@ const OPERATOR_CASES: Array<[name: string, where: FilterCondition, expected: str
// the operator table's inability to say "this operator takes the whole list".
['an empty $in selects nothing', { code: { $in: [] } } as FilterCondition, []],
['an empty $nin selects everything', { code: { $nin: [] } } as FilterCondition, ['1', '2', '3']],
['an empty implicit-equality list selects nothing', { code: [] } as unknown as FilterCondition, []],
// [#16810] `{ code: [] }` — the empty implicit-equality ARRAY — left this
// table when the cell was ruled. It is not a row-set case any more: an
// array in an implicit-equality position is REFUSED on every face, and the
// property this entry carried (the faces agree about it) is asserted below
// as an agreement about the REFUSAL instead of about the empty row set.
['a non-empty $in still selects its members', { code: { $in: ['100'] } } as FilterCondition, ['1', '3']],
];

Expand Down Expand Up @@ -652,6 +656,42 @@ describe('[#5374] operator semantics — the analytics face against the live que
});
}

/**
* [#16810] An ARRAY comparand in an implicit-equality position — refused, and
* refused by BOTH faces with one envelope.
*
* This replaces the row-set case `{ code: [] }` held in `OPERATOR_CASES`
* above. The empty array was the harmless-looking member of the cell: its
* two silent answers happened to coincide at "no rows", which is why nobody
* noticed that a NON-empty one did not. Measured before the refusal landed,
* on a row storing `['a','b']` with the filter `{ tags: ['a','b'] }`: the
* live query path returned the row (mingo deep-equals arrays) and the
* reference matcher returned nothing (`==` between two objects is a
* reference test). One filter, one package, two answers — the #5240 shape.
*
* The assertion is the AGREEMENT, which is what the retired entry was for.
*/
it('an implicit-equality ARRAY comparand is refused, and both faces refuse it alike', async () => {
const where = { code: [] } as unknown as FilterCondition;
const nonEmpty = { code: ['100'] } as unknown as FilterCondition;

for (const [label, filter] of [['empty', where], ['non-empty', nonEmpty]] as const) {
await expect(findIds(filter), `${label}: the live query path did not refuse it`).rejects.toMatchObject({
code: 'INVALID_FILTER',
status: 400,
});
await expect(analyticsIds(filter), `${label}: the analytics face did not refuse it`).rejects.toMatchObject({
code: 'INVALID_FILTER',
status: 400,
});
}

// The list operators are the declared spelling and are untouched — the
// refusal must not read as "this driver stopped accepting arrays".
expect(await findIds({ code: { $in: [] } } as FilterCondition)).toEqual([]);
expect(await findIds({ code: { $nin: [] } } as FilterCondition)).toEqual(['1', '2', '3']);
});

/**
* `contains` and `notContains` partition the table, stated as a predicate over
* every comparand rather than case by case. An inert `$notContains` fails this
Expand Down
Loading
Loading