diff --git a/.changeset/21426-native-number-comparand.md b/.changeset/21426-native-number-comparand.md new file mode 100644 index 00000000000..649e6e4b10f --- /dev/null +++ b/.changeset/21426-native-number-comparand.md @@ -0,0 +1,19 @@ +--- +'@objectstack/service-analytics': minor +--- + +The analytics native-SQL path judges a comparand against a declared number field by the platform's number-comparand rule, the one the data engine's `where` already applies + +Clause-②: no (narrowing) + + + +**BREAKING**: this narrows what the analytics native-SQL face accepts. A query or dataset that compares a declared number field with a comparand the number-comparand rule refuses used to answer 200 with a count on the native face (a 500 on PostgreSQL for a non-numeric string). It now refuses `INVALID_FILTER` / 400 before any statement runs, which is what the engine-aggregate face already answered. It ships as `minor` under the launch-window convention for accept-set narrowings. No export, type or error code changes. + +- **What changed.** A comparand against a `number`, `currency`, `percent`, `rating`, `slider`, `progress` or `summary` column is judged by `numberComparandDoorVerdict` from `@objectstack/spec/data` before the native statement compiles. This covers the query's `where` (including the dataset query's `runtimeFilter`, which is merged into it), each measure's own `filter` and a dataset's own `filter`. The rule runs in the same pass as the boolean rule. + - A numeric string (`'12'`, `'1e3'`) is bound as the number it names, which is what the engine binds. + - Anything else the rule refuses (a string with no numeric reading such as `'abc'`, `''` or `'+5'`, a boolean, or a list where one number belongs) is refused `INVALID_FILTER` / 400 with the rule's own message, before any statement runs. + - A relationship-path member is judged at the related object's declared column. +- **Before.** The native strategy bound the comparand as written. So `{ amount: 'abc' }` counted no rows on SQLite and answered a 500 on PostgreSQL, `{ amount: true }` bound `1` and answered 200, and `{ amount: { $lte: '9999-12-31' } }` counted every row. The engine-aggregate strategy refused all three with 400. +- **What you may notice.** An analytics query or dataset that compared a number field with a value outside the rule's accepted set now refuses instead of answering. Write a number, or a string of exactly that number's JSON spelling (`'12'`). +- **Unchanged.** A number, `null` (the null test), a `{ $field }` reference, a column that is not a number or a boolean, and a host that relays no declared field types (nothing is judged without one). diff --git a/packages/services/service-analytics/src/__tests__/native-sql-number-comparand-door.test.ts b/packages/services/service-analytics/src/__tests__/native-sql-number-comparand-door.test.ts new file mode 100644 index 00000000000..43766765815 --- /dev/null +++ b/packages/services/service-analytics/src/__tests__/native-sql-number-comparand-door.test.ts @@ -0,0 +1,446 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#21426] The native-SQL strategy answers a comparand against a declared + * NUMBER column what the engine's `where` door answers: a numeric string + * (`'12'`, `'1e1'`) narrows to its number, and a string the platform's + * numeric grammar does not read (`'abc'`, `''`, `'+5'`), a boolean or an array + * is refused `INVALID_FILTER` / 400 before a statement runs. The number arm + * joins the boolean arm's walk (#21376): one walk, two arms, the same three + * positions. + * + * ## Measured on the base (`3a6d92f78`), through these doors + * + * Three rows (`amount` 5, 12, 30), counted by the cube read + * (`AnalyticsService.query`, what `POST /api/v1/analytics/query` relays) and + * the dataset door (`AnalyticsService.queryDataset`, what + * `POST /api/v1/analytics/dataset/query` relays, its `runtimeFilter` merged + * into the `where`), each on two faces of the plugin's own composition: the + * native strategy (one raw statement) and the ObjectQL strategy, whose + * `engine.aggregate` runs the engine's `where` door — the target column. + * + * | filter | native, SQLite | native, PostgreSQL 16 | engine door | + * |:--|:--|:--|:--| + * | `{ amount: 'abc' }` | **200, 0** | **500 `DATABASE_ERROR`** | 400 `INVALID_FILTER` | + * | `{ amount: { $lte: '9999-12-31' } }` | **200, 3** | **200, 3** | 400 | + * | `{ amount: { $ne: 'abc' } }` | **200, 3** | **500** | 400 | + * | `{ amount: true }` | **200, 0** | **200, 0** | 400 | + * | `{ amount: 12 }` (the control) | 1 | 1 | 1 | + * | `{ amount: '12' }` | 1, binds `'12'` | 1, binds `'12'` | 1, binds `12` | + * + * The same refusals were missing from each measure's own `filter` and the + * dataset's own scope (a 200 on SQLite, a 500 on PostgreSQL), and the bare-day + * `$lte` met the native window rule for a calendar day at the top of the + * range and bound nothing at all. The strategy now runs the spec's verdict + * (`numberComparandDoorVerdict`) on every filter it compiles. Every parity + * cell below asserts the engine face's answer AND the native face's equality + * with it, and which strategy answered; every narrowing cell asserts the + * native statement bound the numbers the engine handed its driver. + * + * ## Two cells the engine face does not answer with this verdict + * + * Pinned on the native face alone ({@link NATIVE_ONLY_CELLS}), each measured + * on both faces: + * + * - A relationship-path member (`account_credit`, the cube dimension over + * `account.credit`): the engine face refuses every cross-object filter + * (`INVALID_FIELD` / 400, "cannot evaluate a cross-object filter"), so it + * never reaches a comparand. The native face joins, and judges the member at + * the related object's declared column. + * - An array at an ordering operator (`$gt: [10]`): the shared analytics + * lowering hands the engine the list's first member, so the engine door + * never sees the array (the engine face answered 200, 2). The spec's verdict + * refuses a list where one number belongs, and the native face answers it. + * + * Each filter handed in is deep-frozen, and so is each registered dataset's + * own `filter` and its measures' `filter`s: narrowing is copy-on-write, so an + * edit in place would throw. + * + * The PostgreSQL cell runs where `OS_TEST_POSTGRES_URL` is set and is a named + * skip otherwise; no CI step provisions that variable for this package. The + * live cell owns its tables, dropped before and after. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import type { Cube } from '@objectstack/spec/data'; +import type { AnalyticsService } from '../analytics-service.js'; +import { AnalyticsServicePlugin } from '../plugin.js'; + +const OBJECT = 'os21426_amount_ledger'; +const ACCOUNT = 'os21426_amount_account'; + +const ACCOUNT_DEF = { + name: ACCOUNT, + label: 'Number comparand account', + fields: { + name: { name: 'name', type: 'text' as const }, + credit: { name: 'credit', type: 'number' as const }, + }, +}; + +const LEDGER = { + name: OBJECT, + label: 'Number comparand ledger', + fields: { + note: { name: 'note', type: 'text' as const }, + amount: { name: 'amount', type: 'number' as const }, + price: { name: 'price', type: 'currency' as const }, + share: { name: 'share', type: 'percent' as const }, + account: { name: 'account', type: 'lookup' as const, reference: ACCOUNT }, + }, +}; + +const ACCOUNTS = [ + { id: 'a1', name: 'A', credit: 50 }, + { id: 'a2', name: 'B', credit: 500 }, +] as const; + +const ROWS = [ + { id: 'r5', note: 'n', amount: 5, price: 5, share: 0.1, account: 'a1' }, + { id: 'r12', note: 'n', amount: 12, price: 12, share: 0.5, account: 'a2' }, + { id: 'r30', note: 'n', amount: 30, price: 30, share: 0.9, account: 'a2' }, +] as const; + +const CUBE: Cube = { + name: 'os21426_amount_cube', + title: 'Number comparand cube', + sql: OBJECT, + public: true, + measures: { row_count: { type: 'count', sql: '*', label: 'Rows' } }, + dimensions: { + note: { type: 'string', sql: 'note', label: 'Note' }, + amount: { type: 'number', sql: 'amount', label: 'Amount' }, + price: { type: 'number', sql: 'price', label: 'Price' }, + share: { type: 'number', sql: 'share', label: 'Share' }, + account_credit: { type: 'number', sql: 'account.credit', label: 'Account credit' }, + }, +} as Cube; + +/** The inline dataset the dataset door carries. */ +const INLINE = { + name: 'os21426_amount_inline', + label: 'Number comparand inline dataset', + object: OBJECT, + dimensions: [{ name: 'note', field: 'note', type: 'string' }], + measures: [{ name: 'row_count', aggregate: 'count' }], +}; + +function deepFreeze(value: T): T { + if (value !== null && typeof value === 'object' && !Object.isFrozen(value)) { + Object.freeze(value); + for (const child of Object.values(value as Record)) deepFreeze(child); + } + return value; +} + +/** A registered dataset, frozen: its own scope and its measures' filters are the positions under test. */ +const registeredDataset = (name: string, filter: unknown, measureFilter: unknown) => deepFreeze({ + name, + label: name, + object: OBJECT, + ...(filter === undefined ? {} : { filter }), + dimensions: [{ name: 'note', field: 'note', type: 'string' }], + measures: [ + { name: 'row_count', aggregate: 'count' }, + ...(measureFilter === undefined ? [] : [{ name: 'scoped_count', aggregate: 'count', filter: measureFilter }]), + ], +}); + +/** The control: a numeric string in each position narrows, and both faces count alike. */ +const REGISTERED_NARROWED = registeredDataset('os21426_amount_narrowed', { amount: { $gt: '10' } }, { amount: { $lte: '12' } }); +/** One refused comparand per position. */ +const REGISTERED_REFUSED = [ + registeredDataset('os21426_amount_scope_refused', { amount: 'abc' }, undefined), + registeredDataset('os21426_amount_measure_refused', undefined, { amount: { $ne: 'abc' } }), + registeredDataset('os21426_amount_measure_boolean', undefined, { amount: true }), +] as const; + +type Refusal = { code: string; status: number }; +type Answer = number | Refusal; +const REFUSED: Refusal = { code: 'INVALID_FILTER', status: 400 }; + +/** Each filter, the engine door's answer, and the doors it is asked at. */ +const CELLS: ReadonlyArray = [ + // The card's four cells. + [{ amount: 'abc' }, REFUSED, 'no numeric reading'], + [{ amount: { $lte: '9999-12-31' } }, REFUSED, 'a date where a number belongs'], + [{ amount: { $ne: 'abc' } }, REFUSED, 'the negation of a non-number'], + [{ amount: true }, REFUSED, 'a boolean'], + // The controls: a number. + [{ amount: 12 }, 1, 'a number'], + [{ amount: { $gt: 10 } }, 2, 'a number under $gt'], + // Narrowed: the platform's numeric grammar reads each. + [{ amount: '12' }, 1, 'a numeric string'], + [{ amount: { $gt: '10' } }, 2, 'a numeric string under $gt'], + [{ amount: { $gt: '1e1' } }, 2, 'an exponent spelling'], + [{ amount: { $in: ['12', 30] } }, 2, 'a numeric string as a list member'], + [{ amount: { $between: ['10', '40'] } }, 2, 'numeric strings as range bounds'], + [{ $not: { amount: '12' } }, 2, 'under $not'], + [{ price: { $gt: '10' } }, 2, 'a numeric string on a currency field'], + [{ share: { $gt: '0.2' } }, 2, 'a numeric string on a percent field'], + // Refused: no numeric reading, or not a number at all. + [{ amount: false }, REFUSED, 'the other boolean'], + [{ amount: '' }, REFUSED, 'a blank string'], + [{ amount: { $gt: '+5' } }, REFUSED, 'a spelling JSON does not admit'], + [{ amount: { $in: [12, 'abc'] } }, REFUSED, 'a list member with no numeric reading'], + [{ $or: [{ amount: 'abc' }, { note: 'x' }] }, REFUSED, 'under $or'], + [{ price: 'abc' }, REFUSED, 'a currency field'], + [{ share: 'abc' }, REFUSED, 'a percent field'], + // Not the verdict's subject: the null tests. + [{ amount: null }, 0, 'the null test'], + [{ amount: { $ne: null } }, 3, 'the negated null test'], + [{ amount: { $exists: true } }, 3, 'a flag operator'], +]; + +/** + * Cells whose engine face is not this verdict's answer (see the module + * header): the native face's answer alone, with the values it must bind. + */ +const NATIVE_ONLY_CELLS: ReadonlyArray = [ + [{ account_credit: 'abc' }, REFUSED, null, 'a relationship path, judged at the related object\'s number column'], + [{ account_credit: { $gt: '100' } }, 2, [100], 'a relationship path, narrowed at the related object\'s number column'], + [{ amount: { $gt: [10] } }, REFUSED, null, 'a list where one number belongs'], +]; + +/** Narrowing cells: the native statement binds exactly the numbers the engine handed its driver. */ +const NARROWING_CELLS: ReadonlyArray = [ + [{ amount: '12' }, 1, [12]], + [{ amount: { $gt: '1e1' } }, 2, [10]], + [{ amount: { $in: ['12', 30] } }, 2, [12, 30]], + [{ amount: { $between: ['10', '40'] } }, 2, [10, 40]], + [{ price: { $gt: '10' } }, 2, [10]], + [{ share: { $gt: '0.2' } }, 2, [0.2]], +]; + +interface Cell { + id: 'sqlite' | 'pg'; + label: string; + env: string | null; + config: () => Record | null; +} + +const DRIVER_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), + }, +]; + +const quiet = { debug() {}, info() {}, warn() {}, error() {}, child() { return quiet; } }; + +type Face = 'native' | 'engine'; + +/** The member a refusal must name: the judged key the filter spells. */ +const memberOf = (filter: unknown): string => { + const json = JSON.stringify(filter); + return [`${CUBE.name}.amount`, 'account_credit', 'price', 'share', 'amount'].find((m) => json.includes(`"${m}"`)) ?? 'amount'; +}; + +/** Every number or string a filter carries, in document order — the values a driver binds. */ +function boundLeaves(node: unknown): unknown[] { + if (typeof node === 'number' || typeof node === 'string') return [node]; + if (Array.isArray(node)) return node.flatMap(boundLeaves); + if (node !== null && typeof node === 'object') return Object.values(node).flatMap(boundLeaves); + return []; +} + +for (const cell of DRIVER_CELLS) { + const config = cell.config(); + describe.skipIf(!config)( + `[#21426] analytics native SQL — a number comparand answers what the engine door answers (${cell.label})${config ? '' : ` (skipped: set ${cell.env} to run this cell)`}`, + () => { + let driver: any; + let engine: ObjectQL; + /** Raw-SQL statements and engine aggregates that read THIS object, and what each bound. */ + const reads = { rawSql: 0, aggregate: 0, rawArgs: [] as unknown[][], driverWhere: [] as unknown[] }; + const services: Partial> = {}; + + const dropTables = async () => { + if (cell.id !== 'pg') return; + await driver?.execute(`drop table if exists ${OBJECT}`).catch(() => {}); + await driver?.execute(`drop table if exists ${ACCOUNT}`).catch(() => {}); + }; + + /** The total count a face answers, or its refusal envelope, and what read the object. */ + const ask = async (face: Face, run: (svc: AnalyticsService) => Promise<{ rows: unknown[] }>) => { + const before = { rawSql: reads.rawSql, aggregate: reads.aggregate }; + reads.rawArgs.length = 0; + reads.driverWhere.length = 0; + const answer = await run(services[face]!).then( + (res) => (res.rows as Array>).reduce((sum, r) => sum + Number(r.row_count ?? 0), 0) as Answer, + (e: Error & { code?: string; status?: number }) => ({ code: String(e.code), status: Number(e.status), message: e.message }) as Answer, + ); + return { + answer, + rawSql: reads.rawSql - before.rawSql, + aggregate: reads.aggregate - before.aggregate, + rawArgs: reads.rawArgs.flat(), + driverLeaves: reads.driverWhere.flatMap(boundLeaves), + }; + }; + + const cubeRead = (where: unknown) => (svc: AnalyticsService) => + svc.query({ cube: CUBE.name, measures: ['row_count'], where: deepFreeze(structuredClone(where)) } as never) as Promise<{ rows: unknown[] }>; + const datasetRead = (runtimeFilter: unknown) => (svc: AnalyticsService) => + svc.queryDataset(INLINE as never, { measures: ['row_count'], dimensions: ['note'], runtimeFilter: deepFreeze(structuredClone(runtimeFilter)) } as never) as Promise<{ rows: unknown[] }>; + + const strip = (a: Answer) => (typeof a === 'number' ? a : { code: a.code, status: a.status }); + + /** The native face's answer at one door, and — refused — that no statement ran and the words name the member. */ + const expectNative = async (door: (filter: unknown) => (svc: AnalyticsService) => Promise<{ rows: unknown[] }>, filter: unknown, expected: Answer, member: string) => { + const viaNative = await ask('native', door(filter)); + expect(strip(viaNative.answer), 'the native strategy').toEqual(expected); + if (typeof expected === 'number') { + expect(viaNative.rawSql, 'NativeSQLStrategy answered').toBeGreaterThanOrEqual(1); + expect(viaNative.aggregate, 'no engine aggregate on the native face').toBe(0); + } else { + expect(viaNative.rawSql, 'refused before any statement ran').toBe(0); + const message = (viaNative.answer as { message?: string }).message ?? ''; + expect(message).toContain(`'${member}'`); + expect(message).toContain('where'); + } + return viaNative; + }; + + /** Both faces at one door: the engine face answers `engine`, the native face the same. */ + const expectBothFaces = async (door: (filter: unknown) => (svc: AnalyticsService) => Promise<{ rows: unknown[] }>, filter: unknown, engineAnswer: Answer, member = memberOf(filter)) => { + const viaEngine = await ask('engine', door(filter)); + expect(strip(viaEngine.answer), 'the engine door').toEqual(engineAnswer); + expect(viaEngine.rawSql, 'the engine face ran no raw statement').toBe(0); + const viaNative = await expectNative(door, filter, engineAnswer, member); + return { viaEngine, viaNative }; + }; + + beforeAll(async () => { + driver = new SqlDriver(config as any); + await dropTables(); + engine = new ObjectQL({ logger: quiet } as any); + engine.registerDriver(driver, true); + await engine.init(); + engine.registry.registerObject(ACCOUNT_DEF as any); + engine.registry.registerObject(LEDGER as any); + await engine.syncSchemas(); + for (const row of ACCOUNTS) await engine.insert(ACCOUNT, { ...row } as any); + for (const row of ROWS) await engine.insert(OBJECT, { ...row } as any); + + const realExecute = (engine as any).execute.bind(engine); + (engine as any).execute = (sql: unknown, opts?: { object?: string; args?: unknown[] }) => { + if (opts?.object === OBJECT) { + reads.rawSql += 1; + reads.rawArgs.push([...(opts.args ?? [])]); + } + return realExecute(sql, opts); + }; + const realAggregate = engine.aggregate.bind(engine); + (engine as any).aggregate = (object: string, ...rest: unknown[]) => { + if (object === OBJECT) reads.aggregate += 1; + return (realAggregate as any)(object, ...rest); + }; + // What the engine door handed its driver — the target the native statement's binds are held to. + const realDriverAggregate = driver.aggregate.bind(driver); + driver.aggregate = (object: string, query: { where?: unknown }, ...rest: unknown[]) => { + if (object === OBJECT) reads.driverWhere.push(query?.where); + return realDriverAggregate(object, query, ...rest); + }; + + // The plugin's own composition over the real engine: both auto-bridges + // (`native`), and the same narrowed to the engine-aggregate path (`engine`). + for (const [face, caps] of [ + ['native', undefined], + ['engine', () => ({ nativeSql: false, objectqlAggregate: true, inMemory: false })], + ] as const) { + const registered: Record = {}; + await new AnalyticsServicePlugin({ cubes: [CUBE], ...(caps ? { queryCapabilities: caps } : {}) } as any).init({ + getService: (name: string) => (name === 'data' ? engine : registered[name]), + registerService: (name: string, svc: unknown) => { registered[name] = svc; }, + replaceService: (name: string, svc: unknown) => { registered[name] = svc; }, + hook: () => {}, + logger: quiet, + } as never); + services[face] = registered.analytics as AnalyticsService; + for (const dataset of [REGISTERED_NARROWED, ...REGISTERED_REFUSED]) services[face]!.registerDataset(dataset as never); + } + }); + + afterAll(async () => { + await dropTables(); + try { await engine?.destroy(); } catch { /* noop */ } + }); + + describe('the cube read — the `where` of POST /analytics/query', () => { + for (const [filter, engineAnswer, label] of CELLS) { + it(`${JSON.stringify(filter)} (${label}) answers ${JSON.stringify(engineAnswer)}`, async () => { + await expectBothFaces(cubeRead, filter, engineAnswer); + }); + } + + it('the FilterArray spelling and the cube-qualified member are judged too', async () => { + await expectBothFaces(cubeRead, [['amount', '=', 'abc']], REFUSED); + await expectBothFaces(cubeRead, [['amount', '>', '10']], 2); + await expectBothFaces(cubeRead, { [`${CUBE.name}.amount`]: 'abc' }, REFUSED); + }); + + it('a numeric string binds the number the engine handed its driver, never the string', async () => { + for (const [filter, count, binds] of NARROWING_CELLS) { + const { viaEngine, viaNative } = await expectBothFaces(cubeRead, filter, count); + expect(viaEngine.driverLeaves, `${JSON.stringify(filter)}: what the engine door handed its driver`).toEqual(binds); + expect(viaNative.rawArgs, `${JSON.stringify(filter)}: what the native statement bound`).toEqual(binds); + } + }); + + for (const [filter, nativeAnswer, binds, label] of NATIVE_ONLY_CELLS) { + it(`${JSON.stringify(filter)} (${label}) answers ${JSON.stringify(nativeAnswer)} on the native face`, async () => { + const viaNative = await expectNative(cubeRead, filter, nativeAnswer, memberOf(filter)); + if (binds) expect(viaNative.rawArgs, 'what the native statement bound').toEqual(binds); + }); + } + }); + + describe('the dataset door — the `runtimeFilter` of POST /api/v1/analytics/dataset/query', () => { + for (const [filter, engineAnswer, label] of CELLS) { + it(`${JSON.stringify(filter)} (${label}) answers ${JSON.stringify(engineAnswer)}`, async () => { + await expectBothFaces(datasetRead, filter, engineAnswer); + }); + } + }); + + describe("a registered dataset's own scope and measure filter, read by the cube door", () => { + it("`filter: { amount: { $gt: '10' } }` scopes to two rows, and `filter: { amount: { $lte: '12' } }` on a measure counts one of them — both bound as numbers", async () => { + for (const face of ['engine', 'native'] as const) { + const before = reads.rawSql; + reads.rawArgs.length = 0; + const res = await services[face]!.query({ cube: REGISTERED_NARROWED.name, measures: ['row_count', 'scoped_count'] } as never) as { rows: Array> }; + const total = (m: string) => res.rows.reduce((sum, r) => sum + Number(r[m] ?? 0), 0); + expect(total('row_count'), `${face}: the dataset scope keeps two rows`).toBe(2); + expect(total('scoped_count'), `${face}: the measure filter keeps one of them`).toBe(1); + if (face === 'native') { + expect(reads.rawSql - before, 'NativeSQLStrategy answered').toBeGreaterThanOrEqual(1); + const bound = reads.rawArgs.flat(); + expect(bound, 'the scope and the measure filter bind numbers').toEqual(expect.arrayContaining([10, 12])); + expect(bound.filter((v) => typeof v === 'string'), 'no numeric string reached the statement').toEqual([]); + } + } + }); + + for (const dataset of REGISTERED_REFUSED) { + it(`${dataset.name}: ${JSON.stringify(dataset.filter ?? dataset.measures[1]?.filter)} is refused on both faces`, async () => { + for (const face of ['engine', 'native'] as const) { + const before = reads.rawSql; + const answer = await services[face]!.query({ cube: dataset.name, measures: dataset.measures.map((m) => m.name) } as never).then( + () => 'answered' as const, + (e: Error & { code?: string; status?: number }) => ({ code: String(e.code), status: Number(e.status) }), + ); + expect(answer, `${face}: refused`).toEqual(REFUSED); + expect(reads.rawSql - before, `${face}: no statement ran`).toBe(0); + } + }); + } + }); + }, + ); +} diff --git a/packages/services/service-analytics/src/strategies/native-sql-strategy.ts b/packages/services/service-analytics/src/strategies/native-sql-strategy.ts index 1520c77cff3..b007a602d7b 100644 --- a/packages/services/service-analytics/src/strategies/native-sql-strategy.ts +++ b/packages/services/service-analytics/src/strategies/native-sql-strategy.ts @@ -15,17 +15,24 @@ import { SQL_CONST_TRUE, type NormalizedFilterNode, } from './filter-normalizer.js'; -// [#21376] The boolean-comparand verdict the engine's `where` door consults -// (`@objectstack/objectql`'s `boolean-comparand-declared-type-door.ts`), read -// from the same spec module — one verdict, one set of accepted spellings, one -// refusal sentence — and run on every filter this compiler compiles -// ({@link judgedBooleanComparands}). +// [#21376, #21426] The two comparand verdicts the engine's `where` door +// consults at its one field-aware walk — boolean +// (`@objectstack/objectql`'s `boolean-comparand-declared-type-door.ts`) and +// number (`number-comparand-declared-type-door.ts`) — read from the same spec +// modules: one verdict, one set of accepted spellings, one refusal sentence +// each, run as the two arms of one walk on every filter this compiler compiles +// ({@link judgedComparands}). import { BOOLEAN_COMPARAND_DOOR_LIST_OPERATORS, BOOLEAN_COMPARAND_DOOR_SCALAR_OPERATORS, booleanComparandDoorVerdict, booleanComparandFieldVerdict, booleanComparandRefusalMessage, + NUMBER_COMPARAND_DOOR_LIST_OPERATORS, + NUMBER_COMPARAND_DOOR_SCALAR_OPERATORS, + numberComparandDoorVerdict, + numberComparandFieldVerdict, + numberComparandRefusalMessage, } from '@objectstack/spec/data'; import { findCrossFieldComparand, findUninterpretableTemporalMember } from '../comparand-shape.js'; import { assertReadScopeCannotVacate, compileScopedFilterToSql } from '../read-scope-sql.js'; @@ -273,24 +280,36 @@ interface StatementClauses { readonly joins: StatementJoins; } -// ── [#21376] The boolean-comparand verdict, on every filter this compiler compiles ── +// ── [#21376, #21426] The comparand verdicts, on every filter this compiler compiles ── // -// The engine judges a comparand against a declared boolean column at its one -// field-aware filter walk (`@objectstack/objectql`'s -// `boolean-comparand-declared-type-door.ts`), by the spec's verdict -// (`booleanComparandDoorVerdict`, `@objectstack/spec/data`): `true` / `false` -// pass, `"true"` / `"false"`, `"1"` / `"0"` and `1` / `0` narrow to the boolean -// each names, anything else it refuses (`'yes'`, `2`) is `INVALID_FILTER` / 400. This strategy -// compiles its filters to SQL itself, past that walk, so a string reached the -// driver as written: on SQLite a stored boolean is `1` / `0`, and the string -// `'true'` equals neither — `{ flag: 'true' }` counted no row, `{ flag: { $ne: -// 'true' } }` counted every row, and `{ flag: 'yes' }` answered 200 with zero -// where the engine answers 400 (PostgreSQL reads `'yes'` as `true` and counted -// the true rows). So the same verdict runs here, on the caller's `where` (the -// dataset door's `runtimeFilter` arrives merged into it), each measure's own -// `filter` and the dataset's own scope — every filter that reaches -// `compileFilterNode` — and the strategy answers what the engine door answers. -// ⛔ Nothing here reads a spelling: the verdict does. ⛔ No second rule. +// The engine judges a comparand against a declared boolean or number column at +// its one field-aware filter walk (`@objectstack/objectql`'s +// `number-comparand-declared-type-door.ts`, whose walk carries the boolean arm +// too), by the spec's verdicts (`@objectstack/spec/data`): +// +// - boolean (`booleanComparandDoorVerdict`): `true` / `false` pass, `"true"` / +// `"false"`, `"1"` / `"0"` and `1` / `0` narrow to the boolean each names, +// anything else it refuses (`'yes'`, `2`) is `INVALID_FILTER` / 400; +// - number (`numberComparandDoorVerdict`): a number passes, a string the +// platform's numeric grammar reads (`"12"`, `"1e3"`) narrows to its number, +// and a string it does not read (`"abc"`, `""`, `"+5"`), a boolean, a `Date` +// or an array is `INVALID_FILTER` / 400. +// +// This strategy compiles its filters to SQL itself, past that walk, so a +// comparand reached the driver as written. Boolean: on SQLite a stored boolean +// is `1` / `0`, and the string `'true'` equals neither — `{ flag: 'true' }` +// counted no row, and `{ flag: 'yes' }` answered 200 with zero where the engine +// answers 400. Number: `{ amount: 'abc' }` counted no row on SQLite and was a +// `DATABASE_ERROR` / 500 on PostgreSQL, `{ amount: true }` bound `1` and +// answered 200 on both, and `{ amount: { $lte: '9999-12-31' } }` met the bare-day +// window rule and counted every row — each a 400 at the engine door. So the +// same verdicts run here, as the two arms of ONE walk (the engine's shape: the +// two classes are disjoint, so at most one arm judges a member), on the +// caller's `where` (the dataset door's `runtimeFilter` arrives merged into it), +// each measure's own `filter` and the dataset's own scope — every filter that +// reaches `compileFilterNode` — and the strategy answers what the engine door +// answers. ⛔ Nothing here reads a spelling or a number: the verdicts do. ⛔ No +// second rule, and ⛔ no second walk. /** * The declared type of the column a filter member binds against, or @@ -298,10 +317,21 @@ interface StatementClauses { */ type MemberDeclaredType = (member: string) => string | undefined; -/** The operators whose one comparand the verdict judges — the spec's list, never a re-listing. */ -const BOOLEAN_DOOR_SCALAR_OPERATORS: ReadonlySet = new Set(BOOLEAN_COMPARAND_DOOR_SCALAR_OPERATORS); -/** The operators each of whose MEMBERS the verdict judges. */ -const BOOLEAN_DOOR_LIST_OPERATORS: ReadonlySet = new Set(BOOLEAN_COMPARAND_DOOR_LIST_OPERATORS); +/** + * One arm of {@link narrowComparands}: which columns it judges (its spec's + * field verdict), the positions it judges there (its spec's operator lists, + * never a re-listing) and its judgment of ONE comparand at one of them. + */ +interface ComparandArm { + /** Does this arm judge a column of `declaredType`? The spec's field verdict, `judged` alone. */ + readonly judges: (declaredType: string) => boolean; + /** The operators whose one comparand the arm judges. */ + readonly scalarOperators: ReadonlySet; + /** The operators each of whose MEMBERS the arm judges. */ + readonly listOperators: ReadonlySet; + /** The comparand as the verdict leaves it — narrowed or unchanged — or a thrown refusal. */ + readonly judge: (member: string, declaredType: string, comparand: unknown, path: string) => unknown; +} /** A plain object: a filter node or an operator map, never a comparand (a `Date` is data). */ function isPlainFilterNode(value: unknown): value is Record { @@ -311,7 +341,7 @@ function isPlainFilterNode(value: unknown): value is Record { } /** - * The member reader for {@link judgedBooleanComparands}: the declared type the + * The member reader for {@link judgedComparands}: the declared type the * host's `declaredFieldType` hook answers for the column `target` resolves a * member to — the same (object, column) every other declared-type question in * this compiler asks (the datetime lowering, the text-operator constant, @@ -345,24 +375,72 @@ function judgedBooleanComparand(member: string, declaredType: string, comparand: ); } -/** One judged member's constraint, `{ flag: }`, with its comparands judged. Copy-on-write. */ -function narrowedBooleanFieldSpec(member: string, declaredType: string, spec: unknown, path: string): unknown { +/** + * [#21426] One comparand at a judged position on a number column, by the + * spec's verdict: the number a numeric string denotes (so the statement binds + * what the engine door hands its driver — `12`, never `"12"`), the comparand + * unchanged, or a refusal in the same envelope as the boolean arm's + * (`invalidFilterError`, `INVALID_FILTER` / 400) carrying the spec's sentence. + * Every position this compiler compiles is bound by the driver (a measure's + * own `filter` too, inside its conditional aggregate), so the sentence takes + * the spec's default, driver-bound reading. + */ +function judgedNumberComparand(member: string, declaredType: string, comparand: unknown, path: string): unknown { + const verdict = numberComparandDoorVerdict({ type: declaredType }, comparand); + if (verdict.verdict === 'narrows') return verdict.value; + if (verdict.verdict !== 'door-refusal') return comparand; + throw invalidFilterError( + `[analytics] ${numberComparandRefusalMessage({ field: member, declaredType, path, value: comparand, form: verdict.form })}`, + ); +} + +/** [#21426] The number arm: the numeric class (`NUMBER_COMPARAND_DOOR_JUDGED_TYPES`), by its spec's field verdict. */ +const NUMBER_ARM: ComparandArm = { + judges: (type) => numberComparandFieldVerdict({ type }) === 'judged', + scalarOperators: new Set(NUMBER_COMPARAND_DOOR_SCALAR_OPERATORS), + listOperators: new Set(NUMBER_COMPARAND_DOOR_LIST_OPERATORS), + judge: judgedNumberComparand, +}; + +/** [#21376] The boolean arm: the boolean class, by its spec's field verdict. */ +const BOOLEAN_ARM: ComparandArm = { + judges: (type) => booleanComparandFieldVerdict({ type }) === 'judged', + scalarOperators: new Set(BOOLEAN_COMPARAND_DOOR_SCALAR_OPERATORS), + listOperators: new Set(BOOLEAN_COMPARAND_DOOR_LIST_OPERATORS), + judge: judgedBooleanComparand, +}; + +/** + * The arm that judges a column of `declaredType`, or `null`. The two classes + * are disjoint (a column is a number or a boolean, never both), so at most one + * arm answers — the engine walk's own order, number first. A `formula` + * reaches here with no `returnType` (the host relays none) and is `deferred` + * by both verdicts, as the spec defers one; never a list here. + */ +function comparandArmFor(declaredType: string): ComparandArm | null { + if (NUMBER_ARM.judges(declaredType)) return NUMBER_ARM; + if (BOOLEAN_ARM.judges(declaredType)) return BOOLEAN_ARM; + return null; +} + +/** One judged member's constraint, `{ amount: }`, with its comparands judged by `arm`. Copy-on-write. */ +function narrowedFieldSpec(arm: ComparandArm, member: string, declaredType: string, spec: unknown, path: string): unknown { // Not filter structure: the implicit-equality comparand. - if (!isPlainFilterNode(spec)) return judgedBooleanComparand(member, declaredType, spec, path); + if (!isPlainFilterNode(spec)) return arm.judge(member, declaredType, spec, path); // A `{ $field }` reference is not a literal, and a plain object with no `$` - // key is not this verdict's subject — each is left for the face that owns it. + // key is not a verdict's subject — each is left for the face that owns it. if (typeof spec.$field === 'string' || !Object.keys(spec).some((k) => k.startsWith('$'))) return spec; let out: Record | undefined; for (const [op, comparand] of Object.entries(spec)) { - if (BOOLEAN_DOOR_SCALAR_OPERATORS.has(op)) { - const judged = judgedBooleanComparand(member, declaredType, comparand, `${path}.${op}`); + if (arm.scalarOperators.has(op)) { + const judged = arm.judge(member, declaredType, comparand, `${path}.${op}`); if (judged !== comparand) (out ??= { ...spec })[op] = judged; continue; } - if (!BOOLEAN_DOOR_LIST_OPERATORS.has(op) || !Array.isArray(comparand)) continue; + if (!arm.listOperators.has(op) || !Array.isArray(comparand)) continue; let members: unknown[] | undefined; comparand.forEach((value, index) => { - const judged = judgedBooleanComparand(member, declaredType, value, `${path}.${op}[${index}]`); + const judged = arm.judge(member, declaredType, value, `${path}.${op}[${index}]`); if (judged !== value) (members ??= [...comparand])[index] = judged; }); if (members) (out ??= { ...spec })[op] = members; @@ -371,16 +449,16 @@ function narrowedBooleanFieldSpec(member: string, declaredType: string, spec: un } /** - * The lowered condition with every comparand on a declared boolean column - * judged: through `$and`, `$or` and `$not`, at every member key (another `$` - * key at node level is not a member). The positions are the spec's - * (`BOOLEAN_COMPARAND_DOOR_SCALAR_OPERATORS` / `…_LIST_OPERATORS`). A member is - * judged at the column it binds against, so the cube-qualified spelling - * (`.flag`) and a relationship path are judged at their column too. - * Copy-on-write: a subtree nothing narrowed is returned by reference, so a - * filter the dataset registry holds is never edited. + * The lowered condition with every comparand on a declared boolean or number + * column judged by its arm ({@link comparandArmFor}): through `$and`, `$or` + * and `$not`, at every member key (another `$` key at node level is not a + * member), at each arm's spec positions. A member is judged at the column it + * binds against, so the cube-qualified spelling (`.amount`) and a + * relationship path (the related object's declared column) are judged at + * their column too. Copy-on-write: a subtree nothing narrowed is returned by + * reference, so a filter the dataset registry holds is never edited. */ -function narrowBooleanComparands(node: unknown, typeOf: MemberDeclaredType, path: string, depth = 0): unknown { +function narrowComparands(node: unknown, typeOf: MemberDeclaredType, path: string, depth = 0): unknown { if (depth > 32 || !isPlainFilterNode(node)) return node; let out: Record | undefined; for (const [key, value] of Object.entries(node)) { @@ -390,20 +468,19 @@ function narrowBooleanComparands(node: unknown, typeOf: MemberDeclaredType, path if (!Array.isArray(value)) continue; let arms: unknown[] | undefined; value.forEach((arm, index) => { - const walked = narrowBooleanComparands(arm, typeOf, `${here}[${index}]`, depth + 1); + const walked = narrowComparands(arm, typeOf, `${here}[${index}]`, depth + 1); if (walked !== arm) (arms ??= [...value])[index] = walked; }); if (arms) next = arms; } else if (key === '$not') { - next = narrowBooleanComparands(value, typeOf, here, depth + 1); + next = narrowComparands(value, typeOf, here, depth + 1); } else { if (key.startsWith('$')) continue; const declaredType = typeOf(key); - // The spec's field verdict decides which columns are judged — a `formula` - // reaches here with no `returnType` (the host relays none) and is - // `deferred`, as the spec defers one; never a list here. - if (declaredType === undefined || booleanComparandFieldVerdict({ type: declaredType }) !== 'judged') continue; - next = narrowedBooleanFieldSpec(key, declaredType, value, here); + if (declaredType === undefined) continue; + const arm = comparandArmFor(declaredType); + if (!arm) continue; + next = narrowedFieldSpec(arm, key, declaredType, value, here); } if (next !== value) (out ??= { ...node })[key] = next; } @@ -412,20 +489,21 @@ function narrowBooleanComparands(node: unknown, typeOf: MemberDeclaredType, path /** * `source` (a `{ where }` carrier, as {@link normalizeAnalyticsFilterTree} - * takes it) with the spec's boolean verdict applied to its lowered condition: - * `source` itself when nothing narrows (or the host cannot answer), else a - * `{ where }` carrying the narrowed condition. A refusal is thrown. + * takes it) with the spec's boolean and number verdicts applied to its + * lowered condition: `source` itself when nothing narrows (or the host cannot + * answer), else a `{ where }` carrying the narrowed condition. A refusal is + * thrown. * * The condition is lowered by `lowerAnalyticsWhere` — the shared comparand * faces' door, which refuses what it refuses first, in its own words — and * `normalizeAnalyticsFilterTree` lowers the narrowed condition again: the * faces are idempotent on their own output. */ -function judgedBooleanComparands(source: unknown, typeOf: MemberDeclaredType | null): unknown { +function judgedComparands(source: unknown, typeOf: MemberDeclaredType | null): unknown { if (!typeOf) return source; const condition = lowerAnalyticsWhere(source); if (!condition) return source; - const judged = narrowBooleanComparands(condition, typeOf, 'where'); + const judged = narrowComparands(condition, typeOf, 'where'); return judged === condition ? source : { where: judged }; } @@ -1094,10 +1172,10 @@ export class NativeSQLStrategy implements AnalyticsStrategy { // bare-day copy (`buildFilterClause`'s `lte` arm) stays until its deletion // card, and is idempotent on the lowered bound. const lowering = declaredDatetimeLowering(ctx, (member) => this.resolveStorageTarget(cube, member, tableName, joins.referenceOf)); - // [#21376] The boolean-comparand verdict's member reader, asked of the - // SAME target, and applied at the same three filter positions, before - // each is normalized ({@link judgedBooleanComparands}). - const booleanTypeOf = memberDeclaredType(ctx, (member) => this.resolveStorageTarget(cube, member, tableName, joins.referenceOf)); + // [#21376, #21426] The comparand verdicts' member reader (both arms read + // it), asked of the SAME target, and applied at the same three filter + // positions, before each is normalized ({@link judgedComparands}). + const comparandTypeOf = memberDeclaredType(ctx, (member) => this.resolveStorageTarget(cube, member, tableName, joins.referenceOf)); // Build SELECT for measures if (query.measures && query.measures.length > 0) { @@ -1111,7 +1189,7 @@ export class NativeSQLStrategy implements AnalyticsStrategy { const measureFilter = datasetScope?.measureFilters?.[measure]; const predicate = measureFilter ? this.compileFilterNode( - normalizeAnalyticsFilterTree(judgedBooleanComparands({ where: measureFilter }, booleanTypeOf), lowering), + normalizeAnalyticsFilterTree(judgedComparands({ where: measureFilter }, comparandTypeOf), lowering), cube, tableName, joins, @@ -1129,7 +1207,7 @@ export class NativeSQLStrategy implements AnalyticsStrategy { // used to be dropped instead of compiled. const whereClauses: string[] = []; const filterSql = this.compileFilterNode( - normalizeAnalyticsFilterTree(judgedBooleanComparands(query, booleanTypeOf), lowering), + normalizeAnalyticsFilterTree(judgedComparands(query, comparandTypeOf), lowering), cube, tableName, joins, @@ -1146,7 +1224,7 @@ export class NativeSQLStrategy implements AnalyticsStrategy { // predicate with itself selects the same rows. if (datasetScope?.filter) { const scopeSql = this.compileFilterNode( - normalizeAnalyticsFilterTree(judgedBooleanComparands({ where: datasetScope.filter }, booleanTypeOf), lowering), + normalizeAnalyticsFilterTree(judgedComparands({ where: datasetScope.filter }, comparandTypeOf), lowering), cube, tableName, joins,