|
| 1 | +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. |
| 2 | +// |
| 3 | +// [#20981] A per-aggregation `filter` and a `having` REFUSE a non-boolean |
| 4 | +// `$exists` / `$null` with `INVALID_FILTER` / 400, in the words every driver's |
| 5 | +// `where` refuses it in, before any driver is asked. `true` / `false` answer |
| 6 | +// exactly as before. |
| 7 | +// |
| 8 | +// Measured before this change (`origin/main` `7a606a9a3`) through |
| 9 | +// `engine.aggregate` on driver-memory AND driver-sql (better-sqlite3), over a |
| 10 | +// text column `name` holding `'won'` on one row (amount 10) and no value on two |
| 11 | +// (amounts 1 and 100). The engine evaluates both clauses itself, so the two |
| 12 | +// drivers answered identically: |
| 13 | +// |
| 14 | +// | clause · comparand | `$exists` | `$null` | |
| 15 | +// |:--|:--|:--| |
| 16 | +// | filter · `"yes"` / `1` / `"false"` | sum 10 (the valued row) | sum 111 (every row) | |
| 17 | +// | filter · `0` / `null` | sum 101 (the no-value rows) | sum 111 (every row) | |
| 18 | +// | having · `"yes"` / `1` / `"false"` | the `won` group | both groups | |
| 19 | +// | having · `0` / `null` | the null group | both groups | |
| 20 | +// |
| 21 | +// while the same flag in a `where` is refused 400 on both drivers. After it, |
| 22 | +// every cell above is that 400, and the `true` / `false` control keeps the |
| 23 | +// numbers below. |
| 24 | +// |
| 25 | +// This file is the engine-level cell, over a driver of each `having` path's |
| 26 | +// shape — `rows` (no `aggregate()`: the engine reads rows and lowers in memory, |
| 27 | +// the face that evaluates `aggregations[i].filter`) and `native` (the driver |
| 28 | +// aggregates, the engine applies `having`). Every refusal precedes the driver, |
| 29 | +// so the backend does not enter into it; the same answers were measured on |
| 30 | +// driver-memory and driver-sql above. This package does not depend on either |
| 31 | +// driver, and the in-memory driver's test consumers are a closed census |
| 32 | +// (`check:driver-memory-census`), so the SQLite cell — beside its live `where` |
| 33 | +// twin, through `POST /data/:object/query` — is `packages/rest`'s |
| 34 | +// `aggregation-flag-comparand-refusal.test.ts`. |
| 35 | + |
| 36 | +import { describe, it, expect } from 'vitest'; |
| 37 | +import type { EngineAggregateOptions, FilterCondition } from '@objectstack/spec/data'; |
| 38 | +import { ObjectQL } from './engine.js'; |
| 39 | +import { applyHaving, matchesAggregationFilter } from './having-filter.js'; |
| 40 | +import { applyInMemoryAggregation } from './in-memory-aggregation.js'; |
| 41 | + |
| 42 | +const OBJECT = 'os20981_deal'; |
| 43 | + |
| 44 | +const FIELDS = { |
| 45 | + name: { type: 'text' }, |
| 46 | + amount: { type: 'number' }, |
| 47 | +}; |
| 48 | + |
| 49 | +// `a` holds a value; `b` holds null and `c` does not carry the column — both |
| 50 | +// have NO value, the reading `$exists` and `$null` share (#5298). |
| 51 | +const ROWS = [ |
| 52 | + { id: 'a', name: 'won', amount: 10 }, |
| 53 | + { id: 'b', name: null, amount: 1 }, |
| 54 | + { id: 'c', amount: 100 }, |
| 55 | +]; |
| 56 | + |
| 57 | +type Path = 'native' | 'rows'; |
| 58 | + |
| 59 | +/** A driver of one `having` path's shape, counting every read of the object. */ |
| 60 | +function makeDriver(path: Path, rows: ReadonlyArray<Record<string, unknown>>) { |
| 61 | + const reads = { aggregate: 0, find: 0 }; |
| 62 | + const driver: any = { |
| 63 | + name: `${path}-recorder`, |
| 64 | + version: '0.0.0', |
| 65 | + supports: {}, |
| 66 | + async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; }, |
| 67 | + async find() { reads.find += 1; return rows.map((r) => ({ ...r })); }, |
| 68 | + async findOne() { return null; }, |
| 69 | + async create(_o: string, d: any) { return d; }, |
| 70 | + async update(_o: string, _id: string, d: any) { return d; }, |
| 71 | + async delete() { return true; }, |
| 72 | + async count() { return rows.length; }, |
| 73 | + async bulkCreate(_o: string, r: any[]) { return r; }, |
| 74 | + async bulkUpdate() { return []; }, async bulkDelete() {}, |
| 75 | + async beginTransaction() { return { commit: async () => {}, rollback: async () => {} }; }, |
| 76 | + async commit() {}, async rollback() {}, |
| 77 | + }; |
| 78 | + if (path === 'native') { |
| 79 | + driver.aggregate = async (_o: string, ast: any) => { reads.aggregate += 1; return applyInMemoryAggregation([...rows], ast); }; |
| 80 | + } |
| 81 | + return { driver, reads }; |
| 82 | +} |
| 83 | + |
| 84 | +async function makeEngine(path: Path, rows: ReadonlyArray<Record<string, unknown>> = ROWS) { |
| 85 | + const { driver, reads } = makeDriver(path, rows); |
| 86 | + const engine = new ObjectQL(); |
| 87 | + engine.registerDriver(driver, true); |
| 88 | + await engine.init(); |
| 89 | + (engine.registry as any).registerObject({ name: OBJECT, fields: FIELDS }); |
| 90 | + return { engine, reads }; |
| 91 | +} |
| 92 | + |
| 93 | +/** `s` sums `amount` over the rows `filter` selects; `n` counts every row. Always the rows path. */ |
| 94 | +const filterQuery = (filter: unknown): EngineAggregateOptions => ({ |
| 95 | + aggregations: [ |
| 96 | + { function: 'count', alias: 'n' }, |
| 97 | + { function: 'sum', field: 'amount', alias: 's', filter: filter as FilterCondition }, |
| 98 | + ], |
| 99 | +}); |
| 100 | + |
| 101 | +/** Groups by `name`; on the rows path a filtered aggregation forces the engine's own lowering. */ |
| 102 | +const havingQuery = (path: Path, having: unknown): EngineAggregateOptions => ({ |
| 103 | + groupBy: ['name'], |
| 104 | + aggregations: path === 'native' |
| 105 | + ? [{ function: 'count', alias: 'n' }] |
| 106 | + : [{ function: 'count', alias: 'n' }, { function: 'count', alias: 'all', filter: { amount: { $gte: 0 } } }], |
| 107 | + having: having as FilterCondition, |
| 108 | +}); |
| 109 | + |
| 110 | +interface Refusal extends Error { code?: unknown; status?: unknown } |
| 111 | + |
| 112 | +async function refusalOf(run: () => Promise<unknown>): Promise<Refusal> { |
| 113 | + try { |
| 114 | + await run(); |
| 115 | + } catch (e) { |
| 116 | + return e as Refusal; |
| 117 | + } |
| 118 | + throw new Error('expected engine.aggregate to refuse this clause, but it answered'); |
| 119 | +} |
| 120 | + |
| 121 | +function syncRefusalOf(run: () => unknown): Refusal { |
| 122 | + try { |
| 123 | + run(); |
| 124 | + } catch (e) { |
| 125 | + return e as Refusal; |
| 126 | + } |
| 127 | + throw new Error('expected a refusal, but the clause was answered'); |
| 128 | +} |
| 129 | + |
| 130 | +/** |
| 131 | + * The comparand doors' refusal: the ADR-0112 identity, then the drivers' first |
| 132 | + * three sentences — the operator, the field, what arrived (in the drivers' |
| 133 | + * `describeFilterOperand (safeShapePreview)` rendering) and where, and the |
| 134 | + * declaration — then the reason that names the backends. |
| 135 | + */ |
| 136 | +function expectFlagRefusal(err: Refusal, op: '$exists' | '$null', received: string, at: string): void { |
| 137 | + expect(err.code, err.message).toBe('INVALID_FILTER'); |
| 138 | + expect(err.status, err.message).toBe(400); |
| 139 | + expect(err.message.startsWith( |
| 140 | + `Operator "${op}" on field "name" requires a boolean comparand (true or false). ` |
| 141 | + + `Received ${received} at ${at}. ` |
| 142 | + + `@objectstack/spec FieldOperatorsSchema declares ${op} as a boolean. It is refused rather than coerced `, |
| 143 | + ), err.message).toBe(true); |
| 144 | + expect(err.message).toContain('OPPOSITE directions'); |
| 145 | + expect(err.message).toContain('Note "false" the STRING is truthy'); |
| 146 | +} |
| 147 | + |
| 148 | +const OPS = ['$exists', '$null'] as const; |
| 149 | + |
| 150 | +/** The card's five non-booleans, each with the rendering the drivers give it. */ |
| 151 | +const NON_BOOLEANS: ReadonlyArray<readonly [string, unknown, string]> = [ |
| 152 | + ['"yes"', 'yes', 'string ("yes")'], |
| 153 | + ['1', 1, 'number (1)'], |
| 154 | + ['"false" (truthy)', 'false', 'string ("false")'], |
| 155 | + ['0', 0, 'number (0)'], |
| 156 | + ['null', null, 'null (null)'], |
| 157 | + // Not one of the card's five, but reaching the same gate: no earlier door |
| 158 | + // refuses a list here, so it met the old `!!target` read like any other value. |
| 159 | + ['[true] (a list)', [true], 'array ([true])'], |
| 160 | +]; |
| 161 | + |
| 162 | +describe('[#20981] a non-boolean $exists / $null — refused before any driver read, on both positions', () => { |
| 163 | + for (const op of OPS) { |
| 164 | + for (const [label, comparand, received] of NON_BOOLEANS) { |
| 165 | + it(`aggregations[1].filter ${op}: ${label} — 400, the drivers' words, on an empty and a populated table`, async () => { |
| 166 | + for (const rows of [[], ROWS]) { |
| 167 | + const { engine, reads } = await makeEngine('rows', rows); |
| 168 | + const err = await refusalOf(() => engine.aggregate(OBJECT, filterQuery({ name: { [op]: comparand } }))); |
| 169 | + expectFlagRefusal(err, op, received, `aggregations[1].filter.name.${op}`); |
| 170 | + expect(reads, 'the verdict is the filter\'s: no row was read').toEqual({ aggregate: 0, find: 0 }); |
| 171 | + } |
| 172 | + }); |
| 173 | + |
| 174 | + it(`having ${op}: ${label} — 400, the drivers' words, on both having paths`, async () => { |
| 175 | + for (const path of ['native', 'rows'] as const) { |
| 176 | + for (const rows of [[], ROWS]) { |
| 177 | + const { engine, reads } = await makeEngine(path, rows); |
| 178 | + const err = await refusalOf(() => engine.aggregate(OBJECT, havingQuery(path, { name: { [op]: comparand } }))); |
| 179 | + expectFlagRefusal(err, op, received, `having.name.${op}`); |
| 180 | + expect(reads, `${path}: no row was read`).toEqual({ aggregate: 0, find: 0 }); |
| 181 | + } |
| 182 | + } |
| 183 | + }); |
| 184 | + } |
| 185 | + } |
| 186 | + |
| 187 | + // Where the flag sits must not change the verdict: the walk is row-independent, |
| 188 | + // so a branch the per-row walk would short-circuit past is judged too. |
| 189 | + const POSITIONS: ReadonlyArray<readonly [string, (flag: Record<string, unknown>) => Record<string, unknown>, string]> = [ |
| 190 | + ['nested in $and', (flag) => ({ $and: [{ amount: { $gte: 0 } }, { name: flag }] }), '.$and[1].name'], |
| 191 | + ['behind a $or branch that already holds', (flag) => ({ $or: [{ amount: { $gte: 0 } }, { name: flag }] }), '.$or[1].name'], |
| 192 | + ['under $not', (flag) => ({ $not: { name: flag } }), '.$not.name'], |
| 193 | + ['beside a boolean sibling flag', (flag) => ({ name: { $empty: false, ...flag } }), '.name'], |
| 194 | + ]; |
| 195 | + |
| 196 | + for (const [name, wrap, tail] of POSITIONS) { |
| 197 | + it(`${name}: refused at that position, in the per-aggregation filter and in having`, async () => { |
| 198 | + const filterEngine = await makeEngine('rows'); |
| 199 | + expectFlagRefusal( |
| 200 | + await refusalOf(() => filterEngine.engine.aggregate(OBJECT, filterQuery(wrap({ $exists: 'yes' })))), |
| 201 | + '$exists', 'string ("yes")', `aggregations[1].filter${tail}.$exists`, |
| 202 | + ); |
| 203 | + // `having` reads the aggregated row, which carries `name` (the groupBy) |
| 204 | + // but not `amount` — so its wrapper's other branch tests `n` instead. |
| 205 | + const having = JSON.parse(JSON.stringify(wrap({ $null: 1 })).replaceAll('"amount"', '"n"')); |
| 206 | + const havingEngine = await makeEngine('native'); |
| 207 | + expectFlagRefusal( |
| 208 | + await refusalOf(() => havingEngine.engine.aggregate(OBJECT, havingQuery('native', having))), |
| 209 | + '$null', 'number (1)', `having${tail}.$null`, |
| 210 | + ); |
| 211 | + expect(filterEngine.reads.find + havingEngine.reads.aggregate).toBe(0); |
| 212 | + }); |
| 213 | + } |
| 214 | + |
| 215 | + it('a { $field } reference in a flag\'s slot is a non-boolean first, as in $empty\'s', async () => { |
| 216 | + const { engine } = await makeEngine('rows'); |
| 217 | + expectFlagRefusal( |
| 218 | + await refusalOf(() => engine.aggregate(OBJECT, filterQuery({ name: { $exists: { $field: 'amount' } } }))), |
| 219 | + '$exists', 'object ({"$field":"amount"})', 'aggregations[1].filter.name.$exists', |
| 220 | + ); |
| 221 | + const native = await makeEngine('native'); |
| 222 | + expectFlagRefusal( |
| 223 | + await refusalOf(() => native.engine.aggregate(OBJECT, havingQuery('native', { name: { $null: { $field: 'n' } } }))), |
| 224 | + '$null', 'object ({"$field":"n"})', 'having.name.$null', |
| 225 | + ); |
| 226 | + }); |
| 227 | +}); |
| 228 | + |
| 229 | +describe('[#20981] the control — true / false answer exactly as before, on both positions and both paths', () => { |
| 230 | + // `a` (10) has a value; `b` (1) and `c` (100) have none. |
| 231 | + const SUMS: ReadonlyArray<readonly [string, Record<string, unknown>, number]> = [ |
| 232 | + ['$exists: true', { $exists: true }, 10], |
| 233 | + ['$exists: false', { $exists: false }, 101], |
| 234 | + ['$null: true', { $null: true }, 101], |
| 235 | + ['$null: false', { $null: false }, 10], |
| 236 | + ]; |
| 237 | + |
| 238 | + for (const [name, flag, sum] of SUMS) { |
| 239 | + it(`aggregations[1].filter ${name} sums ${sum}`, async () => { |
| 240 | + const { engine, reads } = await makeEngine('rows'); |
| 241 | + expect(await engine.aggregate(OBJECT, filterQuery({ name: flag }))).toEqual([{ n: 3, s: sum }]); |
| 242 | + expect(reads.find).toBe(1); |
| 243 | + }); |
| 244 | + |
| 245 | + const kept = sum === 10 ? ['won'] : ['null']; |
| 246 | + it(`having ${name} keeps the ${kept[0]} group on both paths`, async () => { |
| 247 | + for (const path of ['native', 'rows'] as const) { |
| 248 | + const { engine } = await makeEngine(path); |
| 249 | + const groups = await engine.aggregate(OBJECT, havingQuery(path, { name: flag })); |
| 250 | + expect(groups.map((g: any) => String(g.name)), path).toEqual(kept); |
| 251 | + } |
| 252 | + }); |
| 253 | + } |
| 254 | +}); |
| 255 | + |
| 256 | +describe('[#20981] the published row evaluators refuse per row — the floor for a caller that evaluates rows itself', () => { |
| 257 | + it('applyInMemoryAggregation with fields: $exists "yes" and $null 1 are refused, true / false answer', () => { |
| 258 | + for (const [op, comparand, received] of [['$exists', 'yes', 'string ("yes")'], ['$null', 1, 'number (1)']] as const) { |
| 259 | + const err = syncRefusalOf(() => applyInMemoryAggregation( |
| 260 | + ROWS.map((r) => ({ ...r })), |
| 261 | + filterQuery({ name: { [op]: comparand } }), |
| 262 | + undefined, |
| 263 | + FIELDS, |
| 264 | + )); |
| 265 | + expectFlagRefusal(err, op, received, `aggregations[1].filter.name.${op}`); |
| 266 | + } |
| 267 | + expect(applyInMemoryAggregation(ROWS.map((r) => ({ ...r })), filterQuery({ name: { $exists: true } }), undefined, FIELDS)) |
| 268 | + .toEqual([{ n: 3, s: 10 }]); |
| 269 | + expect(applyInMemoryAggregation(ROWS.map((r) => ({ ...r })), filterQuery({ name: { $null: true } }), undefined, FIELDS)) |
| 270 | + .toEqual([{ n: 3, s: 101 }]); |
| 271 | + }); |
| 272 | + |
| 273 | + it('applyHaving and matchesAggregationFilter refuse a row that reaches the flag, with or without a value', () => { |
| 274 | + const groups = [{ name: 'won', n: 1 }, { name: null, n: 2 }]; |
| 275 | + for (const row of groups) { |
| 276 | + expectFlagRefusal(syncRefusalOf(() => applyHaving([row], { name: { $exists: 'false' } } as never)), |
| 277 | + '$exists', 'string ("false")', 'having.name.$exists'); |
| 278 | + expectFlagRefusal(syncRefusalOf(() => matchesAggregationFilter(row, { name: { $null: 0 } } as never, 2)), |
| 279 | + '$null', 'number (0)', 'aggregations[2].filter.name.$null'); |
| 280 | + } |
| 281 | + expect(applyHaving(groups, { name: { $null: false } })).toEqual([{ name: 'won', n: 1 }]); |
| 282 | + expect(applyHaving(groups, { name: { $exists: false } })).toEqual([{ name: null, n: 2 }]); |
| 283 | + }); |
| 284 | +}); |
0 commit comments