diff --git a/.changeset/20873-aggregation-filter-array-membership.md b/.changeset/20873-aggregation-filter-array-membership.md new file mode 100644 index 00000000000..e619c6f5e3a --- /dev/null +++ b/.changeset/20873-aggregation-filter-array-membership.md @@ -0,0 +1,24 @@ +--- +'@objectstack/objectql': patch +--- + +fix(objectql): a per-aggregation `filter` with `$contains` / `$notContains` on a multi-valued field counts the rows the same `where` finds (#20873) + +Clause-②: no + +`engine.aggregate({ aggregations: [{ …, filter }] })` — and so `POST /api/v1/data/:object/query` +with a per-aggregation `filter` — evaluates that filter in the engine, not in the driver. Its +`$contains` arm failed every value that was not a string, so on a `multiple: true` lookup, +`multiselect`, `checkboxes` or `tags` field a stored array never matched: +`{ owners: { $contains: 'u1' } }` counted 0 on every driver where the same condition as a `where` +found 2 rows, and `$notContains` counted every row, the rows holding the member included. + +On a declared JSON-stored field (a multi-valued field, or a structured-JSON type) both operators +now ask MEMBERSHIP, the reading `FILTER_OPERATORS`' `$contains` docblock declares and `where` gives +on every SQL dialect: `'u1'` is a member of `['u1', 'u2']` and not of `['u10']`, and a member stored +as a number or boolean is named by its text (`'1'` finds `[1, 2]`). `$notContains` is the exact +complement, and a row with no value still satisfies it. A scalar text field keeps the substring +test, unchanged, and so does `having`. + +No query that was refused now answers, and none that answered is refused: only the count of a +per-aggregation `filter` on a multi-valued field moves, to the `where` count. diff --git a/packages/objectql/src/engine-aggregate-filter-array-membership.test.ts b/packages/objectql/src/engine-aggregate-filter-array-membership.test.ts new file mode 100644 index 00000000000..e32c79dfaee --- /dev/null +++ b/packages/objectql/src/engine-aggregate-filter-array-membership.test.ts @@ -0,0 +1,208 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// [#20873] A per-aggregation `filter` answers `$contains` / `$notContains` on a +// DECLARED multi-valued field by MEMBERSHIP — the reading `FILTER_OPERATORS`' +// `$contains` docblock (`@objectstack/spec`) declares for a JSON-stored column, +// and the one `where` gives on every SQL dialect — while a scalar string column +// keeps the substring test. +// +// Measured on the base (`origin/main` `212d613c`) through `engine.aggregate`, +// on a real InMemoryDriver, a real SqlDriver over SQLite and the same over a +// live PostgreSQL 16.14, the six rows below (`owners` a `multiple: true` lookup, +// `tags` a `tags` field, `title` text): +// +// | per-aggregation `filter` | base `m`, all three | SQLite / PostgreSQL `where` | `m` now | +// |:--|:--|:--|:--| +// | `owners $contains 'u1'` (the card) | 0 | 2 (d1, d3) | 2 | +// | `owners $contains 'u10'` | 0 | 1 (d5) | 1 | +// | `owners $notContains 'u1'` | 6 | 4 (d2, d4, d5, d6) | 4 | +// | `tags $contains 'red'` | 0 | 2 (d1, d5) | 2 | +// | `$or` of `$contains` (the any-of spelling) | 0 | 2 | 2 | +// | `$not` of `$contains` | 6 | 4 | 4 | +// | `title $contains 'u1'` (the control) | 3 | 3 | 3 | +// +// `d5` holds `['u10']` and `d3` `['redwood']`: the rows on which membership and +// a substring disagree. `driver-memory`'s own `where` answered 3 for the card +// on that base, by a per-element substring; that face is not this file's (it +// is moving to membership on its own card), and the `where` numbers this file +// holds the evaluator to are the SQL family's. The rows are in the read shape +// `find()` presents on all three backends — measured identical on each: an +// array, or `null`. +// +// The SQLite and PostgreSQL cells run through the public door, beside a live +// `where` twin, in `packages/rest`'s `aggregation-filter-array-membership.test.ts`. + +import { describe, it, expect } from 'vitest'; +import { ObjectQL } from './engine.js'; +import { declaredJsonStoredFields, matchesAggregationFilter } from './having-filter.js'; + +const OBJECT = 'os20873_doc'; + +const FIELDS = { + title: { type: 'text' }, + owners: { type: 'lookup', reference: 'os20873_user', multiple: true }, + tags: { type: 'tags' }, +}; + +const ROWS = [ + { id: 'd1', title: 'u1 memo', owners: ['u1', 'u2'], tags: ['red', 'blue'] }, + { id: 'd2', title: 'none', owners: ['u2'], tags: ['blue'] }, + { id: 'd3', title: 'about u10', owners: ['u3', 'u1'], tags: ['redwood'] }, + { id: 'd4', title: 'x', owners: [], tags: [] }, + { id: 'd5', title: 'u1', owners: ['u10'], tags: ['red'] }, + { id: 'd6', title: null, owners: null, tags: null }, +]; + +/** A driver WITHOUT `aggregate()`: the engine reads rows and lowers in memory, where the filter is evaluated. */ +function makeRowsDriver(rows: readonly Record[]) { + return { + name: 'rows-mock', + version: '0.0.0', + supports: {}, + async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; }, + async find() { return rows.map((row) => ({ ...row })); }, + async findOne() { return null; }, + async create(_o: string, d: any) { return d; }, + async update(_o: string, _id: string, d: any) { return d; }, + async delete() { return true; }, + async count() { return 0; }, + async bulkCreate(_o: string, r: any[]) { return r; }, + async bulkUpdate() { return []; }, async bulkDelete() {}, + async beginTransaction() { return { commit: async () => {}, rollback: async () => {} }; }, + async commit() {}, async rollback() {}, + }; +} + +async function makeEngine(rows: readonly Record[]) { + const engine = new ObjectQL(); + engine.registerDriver(makeRowsDriver(rows) as any, true); + await engine.init(); + (engine.registry as any).registerObject({ name: OBJECT, fields: FIELDS }); + return engine; +} + +/** filter · the rows the SQL family's `where` answers for it, on this fixture. */ +const CASES: ReadonlyArray, readonly string[]]> = [ + ['the card — $contains a member id', { owners: { $contains: 'u1' } }, ['d1', 'd3']], + ['$contains an id another stored id has as a prefix', { owners: { $contains: 'u10' } }, ['d5']], + ['$notContains — the exact complement, the null row included (#5298)', { owners: { $notContains: 'u1' } }, ['d2', 'd4', 'd5', 'd6']], + ['$contains on a tags field — a member, not a substring of one', { tags: { $contains: 'red' } }, ['d1', 'd5']], + ['$notContains on a tags field', { tags: { $notContains: 'red' } }, ['d2', 'd3', 'd4', 'd6']], + ['an $or of $contains — the any-of spelling', { $or: [{ owners: { $contains: 'u1' } }, { owners: { $contains: 'u3' } }] }, ['d1', 'd3']], + ['$not over $contains', { $not: { owners: { $contains: 'u1' } } }, ['d2', 'd4', 'd5', 'd6']], + ['control — $contains on a text field stays a substring', { title: { $contains: 'u1' } }, ['d1', 'd3', 'd5']], + ['control — $notContains on a text field stays a substring', { title: { $notContains: 'u1' } }, ['d2', 'd4', 'd6']], +]; + +describe('[#20873] engine.aggregate — a per-aggregation filter counts a declared multi-valued field by membership', () => { + for (const [name, filter, rows] of CASES) { + it(`${name}: m = ${rows.length}, the rows the SQL where answers`, async () => { + const engine = await makeEngine(ROWS); + const out = await engine.aggregate(OBJECT, { + aggregations: [{ function: 'count', alias: 'n' }, { function: 'count', alias: 'm', filter: filter as never }], + }); + expect(out).toEqual([{ n: 6, m: rows.length }]); + // The same rows, not only the same count. + const fields = FIELDS as Record; + const kept = ROWS.filter((row) => matchesAggregationFilter(row, filter as never, 1, undefined, declaredJsonStoredFields(fields))); + expect(kept.map((row) => row.id)).toEqual(rows); + }); + } + + it('an empty table counts 0 for each, and n is 0 beside it', async () => { + for (const [, filter] of CASES) { + const engine = await makeEngine([]); + const out = await engine.aggregate(OBJECT, { + aggregations: [{ function: 'count', alias: 'n' }, { function: 'count', alias: 'm', filter: filter as never }], + }); + expect(out).toEqual([{ n: 0, m: 0 }]); + } + }); + + it('per group: each group counts the rows whose array holds the member', async () => { + const engine = await makeEngine([ + { id: 'g1', title: 'a', owners: ['u1'] }, + { id: 'g2', title: 'a', owners: ['u10'] }, + { id: 'g3', title: 'b', owners: ['u2', 'u1'] }, + { id: 'g4', title: 'b', owners: null }, + ]); + const out = await engine.aggregate(OBJECT, { + groupBy: ['title'], + aggregations: [{ function: 'count', alias: 'n' }, { function: 'count', alias: 'm', filter: { owners: { $contains: 'u1' } } as never }], + }); + expect([...out].sort((a, b) => String(a.title).localeCompare(String(b.title)))).toEqual([ + { title: 'a', n: 2, m: 1 }, + { title: 'b', n: 2, m: 1 }, + ]); + }); +}); + +describe('[#20873] having — the shared walker keeps the substring reading on an aggregated text column', () => { + it('a $contains over a groupBy text projection keeps the groups whose label holds the text', async () => { + const engine = await makeEngine(ROWS); + const out = await engine.aggregate(OBJECT, { + groupBy: ['title'], + aggregations: [{ function: 'count', alias: 'n' }], + having: { title: { $contains: 'u1' } } as never, + }); + expect(out.map((row: any) => row.title).sort()).toEqual(['about u10', 'u1', 'u1 memo']); + }); +}); + +describe('[#20873] the member reading — the comparand names a member by its text, as on every SQL dialect', () => { + const NUMS = { nums: { type: 'select', multiple: true } }; + const holds = (stored: unknown, comparand: string) => + matchesAggregationFilter({ nums: stored }, { nums: { $contains: comparand } } as never, 0, undefined, declaredJsonStoredFields(NUMS)); + + it.each([ + ['a string member', ['a', 'b'], 'a', true], + ['a number member, named by its text', [1, 2], '1', true], + ['a number member, named by a non-canonical JSON number', [1.5, 0], '1.50', true], + ['a number member, named by an exponent', [100], '1e2', true], + ['a boolean member', [true, false], 'true', true], + ['a null member', [null], 'null', true], + ['a string member that reads as a number', ['1'], '1', true], + ['no member: a leading zero is not a JSON number', [1], '01', false], + ['no member: a padded number is not a JSON number', [1], ' 1', false], + ['no member: a hex spelling is not a JSON number', [16], '0x10', false], + ['no member: a substring of a member', ['redwood'], 'red', false], + ['no member: a nested array', [['u1']], 'u1', false], + ['no member: an object element', [{ k: 'u1' }], 'u1', false], + ['no member: a stored scalar string', 'u1 memo', 'u1', false], + ['no member: an empty array', [], 'u1', false], + ] as const)('%s', (_name, stored, comparand, expected) => { + expect(holds(stored, comparand)).toBe(expected); + }); + + it('$notContains is the exact complement on every one of those, and holds for a null or absent value', () => { + const notHolds = (row: Record, comparand: string) => + matchesAggregationFilter(row, { nums: { $notContains: comparand } } as never, 0, undefined, declaredJsonStoredFields(NUMS)); + expect(notHolds({ nums: [1, 2] }, '1')).toBe(false); + expect(notHolds({ nums: ['redwood'] }, 'red')).toBe(true); + expect(notHolds({ nums: null }, '1')).toBe(true); + expect(notHolds({}, '1')).toBe(true); + }); + + it('the fork is the DECLARATION: an undeclared column holding an array keeps the substring reading', () => { + const row = { owners: ['u1'] }; + expect(matchesAggregationFilter(row, { owners: { $contains: 'u1' } } as never, 0)).toBe(false); + expect(matchesAggregationFilter(row, { owners: { $contains: 'u1' } } as never, 0, undefined, declaredJsonStoredFields(FIELDS))).toBe(true); + }); + + it('the declared population is the spec\'s JSON-stored classes: multi-valued fields and structured-JSON types', () => { + expect([...declaredJsonStoredFields({ + title: { type: 'text' }, + one_owner: { type: 'lookup' }, + owners: { type: 'lookup', multiple: true }, + picks: { type: 'multiselect' }, + tags: { type: 'tags' }, + choice: { type: 'select' }, + choices: { type: 'select', multiple: true }, + meta: { type: 'json' }, + ship_to: { type: 'address' }, + broken: null, + untyped: {}, + })].sort()).toEqual(['choices', 'meta', 'owners', 'picks', 'ship_to', 'tags']); + expect(declaredJsonStoredFields(undefined).size).toBe(0); + }); +}); diff --git a/packages/objectql/src/having-filter.ts b/packages/objectql/src/having-filter.ts index edd1d6b59b0..b77fa6fdf5c 100644 --- a/packages/objectql/src/having-filter.ts +++ b/packages/objectql/src/having-filter.ts @@ -111,6 +111,15 @@ // {@link checkCondition}. Before, both positions compared a temporal comparand // as written, so an ISO instant against a `date` column counted 1 row where the // same condition in a `where` counted 3. +// +// [#20873] …and on a DECLARED multi-valued field, the per-aggregation `filter` +// answers `$contains` / `$notContains` by MEMBERSHIP — the reading +// `FILTER_OPERATORS`' `$contains` docblock (`@objectstack/spec`) declares for a +// JSON-stored column and `where` already gives on every SQL dialect. Before, +// the arm failed every value that was not a string, so a stored array never +// matched: `{ owners: { $contains: 'u1' } }` counted 0 where the same `where` +// counted 2, and `$notContains` counted every row, the members included. See +// {@link storedArrayHasMember} and {@link declaredJsonStoredFields}. import type { FilterCondition } from '@objectstack/spec/data'; // [#20099] The reference's own declaration, so a malformed `addDays` is refused @@ -152,6 +161,10 @@ import { isEmptyFilterValue } from '@objectstack/spec/data'; // the spec, where that rule is declared. import { temporalStorageForm, type TemporalComparandKind } from '@objectstack/core'; import { nextUtcCalendarDay, UNBOUNDED_ABOVE, isUnboundedAbove, type UnboundedAbove } from '@objectstack/spec/data'; +// [#20873] The JSON-stored population — the declared fields on which `$contains` +// asks MEMBERSHIP — from the spec's value-shape classes, the same two +// `driver-sql`'s JSON-column registry is built from. +import { STRUCTURED_JSON_TYPES, isMultiValueField } from '@objectstack/spec/data'; // [#7047] The ADR-0112 envelope this face's refusals used to omit. Shared with // `filter-comparand-shape.ts` rather than re-declared here — see the note on // {@link invalidFilterError} and on {@link unknownOperator} below. @@ -802,6 +815,46 @@ export function declaredFieldClasses(fields: unknown): Map { + const stored = new Set(); + if (!fields || typeof fields !== 'object' || Array.isArray(fields)) return stored; + for (const [name, def] of Object.entries(fields as Record)) { + const type = (def as { type?: unknown } | null)?.type; + if (typeof type !== 'string') continue; + const multiple = (def as { multiple?: unknown }).multiple === true; + if (STRUCTURED_JSON_TYPES.has(type) || isMultiValueField({ type, multiple })) stored.add(name); + } + return stored; +} + /** * [#20176] The storage rule a column of `cls` takes, or `undefined` for a class * that has none (numeric, text, boolean) and for a column whose class the @@ -1239,6 +1292,17 @@ export function applyHaving( * column whose class is temporal has its comparands compared by that column's * storage rule ({@link checkCondition}). Absent, or a column it does not name ⇒ * compared as written. + * + * [#20873] `jsonStored` names the columns declared JSON-stored + * ({@link declaredJsonStoredFields}), on which `$contains` / `$notContains` ask + * MEMBERSHIP. Only the per-aggregation `filter` passes it. `applyHaving` does + * not: an aggregated row has no declaration of its own for a set to be read + * from, a `groupBy` on a multi-valued or structured-JSON field is refused before + * any row exists, and the one aggregated column that can still carry a stored + * array — a `min` / `max` over a multi-valued field — is itself answered three + * ways by the backends (an array on the in-memory aggregation, the serialized + * TEXT on SQLite's native aggregate, a `DATABASE_ERROR` on PostgreSQL), so + * there is no single `where` answer to hold `having` to there. */ export function matchesHaving( row: Record, @@ -1246,22 +1310,23 @@ export function matchesHaving( path = 'having', clause: FilterClause = HAVING_CLAUSE, classes?: ReadonlyMap, + jsonStored?: ReadonlySet, ): boolean { if (!cond || typeof cond !== 'object') return true; for (const [key, value] of Object.entries(cond)) { const here = `${path}.${key}`; if (key === '$and') { const branches = Array.isArray(value) ? value : [value]; - if (!branches.every((c, i) => matchesHaving(row, c, `${here}[${i}]`, clause, classes))) return false; + if (!branches.every((c, i) => matchesHaving(row, c, `${here}[${i}]`, clause, classes, jsonStored))) return false; continue; } if (key === '$or') { const branches = Array.isArray(value) ? value : [value]; - if (!branches.some((c, i) => matchesHaving(row, c, `${here}[${i}]`, clause, classes))) return false; + if (!branches.some((c, i) => matchesHaving(row, c, `${here}[${i}]`, clause, classes, jsonStored))) return false; continue; } if (key === '$not') { - if (matchesHaving(row, value, here, clause, classes)) return false; + if (matchesHaving(row, value, here, clause, classes, jsonStored)) return false; continue; } if (key.startsWith('$')) throw unknownOperator(key, 'logical', [], clause); @@ -1270,7 +1335,11 @@ export function matchesHaving( // same way on purpose: it reads `driver.find()` rows, which are flat too. // [#20099] The row itself goes down too: a `{ $field }` comparand resolves // against it. [#20176] …and the column's storage rule, when it has one. - if (!checkCondition(row?.[key], value, key, here, clause, row, temporalKindOf(classes?.get(key)))) return false; + // [#20873] …and whether the column is declared JSON-stored, which decides + // the question `$contains` / `$notContains` ask of it. + if (!checkCondition( + row?.[key], value, key, here, clause, row, temporalKindOf(classes?.get(key)), jsonStored?.has(key) === true, + )) return false; } return true; } @@ -1293,15 +1362,21 @@ export function matchesHaving( * [#20176] `classes` is the object's declared field classes * ({@link declaredFieldClasses}), so a temporal comparand here is read by the * column's storage rule, as the same comparand in a `where` is by the driver. + * + * [#20873] `jsonStored` is the object's declared JSON-stored fields + * ({@link declaredJsonStoredFields}), so `$contains` / `$notContains` on one of + * them ask MEMBERSHIP, as the same condition in a `where` does on every SQL + * dialect. Absent ⇒ every column keeps the substring reading. */ export function matchesAggregationFilter( row: Record, filter: FilterCondition, index: number, classes?: ReadonlyMap, + jsonStored?: ReadonlySet, ): boolean { const clause = aggregationFilterClause(index); - return matchesHaving(row, filter, clause.root, clause, classes); + return matchesHaving(row, filter, clause.root, clause, classes, jsonStored); } /** @@ -1380,6 +1455,52 @@ function wholeDayUpperBound( return next === null ? undefined : temporalStorageForm(next, 'datetime'); } +/** + * [#20873] The JSON NUMBER grammar, spelled out — the pattern `driver-sql`'s + * `jsonMembershipCandidates` tests a `$contains` comparand against, for its + * reason: `Number()` also accepts `'0x10'`, `' 1 '`, `'Infinity'` and `''`, none + * of which is a JSON number, and admitting them would make the member set + * depend on JS coercion rules no SQL dialect shares. + */ +const JSON_NUMBER_TEXT = /^-?(?:0|[1-9][0-9]*)(?:\.[0-9]+)?(?:[eE][+-]?[0-9]+)?$/; + +/** + * [#20873] Is the `$contains` comparand a MEMBER of the stored array — the + * question `$contains` asks of a declared JSON-stored column + * ({@link declaredJsonStoredFields}). + * + * The comparand is a STRING by contract (`FILTER_OPERATORS`' `$contains` + * docblock), so a member stored as a JSON number or boolean is named by its + * TEXT: `'1'` names the string `'1'` or the number `1`, `'true'` the string or + * `true`, `'null'` the string or `null`, and `'1.50'` the number `1.5`. That is + * the candidate set `driver-sql`'s `jsonMembershipCandidates` binds for every + * dialect — `String(comparand)`, plus the canonical number when the text is a + * JSON number, plus the literal for `true` / `false` / `null` — read here as a + * predicate over JS values instead of as JSON text. + * + * Array-only, as the SQL constructs are: a value that is not an array (a + * scalar, an object, `null`) has no member, and neither does an element that + * is itself an object or an array. + * + * ⚠️ A second copy of one rule, not the shared one: `driver-sql`'s + * `jsonMembershipCandidates` is module-private, and `driver-memory`'s twin is + * module-private too, so neither is importable here. The one shared home would + * be `@objectstack/spec/data`, beside `asciiCaseInsensitiveContains` and + * `isEmptyFilterValue` — the value-level filter rules every JS face already + * reads from there. + */ +function storedArrayHasMember(value: unknown, comparand: unknown): boolean { + if (!Array.isArray(value)) return false; + const text = String(comparand); + const number = JSON_NUMBER_TEXT.test(text) ? Number(text) : Number.NaN; + return value.some((element) => { + if (typeof element === 'string') return element === text; + if (typeof element === 'number') return Number.isFinite(number) && element === number; + if (typeof element === 'boolean' || element === null) return String(element) === text; + return false; + }); +} + /** * One column's condition — implicit equality or an operator object. * @@ -1398,6 +1519,11 @@ function wholeDayUpperBound( * SQLite column in the same canon); rows a driver returns are in it already. * Presence (`$exists`, `$null`), the text operators and a `{ $field }` * reference are not comparands of a value, and are read as before. + * + * [#20873] `jsonStored` is whether the column is declared JSON-stored. Then + * `$contains` asks whether its comparand is a MEMBER of the stored array + * ({@link storedArrayHasMember}) and `$notContains` its exact complement; on + * any other column both keep the substring test. */ function checkCondition( value: any, @@ -1407,6 +1533,7 @@ function checkCondition( clause: FilterClause = HAVING_CLAUSE, row: Record = {}, kind?: TemporalComparandKind, + jsonStored = false, ): boolean { const form = (operand: unknown): unknown => (kind === undefined ? operand : temporalStorageForm(operand, kind)); // Implicit equality (primitives, null, Date, array exact-match) — loose `==` @@ -1521,7 +1648,15 @@ function checkCondition( case '$empty': if (isEmptyFilterValue(value) !== (target === true)) return false; break; - case '$contains': if (typeof value !== 'string' || !value.includes(target)) return false; break; + // [#20873] On a declared JSON-stored column, MEMBERSHIP — the reading + // `where` gives the same condition on every SQL dialect + // (`SqlDriver.applyJsonMembership`). The substring test below failed + // every value that was not a string, so a stored array never matched. + case '$contains': + if (jsonStored + ? !storedArrayHasMember(value, target) + : typeof value !== 'string' || !value.includes(target)) return false; + break; // [#5905] The mirror of `$contains`, NOT its copy-with-a-negated-test. // `$contains` fails a non-string value because "contains" cannot hold for // something that is not text; `$notContains` SUCCEEDS for the same value @@ -1545,7 +1680,18 @@ function checkCondition( // the SQL compilers emit a type-gated constant for a column whose // declared type is in `NON_TEXT_STORED_VALUE_TYPES`. This arm was already // on the ruled side; nothing here moved. - case '$notContains': if (typeof value === 'string' && value.includes(target)) return false; break; + // + // [#20873] On a declared JSON-stored column, the exact complement of the + // membership arm above — what `where` gives on every SQL dialect + // (`col IS NULL OR NOT (…)`). The substring test SUCCEEDED for every + // value that was not a string, so a stored array satisfied + // `$notContains` even when it held the comparand. A row with no value + // still satisfies it (#5298): `null` and an absent column have no member. + case '$notContains': + if (jsonStored + ? storedArrayHasMember(value, target) + : typeof value === 'string' && value.includes(target)) return false; + break; case '$startsWith': if (typeof value !== 'string' || !value.startsWith(target)) return false; break; case '$endsWith': if (typeof value !== 'string' || !value.endsWith(target)) return false; break; // [#6520] `$contains`' case-INSENSITIVE twin, over ASCII case only. Same diff --git a/packages/objectql/src/in-memory-aggregation.ts b/packages/objectql/src/in-memory-aggregation.ts index 1824751db00..17e041d6271 100644 --- a/packages/objectql/src/in-memory-aggregation.ts +++ b/packages/objectql/src/in-memory-aggregation.ts @@ -86,7 +86,7 @@ import { bucketDateKey, compensatedSum } from '@objectstack/core'; import type { QueryAST, GroupByNode, AggregationNode, DateGranularityValue } from '@objectstack/spec/data'; -import { declaredFieldClasses, matchesAggregationFilter } from './having-filter.js'; +import { declaredFieldClasses, declaredJsonStoredFields, matchesAggregationFilter } from './having-filter.js'; /** * Group + aggregate raw rows according to the AST's `groupBy` / @@ -102,6 +102,10 @@ import { declaredFieldClasses, matchesAggregationFilter } from './having-filter. * handed one — the rule the driver applies to the same comparand in a `where` * (having-filter.ts `checkCondition`). Absent (a registry-less caller) ⇒ every * comparand is compared as written, as before. + * + * [#20873] …and `$contains` / `$notContains` on a declared JSON-stored field ask + * MEMBERSHIP (having-filter.ts `declaredJsonStoredFields`), the reading the + * same condition gets in a `where`. Absent ⇒ the substring reading, as before. */ export function applyInMemoryAggregation( rows: any[], @@ -113,13 +117,14 @@ export function applyInMemoryAggregation( const aggregations = (ast.aggregations ?? []) as AggregationNode[]; if (groupBy.length === 0 && aggregations.length === 0) return rows; // [#20176] Read once per call, and only when some aggregation carries a filter. - const filterClasses = fields && aggregations.some((a) => a?.filter && Object.keys(a.filter).length > 0) - ? declaredFieldClasses(fields) - : undefined; + const anyFilter = aggregations.some((a) => a?.filter && Object.keys(a.filter).length > 0); + const filterClasses = fields && anyFilter ? declaredFieldClasses(fields) : undefined; + // [#20873] Read once per call too, from the same declaration. + const filterJsonStored = fields && anyFilter ? declaredJsonStoredFields(fields) : undefined; if (groupBy.length === 0) { // Pure aggregation — single result row. - return [aggregateBucket(rows, aggregations, filterClasses)]; + return [aggregateBucket(rows, aggregations, filterClasses, filterJsonStored)]; } const buckets = new Map; rows: any[] }>(); @@ -147,7 +152,7 @@ export function applyInMemoryAggregation( const out: any[] = []; for (const { key, rows: bucketRows } of buckets.values()) { - const aggValues = aggregateBucket(bucketRows, aggregations, filterClasses); + const aggValues = aggregateBucket(bucketRows, aggregations, filterClasses, filterJsonStored); out.push({ ...key, ...aggValues }); } return out; @@ -182,6 +187,7 @@ function aggregateBucket( allRows: any[], aggregations: AggregationNode[], filterClasses?: ReturnType, + filterJsonStored?: ReadonlySet, ): Record { const out: Record = {}; for (const [index, agg] of aggregations.entries()) { @@ -198,7 +204,7 @@ function aggregateBucket( // still lists, objectui#3136). const aggFilter = agg.filter; const rows = aggFilter && Object.keys(aggFilter).length > 0 - ? allRows.filter((row) => matchesAggregationFilter(row, aggFilter, index, filterClasses)) + ? allRows.filter((row) => matchesAggregationFilter(row, aggFilter, index, filterClasses, filterJsonStored)) : allRows; if (fn === 'count') { // `*` is the count-all sentinel: the Cube `count` measure and a dataset diff --git a/packages/rest/src/aggregation-filter-array-membership.test.ts b/packages/rest/src/aggregation-filter-array-membership.test.ts new file mode 100644 index 00000000000..8fe9fbfb347 --- /dev/null +++ b/packages/rest/src/aggregation-filter-array-membership.test.ts @@ -0,0 +1,198 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#20873] A per-aggregation `filter` with `$contains` / `$notContains` on a + * declared multi-valued field counts the rows the same condition finds as the + * call's `where`, through the door a caller uses: + * `POST /api/v1/data/:object/query` → `RestServer` → + * `ObjectStackProtocolImplementation.findData` → `ObjectQL.aggregate` → a real + * `SqlDriver`. Every row runs beside its `where` TWIN, so a later change to + * either side of the pair — the driver's membership construct or the engine's + * evaluator — breaks the equality here rather than drifting silently. + * + * Measured on the base (`origin/main` `212d613c`) through `engine.aggregate` + * on SQLite and a live PostgreSQL 16.14: every per-aggregation count below was + * 0 for `$contains` and 6 (every row) for `$notContains`, against the `where` + * twin's numbers in the table. The engine evaluated the filter, and its arm + * failed every value that was not a string. + * + * | filter | `where` twin | + * |:--|:--| + * | `owners $contains 'u1'` (the card) | 2 | + * | `owners $contains 'u10'` | 1 | + * | `owners $notContains 'u1'` | 4 | + * | `tags $contains 'red'` | 2 | + * | `tags $notContains 'red'` | 4 | + * | `$or` of `$contains` (the any-of spelling) | 2 | + * | `title $contains 'u1'` (the control, unchanged) | 3 | + * + * InMemoryDriver's evaluator cell is `@objectstack/objectql`'s + * `engine-aggregate-filter-array-membership.test.ts`, over the read shape + * `find()` presents on all three backends (measured identical): this package + * does not depend on the in-memory driver, and that driver's test consumers are + * a ruled, closed census (`check:driver-memory-census`). + * + * ## The dialect axis of THIS file + * + * The SQLite cell always runs. The PostgreSQL and MySQL cells run where + * `OS_TEST_POSTGRES_URL` / `OS_TEST_MYSQL_URL` are set and are a named skip + * otherwise. ⚠️ No CI job provisions those variables for this package, so the + * live cells are red-capable and un-run in CI; the PR that landed this file + * carries their local PostgreSQL run. Each live cell owns its table, dropped + * before and after. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; +import { RestServer } from './rest-server'; + +const OBJECT = 'rest_agg_filter_membership_20873'; + +const DOC = { + name: OBJECT, + label: 'Doc 20873', + fields: { + title: { name: 'title', type: 'text' as const }, + owners: { name: 'owners', type: 'lookup' as const, reference: 'rest_agg_filter_user_20873', multiple: true }, + tags: { name: 'tags', type: 'tags' as const }, + }, +}; + +/** `d5` (`['u10']`) and `d3` (`['redwood']`) are where membership and a substring disagree. */ +const ROWS = [ + { id: 'd1', title: 'u1 memo', owners: ['u1', 'u2'], tags: ['red', 'blue'] }, + { id: 'd2', title: 'none', owners: ['u2'], tags: ['blue'] }, + { id: 'd3', title: 'about u10', owners: ['u3', 'u1'], tags: ['redwood'] }, + { id: 'd4', title: 'x', owners: [], tags: [] }, + { id: 'd5', title: 'u1', owners: ['u10'], tags: ['red'] }, + { id: 'd6', title: null, owners: null, tags: null }, +]; + +/** filter · the `where` twin's count on this fixture. */ +const CASES: ReadonlyArray, number]> = [ + ['the card — $contains a member id', { owners: { $contains: 'u1' } }, 2], + ['$contains an id another stored id has as a prefix', { owners: { $contains: 'u10' } }, 1], + ['$notContains — the exact complement, the null row included', { owners: { $notContains: 'u1' } }, 4], + ['$contains on a tags field — a member, not a substring of one', { tags: { $contains: 'red' } }, 2], + ['$notContains on a tags field', { tags: { $notContains: 'red' } }, 4], + ['an $or of $contains — the any-of spelling', { $or: [{ owners: { $contains: 'u1' } }, { owners: { $contains: 'u3' } }] }, 2], + ['control — $contains on a text field stays a substring', { title: { $contains: 'u1' } }, 3], +]; + +interface Cell { + id: 'sqlite' | 'pg' | 'mysql'; + label: string; + env: string | null; + config: () => Record | null; +} + +const CELLS: readonly Cell[] = [ + { id: 'sqlite', label: 'sqlite', env: null, config: () => ({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }) }, + { + id: 'pg', + label: 'live postgres', + env: 'OS_TEST_POSTGRES_URL', + config: () => (process.env.OS_TEST_POSTGRES_URL ? { client: 'pg', connection: process.env.OS_TEST_POSTGRES_URL } : null), + }, + { + id: 'mysql', + label: 'live mysql', + env: 'OS_TEST_MYSQL_URL', + config: () => (process.env.OS_TEST_MYSQL_URL ? { client: 'mysql2', connection: process.env.OS_TEST_MYSQL_URL } : null), + }, +]; + +function createMockServer() { + const noop = () => {}; + return { get: noop, post: noop, put: noop, delete: noop, patch: noop, use: noop, listen: async () => {}, close: async () => {} }; +} + +function makeRes() { + const res: any = { + write: () => true, end: () => {}, + header: () => res, + status: (code: number) => { res._status = code; return res; }, + json: (body: any) => { res._json = body; return res; }, + }; + return res; +} + +const perAggregation = (filter: unknown) => ({ + aggregations: [{ function: 'count', alias: 'n' }, { function: 'count', alias: 'm', filter }], +}); +const whereTwin = (filter: unknown) => ({ where: filter, aggregations: [{ function: 'count', alias: 'n' }] }); + +for (const cell of CELLS) { + const config = cell.config(); + describe.skipIf(!config)( + `[#20873] POST /data/:object/query — a per-aggregation filter counts a multi-valued field as its where twin — ${cell.label}${config ? '' : ` (skipped: set ${cell.env} to run this cell)`}`, + () => { + let engine: ObjectQL; + let driver: any; + let post: (body: Record) => Promise; + + const dropTables = async () => { + if (cell.id === 'sqlite') return; + await driver?.execute(`drop table if exists ${OBJECT}`).catch(() => {}); + }; + + const boot = async (populated: boolean) => { + if (engine) await engine.destroy().catch(() => {}); + driver = new SqlDriver(config as any); + await dropTables(); + engine = new ObjectQL(); + engine.registerDriver(driver, true); + await engine.init(); + engine.registry.registerObject(DOC as any); + await engine.syncSchemas(); + if (populated) for (const row of ROWS) await engine.insert(OBJECT, { ...row } as any); + + const protocol = new ObjectStackProtocolImplementation(engine as any); + const rest = new RestServer(createMockServer() as any, protocol as any, { api: { requireAuth: false } } as any); + (rest as any).resolveExecCtx = async () => ({ userId: 'test-user' }); + rest.registerRoutes(); + const route = rest.getRoutes().find((r: any) => r.method === 'POST' && r.path === '/api/v1/data/:object/query'); + expect(route).toBeDefined(); + post = async (body) => { + const res = makeRes(); + await route!.handler({ params: { object: OBJECT }, body: JSON.parse(JSON.stringify(body)) } as any, res); + expect(res._status ?? 200, JSON.stringify(res._json)).toBe(200); + return res._json.records as any[]; + }; + }; + + beforeAll(async () => { await boot(true); }, 60_000); + afterAll(async () => { + await dropTables(); + await engine?.destroy().catch(() => {}); + }, 60_000); + + for (const [name, filter, count] of CASES) { + it(`${name}: m = ${count}, the where twin's count`, async () => { + const [twin] = await post(whereTwin(filter)); + expect(twin, 'where twin').toEqual({ n: count }); + expect(await post(perAggregation(filter))).toEqual([{ n: 6, m: twin.n }]); + }, 60_000); + } + + it('having over a groupBy text projection keeps the substring reading', async () => { + const kept = await post({ + groupBy: ['title'], + aggregations: [{ function: 'count', alias: 'n' }], + having: { title: { $contains: 'u1' } }, + }); + expect(kept.map((row) => row.title).sort()).toEqual(['about u10', 'u1', 'u1 memo']); + }, 60_000); + + it('an empty table counts 0 for each, as its where twin does', async () => { + await boot(false); + for (const [, filter] of CASES) { + expect(await post(whereTwin(filter))).toEqual([{ n: 0 }]); + expect(await post(perAggregation(filter))).toEqual([{ n: 0, m: 0 }]); + } + }, 60_000); + }, + ); +}