Skip to content

Commit bc8b5d6

Browse files
committed
fix(driver-memory): read a stored ARRAY as its elements in the equality arm, so both filter faces answer one filter one way
`memory-matcher.ts`'s equality arm ended in `value == condition`. Loose `==` converts a stored ARRAY to a primitive — `['a','b']` becomes the string `"a,b"` — so the reference matcher and the live query path (`InMemoryDriver .find`, through mingo) disagreed about the same filter in BOTH directions: | filter | stored | matcher, before | live path | |-------------------|-------------|-----------------|-----------| | `{ tags: 'a' }` | `['a','b']` | no row | the row | | `{ tags: 'a,b' }` | `['a','b']` | the row | no row | | `{ tags: 'a' }` | `['a']` | the row | the row | The second row is the sharper one: a false positive, a filter written to narrow returning a row it should not, which on a read scope is a permission concern rather than a degraded filter. The first is fail-open the other way and just as silent. A stored array is now read as its ELEMENTS, and each is asked the question the arm asks of a scalar — so an array answers the OR of the answers its elements would give. That is mingo's composition, which is this file's standing tie-break: the reference face converges on the path users actually run instead of inventing a third reading. One level only, measured: mingo does not descend into a nested array, so neither does this face. Refusing the shape was not available — a refusal is raised from the FILTER before any row is seen, and this cell is a property of the stored ROW. `comparandEquals` becomes the entry every arm calls; the previous body is `singleValueEquals`, unchanged, deciding one value against one comparand. The live query path is untouched. Tests: `memory-matcher-scalar-comparand-array-value.test.ts` drives BOTH faces in one process over one fixture — the card's three rows, its firing control and its negative twin, `$eq`/`$ne`, a null comparand against a null member, and the OR-over-elements property over the whole matrix. #16810's pin block, which recorded this behaviour as unchanged so its own refusal could not move it by accident, is rewritten rather than deleted: the three answers move with the value side's ruling, and the invariant the block exists for — the comparand refusal must not reach the value side — is now asserted directly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XTBcV7zZHmokdyQgXjbyEU
1 parent 9c52b1b commit bc8b5d6

4 files changed

Lines changed: 387 additions & 13 deletions

File tree

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
---
2+
"@objectstack/driver-memory": minor
3+
---
4+
5+
fix(driver-memory): a scalar comparand against a stored ARRAY is read as membership on both filter faces, so a filter written to narrow stops returning rows it never selected (#16838)
6+
7+
`memory-matcher.ts`'s equality arm ended in `value == condition`. Loose `==` converts a stored ARRAY to a primitive — `['a','b']` becomes the string `"a,b"` — so this package's reference matcher and its live query path (`InMemoryDriver.find`, through mingo) answered the same filter two different ways, in both directions at once:
8+
9+
| filter | stored value | reference matcher, before | live query path |
10+
|---|---|---|---|
11+
| `{ tags: 'a' }` | `['a','b']` | no row | the row |
12+
| `{ tags: 'a,b' }` | `['a','b']` | the row | no row |
13+
| `{ tags: 'a' }` | `['a']` | the row | the row |
14+
15+
The second row is the sharper one: a **false positive**, a filter written to narrow returning a row it should not, which on a read scope is a permission concern rather than a degraded filter. The first is fail-open in the other direction and just as silent — `if (!rows.length)` cannot tell "genuinely none" from "the predicate asked the wrong question".
16+
17+
**What changes.** A stored array is now read as its elements, and each is asked the question the arm asks of a scalar: the answer for a row storing an array is the OR of the answers for the rows storing its elements. That is MongoDB's array semantics and therefore mingo's, so the reference face converges on the path this package's users actually run rather than on a third reading nobody wrote. One level only — a nested array is not descended into, matching mingo. `$eq` and `$ne` take the same equality as the implicit spelling, so `$ne` stays the exact complement.
18+
19+
**What does not change.** An array in the **comparand** position is still refused (`INVALID_FILTER` / 400) by the shape gate every face of this package runs; this is the VALUE side, which that door does not judge. The live query path is untouched — it already answered membership — so a caller who only ever used `find()` sees no difference. Callers who compared results against the reference matcher, or who ran it directly as a driver double, will see a stored array select on membership instead of on its joined string.

packages/drivers/driver-memory/src/memory-matcher-array-and-date-comparand.test.ts

Lines changed: 50 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,24 @@
2828
* analytics face answer it identically.
2929
*
3030
* ⚠️ The third behaviour, pinned here so a later edit cannot take it away by
31-
* accident: a SCALAR comparand against a stored ARRAY is untouched. `==`
32-
* stringifies the stored array (`['a','b']` becomes `"a,b"`), which is a third
31+
* accident: a SCALAR comparand against a stored ARRAY was untouched. `==`
32+
* stringified the stored array (`['a','b']` becomes `"a,b"`), which is a third
3333
* bad direction of the same operator — but it is on the VALUE side, and the
34-
* comparand door judges comparands. It is recorded, not repaired, and the
35-
* numbers below are the record.
34+
* comparand door judges comparands. It was recorded, not repaired, and the
35+
* numbers below were the record.
36+
*
37+
* [#16838] **That third behaviour has since been repaired, and this file's last
38+
* block moves with it — deliberately, not by accident.** The pin did its job:
39+
* it stated in one place what the VALUE side answered, so the change that moved
40+
* it had to come and say so here rather than sliding through as a side effect
41+
* of the refusal above. The cell was measured on its own card, on both faces —
42+
* the live query path read `['a','b']` as MEMBERSHIP where this face read the
43+
* joined string `"a,b"` — and the reference face converged on the live one, so
44+
* the numbers below are now the AGREEMENT rather than the record of a
45+
* divergence. ⛔ The block is rewritten, never deleted: what it exists to catch
46+
* — this refusal reaching the value side by accident — is still live, and the
47+
* assertion that the two sides stay distinct is the same assertion whichever
48+
* answer the value side gives.
3649
*/
3750

3851
import { describe, it, expect } from 'vitest';
@@ -146,13 +159,38 @@ describe('[#16810] an ARRAY comparand is refused, in the ADR-0112 envelope', ()
146159
});
147160
});
148161

