Skip to content

Commit 69fde55

Browse files
huangyiireneclaude
andauthored
fix(driver-memory): make the $contains family case-exact and answer count_distinct (#7723)
* fix(driver-memory): make the $contains family case-exact and answer count_distinct Clears both remaining driver-memory DEBT rows in scripts/check-driver-conformance.mjs, with the suites that replace them. #6682 — the `$contains` family folded case on nine sites: the AST spelling's four arms (`convertConditionToMongo`), the `$`-spelling's four (`normalizeFieldOperators`) and `filterSubstringPattern`, the #5374 shared rule the analytics face borrows rather than re-derives. The `i` flag folds the WHOLE Unicode range, wider even than the ASCII boundary `$icontains` is held to, so the family returned rows the filter excludes — over-reach on an RLS read scope (#3948). This driver's reference matcher was case-exact all along, so the two folding faces move onto the answer the third already gave (#4706 Q2 = A, #5374). `escapeRegex` is untouched; `$icontains` keeps its ASCII fold in the pattern source. #6814 — `MemoryDriver.computeAggregate` had no `count_distinct` arm, so a function the Query Protocol declares fell to `default: return null` and `aggregate()` resolved with `{ n: null }`. It now counts distinct NON-NULL values. Executing the case-set also corrected the card's reading of the analytics face: `buildAggregator` emitted `{ $addToSet }` and nothing sized it, so the measure answered the raw ARRAY under a field its own metadata types as `number`. Fixed beside it, null-excluded. Both cells are enrolled with real in-process executions of the shared case-sets (`memory-filter-text-conformance.test.ts`, `memory-aggregation-conformance.test.ts`), driving every face of the package. Pins that encoded the fold are flipped to the ruled substance rather than deleted. Gate: 40 covered cells, 0 DEBT, 0 exempt — the ledger is empty for the first time. Fixes #6814 Fixes #6682 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NuzQE844ut8m8vypXcdBYD * test(driver-memory): type the aggregation conformance query instead of erasing it The query-options-erasure ratchet (#4918) went red: the new aggregation suite added 3 counted sites (242 -> 245) by casting `queryFor(c) as any` at each of its three call sites — the two doors of the data face plus the never-answers- null property row. Typed rather than exempted. `queryFor` now declares `DriverQuery` as its return type (import-reachable from `@objectstack/spec/contracts`, the same type `MemoryDriver.find` takes), so all three arguments are checked by `tsc` and no call site needs a cast. The `as unknown as` spelling would have been wrong here: every case in this file is deliberately ON contract — the whole point is that the standard's own vocabulary reaches the driver — so there is no bypassed contract to name. The baseline is NOT raised: the count returns to the 242 ceiling. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NuzQE844ut8m8vypXcdBYD --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent c5ed36c commit 69fde55

12 files changed

Lines changed: 843 additions & 152 deletions
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
---
2+
'@objectstack/driver-memory': patch
3+
---
4+
5+
driver-memory: the `$contains` family is case-SENSITIVE, and `count_distinct` answers a number
6+
7+
Two user-visible answers change on the in-memory driver. Both bring it onto the
8+
answer the SQL family, MongoDB and the protocol already give, so a filter or an
9+
aggregate now means the same thing whether your tests run on this double or your
10+
production runs a real database.
11+
12+
**`$contains` / `$notContains` / `$startsWith` / `$endsWith` no longer fold
13+
case.** They matched with a case-insensitive regex on the query path and on the
14+
analytics face — over the whole Unicode range, wider even than the ASCII
15+
boundary `$icontains` is held to — so `{ name: { $contains: 'acme' } }` returned
16+
`ACME Corp` here and did not on any other backend. This driver's reference
17+
matcher (`match()`) was already case-exact, so the two folding faces have moved
18+
onto the answer the third one always gave. The comparand stays literal: `%`,
19+
`_` and `.` were never wildcards here and still are not.
20+
21+
**This is a ROW-SET change.** If you relied on the fold, write `$icontains`
22+
the operator that spells it, implemented on every backend since #6520 and
23+
folding ASCII case only.
24+
25+
**`count_distinct` answers.** `MemoryDriver.computeAggregate` had no arm for it,
26+
so an aggregation the Query Protocol declares resolved with `{ alias: null }`
27+
no error, no log, no refusal. It now counts distinct NON-NULL values, matching
28+
`COUNT(DISTINCT col)`. The analytics face was wrong in its own way and is fixed
29+
beside it: it collected the distinct values and never sized them, so a
30+
`count_distinct` measure came back as the raw array of values under a field its
31+
own response metadata types as `number`.
32+
33+
Both are held to `@objectstack/spec/data`'s shared case-sets from now on
34+
(`FILTER_TEXT_CASES`, `AGGREGATION_CASES`), executed in process against every
35+
face of the package.

content/docs/protocol/objectql/query-syntax.mdx

Lines changed: 19 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -337,15 +337,16 @@ metacharacters — `{ name: { $icontains: 'a.b' } }` matches `a.b` and not `axb`
337337
compilers). A filter using it means the same thing whether your tests run on the
338338
in-memory double or your production runs SQL.
339339