149-
describe('[#16810] the value side is NOT the comparand side — recorded, not repaired', () => {
150-
it('a scalar comparand against a stored array keeps the answers it had', () => {
151-
// `==` stringifies the stored array. All three lines are the behaviour
152-
// BEFORE this change as well; they are pinned so the refusal above cannot
153-
// silently take the third direction of `==` with it.
154-
expect(match({ tags: ['a', 'b'] }, { tags: 'a' })).toBe(false);
155-
expect(match({ tags: ['a', 'b'] }, { tags: 'a,b' })).toBe(true); // ⚠️ the coercion, still here
156-
expect(match({ tags: ['a'] }, { tags: 'a' })).toBe(true); // ⚠️ and its single-element form
162+
describe('[#16810/#16838] the value side is NOT the comparand side — still two cells, both now answered', () => {
163+
it('a scalar comparand against a stored array is MEMBERSHIP, and is not refused', () => {
164+
// [#16838] The three lines this block pinned as UNCHANGED under #16810,
165+
// with the two that #16838 moved and the one it did not:
166+
//
167+
// before → after
168+
// `{tags:'a'}` vs `['a','b']` false → true the missing membership reading
169+
// `{tags:'a,b'}` vs `['a','b']` true → false the false positive, the sharper half
170+
// `{tags:'a'}` vs `['a']` true → true the firing control, unmoved
171+
//
172+
// They are still asserted here, and still for #16810's reason: this file's
173+
// refusal is about the COMPARAND, and an edit that let it reach the VALUE
174+
// side would turn the first two lines into a throw. Their VALUES track the
175+
// value side's own ruling; the shape of the assertion — an answer, not an
176+
// exception — is what #16810 pinned and it is unchanged.
177+
expect(match({ tags: ['a', 'b'] }, { tags: 'a' })).toBe(true);
178+
expect(match({ tags: ['a', 'b'] }, { tags: 'a,b' })).toBe(false);
179+
expect(match({ tags: ['a'] }, { tags: 'a' })).toBe(true);
180+
});
181+
182+
it('the ARRAY-comparand refusal did not follow the value side — a stored array is still evaluated', () => {
183+
// The invariant this block was created to hold, stated directly rather than
184+
// left to be inferred from the three answers above: the door refuses an
185+
// array in the COMPARAND position and says nothing about a stored one, so a
186+
// scalar comparand against any stored array must ANSWER.
187+
for (const stored of [['a', 'b'], ['a'], [] as unknown[], [null, 'b'], [['a']]]) {
188+
expect(() => match({ tags: stored }, { tags: 'a' }), `stored ${JSON.stringify(stored)} was refused`)
189+
.not.toThrow();
190+
}
191+
// …while the comparand position still refuses, on the same row.
192+
expect(() => match({ tags: ['a', 'b'] }, { tags: ['a', 'b'] })).toThrow(
193+
/requires a single comparable value/,
194+
);
157195
});
158196
});
Lines changed: 249 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,249 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#16838] A SCALAR comparand against a stored ARRAY value — the VALUE side of
5+
* the equality arm, and the third bad direction of `==` that #16810 recorded
6+
* and deliberately did not repair.
7+
*
8+
* # What was measured, and why it is one defect and not two
9+
*
10+
* `checkCondition`'s equality arm ended in `value == condition`. Loose `==`
11+
* converts the stored ARRAY to a primitive, so `['a','b']` becomes the string
12+
* `"a,b"` — and that single conversion produced a disagreement between this
13+
* package's two filter faces in BOTH directions at once:
14+
*
15+
* | filter | stored | reference matcher, BEFORE | live `InMemoryDriver.find` (mingo) |
16+
* |---|---|---|---|
17+
* | `{ tags: 'a' }` | `['a','b']` | `false` — no row | the row |
18+
* | `{ tags: 'a,b' }` | `['a','b']` | `true` — the row | no row |
19+
* | `{ tags: 'a' }` | `['a']` | `true` — the row | the row |
20+
*
21+
* The second row is the sharper one: a FALSE POSITIVE, a filter written to
22+
* narrow returning a row it should not, which on an RLS read scope is a
23+
* permission concern rather than a degraded filter (#3948, and the identical
24+
* notes `memory-matcher.ts` already carries for `$null`, for the malformed
25+
* `$between` shape and for an unknown operator). The third row is the firing
26+
* control: it answers the same on both faces before and after, so a suite that
27+
* went green by never running would not look like a pass.
28+
*
29+
* # Which face was chosen, and why it was not a free choice
30+
*
31+
* The live path's membership reading is MongoDB's array semantics; the
32+
* matcher's string-join reading is an accident of the operator it happens to be
33+
* written with. This file's tie-break is the one `memory-matcher.ts` has used
34+
* since #5240, #5324, #5328 and #5374 — the live mingo path is what this
35+
* package's users actually run, so the reference face converges on it, cell for
36+
* cell. Refusing the shape was the third answer available and is not open here:
37+
* a refusal is raised from the FILTER (`assertFilterConditionShape` walks the
38+
* filter, once, before any row is seen) and this cell is a property of the
39+
* stored ROW, so a refusal would have to fire or not fire depending on the data
40+
* — the very record-dependence #5240 moved the shape walk out of the field loop
41+
* to avoid.
42+
*
43+
* # The rule, stated so it can be checked rather than described
44+
*
45+
* A stored array is read as its elements, and the arm asks each of them the
46+
* question it asks a scalar. That is asserted directly, as a property over the
47+
* whole matrix below: for every case, the answer for a row storing an array
48+
* equals the OR of the answers for the rows storing its elements. It is the
49+
* same composition mingo performs, which is why the two faces agree here by
50+
* construction rather than by coincidence.
51+
*
52+
* ⚠️ One level only, measured rather than reasoned: mingo does not descend into
53+
* a NESTED array, so neither does this face — `[['a']]` does not match `'a'` on
54+
* either face, and that row is in the fixture to hold it.
55+
*/
56+
57+
import { describe, it, expect, beforeAll } from 'vitest';
58+
59+
import { InMemoryDriver } from './memory-driver.js';
60+
import { match } from './memory-matcher.js';
61+
62+
const TABLE = 'array_value_equality';
63+
64+
/**
65+
* One fixture, both faces, one process. The two scalar rows are the card's
66+
* firing control — a comparand that legitimately matches and its negative twin
67+
* — and they are asserted in every case below, so "the filter never ran" and
68+
* "the filter correctly excluded everything" cannot read alike.
69+
*/
70+
const ROWS: ReadonlyArray<Record<string, unknown>> = [
71+
{ id: 'scalar-hit', tags: 'a' },
72+
{ id: 'scalar-miss', tags: 'z' },
73+
{ id: 'array-multi', tags: ['a', 'b'] },
74+
{ id: 'array-single', tags: ['a'] },
75+
{ id: 'array-other', tags: ['b'] },
76+
{ id: 'array-nested', tags: [['a']] },
77+
{ id: 'array-with-null', tags: [null, 'b'] },
78+
{ id: 'array-empty', tags: [] },
79+
];
80+
81+
/**
82+
* Every case names the row set BOTH faces must answer. The expectations are the
83+
* live path's measured answers — see the header for why that is the tie-break.
84+
*/
85+
const CASES: ReadonlyArray<{
86+
name: string;
87+
where: Record<string, unknown>;
88+
expected: string[];
89+
/**
90+
* Whether the case asks the equality question or its NEGATION. The OR-over-
91+
* elements property below is a statement about the equality predicate; `$ne`
92+
* is that predicate's complement, so on an array it means "NO element equals"
93+
* — the AND, not the OR. Marking the polarity states which of the two is
94+
* being asserted instead of leaving a reader to infer it from an operator.
95+
*/
96+
polarity: 'equality' | 'negated';
97+
}> = [
98+
{
99+
name: "{ tags: 'a' } — a scalar comparand is MEMBERSHIP against a stored array",
100+
where: { tags: 'a' },
101+
expected: ['array-multi', 'array-single', 'scalar-hit'],
102+
polarity: 'equality',
103+
},
104+
{
105+
name: "{ tags: 'a,b' } — the JOINED string matches nothing; the false positive is gone",
106+
where: { tags: 'a,b' },
107+
expected: [],
108+
polarity: 'equality',
109+
},
110+
{
111+
name: "{ tags: 'z' } — the firing control's negative twin",
112+
where: { tags: 'z' },
113+
expected: ['scalar-miss'],
114+
polarity: 'equality',
115+
},
116+
{
117+
name: "{ tags: { $eq: 'a' } } — the operator spelling answers as the implicit one",
118+
where: { tags: { $eq: 'a' } },
119+
expected: ['array-multi', 'array-single', 'scalar-hit'],
120+
polarity: 'equality',
121+
},
122+
{
123+
name: "{ tags: { $ne: 'a' } } — and its complement is the exact complement",
124+
where: { tags: { $ne: 'a' } },
125+
expected: ['array-empty', 'array-nested', 'array-other', 'array-with-null', 'scalar-miss'],
126+
polarity: 'negated',
127+
},
128+
{
129+
name: '{ tags: null } — a null comparand finds a null MEMBER, and only that',
130+
where: { tags: null },
131+
expected: ['array-with-null'],
132+
polarity: 'equality',
133+
},
134+
{
135+
name: "{ tags: 'b' } — the member that is not first, so position cannot be what matches",
136+
where: { tags: 'b' },
137+
expected: ['array-multi', 'array-other', 'array-with-null'],
138+
polarity: 'equality',
139+
},
140+
];
141+
142+
const sorted = (ids: readonly string[]): string[] => [...ids].sort((x, y) => x.localeCompare(y));
143+
144+
/** The reference face: `memory-matcher.ts`, one record at a time. */
145+
const referenceIds = (where: Record<string, unknown>): string[] =>
146+
sorted(ROWS.filter((r) => match(r, where)).map((r) => String(r.id)));
147+
148+
describe('[#16838] a scalar comparand against a stored array — both faces, one process', () => {
149+
let driver: InMemoryDriver;
150+
/** The live face: `InMemoryDriver.find`, through `normalizeFilterCondition` and mingo. */
151+
let liveIds: (where: Record<string, unknown>) => Promise<string[]>;
152+
153+
beforeAll(async () => {
154+
driver = new InMemoryDriver({ persistence: false });
155+
await driver.connect();
156+
await driver.syncSchema(TABLE, {
157+
fields: {
158+
id: { type: 'text', name: 'id' },
159+
tags: { type: 'text', name: 'tags' },
160+
},
161+
} as never);
162+
for (const row of ROWS) await driver.create(TABLE, { ...row });
163+
164+
liveIds = async (where) => {
165+
const rows = (await driver.find(TABLE, { fields: ['id'], where } as never)) as Array<Record<string, unknown>>;
166+
return sorted(rows.map((r) => String(r.id)));
167+
};
168+
});
169+
170+
it('the fixture really is all eight rows, arrays included', async () => {
171+
// A case that returns nothing because the seed failed must not read as a
172+
// case that correctly excluded everything.
173+
expect(await liveIds({})).toEqual(sorted(ROWS.map((r) => String(r.id))));
174+
const stored = (await driver.find(TABLE, {} as never)) as Array<Record<string, unknown>>;
175+
expect(stored.find((r) => r.id === 'array-multi')?.tags).toEqual(['a', 'b']);
176+
});
177+
178+
for (const c of CASES) {
179+
it(`${c.name} — the LIVE query path`, async () => {
180+
expect(await liveIds(c.where)).toEqual(sorted(c.expected));
181+
});
182+
183+
it(`${c.name} — the REFERENCE matcher`, () => {
184+
expect(referenceIds(c.where)).toEqual(sorted(c.expected));
185+
});
186+
}
187+
188+
it('both faces answer the whole matrix identically', async () => {
189+
for (const c of CASES) {
190+
expect(await liveIds(c.where), `${c.name}: the live query path and the reference matcher disagree`)
191+
.toEqual(referenceIds(c.where));
192+
}
193+
});
194+
195+
/**
196+
* The card's three rows, spelled exactly as it measured them — `match()`
197+
* directly, one row, one filter — so the numbers in the card and the numbers
198+
* here can be compared without reading the fixture above.
199+
*/
200+
it("the card's own three rows, on the reference matcher", () => {
201+
expect(match({ tags: ['a', 'b'] }, { tags: 'a' })).toBe(true); // was false — the missing membership
202+
expect(match({ tags: ['a', 'b'] }, { tags: 'a,b' })).toBe(false); // was true — the false positive
203+
expect(match({ tags: ['a'] }, { tags: 'a' })).toBe(true); // the firing control, unmoved
204+
});
205+
206+
/**
207+
* The rule the arm implements, asserted as a property rather than described:
208+
* an array answers what the OR of its elements answers. A future edit that
209+
* reintroduces any whole-array conversion breaks this for every case at once,
210+
* not only for the two the card happened to measure.
211+
*/
212+
it('a stored array answers the OR of the answers its ELEMENTS would give', () => {
213+
for (const c of CASES) {
214+
if (c.polarity !== 'equality') continue;
215+
for (const row of ROWS) {
216+
const stored = row.tags;
217+
if (!Array.isArray(stored)) continue;
218+
// One level only: an element that is itself an array is not descended
219+
// into, on either face.
220+
const elementwise = stored.some((element) => !Array.isArray(element) && match({ tags: element }, c.where));
221+
expect(match(row, c.where), `${c.name} / ${String(row.id)}: not the OR over its elements`)
222+
.toBe(elementwise);
223+
}
224+
}
225+
});
226+
227+
/**
228+
* `$ne` is the equality predicate's exact complement, per row — which on an
229+
* array is "NO element equals", the AND rather than the OR. Stated because
230+
* the two spellings share {@link comparandEquals} and a future edit that
231+
* fixed one direction only would leave a stored array both matching and not
232+
* matching the same comparand.
233+
*/
234+
it('$ne is the per-row complement of $eq, arrays included', () => {
235+
for (const comparand of ['a', 'b', 'z', 'a,b', null]) {
236+
for (const row of ROWS) {
237+
expect(
238+
match(row, { tags: { $ne: comparand } }),
239+
`${String(row.id)} / ${JSON.stringify(comparand)}: $ne is not the complement of $eq`,
240+
).toBe(!match(row, { tags: { $eq: comparand } }));
241+
}
242+
}
243+
});
244+
245+
it('a NESTED array is not descended into — one level, on both faces', async () => {
246+
expect(match({ tags: [['a']] }, { tags: 'a' })).toBe(false);
247+
expect(await liveIds({ tags: 'a' })).not.toContain('array-nested');
248+
});
249+
});

0 commit comments

Comments
 (0)