340-
One half of the case rules above is still landing: `$contains` /
340+
The other half of the case rules above has landed too: `$contains` /
341341
`$startsWith` / `$endsWith` / `$notContains` are case-**sensitive** by ruling and
342-
are so on the SQL family and on MongoDB
342+
are so on every backend
343343
[#6682](https://github.com/objectstack-ai/objectstack/issues/6682) removed the
344-
hardcoded `$options: 'i'` that had folded them there. The in-memory driver's
345-
query and analytics faces still fold over the whole Unicode range. Until that
346-
half lands, prefer `$icontains` when you *want* a fold rather than relying on
347-
`$contains` being loose on that backend. The shared standard both halves are
348-
measured against is `FILTER_TEXT_CASES` (`@objectstack/spec/data`).
344+
hardcoded `$options: 'i'` that folded them on MongoDB and the case-insensitive
345+
regex that folded them on the in-memory driver's query and analytics faces.
346+
**If you were relying on `$contains` being loose on the in-memory driver, that
347+
is a row-set change: write `$icontains` when you want a fold.** The shared
348+
standard both halves are measured against is `FILTER_TEXT_CASES`
349+
(`@objectstack/spec/data`), which all five drivers now run.
349350
</Callout>
350351

351352
### `$regex` — removed
@@ -963,19 +964,21 @@ rule in [Case Sensitivity](#case-sensitivity) above. Note what that means for se
963964
a user typing `acme` does not find `ACME Corp`. Only `select` / `status` option
964965
*labels* are matched case-insensitively by the expansion itself.
965966

966-
<Callout type="warn">
967-
**Measured today: one driver still does not match that rule.** The `$contains`
968-
alignment landed in two steps —
967+
<Callout type="info">
968+
**Measured today: every driver matches that rule.** The `$contains` alignment
969+
landed in three steps —
969970
[#6518](https://github.com/objectstack-ai/objectstack/issues/6518) made `SqlDriver`
970971
case-exact per dialect (`GLOB` on the SQLite dialects, `LIKE` unchanged on
971972
Postgres, `LIKE` over a binary cast on MySQL), and
972973
[#6682](https://github.com/objectstack-ai/objectstack/issues/6682) removed
973-
`driver-mongodb`'s hardcoded `$options: 'i'`. `driver-memory`'s query path still
974-
matches with a case-insensitive regex, so running your tests on the in-memory
975-
double can still return rows a SQL or MongoDB deployment would not. Whether the
976-
expansion should emit `$icontains` instead of `$contains` — i.e. whether search is
977-
case-insensitive by definition — is a separate question that rides with that issue,
978-
because it can only be answered once both operators mean one thing everywhere.
974+
`driver-mongodb`'s hardcoded `$options: 'i'` and then the case-insensitive regex
975+
`driver-memory` used on its query and analytics faces. So running your tests on the
976+
in-memory double no longer returns rows a SQL or MongoDB deployment would not —
977+
the divergence this callout warned about is closed, and
978+
`FILTER_TEXT_CASES` holds all five drivers to it. Whether the expansion should emit
979+
`$icontains` instead of `$contains` — i.e. whether search is case-insensitive by
980+
definition — remains a separate open question, and one that can now actually be
981+
answered, since both operators mean one thing everywhere.
979982
</Callout>
980983
`fuzzy`, `boost`, `operator`, `minScore`, `language`, and `highlight` carry
981984
`[EXPERIMENTAL — not enforced]` markers (#4286): the schema accepts them, the
Lines changed: 261 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,261 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#6814] Aggregate-vocabulary conformance for `driver-memory` — the shared
5+
* `@objectstack/spec/data` cases, on rows, executed in process.
6+
*
7+
* The SQL twins (`sql-driver-aggregation-conformance.test.ts`,
8+
* `turso-remote-aggregation-conformance.test.ts`,
9+
* `sqlite-wasm-aggregation-conformance.test.ts`) run this same table against a
10+
* real database. This file is the reason the cases live in the spec package
11+
* rather than beside one driver: an aggregate that answers differently here
12+
* than under SQL pushdown is one query with two numbers, decided by a driver
13+
* capability bit the caller never sees.
14+
*
15+
* ## Why nothing here is a "modelled" evaluation
16+
*
17+
* This driver runs in process, so every case below is a REAL execution — no
18+
* server-free half in the shape `driver-mongodb` needs (#5517), and no emitted-
19+
* string assertion standing in for a number. That matters for the defect this
20+
* file was written against: `computeAggregate` had no `count_distinct` arm at
21+
* all, so the function fell to `default: return null` and `aggregate()` resolved
22+
* with `{ n: null }` — no error, no log, no refusal. Only executing the case
23+
* says so; a lowering-shape assertion has nothing to look at.
24+
*
25+
* ## Both doors of the data face, and the analytics face beside it
26+
*
27+
* `find()` and `aggregate(AST)` are two entries to the same
28+
* `performAggregation`, and objectql's engine uses the second one. Both are
29+
* driven, because "the aggregate works" measured through one door is what let
30+
* this package answer one declared function two ways for as long as it did
31+
* (#5374). The analytics face (`memory-analytics.ts`) is driven in the last
32+
* block for the same reason — it implements `count_distinct` independently, so
33+
* it is a third answer unless something demands they agree.
34+
*
35+
* ## Reverse verification — direction predicted BEFORE it was run
36+
*
37+
* **(A) the `count_distinct` arm removed** (the pre-#6814 state). Predicted:
38+
* the three `count_distinct` cases fail on `null` — the value, not a throw —
39+
* while every arithmetic case stays green, because the missing arm is a silent
40+
* fall-through rather than a broken computation.
41+
*
42+
* **(B) the arm present but written `new Set(values).size`** — null NOT
43+
* excluded, the mistake `driver-mongodb`'s `$addToSet` made (#6814's other
44+
* half). Predicted: `count_distinct(stage)` answers 3 instead of 2 and the
45+
* grouped case answers `west` 3 / `east` 2 instead of 2 / 1, while
46+
* `count_distinct(score)` stays GREEN at 6 — that column has no nulls, so it
47+
* cannot see the mistake. (B) is the direction this file exists for.
48+
*
49+
* Measured after writing the above, of 34:
50+
*
51+
* - **(A) 7 failed / 27 passed.** Every failure was on the VALUE `null`
52+
* (`expected [{ group: null, value: null }] to deeply equal
53+
* [{ group: null, value: 2 }]`), through BOTH doors, plus the
54+
* never-answers-null row — not one on a throw, as predicted. Every
55+
* arithmetic case stayed green. The analytics block stayed green too, which
56+
* is the point of driving the faces separately: this revert is one face's
57+
* defect and the file says which one.
58+
* - **(B) 6 failed / 28 passed**, on `expected 3 to be 2` ungrouped and
59+
* `east` 2 / `west` 3 grouped, through both doors, plus the two analytics
60+
* rows over the same column. `count_distinct(score)` stayed green at 6
61+
* throughout, exactly as predicted — which is why the table carries both
62+
* columns, and why (B) is unreachable by a suite that only tests one.
63+
*
64+
* Pre-fix, on unmodified `origin/main` @ `21888ab`: **11 failed / 23 passed** —
65+
* (A)'s seven plus four more the analytics face contributed on its own account
66+
* (see the last block).
67+
*/
68+
69+
import { describe, it, expect, beforeEach } from 'vitest';
70+
import { AGGREGATION_CASES, AGGREGATION_ROWS } from '@objectstack/spec/data';
71+
import type { AggregationCase, Cube } from '@objectstack/spec/data';
72+
import type { DriverQuery } from '@objectstack/spec/contracts';
73+
import { InMemoryDriver } from './memory-driver.js';
74+
import { MemoryAnalyticsService } from './memory-analytics.js';
75+
76+
const TABLE = 'conformance_agg';
77+
78+
/**
79+
* The case as the `DriverQuery` shape both doors consume.
80+
*
81+
* [#4918] The return type is DECLARED rather than left to inference and erased
82+
* at each call site. Every case here is deliberately ON contract — the whole
83+
* point of the file is that the standard's own vocabulary reaches the driver —
84+
* so there is nothing for an `as any` to bypass, and typing it puts the two
85+
* doors' argument under `tsc` instead of exempting it.
86+
*/
87+
const queryFor = (c: AggregationCase): DriverQuery => ({
88+
aggregations: [{ function: c.function, ...(c.field ? { field: c.field } : {}), alias: 'n' }],
89+
// [#6401] A case carrying `groupByAlias` is sent as the STRUCTURED node, so
90+
// the face receives the union member that declares `alias`. Without this the
91+
// alias axis would send a bare string and pin nothing.
92+
...(c.groupBy
93+
? { groupBy: [c.groupByAlias ? { field: c.groupBy, alias: c.groupByAlias } : c.groupBy] }
94+
: {}),
95+
});
96+
97+
/**
98+
* The rows a case must produce, in the table's own order: `group` ascending for
99+
* a grouped case, one `null`-grouped row otherwise.
100+
*
101+
* [#6401] The group value is read from the column the case SAYS it lands in —
102+
* `groupByAlias ?? groupBy`. Reading `c.groupBy` unconditionally is the mistake
103+
* this axis exists to catch: green on a face that ignores the alias.
104+
*
105+
* `value` is deliberately NOT coerced with `Number()`. The defect this file was
106+
* written against answers `null`, and `Number(null)` is `0` — a coercion here
107+
* would turn "no arm at all" into an ordinary off-by-one and hide the shape of
108+
* the failure.
109+
*/
110+
const actualFor = (c: AggregationCase, rows: Array<Record<string, unknown>>) => {
111+
const groupKey = c.groupByAlias ?? c.groupBy;
112+
return rows
113+
.map((r) => ({ group: groupKey ? String(r[groupKey]) : null, value: r.n }))
114+
.sort((x, y) => String(x.group).localeCompare(String(y.group)));
115+
};
116+
117+
const expectedFor = (c: AggregationCase) =>
118+
[...c.expected]
119+
.map((e) => ({ group: e.group, value: e.value }))
120+
.sort((x, y) => String(x.group).localeCompare(String(y.group)));
121+
122+
async function seed(): Promise<InMemoryDriver> {
123+
const driver = new InMemoryDriver();
124+
for (const row of AGGREGATION_ROWS) await driver.create(TABLE, { ...row });
125+
return driver;
126+
}
127+
128+
describe('[#6814] InMemoryDriver — aggregate vocabulary conformance', () => {
129+
let driver: InMemoryDriver;
130+
beforeEach(async () => { driver = await seed(); });
131+
132+
/**
133+
* The fixture first, read back rather than trusted — a case that answers 2
134+
* because only two rows landed is not a case that deduplicated correctly, and
135+
* the null-bearing column is the one a seed is most likely to mangle.
136+
*/
137+
it('the fixture is all six rows, with the nulls stored AS nulls', async () => {
138+
const rows = await driver.find(TABLE, { orderBy: [{ field: 'id', order: 'asc' }] });
139+
expect(rows.map((r: any) => String(r.id))).toEqual(['1', '2', '3', '4', '5', '6']);
140+
for (const r of rows as any[]) {
141+
const seeded = AGGREGATION_ROWS.find((s) => s.id === String(r.id))!;
142+
expect([r.region, r.stage, r.score], r.id).toEqual([seeded.region, seeded.stage, seeded.score]);
143+
}
144+
// The property every null case hangs off, asserted directly: an empty
145+
// string in place of a null keeps the count_distinct cases green at the
146+
// wrong number.
147+
expect((rows as any[]).filter((r) => r.stage === null)).toHaveLength(2);
148+
});
149+
150+
for (const c of AGGREGATION_CASES) {
151+
it(`find(): ${c.name}`, async () => {
152+
const rows = await driver.find(TABLE, queryFor(c));
153+
expect(actualFor(c, rows as any[]), c.note ?? c.name).toEqual(expectedFor(c));
154+
});
155+
156+
/**
157+
* The SECOND door onto the same computation — objectql's engine calls
158+
* `aggregate(object, AST)`, not `find()`. Two doors that can disagree is
159+
* this package's recurring defect class (#5374), so neither is trusted to
160+
* stand for the other.
161+
*/
162+
it(`aggregate(AST): ${c.name}`, async () => {
163+
const rows = await driver.aggregate(TABLE, queryFor(c));
164+
expect(actualFor(c, rows as any[]), c.note ?? c.name).toEqual(expectedFor(c));
165+
});
166+
}
167+
168+
/**
169+
* The #4157 shape, asserted as a property rather than per case: an aggregate
170+
* the Query Protocol declares must never resolve with `null`. That is what
171+
* `default: return null` produced here — a wrong ANSWER rather than a wrong
172+
* number, and the one failure mode a value comparison per case could be
173+
* "passed" by if a future case-set row ever expected zero.
174+
*/
175+
it('never answers null for a declared aggregate function', async () => {
176+
for (const c of AGGREGATION_CASES) {
177+
const rows = await driver.find(TABLE, queryFor(c));
178+
for (const row of rows as any[]) {
179+
expect(row.n, `${c.name} — a declared function resolving null is the #6814 defect`).not.toBeNull();
180+
expect(typeof row.n, c.name).toBe('number');
181+
}
182+
}
183+
});
184+
});
185+
186+
/**
187+
* [#5374] The ANALYTICS face answers the same function the same way.
188+
*
189+
* This package's recurring defect is not "a face is wrong", it is "the faces
190+
* disagree" — and `count_distinct` was exactly that. #6814 read this face as
191+
* the one that "DOES implement `count_distinct`", which executing it corrects:
192+
* `buildAggregator` emitted `{ $addToSet }` under a comment reading "Will need
193+
* post-processing for count", and no post-processing existed. So the measure
194+
* answered the raw ARRAY — `['won','lost',null]` — under a field
195+
* `measureTypeToFieldType` describes as `number`.
196+
*
197+
* One declared function, three answers: `null` on the data face, an array here,
198+
* and the standard's number nowhere. Aligning the data face alone would have
199+
* left this one free to keep its own.
200+
*/
201+
describe('[#6814] the analytics face answers count_distinct the same number', () => {
202+
const cube: Cube = {
203+
name: 'agg',
204+
title: 'Agg',
205+
sql: TABLE,
206+
measures: {
207+
distinctStage: { name: 'distinct_stage', label: 'Distinct stage', type: 'count_distinct', sql: 'stage' },
208+
distinctScore: { name: 'distinct_score', label: 'Distinct score', type: 'count_distinct', sql: 'score' },
209+
},
210+
dimensions: {
211+
region: { name: 'region', label: 'Region', type: 'string', sql: 'region' },
212+
},
213+
} as unknown as Cube;
214+
215+
let service: MemoryAnalyticsService;
216+
217+
beforeEach(async () => {
218+
const driver = await seed();
219+
service = new MemoryAnalyticsService({ driver, cubes: [cube] });
220+
});
221+
222+
/** The ungrouped pair, against the same numbers `AGGREGATION_CASES` states. */
223+
it('count_distinct(stage) is 2 — distinct NON-NULL values, not 3', async () => {
224+
const result = await service.query({ cube: 'agg', measures: ['agg.distinctStage'] } as any);
225+
expect(result.rows[0]['agg.distinctStage']).toBe(2);
226+
});
227+
228+
it('count_distinct(score) is 6 — the all-distinct control', async () => {
229+
const result = await service.query({ cube: 'agg', measures: ['agg.distinctScore'] } as any);
230+
expect(result.rows[0]['agg.distinctScore']).toBe(6);
231+
});
232+
233+
/**
234+
* Grouped, because a face computing the aggregate over the whole table and
235+
* repeating it per group answers 2/2 and the ungrouped case above cannot see
236+
* it — the same argument `AGGREGATION_CASES`' grouped row is built on.
237+
*/
238+
it('count_distinct(stage) grouped by region is east 1 / west 2', async () => {
239+
const result = await service.query({
240+
cube: 'agg',
241+
measures: ['agg.distinctStage'],
242+
dimensions: ['agg.region'],
243+
} as any);
244+
const byRegion = Object.fromEntries(
245+
result.rows.map((r: any) => [r['agg.region'], r['agg.distinctStage']]),
246+
);
247+
expect(byRegion).toEqual({ east: 1, west: 2 });
248+
});
249+
250+
/**
251+
* The declared TYPE is `number` (`measureTypeToFieldType`), so the value has
252+
* to be one. An `$addToSet` handed back unsized is an ARRAY under a field the
253+
* response describes as numeric — a shape divergence a value comparison alone
254+
* would report as an ordinary wrong number.
255+
*/
256+
it('answers a NUMBER, matching the field type the response declares', async () => {
257+
const result = await service.query({ cube: 'agg', measures: ['agg.distinctStage'] } as any);
258+
expect(result.fields.find((f: any) => f.name === 'agg.distinctStage')?.type).toBe('number');
259+
expect(typeof result.rows[0]['agg.distinctStage']).toBe('number');
260+
});
261+
});

0 commit comments

Comments
 (0)