diff --git a/.changeset/21417-analytics-faces-one-lowering.md b/.changeset/21417-analytics-faces-one-lowering.md new file mode 100644 index 00000000000..2df22edbbb2 --- /dev/null +++ b/.changeset/21417-analytics-faces-one-lowering.md @@ -0,0 +1,33 @@ +--- +"@objectstack/service-analytics": minor +--- + +fix(service-analytics)!: the analytics read scope, the `where` tree and the draft preview take the shared lowering's bound and NULL guards; their own whole-day and NULL-polarity copies are deleted (ADR-0053 D-D1 items 7 to 9) + +Clause-②: no (narrowing) + + + +**BREAKING**: this narrows the rows the native analytics strategy and the draft preview (`queryDataset` with `previewDrafts`) select for a bare-day upper bound on a column the host declares as neither `datetime` nor `date` — a `text` column, for example. It ships as `minor` under the launch-window convention for answer narrowings. No export, published type, accepted input or error code changes. + +**What is deleted.** The native SQL strategy no longer reads a bare `YYYY-MM-DD` `$lte`, a `$between` maximum or an explicit `dateRange` end as "through that whole day" on every column, and no longer drops such a bound on `9999-12-31` whatever the column holds. The whole-day rule is applied once, by the shared `lowerFilterCondition` (`@objectstack/spec/data`), with the column's declared type, the reader the plugin already wires from the engine's registry (`sourceFieldMeta`): a declared `datetime` column keeps the whole day, and every other declared column is compared as written, as the engine compares it. The `/analytics/sql` echo renders the same lowering. + +**The native face now agrees with the engine.** Measured through `AnalyticsService.query` (what `POST /api/v1/analytics/query` relays) in the plugin's own composition, on SQLite and on PostgreSQL 16, over a `text` column `note` holding `'2026-07-27'`, `'2026-07-28'`, `'2026-07-28 late'`, `'n'` and no value: + +- `{ note: { $lte: '9999-12-31' } }` counted every row with a value (4). It now counts 3, the rows the engine's `find` returns: `'n'` sorts above `'9999-12-31'`. +- `{ note: { $lte: '2026-07-28' } }` counted 3, the `'2026-07-28 late'` row included. It now counts 2. +- `$between ['2026-07-28', '2026-07-28']` and a `dateRange` window of the same day counted 2; they now count 1. Their negation through `$not` gains the row the bound lost. + +On a declared `datetime` or `date` column every answer is unchanged, on both strategies. + +**A host with no typed reader** (a strategy context with no `declaredFieldType` hook, or an `AnalyticsService` built without `sourceFieldMeta`) reads every column type-blind, as ADR-0053 D-D1 item 7 prescribes for a seam that cannot read declarations: its native answers do not move. Pass `sourceFieldMeta` (the README shows how) to get the engine's answer on a non-temporal column. + +**The `/analytics/sql` echo.** A `dateRange` window on a declared `date` column now prints the inclusive `<=` the engine runs, where it printed `<` the next day; on a column the host names no type for, it prints the bound the ObjectQL strategy hands the engine, as written. A preset window that stops before its end (`today`, `this_month`, …) now prints `<` its end instant with that instant bound, where it printed `<=` with no value bound. The NULL guards print once where they printed two or three nested copies of the same guard; every row set is unchanged. + +**The draft preview now agrees with the engine too.** `queryDataset` with `previewDrafts` evaluates drafted seed rows in memory; it kept its own whole-day copy, read on every column. It now hands the evaluator the drafted object's declared types (`sourceFieldMeta`), and the shared lowering applies the rule with them: a declared `datetime` column keeps the whole day, any other declared column is compared as written, and a column the host names no type for is read type-blind (ADR-0053 D-D1 item 7). Measured through the plugin's own composition over the same rows, five of the preview's `note` cells moved, each onto the engine's answer: `$lte` a day 3 to 2, `$between` and a window of one day 2 to 1, a window to `9999-12-31` 3 to 2, and the `$not` gains the row. Its `$lte` and `$between` to `9999-12-31` already gave the engine's answer and are unchanged. Every `datetime` and `date` cell is unchanged. + +- A preview window is now the `{ $gte, $lte }` pair the ObjectQL strategy hands the engine, matched like the same bounds in a `where`. Its end used to be read with a `'~'` suffix ("that instant and its own sub-values"), a reading no other face gives. Measured on a `datetime` column over SQLite, a canonical end (`…T10:00:00.000Z`) answers as before and as the engine. An end spelled shorter than the stored value is compared as text, as the preview's `where` already compared it: an end of `…T10:00` or `…T10:00:00` now leaves out the row stored at exactly that instant (the engine keeps it), and leaves out the rows inside that minute or second (the engine leaves them out too; the old reading kept them). Write a window end in full (`2026-07-28T10:00:00.000Z`) to get the engine's rows on the preview. +- A window over rows that hold a `Date` (the BSON storage form a MongoDB-backed draft reads back) is compared as instants, like the preview's `where`; it was compared as the `Date`'s display text. +- A host that wires no `sourceFieldMeta` (or an object the registry does not hold yet) reads every column type-blind. On a `text` column holding a value that sorts above `'9999-12-31'` (`'n'`), a `$lte` or `$between` maximum of `9999-12-31` now keeps that row, as every other type-blind seam does; the deleted copy left it out. + +**Unchanged.** Every answer on a declared `datetime` or `date` column, on the native strategy, the ObjectQL strategy and the draft preview; every answer of the ObjectQL strategy; every answer of the read scope. diff --git a/packages/services/service-analytics/src/__tests__/analytics-faces-one-lowering.test.ts b/packages/services/service-analytics/src/__tests__/analytics-faces-one-lowering.test.ts new file mode 100644 index 00000000000..d5679db8264 --- /dev/null +++ b/packages/services/service-analytics/src/__tests__/analytics-faces-one-lowering.test.ts @@ -0,0 +1,613 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [ADR-0053 D-D1, amended — #5930 step 4] The analytics read scope (F9) and the + * analytics `where` tree with its compilers (F10) keep no copy of the filter + * meaning the shared lowering carries: the whole-day upper bound, the + * `$between` split and the NULL-polarity guards. The lowering + * (`lowerFilterCondition`, `@objectstack/spec/data`) is their one source, run at + * each face's seam with that face's typed column reader: + * + * - **F10, the native strategy** — `declaredDatetimeLowering(ctx, …, + * 'type-blind')`, the context's `declaredFieldType` hook, which the plugin + * answers from the engine's registry (`sourceFieldMeta`). A column the hook + * names no type for is read type-blind (item 7): this face is the last seam + * before its statement runs. The `dateRange` window is the `{ $gte, $lte }` + * pair, lowered by the same reader (item 8). + * - **F10, the ObjectQL strategy** (the engine hand-off and the + * `/analytics/sql` echo) — the same hook; a column it names no type for is + * left as written, for the engine's `where` seam, which reads the object's own + * field map. + * - **F9, the read scope** — `ReadScopeCompileOptions.declaredValueShape`, + * which both of its consumers fill from the context's `declaredValueShape` + * hook (the same `sourceFieldMeta`). + * - **F11, the draft preview** — the drafted object's declared types, which + * `queryDataset`'s preview branch hands `evaluateAnalyticsQueryOverRows` from + * `sourceFieldMeta` (`declaredPreviewLowering`): a declared `datetime` is + * rewritten, any other declared column is compared as written, and a column + * with no declared type reads type-blind. The `dateRange` window is the same + * pair, through the same lowering. + * + * ## Measured on the base (`0b8239111`), through the plugin's own composition + * + * Rows `r1`..`r5` (below). Before this card, the native face widened a bare day + * on EVERY column — its `lte` arm and its window arm read any `YYYY-MM-DD` + * comparand as a calendar day — so on a `text` column it answered differently + * from the engine, on SQLite and on PostgreSQL 16 alike: + * + * | `where` on the text column `note` | native, before | engine and native, now | + * |:--|:--|:--| + * | `$lte '2026-07-28'` | r1, r2, r3 | r1, r2 | + * | `$lte '9999-12-31'` | r1, r2, r3, r4 | r1, r2, r3 | + * | `$between ['2026-07-28', '2026-07-28']` | r2, r3 | r2 | + * | `$not: { $lte '2026-07-28' }` | r4, r5 | r3, r4, r5 | + * | `dateRange ['2026-07-28', '2026-07-28']` | r2, r3 | r2 | + * + * Every `datetime` and `date` cell answered the engine's rows before and + * answers them now, on both faces and both databases; so does every cell on a + * host with no typed reader (below), and every cell of the read scope. + * + * The draft preview (F11) answered the same text cells the native face did, + * through its own type-blind `lteBound`, except the last supported day, where + * it read a value that denotes no instant as written. With the drafted + * object's declared types it now answers every cell the engine does. + * + * The PostgreSQL cell runs where `OS_TEST_POSTGRES_URL` is set and is a named + * skip otherwise. It owns its table, dropped before and after. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { readdirSync, readFileSync, statSync } from 'node:fs'; +import { dirname, join, relative, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { lowerFilterCondition, TEMPORAL_CASES, TEMPORAL_NOW, TEMPORAL_ROWS, type Cube, type FilterCondition } from '@objectstack/spec/data'; +import type { AnalyticsQuery, StrategyContext } from '@objectstack/spec/contracts'; +import { resolveFilterTokens } from '@objectstack/core'; +import { DatasetSchema } from '@objectstack/spec/ui'; +import { AnalyticsService } from '../analytics-service.js'; +import { AnalyticsServicePlugin } from '../plugin.js'; +import { NativeSQLStrategy } from '../strategies/native-sql-strategy.js'; +import { ObjectQLStrategy } from '../strategies/objectql-strategy.js'; +import { NO_DATETIME_COLUMNS, normalizeAnalyticsFilterTree, type NormalizedFilterNode } from '../strategies/filter-normalizer.js'; +import { compileScopedFilterToSql } from '../read-scope-sql.js'; + +const HERE = dirname(fileURLToPath(import.meta.url)); +const SRC = resolve(HERE, '..'); + +// ── The enumeration ────────────────────────────────────────────────────────── + +/** Every non-test source file of this package, path relative to `src/`. */ +function sourceFiles(dir = SRC): string[] { + const out: string[] = []; + for (const name of readdirSync(dir)) { + const path = join(dir, name); + if (statSync(path).isDirectory()) { + if (name !== '__tests__') out.push(...sourceFiles(path)); + } else if (name.endsWith('.ts') && !name.endsWith('.test.ts')) { + out.push(relative(SRC, path)); + } + } + return out.sort(); +} + +/** + * Does `source` USE `name` as code: import it, declare it, or call it? A prose + * mention of a helper (a docblock's `{@link name}` or a backticked name) is + * not a copy of it, and no such mention is an import list, a declaration or a + * call, so no comment stripping is needed to tell them apart. + */ +function uses(source: string, name: string): boolean { + const imported = new RegExp(`import\\s*(type\\s*)?\\{[^}]*\\b${name}\\b[^}]*\\}\\s*from`); + const declared = new RegExp(`\\b(function|const|let|var)\\s+${name}\\b`); + const called = new RegExp(`\\b${name}\\s*\\(`); + return imported.test(source) || declared.test(source) || called.test(source); +} + +/** Which of `names` each file uses, keyed by file; files using none are left out. */ +function holders(names: readonly string[]): Record { + const out: Record = {}; + for (const file of sourceFiles()) { + const source = readFileSync(join(SRC, file), 'utf8'); + const held = names.filter((name) => uses(source, name)); + if (held.length > 0) out[file] = held; + } + return out; +} + +/** + * The whole-day rule's own spellings: the calendar-day primitives a face calls + * to widen a bound itself, and the preview's helper built on them. + */ +const WHOLE_DAY_HELPERS = ['nextUtcCalendarDay', 'isUnboundedAbove', 'UNBOUNDED_ABOVE', 'lteBound'] as const; + +/** The NULL-polarity copies' spellings, as the faces named them. */ +const NULL_POLARITY_HELPERS = [ + 'nullSafeNegationOperand', + 'nullValueSatisfiesOperator', + 'operatorIsNullTotal', + 'nullGuardForFieldSpec', + 'guardFieldEntry', + 'nullSafeNegative', +] as const; + +describe('[#5930 step 4] the enumeration: no analytics face keeps the meaning the shared lowering carries', () => { + it('the scan reads the faces it judges (positive control)', () => { + const files = sourceFiles(); + expect(files).toContain('read-scope-sql.ts'); + expect(files).toContain(join('strategies', 'filter-normalizer.ts')); + expect(files).toContain(join('strategies', 'native-sql-strategy.ts')); + expect(files).toContain(join('strategies', 'objectql-strategy.ts')); + // The seams that run the lowering are where the scan finds it. + expect(Object.keys(holders(['lowerFilterCondition'])).sort()).toEqual([ + 'preview-evaluator.ts', + 'read-scope-sql.ts', + join('strategies', 'filter-normalizer.ts'), + ]); + }); + + it('no face keeps a whole-day helper of its own', () => { + expect(holders(WHOLE_DAY_HELPERS)).toEqual({}); + }); + + it('no face keeps a NULL-polarity copy', () => { + expect(holders(NULL_POLARITY_HELPERS)).toEqual({}); + }); +}); + +// ── One source: each compiler compiles the bound it is handed ──────────────── + +const OBJECT = 'os21417_whole_day'; +const LEDGER = { + name: OBJECT, + label: 'Whole day', + fields: { + signed_at: { name: 'signed_at', type: 'datetime' as const }, + due_on: { name: 'due_on', type: 'date' as const }, + note: { name: 'note', type: 'text' as const }, + }, +}; +const ROWS = [ + { id: 'r1', signed_at: '2026-07-27T10:00:00.000Z', due_on: '2026-07-27', note: '2026-07-27' }, + { id: 'r2', signed_at: '2026-07-28T00:00:00.000Z', due_on: '2026-07-28', note: '2026-07-28' }, + { id: 'r3', signed_at: '2026-07-28T10:00:00.000Z', due_on: '2026-07-28', note: '2026-07-28 late' }, + { id: 'r4', signed_at: '2026-07-29T10:00:00.000Z', due_on: '2026-07-29', note: 'n' }, + { id: 'r5', signed_at: null, due_on: null, note: null }, +]; +const CUBE: Cube = { + name: 'os21417_cube', + title: 'Whole day', + sql: OBJECT, + public: true, + measures: { n: { type: 'count', sql: '*', label: 'n' } }, + dimensions: { + id: { type: 'string', sql: 'id', label: 'Id' }, + signed_at: { type: 'time', sql: 'signed_at', label: 'Signed' }, + due_on: { type: 'time', sql: 'due_on', label: 'Due' }, + note: { type: 'string', sql: 'note', label: 'Note' }, + }, +} as Cube; +const DECLARED: Record = { signed_at: 'datetime', due_on: 'date', note: 'text' }; + +/** A strategy context with this cube and, unless `declared` is omitted, the declared-type hook. */ +const strategyCtx = (declared?: (object: string, field: string) => string | undefined, extra: Record = {}) => ({ + getCube: (name: string) => (name === CUBE.name ? CUBE : undefined), + queryCapabilities: () => ({ nativeSql: true, objectqlAggregate: true, inMemory: false }), + ...(declared ? { declaredFieldType: declared } : {}), + ...extra, +}) as unknown as StrategyContext; +const TYPED = strategyCtx((_o, f) => DECLARED[f]); +const q = (rest: Partial): AnalyticsQuery => + ({ cube: CUBE.name, measures: ['n'], dimensions: ['id'], ...rest }) as AnalyticsQuery; +/** The WHERE of a statement, the grouping cut off. */ +const whereOf = (sql: string): string => sql.replace(/^[\s\S]*? WHERE /, '').replace(/ GROUP BY[\s\S]*$/, ''); + +describe('[#5930 step 4] one source: the native compiler and the echo compile the bound they are handed', () => { + const native = async (ctx: StrategyContext, rest: Partial) => + new NativeSQLStrategy().generateSql(q(rest), ctx); + const echo = async (ctx: StrategyContext, rest: Partial) => + new ObjectQLStrategy().generateSql(q(rest), ctx); + + it('a bare-day $lte: `<` the next day on the declared datetime, as written on every other declared column', async () => { + for (const compile of [native, echo]) { + const at = await compile(TYPED, { where: { signed_at: { $lte: '2026-07-28' } } }); + expect(whereOf(at.sql)).toBe('signed_at < $1'); + expect(at.params).toEqual(['2026-07-29']); + for (const column of ['due_on', 'note']) { + const other = await compile(TYPED, { where: { [column]: { $lte: '2026-07-28' } } }); + expect(whereOf(other.sql), column).toBe(`${column} <= $1`); + expect(other.params, column).toEqual(['2026-07-28']); + } + // The last supported day: `IS NOT NULL` on the datetime only. The text + // column's `$lte '9999-12-31'` is a comparison as written — the cell the + // native face's own copy answered with every row that had a value. + expect(whereOf((await compile(TYPED, { where: { signed_at: { $lte: '9999-12-31' } } })).sql)).toBe('signed_at IS NOT NULL'); + expect(whereOf((await compile(TYPED, { where: { note: { $lte: '9999-12-31' } } })).sql)).toBe('note <= $1'); + } + }); + + it('a $between: split and widened on the declared datetime, inclusive as written elsewhere', async () => { + for (const compile of [native, echo]) { + expect(whereOf((await compile(TYPED, { where: { signed_at: { $between: ['2026-07-28', '2026-07-28'] } } })).sql)) + .toBe('(signed_at >= $1 AND signed_at < $2)'); + expect(whereOf((await compile(TYPED, { where: { note: { $between: ['2026-07-28', '2026-07-28'] } } })).sql)) + .toBe('(note >= $1 AND note <= $2)'); + } + }); + + it('a dateRange window: the same pair, through the same reader (item 8)', async () => { + const window = (dimension: string, end = '2026-07-28'): Partial => + ({ timeDimensions: [{ dimension, dateRange: ['2026-07-28', end] as [string, string] }] }); + for (const compile of [native, echo]) { + const at = await compile(TYPED, window('signed_at')); + expect(whereOf(at.sql)).toBe('(signed_at >= $1 AND signed_at < $2)'); + expect(at.params).toEqual(['2026-07-28', '2026-07-29']); + expect(whereOf((await compile(TYPED, window('signed_at', '9999-12-31'))).sql)).toBe('(signed_at >= $1 AND signed_at IS NOT NULL)'); + for (const column of ['due_on', 'note']) { + const other = await compile(TYPED, window(column)); + expect(whereOf(other.sql), column).toBe(`(${column} >= $1 AND ${column} <= $2)`); + expect(other.params, column).toEqual(['2026-07-28', '2026-07-28']); + } + } + }); + + /** The null predicates a tree holds, and the `$null` flags a condition holds. */ + const nullLeaves = (node: NormalizedFilterNode | null): number => { + if (!node) return 0; + if (node.kind === 'leaf') return node.operator === 'set' || node.operator === 'notSet' ? 1 : 0; + if (node.kind === 'not') return nullLeaves(node.child); + if (node.kind === 'and' || node.kind === 'or') return node.children.reduce((n, c) => n + nullLeaves(c), 0); + return 0; + }; + const nullFlags = (condition: unknown): number => (JSON.stringify(condition).match(/"\$null"/g) ?? []).length; + /** Wheres whose every null predicate is a guard: no `$null`, `$exists` or null comparand of the author's. */ + const GUARDED: FilterCondition[] = [ + { $not: { note: '2026-07-28' } }, + { note: { $ne: 'n' } }, + { note: { $nin: ['n'] } }, + { note: { $notContains: 'late' } }, + { $not: { $or: [{ note: 'n' }, { due_on: { $ne: '2026-07-28' } }] } }, + { $not: { signed_at: { $gt: '2026-07-28' }, note: { $ne: 'n' } } }, + { $or: [{ note: { $ne: 'n' } }, { $not: { due_on: { $in: ['2026-07-28'] } } }] }, + ]; + + it('F10: every null predicate in the tree is one the shared lowering wrote', () => { + for (const where of GUARDED) { + const lowered = lowerFilterCondition(where, NO_DATETIME_COLUMNS); + expect(nullFlags(lowered), JSON.stringify(where)).toBeGreaterThan(0); + expect(nullLeaves(normalizeAnalyticsFilterTree({ where }, NO_DATETIME_COLUMNS)), JSON.stringify(where)).toBe(nullFlags(lowered)); + } + }); + + it('F9: every null test in the read scope is one the shared lowering wrote', () => { + for (const where of GUARDED) { + const lowered = lowerFilterCondition(where, NO_DATETIME_COLUMNS); + const { sql } = compileScopedFilterToSql(where, OBJECT); + expect((sql.match(/ IS (NOT )?NULL/g) ?? []).length, JSON.stringify(where)).toBe(nullFlags(lowered)); + } + }); +}); + +// ── The typed drivers' answer, on every face, over a real engine ───────────── + +type Ids = string; +/** Each cell: a label, the query, and the rows the engine's `find` answers for it. */ +const CELLS: ReadonlyArray, engine: Ids]> = [ + ['datetime $lte a day', { where: { signed_at: { $lte: '2026-07-28' } } }, 'r1,r2,r3'], + ['datetime $lte the last day', { where: { signed_at: { $lte: '9999-12-31' } } }, 'r1,r2,r3,r4'], + ['datetime $between one day', { where: { signed_at: { $between: ['2026-07-28', '2026-07-28'] } } }, 'r2,r3'], + ['datetime $not $lte a day', { where: { $not: { signed_at: { $lte: '2026-07-28' } } } }, 'r4,r5'], + ['datetime window one day', { timeDimensions: [{ dimension: 'signed_at', dateRange: ['2026-07-28', '2026-07-28'] }] }, 'r2,r3'], + ['datetime window to the last day', { timeDimensions: [{ dimension: 'signed_at', dateRange: ['2026-07-28', '9999-12-31'] }] }, 'r2,r3,r4'], + ['date $lte a day', { where: { due_on: { $lte: '2026-07-28' } } }, 'r1,r2,r3'], + ['date $lte the last day', { where: { due_on: { $lte: '9999-12-31' } } }, 'r1,r2,r3,r4'], + ['date $between one day', { where: { due_on: { $between: ['2026-07-28', '2026-07-28'] } } }, 'r2,r3'], + ['date window one day', { timeDimensions: [{ dimension: 'due_on', dateRange: ['2026-07-28', '2026-07-28'] }] }, 'r2,r3'], + ['text $lte a day', { where: { note: { $lte: '2026-07-28' } } }, 'r1,r2'], + ['text $lte the last day', { where: { note: { $lte: '9999-12-31' } } }, 'r1,r2,r3'], + ['text $between one day', { where: { note: { $between: ['2026-07-28', '2026-07-28'] } } }, 'r2'], + ['text $between to the last day', { where: { note: { $between: ['2026-07-28', '9999-12-31'] } } }, 'r2,r3'], + ['text $not $lte a day', { where: { $not: { note: { $lte: '2026-07-28' } } } }, 'r3,r4,r5'], + ['text window one day', { timeDimensions: [{ dimension: 'note', dateRange: ['2026-07-28', '2026-07-28'] }] }, 'r2'], + ['text window to the last day', { timeDimensions: [{ dimension: 'note', dateRange: ['2026-07-28', '9999-12-31'] }] }, 'r2,r3'], + ['text $ne a value', { where: { note: { $ne: 'n' } } }, 'r1,r2,r3,r5'], +]; + +/** The engine's own `where` for a cell: a window is the `{ $gte, $lte }` pair. */ +const engineWhere = (query: Partial): Record => { + if (query.where) return query.where as Record; + const td = query.timeDimensions![0]; + const [start, end] = td.dateRange as string[]; + return { [td.dimension]: { $gte: start, $lte: end } }; +}; + +const quiet = { debug() {}, info() {}, warn() {}, error() {}, child() { return quiet; } }; +const ids = (rows: Array>): Ids => rows.map((r) => String(r.id)).sort().join(','); + +interface DbCell { id: 'sqlite' | 'pg'; label: string; env: string | null; config: () => Record | null } +const DB_CELLS: readonly DbCell[] = [ + { 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), + }, +]; + +/** The plugin's own composition over `engine`: the native face, and the same narrowed to the engine aggregate. */ +async function composeFaces(engine: ObjectQL, cubes: Cube[]): Promise<{ native: AnalyticsService; objectql: AnalyticsService }> { + const faces: Record = {}; + for (const [face, caps] of [ + ['native', undefined], + ['objectql', () => ({ nativeSql: false, objectqlAggregate: true, inMemory: false })], + ] as const) { + const registered: Record = {}; + await new AnalyticsServicePlugin({ cubes, ...(caps ? { queryCapabilities: caps } : {}) } as never).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); + faces[face] = registered.analytics as AnalyticsService; + } + return faces as { native: AnalyticsService; objectql: AnalyticsService }; +} + +for (const cell of DB_CELLS) { + const config = cell.config(); + describe.skipIf(!config)( + `[#5930 step 4] a bare day answers the typed drivers' rows on every analytics face (${cell.label})${config ? '' : ` (skipped: set ${cell.env} to run this cell)`}`, + () => { + let driver: SqlDriver; + let engine: ObjectQL; + let faces: { native: AnalyticsService; objectql: AnalyticsService }; + /** The raw statements the native face ran on the object, to prove which face answered. */ + let rawStatements = 0; + const drop = async () => { + if (cell.id === 'pg') await (driver as any)?.execute(`drop table if exists ${OBJECT}`).catch(() => {}); + }; + + beforeAll(async () => { + driver = new SqlDriver(config as never); + await drop(); + engine = new ObjectQL({ logger: quiet } as never); + engine.registerDriver(driver as never, true); + await engine.init(); + engine.registry.registerObject(LEDGER as never); + await engine.syncSchemas(); + for (const row of ROWS) await engine.insert(OBJECT, { ...row } as never); + const realExecute = (engine as any).execute.bind(engine); + (engine as any).execute = (sql: string, opts?: { object?: string }) => { + if (opts?.object === OBJECT) rawStatements += 1; + return realExecute(sql, opts); + }; + faces = await composeFaces(engine, [CUBE]); + }); + afterAll(async () => { + await drop(); + try { await engine?.destroy(); } catch { /* noop */ } + }); + + const viaFace = async (face: 'native' | 'objectql', query: Partial) => { + const before = rawStatements; + const res = await faces[face].query(q(query) as never); + return { ids: ids(res.rows as Array>), raw: rawStatements - before }; + }; + + for (const [label, query, expected] of CELLS) { + it(`${label}: ${expected}`, async () => { + expect(ids(await engine.find(OBJECT, { where: engineWhere(query), fields: ['id'] } as never)), 'engine.find').toBe(expected); + const native = await viaFace('native', query); + expect(native.ids, 'the native face').toBe(expected); + expect(native.raw, 'the native strategy answered').toBeGreaterThanOrEqual(1); + const objectql = await viaFace('objectql', query); + expect(objectql.ids, 'the ObjectQL face').toBe(expected); + expect(objectql.raw, 'the engine aggregate answered').toBe(0); + }); + } + + // F9 binds a comparand as written (no storage-form coercion), so its + // datetime cells are pinned on SQLite, whose stored form is ISO text. + // PostgreSQL reads a bare-day text bound in the session's zone; that is + // the read scope's temporal-coercion question, not its lowering. + it.skipIf(cell.id !== 'sqlite')('the read scope (F9), typed by its declared value shape, answers the same rows', async () => { + const declaredValueShape = (field: string) => (DECLARED[field] ? { type: DECLARED[field], multiple: false } : undefined); + for (const [label, query, expected] of CELLS) { + if (!query.where) continue; + const { sql, params } = compileScopedFilterToSql(query.where as FilterCondition, OBJECT, { declaredValueShape, dialect: 'sqlite' }); + const rows = await (engine as any).execute(`select "id" from "${OBJECT}" where ${sql}`, { args: params, object: OBJECT }); + expect(ids(rows as Array>), label).toBe(expected); + } + }); + + it.skipIf(cell.id !== 'sqlite')('a host with no typed reader keeps the type-blind reading on the native face (ADR-0053 D-D1 item 7)', async () => { + // The native strategy over a context with no `declaredFieldType` hook, + // and over one whose hook names no type (an `AnalyticsService` built + // without `sourceFieldMeta`): every column is read type-blind. So the + // datetime and date cells keep the engine's rows, and the text cells + // keep the answer the deleted copy gave — the cells where "answer as + // the typed drivers" cannot hold without a reader, because a reader is + // what tells a text column from a datetime one. + const raw = async (_object: string, sql: string, params: unknown[]) => + (engine as any).execute(sql.replace(/\$(\d+)/g, '?'), { args: params, object: OBJECT }); + const typeBlind: Record = { + 'text $lte a day': 'r1,r2,r3', + 'text $lte the last day': 'r1,r2,r3,r4', + 'text $between one day': 'r2,r3', + 'text $between to the last day': 'r2,r3,r4', + 'text $not $lte a day': 'r4,r5', + 'text window one day': 'r2,r3', + 'text window to the last day': 'r2,r3,r4', + }; + for (const hook of [undefined, () => undefined]) { + const ctx = strategyCtx(hook, { executeRawSql: raw }); + for (const [label, query, expected] of CELLS) { + const res = await new NativeSQLStrategy().execute(q(query), ctx); + expect(ids(res.rows as Array>), `${label} (${hook ? 'a hook naming no type' : 'no hook'})`) + .toBe(typeBlind[label] ?? expected); + } + } + }); + }, + ); +} + +describe('[#5930 step 4] the ObjectQL face hands a column with no declared type to the engine as written', () => { + it('a bare-day $lte and a window reach engine.aggregate unrewritten', async () => { + const seen: unknown[] = []; + const ctx = strategyCtx(undefined, { + executeAggregate: async (_object: string, opts: { filter?: unknown }) => { seen.push(opts.filter); return []; }, + }); + await new ObjectQLStrategy().execute(q({ where: { note: { $lte: '2026-07-28' } } }), ctx); + await new ObjectQLStrategy().execute(q({ timeDimensions: [{ dimension: 'signed_at', dateRange: ['2026-07-28', '2026-07-28'] }] }), ctx); + expect(seen).toEqual([ + { note: { $lte: '2026-07-28' } }, + { signed_at: { $gte: '2026-07-28', $lte: '2026-07-28' } }, + ]); + }); +}); + +// ── The draft preview (F11), through the door that reaches it ──────────────── + +/** + * `AnalyticsService.queryDataset` with `previewDrafts`, over drafted seed rows + * (`draftRowsResolver`): the preview branch hands the evaluator the drafted + * object's declared types from `sourceFieldMeta`. The live path is not wired, + * so an answer can only come from the preview. + */ +describe('[#5930 step 4] the draft preview (F11) answers the typed drivers\' rows', () => { + const DATASET = DatasetSchema.parse({ + name: 'os21417_preview', + label: 'Whole day preview', + object: OBJECT, + dimensions: [ + { name: 'id', field: 'id', type: 'string' }, + { name: 'signed_at', field: 'signed_at', type: 'date' }, + { name: 'due_on', field: 'due_on', type: 'date' }, + { name: 'note', field: 'note', type: 'string' }, + ], + measures: [{ name: 'row_count', aggregate: 'count' }], + }); + const service = (sourceFieldMeta?: (object: string, field: string) => { type: string } | undefined) => + new AnalyticsService({ + ...(sourceFieldMeta ? { sourceFieldMeta } : {}), + queryCapabilities: () => ({ nativeSql: false, objectqlAggregate: true, inMemory: false }), + executeAggregate: async () => { throw new Error('the live path ran: the preview did not answer'); }, + draftRowsResolver: async (object: string) => (object === OBJECT ? ROWS.map((r) => ({ ...r })) : null), + } as never); + const DECLARING = service((object, field) => (object === OBJECT && DECLARED[field] ? { type: DECLARED[field] } : undefined)); + const viaPreview = async (svc: AnalyticsService, query: Partial): Promise => { + const selection = { + dimensions: ['id'], + measures: ['row_count'], + ...(query.where ? { runtimeFilter: query.where } : {}), + ...(query.timeDimensions ? { timeDimensions: query.timeDimensions } : {}), + }; + const res = await svc.queryDataset(DATASET as never, selection as never, { tenantId: 'org_A' } as never, { previewDrafts: true }); + return ids(res.rows as Array>); + }; + + for (const [label, query, expected] of CELLS) { + it(`${label}: ${expected}`, async () => { + expect(await viaPreview(DECLARING, query)).toBe(expected); + }); + } + + it('a host that names no declared type reads every column type-blind (ADR-0053 D-D1 item 7)', async () => { + // The text cells keep the type-blind answer — the cells where "answer as + // the typed drivers" cannot hold without a reader — and the datetime and + // date cells keep the engine's rows. + const typeBlind: Record = { + 'text $lte a day': 'r1,r2,r3', + 'text $lte the last day': 'r1,r2,r3,r4', + 'text $between one day': 'r2,r3', + 'text $between to the last day': 'r2,r3,r4', + 'text $not $lte a day': 'r4,r5', + 'text window one day': 'r2,r3', + 'text window to the last day': 'r2,r3,r4', + }; + const undeclared = service(); + for (const [label, query, expected] of CELLS) { + expect(await viaPreview(undeclared, query), label).toBe(typeBlind[label] ?? expected); + } + }); +}); + +// ── The temporal conformance matrix, on every face ─────────────────────────── + +/** + * `TEMPORAL_CASES` through the native face and the ObjectQL face of the + * plugin's composition, and through the read scope, over one engine on SQLite. + * The native face over a context with no hook runs the same matrix in + * `native-sql-temporal-conformance.test.ts`. Dimension ids match the fixture's + * properties (`at` / `on`); the columns do not, so a member is resolved, not + * echoed. + */ +describe('[#5930 step 4] the temporal conformance matrix answers on every analytics face (sqlite)', () => { + const TEMPORAL = 'os21417_temporal'; + const COLUMN: Record = { at: 'happened_at', on: 'happened_on' }; + const temporalCube = { + name: 'os21417_temporal_cube', + title: 'Temporal', + sql: TEMPORAL, + public: true, + measures: { n: { type: 'count', sql: '*', label: 'n' } }, + dimensions: { + id: { type: 'string', sql: 'id', label: 'Id' }, + at: { type: 'time', sql: COLUMN.at, label: 'At' }, + on: { type: 'time', sql: COLUMN.on, label: 'On' }, + }, + } as unknown as Cube; + let engine: ObjectQL; + let faces: { native: AnalyticsService; objectql: AnalyticsService }; + const resolveTokens = (filter: T): T => resolveFilterTokens(filter, { now: new Date(TEMPORAL_NOW) }); + /** The case's filter on the columns, for the read scope, which reads columns. */ + const onColumns = (filter: FilterCondition): FilterCondition => + Object.fromEntries(Object.entries(filter).map(([k, v]) => [COLUMN[k] ?? k, v])) as FilterCondition; + + beforeAll(async () => { + const driver = new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true } as never); + engine = new ObjectQL({ logger: quiet } as never); + engine.registerDriver(driver as never, true); + await engine.init(); + engine.registry.registerObject({ + name: TEMPORAL, + label: 'Temporal', + fields: { + [COLUMN.at]: { name: COLUMN.at, type: 'datetime' }, + [COLUMN.on]: { name: COLUMN.on, type: 'date' }, + }, + } as never); + await engine.syncSchemas(); + for (const r of TEMPORAL_ROWS) await engine.insert(TEMPORAL, { id: r.id, [COLUMN.at]: r.at, [COLUMN.on]: r.on } as never); + faces = await composeFaces(engine, [temporalCube]); + }); + afterAll(async () => { + try { await engine?.destroy(); } catch { /* noop */ } + }); + + const idsOn = async (face: 'native' | 'objectql', rest: Partial) => + ids((await faces[face].query({ cube: temporalCube.name, measures: ['n'], dimensions: ['id'], ...rest } as never)).rows as Array>); + + for (const c of TEMPORAL_CASES) { + it(c.name, async () => { + const expected = [...c.expected].sort().join(','); + for (const face of ['native', 'objectql'] as const) { + expect(await idsOn(face, { where: c.filter }), `${face}: ${c.note ?? ''}`).toBe(expected); + if (c.tokenFilter) expect(await idsOn(face, { where: resolveTokens(c.tokenFilter) }), `${face}, tokens`).toBe(expected); + if (c.dateRange) { + expect(await idsOn(face, { timeDimensions: [{ dimension: c.field, dateRange: resolveTokens(c.dateRange) }] }), `${face}, dateRange`).toBe(expected); + } + } + const { sql, params } = compileScopedFilterToSql(onColumns(c.filter), TEMPORAL, { + declaredValueShape: (field) => (field === COLUMN.at ? { type: 'datetime', multiple: false } : field === COLUMN.on ? { type: 'date', multiple: false } : undefined), + dialect: 'sqlite', + }); + const rows = await (engine as any).execute(`select "id" from "${TEMPORAL}" where ${sql}`, { args: params, object: TEMPORAL }); + expect(ids(rows as Array>), 'the read scope').toBe(expected); + }); + } +}); diff --git a/packages/services/service-analytics/src/__tests__/cross-field-reference-refusal.test.ts b/packages/services/service-analytics/src/__tests__/cross-field-reference-refusal.test.ts index 057f4d26d14..c9f12adbed5 100644 --- a/packages/services/service-analytics/src/__tests__/cross-field-reference-refusal.test.ts +++ b/packages/services/service-analytics/src/__tests__/cross-field-reference-refusal.test.ts @@ -215,21 +215,14 @@ describe("[#7598] the #5222 corpus's SUPPORTED arm is ROUTED by the `where` door kind: 'leaf', member: 'amount', operator: 'notEquals', values: [{ $field: 'budget' }], }); // …and the literal keeps its guard. This pair is the whole claim. - // [ADR-0053 D-D1, amended — #5930 step 3] The guard now arrives twice: the - // shared lowering's NULL escape (outer), around this face's own interim - // copy of it (inner) — the same rows, until the copy's deletion card. The - // reference above gets neither, from either. + // [ADR-0053 D-D1, amended — #5930 step 4] The guard arrives once: the + // shared lowering's NULL escape, its one source since this face's own copy + // was deleted. The reference above gets none. expect(tree({ amount: { $ne: 5 } })).toEqual({ kind: 'or', children: [ { kind: 'leaf', member: 'amount', operator: 'notSet', values: [] }, - { - kind: 'or', - children: [ - { kind: 'leaf', member: 'amount', operator: 'notSet', values: [] }, - { kind: 'leaf', member: 'amount', operator: 'notEquals', values: [5] }, - ], - }, + { kind: 'leaf', member: 'amount', operator: 'notEquals', values: [5] }, ], }); }); diff --git a/packages/services/service-analytics/src/__tests__/filter-normalizer-mixed-wrapper.test.ts b/packages/services/service-analytics/src/__tests__/filter-normalizer-mixed-wrapper.test.ts index 4e50b14a94e..633881039ee 100644 --- a/packages/services/service-analytics/src/__tests__/filter-normalizer-mixed-wrapper.test.ts +++ b/packages/services/service-analytics/src/__tests__/filter-normalizer-mixed-wrapper.test.ts @@ -52,12 +52,14 @@ * gate shows up there as a throw. * * `the #5146 rewrite cannot swallow the wrapper` is the gate-side question, - * same as #6386's: the gate sits in `fieldLeaves`, downstream of - * `nullSafeNegationOperand`. For a MIXED wrapper the carry-through is - * structural: a non-`$` key never satisfies `operatorIsNullTotal`, so - * `nullGuardForFieldSpec` never answers `none` for one — the disposition is - * always `requireValue`/`allowNull`, both of which push the spec by - * reference, so the gate always sees the author's wrapper. + * same as #6386's: the gate sits in `fieldLeaves`, downstream of the rewrite — + * the shared lowering's rule 3 (`lowerFilterCondition`, `filter-lowering.ts`), + * since #5930 step 4 deleted this module's own copy of it. For a MIXED wrapper + * the carry-through is structural: a non-`$` key is never total for a missing + * value (the lowering's `operatorIsNullTotal`), so its `nullGuardForFieldSpec` + * never answers `none` for one — the disposition is always + * `requireValue`/`allowNull`, both of which carry the spec by reference, so the + * gate always sees the author's wrapper. * * ## Reverse verification — direction predicted BEFORE running * @@ -263,11 +265,12 @@ describe('[#6444] a mixed $/non-$ field wrapper is ONE refusal', () => { }); describe('[#6444] the #5146 rewrite cannot swallow the wrapper', () => { - // The gate lives in `fieldLeaves`, DOWNSTREAM of `nullSafeNegationOperand`. - // A mixed wrapper reaches it because a non-$ key never satisfies - // `operatorIsNullTotal`, so `nullGuardForFieldSpec` never answers `none` for - // one — `requireValue` and `allowNull` both push the author's spec by - // REFERENCE. One case per rewrite path that can carry a mixed wrapper. + // The gate lives in `fieldLeaves`, DOWNSTREAM of the `$not` rewrite (the + // shared lowering's rule 3 since #5930 step 4). A mixed wrapper reaches it + // because a non-$ key never satisfies the lowering's `operatorIsNullTotal`, + // so its `nullGuardForFieldSpec` never answers `none` for one — + // `requireValue` and `allowNull` both carry the author's spec by REFERENCE. + // One case per rewrite path that can carry a mixed wrapper. const REWRITE_PATHS: Array<{ name: string; where: unknown; field: string }> = [ { name: '`requireValue` — pushes {k: {$null: false}}, {k: spec}; spec kept by reference', diff --git a/packages/services/service-analytics/src/__tests__/filter-normalizer-not-null-safe.test.ts b/packages/services/service-analytics/src/__tests__/filter-normalizer-not-null-safe.test.ts index 6d920609dfb..7cdd6ec4e29 100644 --- a/packages/services/service-analytics/src/__tests__/filter-normalizer-not-null-safe.test.ts +++ b/packages/services/service-analytics/src/__tests__/filter-normalizer-not-null-safe.test.ts @@ -324,14 +324,14 @@ describe('[#5325] analytics `where` — NULL-safe `$not` and the boolean identit describe('a NULL column does not satisfy the negated condition', () => { it('the guard rides the LEAF, so the emitted SQL negates a TOTAL predicate', async () => { const { sql } = await sqlFor({ $not: { stage: 'won' } }); - // [ADR-0053 D-D1, amended — #5930 step 3] The shared lowering totalises - // the `$not` operand first (`{ stage: { $null: false } }` beside the - // leaf); this face's own copy then guards the leaf again. `X AND (X AND - // Y)` ≡ `X AND Y`: the predicate, and the ids above, are unchanged until - // the copy's deletion card. Asserted as emitted, for this file's reason. + // [ADR-0053 D-D1, amended — #5930 step 4] The shared lowering totalises + // the `$not` operand (`{ stage: { $null: false } }` beside the leaf), and + // it is the guard's one source: this face's own copy, which guarded the + // leaf a second time, is deleted. Asserted as emitted, for this file's + // reason. expect(sql).toBe( 'SELECT id AS "id", COUNT(*) AS "total" FROM "deal" ' + - 'WHERE NOT ((stage IS NOT NULL AND (stage IS NOT NULL AND stage = $1))) GROUP BY id', + 'WHERE NOT ((stage IS NOT NULL AND stage = $1)) GROUP BY id', ); }); @@ -361,18 +361,15 @@ describe('[#5325] analytics `where` — NULL-safe `$not` and the boolean identit // filter excludes. `{$not: {$ne: 'won'}}` means "stage IS won". expect(await ids({ $not: { stage: { $ne: 'won' } } })).toEqual(['1']); expect(await ids({ $not: { stage: { $nin: ['won'] } } })).toEqual(['1']); - // [#5298] The guard now appears TWICE: `nullSafeNegationOperand`'s - // `allowNull` arm wraps the field spec, and `fieldLeaves` wraps the `$ne` - // leaf itself because the operator is NULL-safe everywhere now, not only - // under a `$not`. `X OR (X OR Y)` ≡ `X OR Y`, so the predicate is the one - // this case has always asserted — the two id sets above are the guarantee, - // and they are unchanged. Asserted as it is actually emitted rather than - // trimmed to the prettier form: a pin that describes SQL the compiler does - // not produce is how the next reader learns to distrust this file. - // [ADR-0053 D-D1, amended — #5930 step 3] …and a third time: the shared - // lowering's own `allowNull` escape on the `$not` operand, outermost. + // [ADR-0053 D-D1, amended — #5930 step 4] The guard appears ONCE: the + // shared lowering's `allowNull` escape on the `$not` operand. This face's + // two copies of it (the `$not`-operand rewrite and the #5298 leaf wrap, + // which made it three) are deleted; the two id sets above are the + // guarantee, and they are unchanged. Asserted as it is actually emitted: + // a pin that describes SQL the compiler does not produce is how the next + // reader learns to distrust this file. const { sql } = await sqlFor({ $not: { stage: { $ne: 'won' } } }); - expect(sql).toContain('NOT ((stage IS NULL OR (stage IS NULL OR (stage IS NULL OR stage != $1))))'); + expect(sql).toContain('NOT ((stage IS NULL OR stage != $1))'); }); it('`$not` of an ordering comparison returns the NULL rows', async () => { @@ -450,10 +447,9 @@ describe('[#5325] analytics `where` — NULL-safe `$not` and the boolean identit // generated SQL only — `region` is not a column of this fixture, which is // the point: both halves resolve to ONE member. const { sql } = await sqlFor({ $not: { 'account.region': 'NA' } }); - // [ADR-0053 D-D1, amended — #5930 step 3] The shared lowering's guard - // lands on the dotted member (outer), and this face's own copy adds its - // own (inner) — never a guard on `account` itself. - expect(sql).toContain('NOT (("account"."region" IS NOT NULL AND ("account"."region" IS NOT NULL AND "account"."region" = $1)))'); + // [ADR-0053 D-D1, amended — #5930 step 4] The shared lowering's guard, + // its one source, lands on the dotted member — never on `account` itself. + expect(sql).toContain('NOT (("account"."region" IS NOT NULL AND "account"."region" = $1))'); expect(sql).not.toContain('"deal"."account" IS NOT NULL'); expect(sql).not.toMatch(/(^|[^."])account IS NOT NULL/); // [#20887] REPLACED spelling. This case wrote the NESTED form @@ -509,15 +505,15 @@ describe('[#5325] analytics `where` — NULL-safe `$not` and the boolean identit // Not a dialect equivalent (`IS DISTINCT FROM` / `<=>`): `NOT LIKE` has no // such form, so the family would have needed two shapes. The cost-list // measurement (#5298 §2/§3) found the query plans identical either way. - // [ADR-0053 D-D1, amended — #5930 step 3] The shared lowering's NULL - // escape (outer) now arrives around this face's own copy (inner): the - // same OR expansion, twice, the same rows, until the copy's deletion card. + // [ADR-0053 D-D1, amended — #5930 step 4] The shared lowering's NULL + // escape is the expansion's one source: this face's own copy, which + // wrapped the leaf a second time, is deleted. The same rows. expect((await sqlFor({ stage: { $ne: 'won' } })).sql) - .toContain('WHERE (stage IS NULL OR (stage IS NULL OR stage != $1))'); + .toContain('WHERE (stage IS NULL OR stage != $1)'); expect((await sqlFor({ stage: { $nin: ['won'] } })).sql) - .toContain('WHERE (stage IS NULL OR (stage IS NULL OR stage NOT IN ($1)))'); + .toContain('WHERE (stage IS NULL OR stage NOT IN ($1))'); expect((await sqlFor({ stage: { $notContains: 'wo' } })).sql) - .toContain('WHERE (stage IS NULL OR (stage IS NULL OR stage NOT LIKE $1 ESCAPE $2))'); + .toContain('WHERE (stage IS NULL OR stage NOT LIKE $1 ESCAPE $2)'); }); it('the ObjectQL path and the display SQL agree with the raw-SQL path', async () => { @@ -534,8 +530,8 @@ describe('[#5325] analytics `where` — NULL-safe `$not` and the boolean identit it('positive comparisons take NO guard — the polarity table decides, not a name list', async () => { // A blanket null escape would hand back the rows these filters exclude. - // `$eq` / `$in` / `$contains` are the family `nullValueSatisfiesOperator` - // answers `false` for, and they compile byte-identically to before. + // `$eq` / `$in` / `$contains` are the family the shared lowering's + // `nullValueSatisfiesOperator` answers `false` for, and they compile byte-identically to before. expect((await sqlFor({ stage: { $eq: 'won' } })).sql).toContain('WHERE stage = $1'); expect((await sqlFor({ stage: { $in: ['won'] } })).sql).toContain('WHERE stage IN ($1)'); expect((await sqlFor({ stage: { $contains: 'wo' } })).sql) @@ -547,7 +543,7 @@ describe('[#5325] analytics `where` — NULL-safe `$not` and the boolean identit it('the operators that are already TOTAL are not wrapped either', async () => { // `$ne: null` compiles to `set` (`IS NOT NULL`), which is two-valued by // construction — wrapping it would turn "stage has a value" into a - // tautology. `operatorIsNullTotal` is what keeps the two apart, and it + // tautology. The shared lowering's `operatorIsNullTotal` is what keeps the two apart, and it // reads the COMPARAND, which is why a hard-coded list of three operator // NAMES would have been wrong here as well as duplicated. expect((await sqlFor({ stage: { $ne: null } })).sql).toContain('WHERE stage IS NOT NULL'); @@ -604,21 +600,24 @@ describe('[#5325] analytics `where` — NULL-safe `$not` and the boolean identit // behind the engine — NULL-safe or not — admits the same rows. Rendering it // only in the SQL strategy would have made the answer depend on the driver. // [#20918] It travels in the engine's own spelling, `{ $null: false }`. - expect(JSON.stringify(lastEngineFilter)).toContain('{"stage":{"$null":false}}'); - expect(JSON.stringify(lastEngineFilter)).toContain('$not'); - }); - - it('DOUBLE-guarding is idempotent — the stand-in engine guards again', async () => { - // `compileScopedFilterToSql` runs its OWN `nullSafeNegationOperand` over - // the condition this path already guarded, so the executed SQL carries the - // guard twice. `NOT (c IS NOT NULL AND (c IS NOT NULL AND c = v))` is the - // same predicate as the single-guarded form: redundant, not wrong. That is - // the trade the module header names — one extra conjunct for portability. + // [ADR-0053 D-D1, amended — #5930 step 4] Once, from the shared lowering: + // the conjunct sits beside the leaf in the `$not` operand. + expect(lastEngineFilter).toEqual({ $and: [{ $not: { stage: { $null: false }, $and: [{ stage: 'won' }] } }] }); + }); + + it('a second lowering of the guarded condition adds no guard — the stand-in engine lowers again', async () => { + // [ADR-0053 D-D1, amended — #5930 step 4] `compileScopedFilterToSql` runs + // the shared lowering at its entry over the condition this path already + // guarded. The lowering is idempotent — it reads the `{ stage: { $null: + // false } }` conjunct beside the leaf as the guard it would write — so the + // executed SQL carries the guard ONCE. Until #5930 step 4 that compiler + // also ran its own copy of the rewrite, which guarded the leaf a second + // time: redundant, not wrong, and deleted with the copy. const { sql } = compileScopedFilterToSql( lastEngineFilter as FilterCondition, 'deal', ); - expect(sql.match(/IS NOT NULL/g)?.length).toBeGreaterThanOrEqual(2); + expect(sql.match(/IS NOT NULL/g)?.length).toBe(1); expect(run(`SELECT "id" FROM "deal" AS "deal" WHERE ${sql}`, ['won'])).toEqual(['2', '3', '4']); }); diff --git a/packages/services/service-analytics/src/__tests__/filter-normalizer-undefined-comparand.test.ts b/packages/services/service-analytics/src/__tests__/filter-normalizer-undefined-comparand.test.ts index c28f74f5f5a..bb6c971ca42 100644 --- a/packages/services/service-analytics/src/__tests__/filter-normalizer-undefined-comparand.test.ts +++ b/packages/services/service-analytics/src/__tests__/filter-normalizer-undefined-comparand.test.ts @@ -51,7 +51,8 @@ * both before and after, tree for tree. * * `the #5146 rewrite cannot swallow the leaf` is the gate-SIDE question. The gate - * sits in `fieldLeaves`, downstream of `nullSafeNegationOperand`, so whether row + * sits in `fieldLeaves`, downstream of the `$not` rewrite (the shared lowering's + * rule 3 since #5930 step 4 deleted this module's copy), so whether row * three throws or merely changes shape depends on the rewrite carrying the * author's spec through. Measured, not assumed — PR #6390 hit the same trap on * the sibling door, and its reasoning does not transfer (that module's polarity @@ -414,7 +415,8 @@ describe('[#6386] the `null` control group does not move', () => { }); describe('[#6386] the #5146 rewrite cannot swallow the leaf — the gate side is load-bearing', () => { - // The gate lives in `fieldLeaves`, DOWNSTREAM of `nullSafeNegationOperand`, so + // The gate lives in `fieldLeaves`, DOWNSTREAM of the `$not` rewrite (the shared + // lowering's rule 3 since #5930 step 4), so // `{$not: {…}}` throws only if the rewrite carries the author's spec through. // One case per rewrite path that can carry a swept comparand. const REWRITE_PATHS: Array<{ name: string; where: unknown; path: string }> = [ @@ -446,7 +448,7 @@ describe('[#6386] the #5146 rewrite cannot swallow the leaf — the gate side is ]; // [#20035] RE-JUDGED. The shared type face (#7872) now answers first, on the - // author's OWN condition, before `nullSafeNegationOperand` rewrites anything: + // author's OWN condition, before the `$not` rewrite rewrites anything: // `lowerAnalyticsWhere` runs it ahead of `buildNode`. So the refusal names // the `$not` path the author wrote (`where.$not.d`) — the question "can the // rewrite swallow the leaf before the gate sees it" is answered upstream of @@ -462,7 +464,8 @@ describe('[#6386] the #5146 rewrite cannot swallow the leaf — the gate side is } it('there is no `none`-disposition case to write, and this is why', () => { - // `nullGuardForFieldSpec` answers 'none' only when EVERY operator satisfies + // The rewrite's `nullGuardForFieldSpec` (the shared lowering's since #5930 + // step 4) answers 'none' only when EVERY operator satisfies its // `operatorIsNullTotal`, which for an `undefined` comparand is false on every // operator this gate sweeps ($eq/$ne compare `value === null`; $in/$nin need // an empty array). So the only field specs reaching 'none' while holding an diff --git a/packages/services/service-analytics/src/__tests__/native-sql-temporal-conformance.test.ts b/packages/services/service-analytics/src/__tests__/native-sql-temporal-conformance.test.ts index 078f9cc0dd0..9582beeac3c 100644 --- a/packages/services/service-analytics/src/__tests__/native-sql-temporal-conformance.test.ts +++ b/packages/services/service-analytics/src/__tests__/native-sql-temporal-conformance.test.ts @@ -136,39 +136,64 @@ describe('NativeSQLStrategy — temporal conformance', () => { db?.close(); }); - /** Group by `id` so the result rows ARE the matched row ids. */ - const idsFor = async (query: Omit) => { - const result = await new NativeSQLStrategy().execute( - { cube: 'conformance', measures: ['total'], dimensions: ['id'], ...query } as AnalyticsQuery, - ctx, - ); - return result.rows.map((r) => String(r.id)).sort(); - }; + /** + * [ADR-0053 D-D1, amended — #5930 step 4] The matrix runs twice: over the + * context above, which wires no declared-type hook (the strategy then reads + * every column type-blind, item 7), and over the same context with the hook + * the plugin wires (`happened_at` a `datetime`, `happened_on` a `date`), the + * reader the shared lowering applies in production. The whole-day rule is the + * lowering's alone since this face's own copy was deleted, so both readers + * must answer every case. + */ + const READERS: Array<[string, () => StrategyContext]> = [ + ['no declared-type hook', () => ctx], + [ + 'the declared-type hook', + () => ({ + ...ctx, + declaredFieldType: (_object: string, field: string) => + field === 'happened_at' ? 'datetime' : field === 'happened_on' ? 'date' : undefined, + }) as StrategyContext, + ], + ]; - for (const c of TEMPORAL_CASES) { - it(c.name, async () => { - expect(await idsFor({ where: c.filter }), c.note).toEqual([...c.expected].sort()); - }); + for (const [reader, ctxOf] of READERS) { + /** Group by `id` so the result rows ARE the matched row ids. */ + const idsFor = async (query: Omit) => { + const result = await new NativeSQLStrategy().execute( + { cube: 'conformance', measures: ['total'], dimensions: ['id'], ...query } as AnalyticsQuery, + ctxOf(), + ); + return result.rows.map((r) => String(r.id)).sort(); + }; - // The D-A3 token axis (#4081): the same case spelled in relative tokens, - // resolved at the pinned instant, must reach the same rows. - if (c.tokenFilter) { - it(`${c.name} — via relative tokens`, async () => { - expect(await idsFor({ where: resolveTokens(c.tokenFilter) }), c.note).toEqual( - [...c.expected].sort(), - ); - }); - } - - // The dashboard-window path — the shape #3650 dropped entirely. No - // granularity, or `canHandle` correctly declines to the ObjectQL strategy. - if (c.dateRange) { - it(`${c.name} — via timeDimensions.dateRange`, async () => { - expect( - await idsFor({ timeDimensions: [{ dimension: c.field, dateRange: resolveTokens(c.dateRange) }] }), - c.note, - ).toEqual([...c.expected].sort()); - }); - } + describe(reader, () => { + for (const c of TEMPORAL_CASES) { + it(c.name, async () => { + expect(await idsFor({ where: c.filter }), c.note).toEqual([...c.expected].sort()); + }); + + // The D-A3 token axis (#4081): the same case spelled in relative tokens, + // resolved at the pinned instant, must reach the same rows. + if (c.tokenFilter) { + it(`${c.name} — via relative tokens`, async () => { + expect(await idsFor({ where: resolveTokens(c.tokenFilter) }), c.note).toEqual( + [...c.expected].sort(), + ); + }); + } + + // The dashboard-window path — the shape #3650 dropped entirely. No + // granularity, or `canHandle` correctly declines to the ObjectQL strategy. + if (c.dateRange) { + it(`${c.name} — via timeDimensions.dateRange`, async () => { + expect( + await idsFor({ timeDimensions: [{ dimension: c.field, dateRange: resolveTokens(c.dateRange) }] }), + c.note, + ).toEqual([...c.expected].sort()); + }); + } + } + }); } }); diff --git a/packages/services/service-analytics/src/__tests__/objectql-contains-canonical-operator.test.ts b/packages/services/service-analytics/src/__tests__/objectql-contains-canonical-operator.test.ts index d404cb9acce..050daad0681 100644 --- a/packages/services/service-analytics/src/__tests__/objectql-contains-canonical-operator.test.ts +++ b/packages/services/service-analytics/src/__tests__/objectql-contains-canonical-operator.test.ts @@ -265,11 +265,11 @@ describe('[#5557] `contains` reaches the engine as `$contains`, comparand taken // the null-predicate disjunct. What THIS case asserts is unaffected and // still exact: the operator key is the declared `$notContains` and the // comparand is the author's literal `'a.b'`, not a `$regex` pattern. - // [ADR-0053 D-D1, amended — #5930 step 3] …twice now: the shared - // lowering's escape around this face's own copy. Same operator key, same - // literal comparand, same rows. + // [ADR-0053 D-D1, amended — #5930 step 4] …once: the shared lowering's + // escape, its one source since this face's own copy was deleted. Same + // operator key, same literal comparand, same rows. expect(await engineFilter({ stage: { $notContains: 'a.b' } })).toEqual({ - $and: [{ $or: [{ stage: { $null: true } }, { $or: [{ stage: { $null: true } }, { stage: { $notContains: 'a.b' } }] }] }], + $and: [{ $or: [{ stage: { $null: true } }, { stage: { $notContains: 'a.b' } }] }], }); expect(await engineFilter({ stage: { $startsWith: 'a.b' } })).toEqual({ stage: { $startsWith: 'a.b' }, diff --git a/packages/services/service-analytics/src/__tests__/objectql-daterange.test.ts b/packages/services/service-analytics/src/__tests__/objectql-daterange.test.ts index 76f6a9feff0..5e81f6cbd44 100644 --- a/packages/services/service-analytics/src/__tests__/objectql-daterange.test.ts +++ b/packages/services/service-analytics/src/__tests__/objectql-daterange.test.ts @@ -60,7 +60,7 @@ type AggOpts = { function matches(row: Row, filter: Record): boolean { return Object.entries(filter).every(([key, cond]) => { if (key === '$and') return (cond as Record[]).every((sub) => matches(row, sub)); - // [#5298] `fieldLeaves` emits a NULL-safe `$ne` as `$or: [{ field: { $null: true } }, { field: { $ne } }]`, + // [#5298] The shared lowering writes a NULL-safe `$ne` as `$or: [{ field: { $null: true } }, { field: { $ne } }]`, // so a real query genuinely hands this double an `$or` — it is not dormant here. if (key === '$or') return (cond as Record[]).some((sub) => matches(row, sub)); if (key.startsWith('$')) throw new Error(`test bridge: unhandled operator ${key}`); @@ -344,18 +344,16 @@ describe('ObjectQLStrategy — window ∧ where on one field (#3650)', () => { ctx, ); - // [#5298] The `$ne` operand arrives NULL-safe: `fieldLeaves` emits it as an - // `or` of the null predicate with the comparison, so "stage is not lost" - // keeps the rows that have no stage — the answer every other backend gives. - // What this case is about is unchanged and still visible: BOTH operands - // survive, the second as its own `$and` conjunct rather than overwriting the - // bare equality. - // [ADR-0053 D-D1, amended — #5930 step 3] The null predicate now arrives - // twice, the shared lowering's escape around this face's own copy — the - // same rows; both operands still survive. + // [#5298] The `$ne` operand arrives NULL-safe: the shared lowering writes + // it as an `$or` of the null predicate with the comparison (once — this + // face's own copy of the escape is deleted, #5930 step 4), so "stage is not + // lost" keeps the rows that have no stage — the answer every other backend + // gives. What this case is about is unchanged and still visible: BOTH + // operands survive, the second as its own `$and` conjunct rather than + // overwriting the bare equality. expect(seen[0].filter).toEqual({ stage: 'won', - $and: [{ $or: [{ stage: { $null: true } }, { $or: [{ stage: { $null: true } }, { stage: { $ne: 'lost' } }] }] }], + $and: [{ $or: [{ stage: { $null: true } }, { stage: { $ne: 'lost' } }] }], }); }); }); @@ -390,10 +388,21 @@ describe('DatasetExecutor compareTo over the ObjectQL path (#3650)', () => { }); }); +/** + * [ADR-0053 D-D1 item 8, amended — #5930 step 4] The echo renders each window + * through the shared lowering, with the reader `execute()` hands the engine's + * `where` seam: the host's declared type of the column (`sourceFieldMeta`, the + * hook the plugin answers from the engine's registry). These hosts declare + * `close_date` a `datetime`, the column the half-open render is for. + */ +const DECLARED_DATETIME = { + sourceFieldMeta: (_object: string, field: string) => (field === 'close_date' ? { type: 'datetime' } : undefined), +}; + describe('ObjectQLStrategy.generateSql — window rendering (#3650)', () => { it('renders the window as a parameterised half-open pair', async () => { const seen: AggOpts[] = []; - const svc = makeService(seen); + const svc = makeService(seen, DECLARED_DATETIME); const { sql, params } = await svc.generateSql!({ cube: 'sales', @@ -419,7 +428,7 @@ describe('ObjectQLStrategy.generateSql — window rendering (#3650)', () => { it('numbers window placeholders after the caller\'s own filters', async () => { const seen: AggOpts[] = []; - const svc = makeService(seen); + const svc = makeService(seen, DECLARED_DATETIME); const { sql, params } = await svc.generateSql!({ cube: 'sales', @@ -438,7 +447,7 @@ describe('ObjectQLStrategy.generateSql — window rendering (#3650)', () => { // the five-digit '10000-01-01' as the upper bound — SQL that answers no rows // on SQLite, where the column is ISO text that sorts above it. it('renders a window ending on the last supported day with no upper bound; 9999-12-30 keeps one', async () => { - const svc = makeService([]); + const svc = makeService([], DECLARED_DATETIME); const echo = (end: string) => svc.generateSql!({ cube: 'sales', dimensions: ['stage'], @@ -446,8 +455,10 @@ describe('ObjectQLStrategy.generateSql — window rendering (#3650)', () => { timeDimensions: [{ dimension: 'close_date', dateRange: ['2026-01-01', end] }], }); + // [#5930 step 4] The shared lowering keeps only "has a value" beside the + // start, `{ $gte, $null: false }`: the same rows as the start alone. const last = await echo('9999-12-31'); - expect(last.sql).toContain('(close_date >= $1)'); + expect(last.sql).toContain('(close_date >= $1 AND close_date IS NOT NULL)'); expect(last.sql).not.toContain('close_date <'); expect(last.params).toEqual(['2026-01-01']); @@ -455,6 +466,38 @@ describe('ObjectQLStrategy.generateSql — window rendering (#3650)', () => { expect(control.sql).toContain('(close_date >= $1 AND close_date < $2)'); expect(control.params).toEqual(['2026-01-01', '9999-12-31']); }); + + it('[#5930 step 4] renders the bound execute() hands the engine: inclusive on a declared `date`, and as written where the host names no type', async () => { + const echo = (overrides: Record) => makeService([], overrides).generateSql!({ + cube: 'sales', + dimensions: ['stage'], + measures: ['revenue'], + timeDimensions: [{ dimension: 'close_date', dateRange: ['2026-01-01', '2026-01-31'] }], + }); + const asWritten = { sql: '(close_date >= $1 AND close_date <= $2)', params: ['2026-01-01', '2026-01-31'] }; + // A declared `date`: the engine's seam compares a calendar day as written. + const onDate = await echo({ sourceFieldMeta: (_o: string, f: string) => (f === 'close_date' ? { type: 'date' } : undefined) }); + expect(onDate.sql).toContain(asWritten.sql); + expect(onDate.params).toEqual(asWritten.params); + // No declared type: this face hands the engine `$lte` as written and the + // engine's seam, which reads the object's own field map, lowers it; the + // echo prints what this face hands it. + const undeclared = await echo({}); + expect(undeclared.sql).toContain(asWritten.sql); + expect(undeclared.params).toEqual(asWritten.params); + }); + + it('[#5930 step 4] a resolved preset that stops before its end renders `<` its end instant', async () => { + const { sql, params } = await makeService([], DECLARED_DATETIME).generateSql!({ + cube: 'sales', + dimensions: ['stage'], + measures: ['revenue'], + timeDimensions: [{ dimension: 'close_date', dateRange: 'this_year' }], + }); + expect(sql).toContain('(close_date >= $1 AND close_date < $2)'); + expect(params).toHaveLength(2); + expect(params.every((p) => typeof p === 'string' && /T00:00:00\.000Z$/.test(p as string))).toBe(true); + }); }); describe('ObjectQLStrategy — cross-object FK-expand carries the window (#3650 × #3654)', () => { diff --git a/packages/services/service-analytics/src/__tests__/preview-evaluator.test.ts b/packages/services/service-analytics/src/__tests__/preview-evaluator.test.ts index 7cbf9e3db20..8b02ed7c090 100644 --- a/packages/services/service-analytics/src/__tests__/preview-evaluator.test.ts +++ b/packages/services/service-analytics/src/__tests__/preview-evaluator.test.ts @@ -10,6 +10,7 @@ import { DatasetSchema } from '@objectstack/spec/ui'; import { AnalyticsService } from '../analytics-service.js'; import { evaluateAnalyticsQueryOverRows, bucketDate, matchesWhere } from '../preview-evaluator.js'; import type { Cube } from '@objectstack/spec/data'; +import type { AnalyticsQuery } from '@objectstack/spec/contracts'; const SEED_ROWS = [ { title: 'Flight', amount: 1200, category: 'travel', spent_on: '2026-05-03' }, @@ -151,19 +152,34 @@ describe('evaluateAnalyticsQueryOverRows', () => { expect(matchesWhere({ a: 'Hello World' }, { a: { $contains: 'world' } })).toBe(true); }); - it('a bare-day $lte covers the whole day on a timestamp value (#3777)', () => { - // Same translation the SQL paths apply — the preview must agree, or a - // drafted chart shows different numbers than the published one. - expect(matchesWhere({ at: '2026-07-28T21:40:00.000Z' }, { at: { $lte: '2026-07-28' } })).toBe(true); - expect(matchesWhere({ at: '2026-07-29T00:00:00.000Z' }, { at: { $lte: '2026-07-28' } })).toBe(false); - // A plain date value is unchanged (string ordering makes the two forms - // equivalent there). - expect(matchesWhere({ on: '2026-07-28' }, { on: { $lte: '2026-07-28' } })).toBe(true); - expect(matchesWhere({ on: '2026-07-29' }, { on: { $lte: '2026-07-28' } })).toBe(false); + it('a bare-day $lte covers the whole day on a declared datetime — the shared lowering\'s rule, not the matcher\'s (#3777, #5930 step 4)', () => { + // [ADR-0053 D-D1, amended — #5930 step 4] `matchesWhere` compares the bound + // it is handed, like every other operator: a bare day is that day's + // midnight to it. The whole day is the shared lowering's, which + // `evaluateAnalyticsQueryOverRows` runs with the drafted object's declared + // types — the same answer the SQL paths give, so a drafted chart shows the + // numbers the published one will. + expect(matchesWhere({ at: '2026-07-28T21:40:00.000Z' }, { at: { $lte: '2026-07-28' } })).toBe(false); + const CUBE = { name: 'c', sql: 'c', dimensions: { id: { sql: 'id', type: 'string' } }, measures: { n: { sql: '*', type: 'count' } } } as unknown as Cube; + const rows = [ + { id: 'late', at: '2026-07-28T21:40:00.000Z', on: '2026-07-28', note: '2026-07-28 late' }, + { id: 'next', at: '2026-07-29T00:00:00.000Z', on: '2026-07-29', note: '2026-07-29' }, + ]; + const declared = (field: string) => ({ at: 'datetime', on: 'date', note: 'text' } as Record)[field]; + const ids = (where: Record, declaredType?: (field: string) => string | undefined) => + evaluateAnalyticsQueryOverRows({ measures: ['n'], dimensions: ['id'], where } as AnalyticsQuery, CUBE, rows.map((r) => ({ ...r })), declaredType) + .rows.map((r) => String(r.id)).sort(); + // A declared datetime: the whole final day, and not the next midnight. + expect(ids({ at: { $lte: '2026-07-28' } }, declared)).toEqual(['late']); + // A declared date: the comparison as written, which orders exactly as the whole day. + expect(ids({ on: { $lte: '2026-07-28' } }, declared)).toEqual(['late']); + // A declared text column: as written, as the engine compares it — the + // `'2026-07-28 late'` text sorts after `'2026-07-28'`. + expect(ids({ note: { $lte: '2026-07-28' } }, declared)).toEqual([]); + // No declared type: type-blind (ADR-0053 D-D1 item 7). + expect(ids({ note: { $lte: '2026-07-28' } })).toEqual(['late']); // Full-ISO bounds keep instant semantics. - expect( - matchesWhere({ at: '2026-07-28T21:40:00.000Z' }, { at: { $lte: '2026-07-28T12:00:00.000Z' } }), - ).toBe(false); + expect(ids({ at: { $lte: '2026-07-28T12:00:00.000Z' } }, declared)).toEqual([]); }); it('a timeDimension dateRange keeps the final day of the window on timestamps (#3777)', () => { diff --git a/packages/services/service-analytics/src/__tests__/preview-temporal-conformance.test.ts b/packages/services/service-analytics/src/__tests__/preview-temporal-conformance.test.ts index ec3be03bc1d..a4710314f54 100644 --- a/packages/services/service-analytics/src/__tests__/preview-temporal-conformance.test.ts +++ b/packages/services/service-analytics/src/__tests__/preview-temporal-conformance.test.ts @@ -13,8 +13,14 @@ * the preview and the driver disagree about a window, the numbers jump across * the publish boundary — the continuity the preview exists to provide. * - * Type-blind, like `formula`: it evaluates a bare row with no schema, so both - * the `datetime` and `date` cases run against the raw string values. + * [ADR-0053 D-D1, amended — #5930 step 4] The whole-day bound is the shared + * lowering's on this face, applied by `evaluateAnalyticsQueryOverRows` with the + * drafted object's declared types (`queryDataset` hands it `sourceFieldMeta`); + * the matcher compares what it is handed. So the matrix runs through the + * evaluator twice: with the declared types (`at` a `datetime`, `on` a `date`), + * the reader production hands it, and with none, where every column reads + * type-blind (item 7). Both readers must answer every case. The `Field.time` + * cases carry no bare day, so the matcher answers them as written. */ import { describe, it, expect } from 'vitest'; @@ -55,59 +61,63 @@ const nativeRows = TEMPORAL_ROWS.map((r) => ({ at: r.writerForm === 'native' ? new Date(r.at) : r.at, })); -describe('preview-evaluator — temporal conformance', () => { - for (const c of TEMPORAL_CASES) { - it(c.name, () => { - const got = TEMPORAL_ROWS.filter((r) => matchesWhere(r as any, c.filter as any)).map((r) => r.id); - expect(got, c.note).toEqual(c.expected); - }); +const CUBE = { + name: 'conformance_ds', + sql: 'conformance', + dimensions: { id: { type: 'string', sql: 'id' } }, + measures: { count: { type: 'count', sql: '*' } }, +} as unknown as Cube; - it(`${c.name} — on a native-writer (BSON Date) row population`, () => { - const got = nativeRows.filter((r) => matchesWhere(r as any, c.filter as any)).map((r) => r.id); - expect(got, c.note).toEqual(c.expected); - }); +const READERS: Array<[string, ((field: string) => string | undefined) | undefined]> = [ + ['the declared types', (field) => (field === 'at' ? 'datetime' : field === 'on' ? 'date' : undefined)], + ['no declared type (type-blind)', undefined], +]; + +/** The ids a query selects through the evaluator, grouped by `id` so the output rows ARE the ids. */ +const idsFor = ( + rest: Record, + rows: ReadonlyArray>, + declaredType: ((field: string) => string | undefined) | undefined, +): string[] => + evaluateAnalyticsQueryOverRows( + { measures: ['count'], dimensions: ['id'], ...rest } as never, + CUBE, + rows.map((r) => ({ ...r })), + declaredType, + ).rows.map((r) => String(r.id)).sort(); - // The D-A3 token axis (#4081): the same case spelled in relative tokens, - // resolved at the pinned instant, must reach the same rows. - if (c.tokenFilter) { - it(`${c.name} — via relative tokens`, () => { - const where = resolveTokens(c.tokenFilter); - const got = TEMPORAL_ROWS.filter((r) => matchesWhere(r as any, where as any)).map((r) => r.id); - expect(got, c.note).toEqual(c.expected); +for (const [reader, declaredType] of READERS) { + describe(`preview-evaluator — temporal conformance, read with ${reader}`, () => { + for (const c of TEMPORAL_CASES) { + const expected = [...c.expected].sort(); + it(c.name, () => { + expect(idsFor({ where: c.filter }, TEMPORAL_ROWS as never, declaredType), c.note).toEqual(expected); }); - } - } -}); -describe('preview-evaluator — timeDimensions.dateRange temporal conformance', () => { - // The dashboard-window path — the surface #3650 broke (the range was - // dropped entirely and every row charted). Windows tagged with a - // `dateRange` spelling run through the full evaluator, grouped by `id` so - // the output rows ARE the matched row ids. - const CUBE = { - name: 'conformance_ds', - sql: 'conformance', - dimensions: { id: { type: 'string', sql: 'id' } }, - measures: { count: { type: 'count', sql: '*' } }, - } as unknown as Cube; + it(`${c.name} — on a native-writer (BSON Date) row population`, () => { + expect(idsFor({ where: c.filter }, nativeRows as never, declaredType), c.note).toEqual(expected); + }); - for (const c of TEMPORAL_CASES) { - if (!c.dateRange) continue; - it(`${c.name} — via timeDimensions.dateRange`, () => { - const result = evaluateAnalyticsQueryOverRows( - { - measures: ['count'], - dimensions: ['id'], - timeDimensions: [{ dimension: c.field, dateRange: resolveTokens(c.dateRange) }], - }, - CUBE, - TEMPORAL_ROWS.map((r) => ({ ...r })), - ); - const got = result.rows.map((r) => String(r.id)).sort(); - expect(got, c.note).toEqual([...c.expected].sort()); - }); - } -}); + // The D-A3 token axis (#4081): the same case spelled in relative tokens, + // resolved at the pinned instant, must reach the same rows. + if (c.tokenFilter) { + it(`${c.name} — via relative tokens`, () => { + expect(idsFor({ where: resolveTokens(c.tokenFilter) }, TEMPORAL_ROWS as never, declaredType), c.note).toEqual(expected); + }); + } + + // The dashboard-window path — the surface #3650 broke (the range was + // dropped entirely and every row charted). + if (c.dateRange) { + it(`${c.name} — via timeDimensions.dateRange`, () => { + const window = { timeDimensions: [{ dimension: c.field, dateRange: resolveTokens(c.dateRange) }] }; + expect(idsFor(window, TEMPORAL_ROWS as never, declaredType), c.note).toEqual(expected); + expect(idsFor(window, nativeRows as never, declaredType), `${c.note} (BSON Date rows)`).toEqual(expected); + }); + } + } + }); +} describe('preview-evaluator — Field.time conformance', () => { for (const c of TEMPORAL_TIME_CASES) { diff --git a/packages/services/service-analytics/src/__tests__/read-scope-boolean-flag-comparand.test.ts b/packages/services/service-analytics/src/__tests__/read-scope-boolean-flag-comparand.test.ts index b0508d18e35..d75b2af1f6e 100644 --- a/packages/services/service-analytics/src/__tests__/read-scope-boolean-flag-comparand.test.ts +++ b/packages/services/service-analytics/src/__tests__/read-scope-boolean-flag-comparand.test.ts @@ -74,7 +74,8 @@ * * `describe('the polarity table …')` covers the second half of the change — * `nullValueSatisfiesOperator`'s `$null` / `$exists` arms moving from truthiness - * to identity — and is honest about what can and cannot be observed from + * to identity (this compiler's own table then; the shared lowering's since + * #5930 step 4 deleted the copy, which reads them by identity too) — and is honest about what can and cannot be observed from * outside; see its own comment. */ @@ -278,23 +279,23 @@ describe('[#6387] the polarity table moved WITH the emitter (#5146 / #5298)', () const sql = (f: FilterCondition) => compileScopedFilterToSql(f, ALIAS).sql; it('allowNull polarity: a NULL column satisfies $null: true', () => { - // `$nin` makes the constraint non-total, so `nullGuardForFieldSpec` has to + // `$nin` makes the constraint non-total, so the `$not` rewrite has to // consult the table; `$null: true` says a NULL row DOES satisfy it, so the // leaf is guarded with `IS NULL OR (…)`. - // [ADR-0053 D-D1, amended — #5930 step 3] The shared lowering, at this - // compiler's entry, reads the same table and lays the same `allowNull` - // guard on first (outer); this compiler's own copy then adds its own - // (inner). Same predicate, until the copy's deletion card. + // [ADR-0053 D-D1, amended — #5930 step 4] The rewrite and its table are + // the shared lowering's, run at this compiler's entry: its one source since + // this compiler's own copy (which added a second guard inside) was deleted. expect(sql({ $not: { d: { $null: true, $nin: ['x'] } } })).toBe( - 'NOT ((("t"."d" IS NULL OR (("t"."d" IS NULL OR ("t"."d" IS NULL AND ("t"."d" IS NULL OR "t"."d" NOT IN (?))))))))', + 'NOT ((("t"."d" IS NULL OR ("t"."d" IS NULL AND "t"."d" NOT IN (?)))))', ); }); it('requireValue polarity: a NULL column does NOT satisfy $null: false', () => { - // [ADR-0053 D-D1, amended — #5930 step 3] The shared lowering's - // `requireValue` guard, then this compiler's own — see the row above. + // [ADR-0053 D-D1, amended — #5930 step 4] The shared lowering's + // `requireValue` guard, once — see the row above. The second `IS NOT NULL` + // is the `$null: false` operator itself. expect(sql({ $not: { d: { $null: false, $nin: ['x'] } } })).toBe( - 'NOT (("t"."d" IS NOT NULL AND ("t"."d" IS NOT NULL AND ("t"."d" IS NOT NULL AND ("t"."d" IS NULL OR "t"."d" NOT IN (?))))))', + 'NOT (("t"."d" IS NOT NULL AND ("t"."d" IS NOT NULL AND "t"."d" NOT IN (?))))', ); }); diff --git a/packages/services/service-analytics/src/__tests__/read-scope-not-null-safe.test.ts b/packages/services/service-analytics/src/__tests__/read-scope-not-null-safe.test.ts index 37056bdfe1b..9445f9586b2 100644 --- a/packages/services/service-analytics/src/__tests__/read-scope-not-null-safe.test.ts +++ b/packages/services/service-analytics/src/__tests__/read-scope-not-null-safe.test.ts @@ -243,11 +243,11 @@ describe('[#5297] read-scope `$not` — boolean identities and NULL safety', () it('the guard rides the leaf, so the emitted SQL negates a TOTAL predicate', () => { const { sql } = compileScopedFilterToSql({ $not: { stage: 'won' } } as FilterCondition, ALIAS); - // [ADR-0053 D-D1, amended — #5930 step 3] The shared lowering at this - // compiler's entry totalises the operand first; this compiler's own copy - // guards the leaf again. `X AND (X AND Y)` ≡ `X AND Y` until the copy's - // deletion card; the id sets in this block are the guarantee. - expect(sql).toBe('NOT (("t"."stage" IS NOT NULL AND ("t"."stage" IS NOT NULL AND "t"."stage" = ?)))'); + // [ADR-0053 D-D1, amended — #5930 step 4] The shared lowering at this + // compiler's entry totalises the operand, and it is the guard's one + // source: this compiler's own copy, which guarded the leaf a second time, + // is deleted. The id sets in this block are the guarantee. + expect(sql).toBe('NOT (("t"."stage" IS NOT NULL AND "t"."stage" = ?))'); }); it('`$not` over MULTIPLE columns admits a row that is NULL in EITHER', () => { @@ -406,10 +406,10 @@ describe('[#5297] read-scope `$not` — boolean identities and NULL safety', () * different row sets — the defect, not a smaller version of the fix. */ it('$ne / $nin / $notContains are NULL-safe outside a $not too (#5298)', () => { - // [ADR-0053 D-D1, amended — #5930 step 3] The shared lowering's NULL - // escape (outer) around this compiler's own (inner): the same rows. + // [ADR-0053 D-D1, amended — #5930 step 4] The shared lowering's NULL + // escape, once — this compiler's own wrap is deleted. The same rows. expect(compileScopedFilterToSql({ stage: { $ne: 'won' } } as FilterCondition, ALIAS).sql) - .toBe('(("t"."stage" IS NULL OR ("t"."stage" IS NULL OR "t"."stage" <> ?)))'); + .toBe('(("t"."stage" IS NULL OR "t"."stage" <> ?))'); expect(ids({ stage: { $ne: 'won' } })).toEqual(['2', '3', '4']); expect(ids({ stage: { $nin: ['won'] } })).toEqual(['2', '3', '4']); expect(ids({ stage: { $notContains: 'wo' } })).toEqual(['2', '3', '4']); diff --git a/packages/services/service-analytics/src/__tests__/read-scope-placeholder-three-faces.test.ts b/packages/services/service-analytics/src/__tests__/read-scope-placeholder-three-faces.test.ts index 6b3bc33f509..d7ce4d9eaa3 100644 --- a/packages/services/service-analytics/src/__tests__/read-scope-placeholder-three-faces.test.ts +++ b/packages/services/service-analytics/src/__tests__/read-scope-placeholder-three-faces.test.ts @@ -482,11 +482,11 @@ describe('[#20075] `compileScopedFilterToSql` — the public export', () => { const UNCHANGED: Array<{ scope: unknown; sql: string; params: unknown[] }> = [ { scope: { owner: 'u_me' }, sql: '"deal"."owner" = ?', params: ['u_me'] }, { - // [ADR-0053 D-D1, amended — #5930 step 3] The shared lowering's NULL - // escape now wraps this compiler's own: the same rows, with or without - // a context — which is what this row is about. + // [ADR-0053 D-D1, amended — #5930 step 4] The shared lowering's NULL + // escape, the guard's one source: the same rows, with or without a + // context — which is what this row is about. scope: { owner: { $ne: 'u_me' } }, - sql: '(("deal"."owner" IS NULL OR ("deal"."owner" IS NULL OR "deal"."owner" <> ?)))', + sql: '(("deal"."owner" IS NULL OR "deal"."owner" <> ?))', params: ['u_me'], }, { scope: { region: { $in: ['emea', 'amer'] } }, sql: '"deal"."region" IN (?, ?)', params: ['emea', 'amer'] }, @@ -508,7 +508,7 @@ describe('[#20075] `compileScopedFilterToSql` — the public export', () => { it('with a context, a placeholder binds its resolved value', () => { expect(compile({ owner: { $ne: '{current_user_id}' } }, { context: MEMBER })).toEqual({ - sql: '(("deal"."owner" IS NULL OR ("deal"."owner" IS NULL OR "deal"."owner" <> ?)))', + sql: '(("deal"."owner" IS NULL OR "deal"."owner" <> ?))', params: ['u_me'], }); }); diff --git a/packages/services/service-analytics/src/__tests__/read-scope-undefined-comparand.test.ts b/packages/services/service-analytics/src/__tests__/read-scope-undefined-comparand.test.ts index f99f3eedbc0..a22d5032d6b 100644 --- a/packages/services/service-analytics/src/__tests__/read-scope-undefined-comparand.test.ts +++ b/packages/services/service-analytics/src/__tests__/read-scope-undefined-comparand.test.ts @@ -124,16 +124,17 @@ const REFUSED: Array<{ name: string; filter: FilterCondition; path: string; wasS * * Every one of these is a DECLARED comparand whose meaning is settled, and every * one of them sits one `===` away from the value being refused: the module's - * emitter arms (`$eq`/`$ne`), `operatorIsNullTotal` and - * `nullValueSatisfiesOperator` all branch on `value === null`. A refusal + * emitter arms (`$eq`/`$ne`) and the shared lowering's `operatorIsNullTotal` + * and `nullValueSatisfiesOperator` (this module's own copies until #5930 step + * 4) all branch on `value === null`. A refusal * written one character wider takes this whole table with it, and — because * `IS NULL` lowering is what an RLS policy uses to scope unowned rows — it would * take it with a 500 on a policy that is correct. * * The `$not` rows are here for the second failure mode: the #5146 rewrite - * reaches leaves through `nullSafeNegationOperand`, so a guard placed on the - * wrong side of it changes the SHAPE rather than throwing, which no - * throw-assertion would catch. + * (the shared lowering's rule 3, at the compiler's entry) reaches every leaf, + * so a guard placed on the wrong side of it changes the SHAPE rather than + * throwing, which no throw-assertion would catch. */ const NULL_CONTROL: Array<{ name: string; filter: FilterCondition; sql: string; params: unknown[] }> = [ { name: '{ d: null } — the implicit null predicate', filter: { d: null }, sql: '"t"."d" IS NULL', params: [] }, diff --git a/packages/services/service-analytics/src/__tests__/text-match-sqlite-nul.test.ts b/packages/services/service-analytics/src/__tests__/text-match-sqlite-nul.test.ts index c077c8032bd..763a5efe9d0 100644 --- a/packages/services/service-analytics/src/__tests__/text-match-sqlite-nul.test.ts +++ b/packages/services/service-analytics/src/__tests__/text-match-sqlite-nul.test.ts @@ -310,11 +310,11 @@ describe('[#20025] the compiled constructs, per shape', () => { it('`$contains` / `$notContains` / `$icontains` take instr() and bind the comparand raw', async () => { expect(scope({ v: { $contains: 'a*b' } } as FilterCondition)) .toEqual({ sql: 'instr("t"."v", ?) > 0', params: ['a*b'] }); - // The #5298 NULL-safe wrapper composes around the negated construct unchanged - // — [ADR-0053 D-D1, amended — #5930 step 3] inside the shared lowering's own - // escape, which now reaches this compiler first. + // The #5298 NULL-safe escape composes around the negated construct unchanged + // — [ADR-0053 D-D1, amended — #5930 step 4] the shared lowering's, its one + // source since this compiler's own wrapper was deleted. expect(scope({ v: { $notContains: 'a*b' } } as FilterCondition)) - .toEqual({ sql: '(("t"."v" IS NULL OR ("t"."v" IS NULL OR NOT (instr("t"."v", ?) > 0))))', params: ['a*b'] }); + .toEqual({ sql: '(("t"."v" IS NULL OR NOT (instr("t"."v", ?) > 0)))', params: ['a*b'] }); expect(scope({ v: { $icontains: 'A*b' } } as FilterCondition)) .toEqual({ sql: 'instr(lower("t"."v"), lower(?)) > 0', params: ['A*b'] }); const n = await native({ v: { $contains: 'a*b' } } as FilterCondition); diff --git a/packages/services/service-analytics/src/__tests__/text-operator-case-exactness.test.ts b/packages/services/service-analytics/src/__tests__/text-operator-case-exactness.test.ts index b1ac0d138d4..9593a244d9e 100644 --- a/packages/services/service-analytics/src/__tests__/text-operator-case-exactness.test.ts +++ b/packages/services/service-analytics/src/__tests__/text-operator-case-exactness.test.ts @@ -291,12 +291,12 @@ describe('[#15684] the compiled TEXT, per dialect', () => { expect(out.params).toEqual(['acme']); expect(compileScopedFilterToSql({ name: { $contains: 'acme' } } as FilterCondition, 't', { dialect: 'sqlite' })) .toEqual({ sql: 'instr("t"."name", ?) > 0', params: ['acme'] }); - // `$notContains` keeps the read scope's NULL-safe wrapper around the + // `$notContains` keeps the read scope's NULL-safe escape around the // negated construct — the polarity moved, the #5298 rule did not. - // [ADR-0053 D-D1, amended — #5930 step 3] …inside the shared lowering's own - // escape, which now reaches this compiler first. + // [ADR-0053 D-D1, amended — #5930 step 4] The escape is the shared + // lowering's, its one source since this compiler's own wrapper was deleted. expect(compileScopedFilterToSql({ name: { $notContains: 'acme' } } as FilterCondition, 't', { dialect: 'sqlite' })) - .toEqual({ sql: '(("t"."name" IS NULL OR ("t"."name" IS NULL OR NOT (instr("t"."name", ?) > 0))))', params: ['acme'] }); + .toEqual({ sql: '(("t"."name" IS NULL OR NOT (instr("t"."name", ?) > 0)))', params: ['acme'] }); const starts = await nativeSql({ name: { $startsWith: 'ACME' } }, 'sqlite'); expect(starts.sql).toContain('WHERE name GLOB $1'); expect(starts.params).toEqual(['ACME*']); diff --git a/packages/services/service-analytics/src/__tests__/text-operator-non-text-column.test.ts b/packages/services/service-analytics/src/__tests__/text-operator-non-text-column.test.ts index e09c4359d66..7f9146727f2 100644 --- a/packages/services/service-analytics/src/__tests__/text-operator-non-text-column.test.ts +++ b/packages/services/service-analytics/src/__tests__/text-operator-non-text-column.test.ts @@ -179,15 +179,16 @@ describe('[#14079] read-scope-sql compiles the contract\'s constant for a declar }); it('composes with the NULL-safe $not rewrite: the negation of the constant is total', () => { - // `nullSafeNegationOperand` guards the leaf first, then the constant - // replaces the LIKE: TRUE for every row, what the JS faces answer for - // `!contains` on a number; its mirror is FALSE for every row. - // [ADR-0053 D-D1, amended — #5930 step 3] …after the shared lowering's own - // guard on the operand, which reaches this compiler first. + // The `$not` rewrite guards the leaf first, then the constant replaces the + // LIKE: TRUE for every row, what the JS faces answer for `!contains` on a + // number; its mirror is FALSE for every row. + // [ADR-0053 D-D1, amended — #5930 step 4] The rewrite is the shared + // lowering's, at this compiler's entry: one guard, its one source since + // this compiler's own copy (a second guard inside) was deleted. expect(compile({ $not: { score: { $contains: '5' } } } as FilterCondition).sql) - .toBe('NOT (("t"."score" IS NOT NULL AND ("t"."score" IS NOT NULL AND 1 = 0)))'); + .toBe('NOT (("t"."score" IS NOT NULL AND 1 = 0))'); expect(compile({ $not: { score: { $notContains: '5' } } } as FilterCondition).sql) - .toBe('NOT ((("t"."score" IS NULL OR (("t"."score" IS NULL OR 1 = 1)))))'); + .toBe('NOT ((("t"."score" IS NULL OR 1 = 1)))'); }); it('a text column beside it is untouched, and params stay aligned with the LIKE that IS bound', () => { @@ -199,10 +200,11 @@ describe('[#14079] read-scope-sql compiles the contract\'s constant for a declar it('without the option, or when the column is text, the LIKE is byte-identical to before', () => { expect(compileScopedFilterToSql({ score: { $contains: '5' } } as FilterCondition, ALIAS)) .toEqual({ sql: '"t"."score" LIKE ? ESCAPE ?', params: ['%5%', '\\'] }); - // [ADR-0053 D-D1, amended — #5930 step 3] The shared lowering's NULL escape - // around this compiler's own; the LIKE inside is the same bytes. + // [ADR-0053 D-D1, amended — #5930 step 4] The shared lowering's NULL + // escape, once (this compiler's own copy is deleted); the LIKE inside is + // the same bytes. expect(compile({ name: { $notContains: '5' } } as FilterCondition).sql) - .toBe('(("t"."name" IS NULL OR ("t"."name" IS NULL OR "t"."name" NOT LIKE ? ESCAPE ?)))'); + .toBe('(("t"."name" IS NULL OR "t"."name" NOT LIKE ? ESCAPE ?))'); }); it('a comparand the contract refuses is refused AHEAD of the constant', () => { diff --git a/packages/services/service-analytics/src/__tests__/where-boolean-flag-refusal.test.ts b/packages/services/service-analytics/src/__tests__/where-boolean-flag-refusal.test.ts index 641bdc7a561..de9b4b897b0 100644 --- a/packages/services/service-analytics/src/__tests__/where-boolean-flag-refusal.test.ts +++ b/packages/services/service-analytics/src/__tests__/where-boolean-flag-refusal.test.ts @@ -241,7 +241,7 @@ const SELECT = 'SELECT id AS "id", COUNT(*) AS "n" FROM "deal" WHERE '; const TAIL = ' GROUP BY id'; const notSet = { kind: 'leaf', member: 'stage', operator: 'notSet', values: [] }; const set = { kind: 'leaf', member: 'stage', operator: 'set', values: [] }; -const neLost = { kind: 'or', children: [notSet, { kind: 'leaf', member: 'stage', operator: 'notEquals', values: ['lost'] }] }; +const neLost = { kind: 'leaf', member: 'stage', operator: 'notEquals', values: ['lost'] }; interface ControlFamily { tree: unknown; @@ -259,14 +259,15 @@ const IS_NULL: Record<'top' | 'not' | 'notNe', ControlFamily> = { engine: { $and: [{ $not: { stage: { $null: true } } }] }, rows: ['r1', 'r3'], }, - // [ADR-0053 D-D1, amended — #5930 step 3] The shared lowering's `allowNull` - // escape on the `$not` operand now wraps this face's own copy of it: one more - // `stage IS NULL OR …` outermost. The rows are the family's, unchanged. + // [ADR-0053 D-D1, amended — #5930 step 4] The shared lowering's `allowNull` + // escape on the `$not` operand, and nothing else: this face's own copies of + // the escape (around the operand, and around the `$ne` leaf) are deleted. + // The rows are the family's, unchanged. notNe: { - tree: { kind: 'not', child: { kind: 'or', children: [notSet, { kind: 'or', children: [notSet, { kind: 'and', children: [notSet, neLost] }] }] } }, - sql: `${SELECT}NOT ((stage IS NULL OR (stage IS NULL OR (stage IS NULL AND (stage IS NULL OR stage != $1)))))${TAIL}`, + tree: { kind: 'not', child: { kind: 'or', children: [notSet, { kind: 'and', children: [notSet, neLost] }] } }, + sql: `${SELECT}NOT ((stage IS NULL OR (stage IS NULL AND stage != $1)))${TAIL}`, params: ['lost'], - engine: { $and: [{ $not: { $or: [{ stage: { $null: true } }, { $or: [{ stage: { $null: true } }, { stage: { $null: true }, $and: [{ $or: [{ stage: { $null: true } }, { stage: { $ne: 'lost' } }] }] }] }] } }] }, + engine: { $and: [{ $not: { $or: [{ stage: { $null: true } }, { stage: { $null: true, $ne: 'lost' } }] } }] }, rows: ['r1', 'r3'], }, }; @@ -279,13 +280,14 @@ const IS_NOT_NULL: Record<'top' | 'not' | 'notNe', ControlFamily> = { engine: { $and: [{ $not: { stage: { $null: false } } }] }, rows: ['r2'], }, - // [ADR-0053 D-D1, amended — #5930 step 3] …and the `requireValue` guard the - // same way: one more `stage IS NOT NULL AND …`. Rows unchanged. + // [ADR-0053 D-D1, amended — #5930 step 4] …and the `requireValue` guard the + // same way, once. The second `stage IS NOT NULL` is the `$null: false` + // operator itself. Rows unchanged. notNe: { - tree: { kind: 'not', child: { kind: 'and', children: [set, { kind: 'and', children: [set, { kind: 'and', children: [set, neLost] }] }] } }, - sql: `${SELECT}NOT ((stage IS NOT NULL AND (stage IS NOT NULL AND (stage IS NOT NULL AND (stage IS NULL OR stage != $1)))))${TAIL}`, + tree: { kind: 'not', child: { kind: 'and', children: [set, { kind: 'and', children: [set, neLost] }] } }, + sql: `${SELECT}NOT ((stage IS NOT NULL AND (stage IS NOT NULL AND stage != $1)))${TAIL}`, params: ['lost'], - engine: { $and: [{ $not: { stage: { $null: false }, $and: [{ stage: { $null: false } }, { stage: { $null: false } }, { $or: [{ stage: { $null: true } }, { stage: { $ne: 'lost' } }] }] } }] }, + engine: { $and: [{ $not: { stage: { $null: false, $ne: 'lost' }, $and: [{ stage: { $null: false } }] } }] }, rows: ['r2', 'r3'], }, }; diff --git a/packages/services/service-analytics/src/__tests__/where-door-shared-lowering-seam.test.ts b/packages/services/service-analytics/src/__tests__/where-door-shared-lowering-seam.test.ts index 063c0ddfdb6..e415d2280e4 100644 --- a/packages/services/service-analytics/src/__tests__/where-door-shared-lowering-seam.test.ts +++ b/packages/services/service-analytics/src/__tests__/where-door-shared-lowering-seam.test.ts @@ -24,15 +24,20 @@ * F10 can read declared types through the strategy context's * `declaredFieldType` hook, so it rewrites the whole-day bound on a member * whose column is declared `datetime` and nowhere else — the engine seam's - * scope, which is what the ObjectQL hand-off meets next. A context with no hook - * reads no member as `datetime`. + * scope, which is what the ObjectQL hand-off meets next. [#5930 step 4] A + * column the hook cannot name a type for is read per strategy + * (`declaredDatetimeLowering`'s `undeclared` argument): type-blind on the + * native strategy, the last seam before its statement runs, and as written on + * the ObjectQL strategy, whose engine seam reads the declaration. * - * F11 evaluates drafted rows with no schema. Its lowering reads no member as - * `datetime`: its own bound copy (`lteBound`) keeps answering the whole-day - * rule until its deletion card, and a type-blind rewrite here would move one - * cell — `$lte` on the last supported day over a non-temporal value that sorts - * above it — away from the typed drivers' answer. The NULL-polarity guards - * apply on both faces whatever the type. + * F11 evaluates drafted rows, which carry no schema of their own. [#5930 step + * 4] Its reader is the drafted object's declared types, which `queryDataset`'s + * preview branch hands it from `sourceFieldMeta` + * (`declaredPreviewLowering`): a declared `datetime` is rewritten, any other + * declared column is compared as written, and a column with no declared type + * (or a caller that hands none, as below) reads type-blind. Its own bound copy + * (`lteBound`) is deleted. The NULL-polarity guards apply on both faces + * whatever the type. */ import { describe, it, expect } from 'vitest'; @@ -98,18 +103,13 @@ describe('[ADR-0053 D-D1 amended — #5930 step 3] F10: the where → tree face }); it('a negative-polarity leaf reaches the tree inside the NULL escape the seam emits, whatever the type', () => { - // The outer disjunction is the seam's `{ $or: [{ stage: { $null: true } }, - // { stage: { $ne: 'won' } }] }`. The inner one is this face's own interim - // copy of the same guard (`fieldLeaves`, #5298), which still wraps the - // `$ne` it meets: idempotent in rows (a guard of a guarded leaf admits the - // same rows), and removed by the face's deletion card — which updates this - // row to the single disjunction. + // The disjunction is the seam's `{ $or: [{ stage: { $null: true } }, + // { stage: { $ne: 'won' } }] }`. [#5930 step 4] It is the guard's one + // source: this face's own interim copy (`fieldLeaves`' #5298 wrap, which + // nested a second disjunction inside it) is deleted. expect(tree({ stage: { $ne: 'won' } }, UNTYPED)).toEqual({ kind: 'or', - children: [ - leaf('stage', 'notSet', []), - { kind: 'or', children: [leaf('stage', 'notSet', []), leaf('stage', 'notEquals', ['won'])] }, - ], + children: [leaf('stage', 'notSet', []), leaf('stage', 'notEquals', ['won'])], }); }); @@ -222,7 +222,7 @@ describe('[ADR-0053 D-D1 amended — #5930 step 3] F11: the draft preview evalua expect(previewIds({ stage: { $null: false } }, ROWS)).toEqual(['p1']); }); - it('a bare-day bound is still answered through the whole named day (the face\'s own copy)', () => { + it('a bare-day bound is answered through the whole named day — the lowering\'s, read type-blind with no declared type', () => { const rows = [{ id: 'd27', at: '2026-07-27T10:00:00.000Z' }, { id: 'd28', at: '2026-07-28T10:00:00.000Z' }, { id: 'd29', at: '2026-07-29T10:00:00.000Z' }]; expect(previewIds({ at: { $lte: '2026-07-28' } }, rows)).toEqual(['d27', 'd28']); expect(previewIds({ at: { $between: ['2026-07-28', '2026-07-28'] } }, rows)).toEqual(['d28']); diff --git a/packages/services/service-analytics/src/analytics-service.ts b/packages/services/service-analytics/src/analytics-service.ts index 516e1aad0b0..de5f8bf2d17 100644 --- a/packages/services/service-analytics/src/analytics-service.ts +++ b/packages/services/service-analytics/src/analytics-service.ts @@ -2470,7 +2470,7 @@ export class AnalyticsService implements IAnalyticsService { { filter: compiled.filter, measureFilters: compiled.measureFilters }, context, ); - return evaluateAnalyticsQueryOverRows(q, compiled.cube, seedRows!); + return evaluateAnalyticsQueryOverRows(q, compiled.cube, seedRows!, (field) => this.sourceFieldMeta?.(dataset.object, field)?.type); }, } as IAnalyticsService; const previewResult = await new DatasetExecutor(previewService).execute(compiled, selection, context); diff --git a/packages/services/service-analytics/src/empty-operator-sql.ts b/packages/services/service-analytics/src/empty-operator-sql.ts index 05d539c7647..79587e10700 100644 --- a/packages/services/service-analytics/src/empty-operator-sql.ts +++ b/packages/services/service-analytics/src/empty-operator-sql.ts @@ -46,8 +46,9 @@ * * Every predicate is TOTAL — TRUE or FALSE for every row, never UNKNOWN — * because both polarities spell their NULL case out. So a `$not` over `$empty` - * needs no NULL guard (`operatorIsNullTotal` answers `true` for it on both - * faces), and `NOT (…)` is the exact complement. + * needs no NULL guard (the shared lowering's `operatorIsNullTotal`, the NULL + * rule's one source on both faces since #5930 step 4, answers `true` for it), + * and `NOT (…)` is the exact complement. * * `L` has no construct on the `'unknown'` dialect: no JSON test parses on all * three dialects, and the text-match family's `unknown` arm (a construct that diff --git a/packages/services/service-analytics/src/non-text-column.ts b/packages/services/service-analytics/src/non-text-column.ts index 28b0238e373..662767859a3 100644 --- a/packages/services/service-analytics/src/non-text-column.ts +++ b/packages/services/service-analytics/src/non-text-column.ts @@ -39,7 +39,8 @@ * * A row with no value already satisfies `$notContains` (#5298) and fails every * positive operator, so `1 = 1` / `1 = 0` agree with the null polarity on every - * row. Under `$not`, the leaf is totalised first (`nullSafeNegationOperand`): + * row. Under `$not`, the leaf is totalised first (by the shared lowering, the + * NULL rule's one source since #5930 step 4): * `NOT (col IS NOT NULL AND 1 = 0)` is TRUE for every row — what the JS faces * answer for `!contains` on a number — and `NOT (col IS NULL OR 1 = 1)` is * FALSE for every row, what they answer for `!notContains`. diff --git a/packages/services/service-analytics/src/preview-evaluator.ts b/packages/services/service-analytics/src/preview-evaluator.ts index 228976a23bd..89cc78eb510 100644 --- a/packages/services/service-analytics/src/preview-evaluator.ts +++ b/packages/services/service-analytics/src/preview-evaluator.ts @@ -32,10 +32,8 @@ import { bucketDateKey, - nextUtcCalendarDay, resolveAnalyticsDateRangeString, utcInstantMs, - isUnboundedAbove, compensatedSum, type BucketGranularity, } from '@objectstack/core'; @@ -48,9 +46,9 @@ import { explicitDateRangeWindow } from './date-range-array-arm.js'; // same reason: one rule, one spelling, on both faces. [#20010] And, through // the same gate, every other arm of the shared comparand-shape face. [#20035] // And the comparand-TYPE face, through the same gate again. -import { invalidFilterError, normalizeWhereComparands, NO_DATETIME_COLUMNS } from './strategies/filter-normalizer.js'; +import { invalidFilterError, normalizeWhereComparands } from './strategies/filter-normalizer.js'; import type { AnalyticsQuery, AnalyticsResult } from '@objectstack/spec/contracts'; -import { emptyGroupValueFor, lowerFilterCondition, type Cube } from '@objectstack/spec/data'; +import { emptyGroupValueFor, lowerFilterCondition, type Cube, type FilterLoweringOptions } from '@objectstack/spec/data'; type Row = Record; @@ -87,29 +85,6 @@ function compare(a: unknown, b: unknown): number { return String(a) < String(b) ? -1 : String(a) > String(b) ? 1 : 0; } -/** - * The inclusive-upper-bound comparison, with the calendar-day rule (#3777): a - * bare-day bound means "through that whole day", so it is evaluated half-open - * against the next day. String ordering makes `< nextDay` equivalent to - * `<= day` for plain date values, so this needs no field-type lookup — which - * matters here, because the preview sees drafted rows with no schema. - * - * Shared by `$lte` and the max of `$between` so the two cannot drift apart. - * - * [#20600] `9999-12-31`, the last supported day, has no next day to compare - * against (`UNBOUNDED_ABOVE`): every instant the platform stores is on or - * before it, so a value that denotes an instant ({@link utcInstantMs}) is - * inside the bound, and any other value keeps the comparison as written — the - * same reading `formula`'s `check` evaluator gives, so the two type-blind - * surfaces answer one bound alike. - */ -function lteBound(value: unknown, bound: unknown): boolean { - const nextDay = nextUtcCalendarDay(bound); - if (isUnboundedAbove(nextDay)) return utcInstantMs(value) !== null || compare(value, bound) <= 0; - if (nextDay != null) return compare(value, nextDay) < 0; - return compare(value, bound) <= 0; -} - /** One field operator's predicate, over one row's value. */ type PreviewPredicate = (value: unknown, expected: unknown) => boolean; @@ -157,24 +132,27 @@ const PREVIEW_FIELD_OPERATORS = new Map([ ['$gt', (value, expected) => value != null && compare(value, expected) > 0], ['$gte', (value, expected) => value != null && compare(value, expected) >= 0], ['$lt', (value, expected) => value != null && compare(value, expected) < 0], - ['$lte', (value, expected) => { - if (value == null) return false; - // A bare-day upper bound means "through that whole day" (#3777): the SQL - // paths compile it half-open (`< day+1`), and the preview must agree or - // a drafted chart shows different numbers than the published one. String - // ordering makes `< nextDay` equivalent to `<= day` for plain date - // values, so no type lookup is needed here either. - return lteBound(value, expected); - }], + // [ADR-0053 D-D1, amended — #5930 step 4] `$lte` and `$between` compare the + // bound they are handed, as every other ordering operator here does. A + // bare-day upper bound means "through that whole day" (#3777) on a + // `datetime` column, and the shared lowering has already rewritten such a + // bound to `$lt` the next day (or, on the last supported day, to + // `$null: false`) — with the column's declared type in hand — before + // {@link evaluateAnalyticsQueryOverRows} reads a row. A `$lte` or `$between` + // that reaches this table is on a column declared something else (`date`, + // text, a number), where the comparison as written is the typed drivers' + // answer. This face kept a type-blind copy of the rule (`lteBound`) until + // #5930 step 4. + ['$lte', (value, expected) => value != null && compare(value, expected) <= 0], ['$between', (value, expected) => { // Was absent, so it fell to the permissive `default` and matched EVERY // row — a drafted chart with a range filter silently charted the whole // dataset, then changed at publish (found by the ADR-0053 D-A3 matrix, - // #4081). The max takes the same whole-day rule as `$lte`. + // #4081). Inclusive at both ends, as written (see the note above). if (value == null || !Array.isArray(expected) || expected.length !== 2) return false; const [min, max] = expected; if (min == null || max == null) return false; - return compare(value, min) >= 0 && lteBound(value, max); + return compare(value, min) >= 0 && compare(value, max) <= 0; }], // [ADR-0053 D-D1, amended 2026-09-30 — #5930 step 3] `$null` — the one // operator the shared lowering emits that this face did not evaluate: the @@ -626,9 +604,10 @@ export interface PreviewDateRangeWindow { * * The ARRAY arm is the CALLER's explicit window and is untouched, bound for * bound, with the inclusive upper reading it has always had (#16179). Its - * bare-day widening (#3777) stays in the predicate below rather than moving - * here: that is a per-face calendar translation, not a window this vocabulary - * resolved. + * bare-day end means the whole day on a `datetime` column (#3777); that is the + * shared lowering's rule, applied where {@link evaluateAnalyticsQueryOverRows} + * expresses the window as the `{ $gte, $lte }` pair (ADR-0053 D-D1 item 8), not + * a translation of this function's. * * @throws the ADR-0112 envelope for a string outside `DATE_RANGE_PRESETS`. */ @@ -650,16 +629,50 @@ export function lowerPreviewDateRange( return { start, end, endExclusive: false }; } +/** + * [ADR-0053 D-D1, amended — #5930 step 4] The draft preview's column-type + * reader for the shared lowering (item 7), built from the host's declared type + * of a column of the dataset's object — `sourceFieldMeta`, the same hook the + * strategies' `declaredFieldType` reads, handed in by `queryDataset`'s preview + * branch. Drafted seed rows carry no schema of their own; the object they are + * drafted for does. A caller that hands no declared type cannot read + * declarations at all, and every column then reads type-blind. + * + * - a column declared `datetime` is rewritten: a bare-day upper bound means + * the whole day, and a `$between` splits; + * - a column declared any other type is compared as written, as the typed + * drivers compare it; + * - a column the host names no type for (an object the registry does not + * hold yet, a key that is not a column) is read type-blind, item 7's reading + * for a seam that cannot read the declaration. + */ +export function declaredPreviewLowering(declaredType?: (field: string) => string | undefined): FilterLoweringOptions { + return { + isDatetimeColumn: (field) => { + const type = declaredType?.(field); + return typeof type !== 'string' || type === '' ? true : type === 'datetime'; + }, + }; +} + /** * Evaluate `query` over `rows` using the cube's measure/dimension specs. * Mirrors the engine strategies' output contract: rows keyed by bare * measure/dimension names, `fields` describing each output column. + * + * `declaredType` is the host's declared type of a column of the drafted + * object, which {@link declaredPreviewLowering} turns into this face's reader + * for the shared lowering; the production caller always passes it. A caller + * that passes none cannot read declarations, and every column then reads + * type-blind (ADR-0053 D-D1 item 7). */ export function evaluateAnalyticsQueryOverRows( query: AnalyticsQuery, cube: Cube, rows: Row[], + declaredType?: (field: string) => string | undefined, ): AnalyticsResult { + const lowering = declaredPreviewLowering(declaredType); // 1. Row-level filters: `where`, then timeDimension dateRanges. // [#19810] The operator vocabulary is decided BEFORE the rows are read, so an // unevaluable predicate refuses over an empty seed draft too — see @@ -680,20 +693,17 @@ export function evaluateAnalyticsQueryOverRows( // answered EVERY row; and a bigint within 2^53 was ordered as text // (`{ amt: { $gt: 2n } }` lost `amt = 10`), where publish narrows it to its // number and serves the right rows. - // [ADR-0053 D-D1, amended 2026-09-30 — #5930 step 3] Then the shared + // [ADR-0053 D-D1, amended — #5930 steps 3 and 4] Then the shared // `FilterCondition → FilterCondition` lowering, on what the door admitted // and before the vocabulary gate reads it — the `where` door's seam for this // face (the amendment's item 2), with filter tokens already resolved by the - // `DatasetExecutor` that calls this evaluator (item 3). Its column-type - // reader is {@link NO_DATETIME_COLUMNS} (item 7): drafted rows carry no - // schema, so no member is read as `datetime` and {@link lteBound} keeps - // answering the whole-day rule as this face's own copy. A type-blind rewrite - // here would move one cell away from the typed drivers — `$lte` on the last - // supported day over a non-temporal value sorting above it. The NULL guards - // apply whatever the type; measured, they move only the rows this face read - // through `String()` — a row with no value against the text `'null'` or - // `'undefined'` — onto every driver's answer. - const where = lowerFilterCondition(normalizeWhereComparands(query.where), NO_DATETIME_COLUMNS); + // `DatasetExecutor` that calls this evaluator (item 3). It is the one source + // of the whole-day bound, the `$between` split and the NULL guards on this + // face, read with the reader above: {@link matchesWhere} compares what it is + // handed. Measured when the NULL guards arrived (step 3), they moved only the + // rows this face read through `String()` — a row with no value against the + // text `'null'` or `'undefined'` — onto every driver's answer. + const where = lowerFilterCondition(normalizeWhereComparands(query.where), lowering); assertPreviewCanEvaluate(where); let filtered = rows.filter((r) => matchesWhere(r, where)); const timeDims = query.timeDimensions ?? []; @@ -703,30 +713,22 @@ export function evaluateAnalyticsQueryOverRows( if (!td.dateRange) continue; // [#16322] One lowering for both arms — the closed preset vocabulary, or // the caller's explicit window — and a refusal for anything else. - const explicit = Array.isArray(td.dateRange); const { start, end, endExclusive } = lowerPreviewDateRange(td.dateRange, query.timezone); - // Bare-day end → half-open `< day+1`, the same translation the SQL - // paths apply (#3777); a full-timestamp end keeps the historical - // `'~'`-suffix trick (inclusive of that instant's own sub-values). - // ⛔ Neither reaches a RESOLVED preset window: it states its own upper - // reading and is never a bare day — the ten calendar presets stop BEFORE - // their end instant, the three rolling ones end at NOW and reach it. - // [#20600] A bare end on the last supported day has no next day to stop - // before: every value is inside it, so the window keeps its start alone. - const nextDay = explicit ? nextUtcCalendarDay(end) : null; - filtered = filtered.filter((r) => { - const v = String(r[field] ?? ''); - const inUpper = endExclusive - ? v < end - : isUnboundedAbove(nextDay) - ? true - : nextDay != null - ? v < nextDay - : explicit - ? v <= `${end}~` - : v <= end; - return v >= start && inUpper; - }); + // [ADR-0053 D-D1 item 8, amended — #5930 step 4] The window is the + // `{ $gte, $lte }` pair the ObjectQL strategy hands the engine (the + // `{ $gte, $lt }` pair of a resolved preset that stops before its end), + // on the column the rows carry, through the same lowering and reader as + // the `where`, and matched by the same {@link matchesWhere}. So a bare-day + // explicit end on a `datetime` column, or on one the reader cannot name, + // covers the whole day (#3777) and drops on the last supported day + // (#20600), and on a column declared anything else it is inclusive as + // written. A resolved preset's ends are instants, which the lowering never + // widens. This face kept its own copy of the rule here until #5930 step + // 4, with a `'~'`-suffix reading of a full-timestamp end ("inclusive of + // that instant's own sub-values") that no other face gives. + const bounds = endExclusive ? { $gte: start, $lt: end } : { $gte: start, $lte: end }; + const window = lowerFilterCondition({ [field]: bounds }, lowering); + filtered = filtered.filter((r) => matchesWhere(r, window)); } // 2. Grouping keys: each selected dimension (time dims bucketed). diff --git a/packages/services/service-analytics/src/read-scope-sql.ts b/packages/services/service-analytics/src/read-scope-sql.ts index b142049d7b3..012f458156f 100644 --- a/packages/services/service-analytics/src/read-scope-sql.ts +++ b/packages/services/service-analytics/src/read-scope-sql.ts @@ -93,20 +93,31 @@ import { * `filter-normalizer` with the reduction (see the note at the `length === 0` * branch in {@link compileNode} for why). Reduction happens structurally over * the whole tree, and it composes with the #5146 NULL-safe `$not` rewrite as - * "reduce first": {@link nullSafeNegationOperand} maps combinator arrays - * element-wise (an empty array stays empty, a `{}` leaf has no field to + * "reduce first": the rewrite (the shared lowering's, below) maps combinator + * arrays element-wise (an empty array stays empty, a `{}` leaf has no field to * guard), so the identity a constant reduces to is untouched by the rewrite * and the rewrite only ever guards leaves that survive it. * - * ## `$not` is NULL-safe (#5146) + * ## `$not` and the negative-polarity operators are NULL-safe (#5146, #5298) * * SQL is three-valued and a `WHERE` keeps only TRUE, so a bare `NOT (col = ?)` - * drops every row whose `col` is NULL — while `driver-memory` and `formula` - * (and, since #5296, `driver-sql`) return those rows. One read scope, two - * visible sets, chosen by which backend answered. #5146 ruled the JS answer - * canonical; {@link nullSafeNegationOperand} here is the same rewrite - * `sql-driver.ts` applies, so an analytics query and an ordinary `find()` scope - * the same rows. + * or `col <> ?` drops every row whose `col` is NULL — while `driver-memory`, + * `formula` and `driver-sql` return those rows. One read scope, two visible + * sets, chosen by which backend answered. #5146 ruled the JS answer canonical + * for `$not`, and #5298 for `$ne` / `$nin` / `$notContains` — and an RLS rule + * is evaluated on BOTH sides, read here and by `formula`'s + * `matchesFilterCondition` for the write-side `check`, so one rule admitting + * two row sets is the security defect #5146 named. + * + * [ADR-0053 D-D1, amended — #5930 step 4] The ONE source of both rules is the + * shared lowering (`lowerFilterCondition`, `@objectstack/spec/data`, its rule + * 3), which {@link compileScopedFilterToSql} runs at its entry before a single + * clause compiles — the same rewrite the engine and the RLS compile seam run, + * so an analytics query and an ordinary `find()` scope the same rows. Every + * path into {@link compileNode} passes through it. This compiler kept its own + * copy of both rules (a `$not`-operand rewrite with its three polarity tables, + * and an `IS NULL OR` wrap on `$ne` / `$nin` / `$notContains`) until this + * face's deletion card; it compiles each operator as written now. * * ## The LIKE family compares LITERALS (#5567) * @@ -238,7 +249,7 @@ import { * family. `translateFieldOperators` passes `$nin` straight through and * compiles `$notContains` to `{ $not: { $regex } }`, and both match a * missing or null field — so it has always answered as this compiler does - * through {@link nullValueSatisfiesOperator}. + * (through the shared lowering's NULL-polarity table since #5930 step 4). * (b) `driver-memory` was the one real holdout, and only on its REFERENCE * matcher; its live mingo query path already agreed. #13166 aligned that * matcher, so on the null SEMANTICS cell nothing answers differently now. @@ -794,10 +805,10 @@ export function compileScopedFilterToSql( // face the whole-day upper bound it never applied: a bare-day `$lte` (or a // `$between` maximum) on a declared `datetime` column compiles `< next-day`, // in the calendar-string domain, and answers the rows `SqlDriver.find` does - // on the same filter. It also lays the NULL-polarity guards on as structure, - // which this compiler's own copies (`nullSafeNegative`, - // `nullSafeNegationOperand`) already answered: idempotent in rows, and - // removed by this face's deletion card. + // on the same filter. It also lays the NULL-polarity guards on as structure + // (#5146, #5298), and since #5930 step 4 it is their ONE source on this + // face: {@link compileNode} and {@link compileOperator} compile what they are + // handed (see the module header). // // The shared comparand faces below still judge the scope AS WRITTEN, after // compilation (#20018's order). The lowering never refuses and never turns a @@ -1240,12 +1251,12 @@ function compileNode(node: unknown, qAlias: string, params: unknown[], opts: Rea const joiner = key === '$and' ? ' AND ' : ' OR '; clauses.push(`(${kept.map((c) => c.sql).join(joiner)})`); } else if (key === '$not') { - // NULL-safe negation (#5146): totalise the operand's leaves first, so - // `NOT (…)` can never be UNKNOWN and this compiler admits the same rows - // `driver-sql` / `driver-memory` / `formula` admit. A non-node operand is - // left alone so `compileNode` still rejects it with its own message. - const operand = isFilterNode(value) ? nullSafeNegationOperand(value) : value; - const inner = compileSub(operand, qAlias, opts); + // NULL-safe negation (#5146): the operand's leaves arrive TOTAL — the + // shared lowering guarded each one at this compiler's entry — so + // `NOT (…)` can never be UNKNOWN and this compiler admits the rows + // `driver-sql` / `driver-memory` / `formula` admit. A non-node operand + // still reaches `compileNode`, which rejects it with its own message. + const inner = compileSub(value, qAlias, opts); if (inner.sql.length === 0) { // `NOT TRUE ≡ FALSE`. Emitting nothing here is what let a `{$not: {}}` // read scope through `applyReadScope`'s `if (!sql) return;` and ran the @@ -1441,31 +1452,6 @@ function membershipMatch( return sql; } -/** - * [#5298] Wrap a negative-polarity value test so a row whose column has no value - * SATISFIES it: `(col IS NULL OR )`. - * - * The read-scope twin of `driver-sql`'s `applyNullSafeNegative`, and the reason - * this compiler had to move in the same PR rather than a later one: an RLS rule - * is authored once and evaluated on BOTH sides — this file lowers it for the - * read path while `formula`'s `matchesFilterCondition` evaluates it for the - * write-side `check`. Leaving the two on different answers for `$ne` is one - * permission rule admitting two different row sets, which is the security - * defect #5146 named for `$not` and #5298 ruled for the rest. - * - * OR-expansion rather than `IS DISTINCT FROM` / `IS NOT` / `<=>`, for the three - * reasons recorded on the driver-side twin: `NOT LIKE` has no such form, the - * SQLite spelling depends on an engine version nothing here pins, and the - * measured query plans are identical either way. - * - * The parentheses are not optional. {@link compileField} joins a field's - * operators with bare ` AND `, so an unwrapped `col IS NULL OR …` would bind - * looser than that AND and silently widen the whole scope. - */ -function nullSafeNegative(col: string, test: string): string { - return `(${col} IS NULL OR ${test})`; -} - /** * [#5234] The comparand-SHAPE gate for this door. * @@ -1688,21 +1674,22 @@ function undefinedComparandError(field: string, path: string): Error { * conditional on evaluation order. THIS compiler has no such blind spot — * {@link compileNode} `.map()`s every `$and`/`$or` child into its own buffer * BEFORE any identity is applied (the `$or` TRUE-absorption and the `$and` - * identity filter both read the fully-compiled list), and - * {@link nullSafeNegationOperand} rewrites a `$not` operand without dropping a - * single leaf. Every comparand therefore reaches `compileField`, which is also + * identity filter both read the fully-compiled list), and the shared lowering + * at {@link compileScopedFilterToSql}'s entry rewrites a `$not` operand without + * dropping a single leaf (each guard carries the field's spec through by + * reference). Every comparand therefore reaches `compileField`, which is also * the only path to {@link bind} — one gate, on the one road. * * The other half of `driver-sql`'s "runs FIRST" argument does not transfer * either, and that is worth stating rather than copying: there, the refusal had * to precede the `$not` rewrite because the polarity tables spelled `=== null` * while the `$ne` emitter spelled `== null`, so the two disagreed about - * `undefined` itself. Here {@link nullValueSatisfiesOperator}, - * {@link operatorIsNullTotal} and every arm of {@link compileOperator} spell it - * `=== null` alike, so the tables and the emitter agree that `undefined` is "a - * value" — the rewrite for a `{ $not: … }` operand runs, produces a leaf, and - * that leaf is refused. Nothing inconsistent is being outrun; the silent NULL - * bind is. + * `undefined` itself. Here the shared lowering's NULL-polarity table (#5930 + * step 4: this compiler kept its own copy until then) and every arm of + * {@link compileOperator} spell it `=== null` alike, so the table and the + * emitter agree that `undefined` is "a value" — the rewrite for a + * `{ $not: … }` operand runs, produces a leaf, and that leaf is refused. + * Nothing inconsistent is being outrun; the silent NULL bind is. */ function assertDefinedComparands(field: string, spec: unknown): void { const root = `"${field}"`; @@ -1844,13 +1831,14 @@ function nonBooleanFlagComparandError(op: string, field: string, path: string): * Same reason {@link assertDefinedComparands} sits here: {@link compileField} is * the one road every field constraint travels, because {@link compileNode} * `.map()`s every child into its own buffer BEFORE any boolean identity is - * applied, so no sibling can absorb a malformed one. It runs AFTER - * {@link nullSafeNegationOperand} for a `$not` operand — harmless, and worth - * stating: that rewrite consults {@link nullValueSatisfiesOperator}, which now - * reads these two by identity, so a non-boolean is classified before it is - * refused. The classification is DISCARDED either way (the leaf still reaches - * `compileField` and still throws), and the rewrite's own synthesised leaves - * (`{ $null: false }`, `{ $null: true }`) are literal booleans by construction. + * applied, so no sibling can absorb a malformed one. It runs AFTER the shared + * lowering's `$not` rewrite (at {@link compileScopedFilterToSql}'s entry) — + * harmless, and worth stating: that rewrite consults its NULL-polarity table, + * which reads these two by identity, so a non-boolean is classified before it + * is refused. The classification is DISCARDED either way (the leaf still + * reaches `compileField` and still throws), and the rewrite's own synthesised + * leaves (`{ $null: false }`, `{ $null: true }`) are literal booleans by + * construction. * * [#20445] `$empty` is the third flag, and it joins the gate on the day its arm * lands rather than after a flip is measured: the spec declares it @@ -2030,9 +2018,11 @@ function compileOperator( // [#19975] `val` is never a list here: {@link assertNoListInEqualitySlot} // refused one at {@link compileField}, before this emitter runs. case '$eq': return val === null ? `${col} IS NULL` : `${col} = ${bind(params, val)}`; - // [#5298] `$ne: null` stays `IS NOT NULL` — already total, and "has any - // value" is false for a row that has none. Only the comparison is guarded. - case '$ne': return val === null ? `${col} IS NOT NULL` : nullSafeNegative(col, `${col} <> ${bind(params, val)}`); + // [#5298] `$ne: null` is `IS NOT NULL` — already total, and "has any + // value" is false for a row that has none. A `$ne` of a value arrives + // inside the NULL escape the shared lowering wrote around it (see the + // module header), so the comparison compiles as written here. + case '$ne': return val === null ? `${col} IS NOT NULL` : `${col} <> ${bind(params, val)}`; case '$gt': return `${col} > ${bind(params, val)}`; case '$gte': return `${col} >= ${bind(params, val)}`; case '$lt': return `${col} < ${bind(params, val)}`; @@ -2054,9 +2044,9 @@ function compileOperator( // header's #13571 section before "harmonising" the two arms. if (val.length === 0) throw readScopeCompileError(`[read-scope-sql] $nin for "${field}" is empty — an empty exclusion excludes nothing and would compile the read scope to constant TRUE (fail-closed).`); assertCompilableMembers(op, field, val); - // [#5298] NULL-safe: "not among this list" holds vacuously for a value - // that is not there. - return nullSafeNegative(col, `${col} NOT IN (${val.map((v) => bind(params, v)).join(', ')})`); + // [#5298] "Not among this list" holds vacuously for a value that is not + // there: the shared lowering's NULL escape around this leaf says so. + return `${col} NOT IN (${val.map((v) => bind(params, v)).join(', ')})`; } case '$between': { if (!Array.isArray(val) || val.length !== 2) throw readScopeCompileError(`[read-scope-sql] $between for "${field}" needs [min,max] (fail-closed).`); @@ -2124,17 +2114,16 @@ function compileOperator( assertIcontainsComparandNotRefused(op, field, val); return textOverNonTextColumn(op, field, opts) ?? textMatch(col, 'contains', val, false, params, opts, true); - // [#5298] NULL-safe: `NOT LIKE` is UNKNOWN for a NULL column, and "does not - // contain" is true of a value that is not there. - // [#20987] The same wrapper around the negated MEMBERSHIP test on a column - // declared multi-valued or JSON-stored, `driver-sql`'s NULL rule. + // [#5298] `NOT LIKE` is UNKNOWN for a NULL column, and "does not contain" + // is true of a value that is not there: the shared lowering's NULL escape + // around this leaf says so, for the text test and — [#20987] — for the + // negated MEMBERSHIP test on a column declared multi-valued or JSON-stored + // alike, `driver-sql`'s NULL rule. case '$notContains': assertRenderableText(op, field, val); return textOverNonTextColumn(op, field, opts) - ?? nullSafeNegative( - col, - membershipMatch(col, op, val, field, params, opts) ?? textMatch(col, 'contains', val, true, params, opts), - ); + ?? membershipMatch(col, op, val, field, params, opts) + ?? textMatch(col, 'contains', val, true, params, opts); case '$startsWith': assertRenderableText(op, field, val); return textOverNonTextColumn(op, field, opts) ?? textMatch(col, 'starts', val, false, params, opts); @@ -2147,7 +2136,7 @@ function compileOperator( // not the "anything truthy is IS NULL" rule it used to be. That old rule is // what put the STRING `"false"` on the side opposite the `false` it was // written to mean; the identity spelling cannot, and it is the spelling - // {@link nullValueSatisfiesOperator} now mirrors (#5146 / #5298). + // the shared lowering's NULL-polarity table reads (#5146 / #5298). case '$null': return val === true ? `${col} IS NULL` : `${col} IS NOT NULL`; case '$exists': return val === true ? `${col} IS NOT NULL` : `${col} IS NULL`; // [#20445] `val` is a boolean here too — the same gate refused anything @@ -2219,196 +2208,3 @@ function compileEmptyOperator( } return sql; } - -// ── [#5146] NULL-safe `$not` ───────────────────────────────────────────────── - -/** - * What one field constraint needs so its compiled SQL is TOTAL — TRUE or FALSE - * for every row, never UNKNOWN. - * - * - `'none'` — already total (`IS NULL` / `IS NOT NULL`), or a shape - * this compiler refuses outright, which must keep refusing. - * - `'requireValue'` — a NULL column does NOT satisfy it: `col IS NOT NULL AND (…)`. - * - `'allowNull'` — a NULL column DOES satisfy it: `col IS NULL OR (…)`. - */ -type NullGuard = 'none' | 'requireValue' | 'allowNull'; - -/** - * Does a NULL column satisfy this one operator, under the semantics the JS - * backends (`driver-memory`'s `match`, `formula`'s `matchesFilterCondition`) - * give it? They evaluate a missing value in ordinary two-valued JS — `undefined - * !== 'won'` is simply `true` — and #5146 ruled that answer canonical. - * - * This is `sql-driver.ts`'s `nullValueSatisfiesOperator` table, entry for entry, - * with ONE deliberate difference that comes from THIS file's emitter rather than - * from a different reading of #5146: - * - * - `$between` exists in this compiler and not in that table; it is a - * positive comparison, so it takes the default (a value that is not there - * does not lie between two bounds) exactly as the other comparisons do. - * - * ⚠️ [#6387] There used to be a SECOND difference, and its removal is half of - * that change rather than a tidy-up. `$null` / `$exists` were read here by - * TRUTHINESS — `Boolean(value)` / `!value` — because {@link compileOperator} - * wrote them as `val ? … : …`, while `driver-sql` read them by identity because - * its emitter did. That was correct under the invariant #5146 / #5298 state: - * each polarity table pins the spelling of ITS OWN emitter, not the other - * file's. So when the emitter stopped guessing at a non-boolean, these two arms - * had to move WITH it in the same change — leaving them truthy would have - * broken the invariant silently, at its own definition, with nothing red. The - * divergence is gone now because its cause is: both emitters read the declared - * boolean domain, so both tables spell it by identity, and the two files agree - * on every arm for the first time. - * - * The default is the large positive-comparison family (`$gt`/`$in`/`$contains`/ - * …), every member of which answers `false` for a value that is not there. An - * operator this compiler does not support also lands here; it is guarded and - * then still throws from {@link compileOperator}, so fail-closed is preserved. - */ -function nullValueSatisfiesOperator(op: string, value: unknown): boolean { - switch (op) { - // `$eq: null` IS the null predicate; any other comparand is a value test. - case '$eq': return value === null; - // Mirror image: `$ne: null` compiles to `IS NOT NULL`, which a NULL fails. - case '$ne': return value !== null; - // [#6387] Identity, matching this file's emitter (see the note above). - // `assertBooleanFlagComparands` refuses anything but `true` / `false` before - // this table is consulted, so each arm is an exhaustive TWO-WAY choice over - // the declared domain — and the strict spelling is chosen over the lenient - // one it replaces for the reason #5347 gave: `Boolean(value)` and - // `value === true` are equivalent only while the gate upstream holds, and - // the lenient spelling would quietly resume answering for shapes nobody - // ruled on if that gate were ever moved. A NULL column satisfies `$null` - // exactly when the author asked for null… - case '$null': return value === true; - // …and satisfies `$exists` exactly when the author asked for "no value". - // `$null: true` and `$exists: false` are the same question, so these two - // arms are correctly each other's MIRROR, not each other's copy (#5369). - case '$exists': return value === false; - // [#20445] Null is empty on every row of the ruled table, so a NULL column - // satisfies `$empty: true` and fails its complement — by identity, as the - // arm reads it, behind the same boolean gate. - case '$empty': return value === true; - // Negative-polarity set / substring tests hold vacuously for an absent value. - case '$nin': return true; - // `$notContains` is the one operator where the two JS backends disagree for - // a null-valued field (`driver-memory` answers false, `formula` true). - // `formula` is followed because `driver-sql` follows it, so this compiler - // does not cast a vote on a disagreement that is filed elsewhere. - case '$notContains': return true; - default: return false; - } -} - -/** Is this operator's compiled SQL already total for a NULL column? */ -function operatorIsNullTotal(op: string, value: unknown): boolean { - switch (op) { - // Compile to `IS NULL` / `IS NOT NULL` — two-valued by construction. - case '$null': - case '$exists': - return true; - // [#20445] Both polarities spell their NULL case out (`col IS NULL OR …` / - // `col IS NOT NULL AND …`, `empty-operator-sql.ts`), so the arm is TOTAL. - case '$empty': - return true; - // A null comparand makes these null PREDICATES too, not comparisons. - case '$eq': - case '$ne': - return value === null; - default: - return false; - } -} - -/** - * The guard one field constraint needs. A constraint is the AND of its - * operators, so it is total when every operator is, and a NULL column satisfies - * it only when it satisfies all of them. - */ -function nullGuardForFieldSpec(spec: unknown): NullGuard { - // `{ field: null }` compiles to `IS NULL` — already total. - if (spec === null) return 'none'; - // A scalar / Date is an implicit `=`; a NULL column fails it. A bare array is - // REFUSED by `compileField`; classifying it here keeps that refusal reachable - // (the unrewritten `{field: […]}` conjunct still throws its own message). - if (typeof spec !== 'object' || spec instanceof Date || Array.isArray(spec)) return 'requireValue'; - const entries = Object.entries(spec as Record); - // `{ field: {} }` and any non-`$` key are shapes `compileField` throws on. - // Passing them through unrewritten is what preserves the exact error; a guard - // wrapped around them would only change which message the caller sees. - if (entries.length === 0) return 'none'; - let total = true; - let nullSatisfies = true; - for (const [op, value] of entries) { - if (!operatorIsNullTotal(op, value)) total = false; - if (!nullValueSatisfiesOperator(op, value)) nullSatisfies = false; - } - if (total) return 'none'; - return nullSatisfies ? 'allowNull' : 'requireValue'; -} - -/** - * [#5146] Rewrite the operand of a `$not` so every leaf compiles to a TOTAL - * predicate — which is what makes `NOT (…)` mean here what it means in - * `driver-memory`, `formula` and (since #5296) `driver-sql`. - * - * # Why the guard rides the LEAF, not the `NOT` - * - * For a flat operand `NOT (a IS NOT NULL AND a = ?)` and `NOT (a = ?) OR a IS - * NULL` are the same predicate. They stop being the same as soon as the operand - * nests: hoisting the guard above a `$not` whose operand is a `$or` re-admits - * rows the JS backends exclude — a NULL `a` would satisfy the whole negation - * even when the `$or`'s OTHER branch is satisfied. Totalising each leaf makes - * the rewrite compositional instead: De Morgan is sound over two-valued leaves, - * so `$and`, `$or` and a nested `$not` all stay correct with no special cases. - * On an RLS lowering that difference is rows a policy excludes becoming visible, - * so it is the whole reason this is a rewrite and not a suffix. - * - * # Why polarity is per operator - * - * A blanket `OR col IS NULL` would WIDEN the negative-polarity operators: - * `{$not: {a: {$ne: 5}}}` means "a is 5", and both JS backends exclude a NULL - * row from it. Adding an unconditional null escape there would hand back exactly - * the rows the scope excludes. So each leaf is guarded in the direction its own - * operator answers, per {@link nullValueSatisfiesOperator}. - * - * The rewrite runs ONLY inside a `$not`; an ordinary comparison's SQL is - * untouched, so nothing outside a negation changes shape. A nested `$not` is - * left alone on purpose — its own branch totalises its operand, and - * `NOT ` is itself total, so recursing would stack a redundant guard on - * the same column. - */ -function nullSafeNegationOperand(node: Record): Record { - const out: Record = {}; - const guarded: unknown[] = []; - for (const [key, value] of Object.entries(node)) { - if ((key === '$and' || key === '$or') && Array.isArray(value)) { - // A non-node element is passed through so `compileNode` still rejects it. - out[key] = value.map((element) => (isFilterNode(element) ? nullSafeNegationOperand(element) : element)); - continue; - } - if (key.startsWith('$')) { - // `$not` (handled by its own branch) and anything else `$`-prefixed keep - // whatever this compiler does with them today — the rewrite rules on NULL, - // not on the operator vocabulary, and an unknown one must still throw. - out[key] = value; - continue; - } - const guard = nullGuardForFieldSpec(value); - if (guard === 'none') { - out[key] = value; - } else if (guard === 'requireValue') { - // `col IS NOT NULL AND (…)` — both conjuncts of the enclosing node. - guarded.push({ [key]: { $null: false } }, { [key]: value }); - } else { - // `col IS NULL OR (…)` — one conjunct, so the OR binds tighter than the - // AND this node's keys form. - guarded.push({ $or: [{ [key]: { $null: true } }, { [key]: value }] }); - } - } - if (guarded.length > 0) { - const existing = Array.isArray(out.$and) ? out.$and : []; - out.$and = [...existing, ...guarded]; - } - return out; -} diff --git a/packages/services/service-analytics/src/strategies/filter-normalizer.ts b/packages/services/service-analytics/src/strategies/filter-normalizer.ts index cf47eb8f5ff..01912b019d3 100644 --- a/packages/services/service-analytics/src/strategies/filter-normalizer.ts +++ b/packages/services/service-analytics/src/strategies/filter-normalizer.ts @@ -31,9 +31,9 @@ * `$contains` `$notContains` `$startsWith` `$endsWith`; * - value-DEPENDENT, so resolved explicitly rather than through the map — * `$null` and `$exists`, whose meaning flips with their boolean; - * - lowered — `$between`, which becomes its two bounds so each strategy's - * existing upper-bound handling applies the calendar-day whole-day rule - * (see the note at the lowering); + * - lowered — `$between`, which becomes its two bounds; the calendar-day + * whole-day rule on its maximum is the shared lowering's (see the note at + * the `$between` arm of {@link fieldLeaves}); * - structural — `$and` / `$or` / `$not`, carried as tree nodes; * - anything else THROWS. An operator outside the vocabulary is a caller * error, and a loud one beats a silently widened read — the call @@ -74,59 +74,25 @@ * see the note inside {@link buildNode}'s combinator branch for the history * and the reasoning the ruling adopted. * - * # `$not` is NULL-safe (#5146) + * # `$not` and the negative-polarity operators are NULL-safe (#5146, #5298) * * SQL is three-valued and a `WHERE` keeps only TRUE, so a bare `NOT (col = ?)` - * drops every row whose `col` is NULL — while `driver-memory`, `formula` and - * (since #5296) `driver-sql` return those rows. One widget filter, two row sets, - * chosen by whichever backend answered. #5146 ruled the JS answer canonical, and - * {@link nullSafeNegationOperand} applies the same leaf-wise totalisation - * `sql-driver.ts` and `read-scope-sql.ts` apply. - * - * The rewrite lives HERE rather than in `native-sql-strategy` on purpose: at - * this layer the guard is STRUCTURE (one more `{col: {$null: false}}` conjunct), - * not a SQL trick, so it survives `filterNodeToCondition` handing the tree to - * the ObjectQL engine and holds on any driver behind it — including one that is - * not NULL-safe by itself. Guarding only in the SQL strategy would make "what - * does this widget's `$not` mean" depend on which backend caught it, which is - * what #5146 spent a round eliminating. The cost is that the engine path can - * guard twice (this rewrite, then `driver-sql`'s own); that is idempotent — - * `NOT (c IS NOT NULL AND (c IS NOT NULL AND c = v))` is the same predicate — - * so it buys portability for one redundant conjunct. - * - * # `$ne` / `$nin` / `$notContains` are NULL-safe too (#5298) - * - * Same rule, same reason, one ruling later. The operators that carry their OWN - * negation had the defect #5146 fixed for `$not`: a bare `col <> ?` is UNKNOWN - * for a NULL column and the `WHERE` drops the row, while the JS backends return - * it. Measured on this package's own fixture before the fix (#5977), for - * `{stage: {$ne: 'won'}}` over rows whose `stage` is NULL: - * - * | path | was | now (= JS family) | - * |----------------------------------------|-----------|-------------------| - * | `NativeSQLStrategy` (raw SQL) | `2` | `2,3,4` | - * | `ObjectQLStrategy` display SQL echo | `2` | `2,3,4` | - * | `ObjectQLStrategy` → engine condition | `2,3,4` | `2,3,4` | - * - * The engine column was already right, and that is the whole argument for - * fixing it HERE: it was right because `driver-sql` guards for itself (#5962), - * so the Cube face's answer depended on which compiler downstream caught the - * leaf — three emitters, two answers. `fieldLeaves` now emits the guard as - * STRUCTURE, an `or` of `notSet` with the comparison, so all three compile the - * same predicate and none of them needs to know the rule. That is the same - * trade the `$not` rewrite above took, including its cost: the engine path - * guards twice, which is idempotent (`c IS NULL OR (c IS NULL OR c <> v)`). - * - * Which operators get the guard is NOT a new list — it is - * {@link nullValueSatisfiesOperator} and {@link operatorIsNullTotal}, the same - * pair `nullGuardForFieldSpec` consults for the `$not` rewrite, asked about one - * operator instead of a whole field spec. A leaf is guarded exactly when a NULL - * value SATISFIES the operator and the compiled leaf is not already total, which - * is that pair's `allowNull` verdict. Hard-coding the three names would have put - * a second polarity table in this file, free to drift from the first — and the - * `$eq`/`$ne` arms of the existing one already turn on the COMPARAND (`$ne: - * null` compiles to `set`, which is total and must never be widened), so a name - * list would have been wrong as well as duplicated. + * or `col <> ?` drops every row whose `col` is NULL, while `driver-memory`, + * `formula` and `driver-sql` return those rows. #5146 ruled the JS answer + * canonical for `$not`; #5298 ruled it for `$ne` / `$nin` / `$notContains`. + * + * [ADR-0053 D-D1, amended — #5930 step 4] The ONE source of both rules is the + * shared lowering, `lowerFilterCondition` (`@objectstack/spec/data`, its rule + * 3), which {@link normalizeAnalyticsFilterTree} runs before {@link buildNode} + * reads the condition. It lays each guard on as STRUCTURE: a `{ col: { $null: + * false } }` conjunct beside a leaf of a `$not` operand that no missing value + * satisfies, and the escape `{ $or: [{ col: { $null: true } }, { col: spec }] }` + * around a leaf that a missing value does satisfy. As structure, the guard + * survives `filterNodeToCondition` handing the tree to the ObjectQL engine, and + * every compiler of the tree reads one predicate. This module kept its own copy + * of both rules (a `$not`-operand rewrite and a per-leaf wrap, with their + * polarity tables) until this face's deletion card. It restates neither now, so + * a ruling on what a missing value satisfies is made in one place. * * # A `null` COMPARAND is a null predicate, not a value (#5332) * @@ -142,10 +108,11 @@ * * The pair is not merely similar to `{$null: true|false}` — `driver-mongodb` * TRANSLATES `$null` into it — so {@link fieldLeaves} now emits the same - * `notSet` / `set` leaves for all three spellings, and the #5146 guard table - * moved in the same commit (see {@link nullValueSatisfiesOperator}); a guard that - * still described the old emitter would have negated an always-false conjunction - * and answered `{$not: {stage: {$eq: null}}}` with every row. + * `notSet` / `set` leaves for all three spellings. The #5146 guard table moved + * in the same commit (it is the shared lowering's now, which reads a `null` + * comparand of `$eq` / `$ne` as already total); a guard that still described the + * old emitter would have negated an always-false conjunction and answered + * `{$not: {stage: {$eq: null}}}` with every row. * * # A comparand keeps its own TYPE — there is no round trip any more (#5526) * @@ -487,7 +454,6 @@ import type { StrategyContext } from '@objectstack/spec/contracts'; import type { DatasetScopedStrategyContext } from './types.js'; import { StandardErrorCode } from '@objectstack/spec/api'; import { - CROSS_FIELD_COMPARISON_OPERATORS, fieldReferenceBetweenBoundMessage, isBindableComparand, isFieldReference, @@ -976,26 +942,24 @@ function undefinedComparandError(field: string, path: string): Error { * gate covers all three consumers of the tree at once — the same argument * {@link assertCompilableComparand} makes one function below. * - * That places it DOWNSTREAM of {@link nullSafeNegationOperand}, and for row three - * of the header's table that choice is the whole question: a gate on the far side - * of the #5146 rewrite refuses, while a rewrite that could swallow the leaf first - * would leave a CHANGED SHAPE for the gate to bless. Measured rather than - * assumed, because the same trap cost PR #6390 a lap on the sibling door — and - * the reasoning there does NOT transfer, since the two modules' polarity tables - * are spelled differently (that one is uniformly `=== null`; this one mixes - * `=== null` for `$eq`/`$ne` with IDENTITY reads for `$null`/`$exists`). What the - * measurement shows here is that the rewrite never drops a leaf: every guard - * disposition — `requireValue` pushes `{k: {$null: false}}, {k: spec}`, - * `allowNull` pushes `{$or: [{k: {$null: true}}, {k: spec}]}`, `none` writes - * `out[k] = spec` — carries `spec` through by reference, so the author's - * `undefined` always reaches this gate and always throws. Pinned in + * That places it DOWNSTREAM of the #5146 rewrite, and for row three of the + * header's table that choice is the whole question: a gate on the far side of + * the rewrite refuses, while a rewrite that could swallow the leaf first would + * leave a CHANGED SHAPE for the gate to bless. [#5930 step 4] The rewrite is the + * shared lowering's rule 3 now (`lowerFilterCondition`, run by + * {@link normalizeAnalyticsFilterTree} before {@link buildNode}), and it never + * drops a leaf either: every guard disposition — `requireValue` adds + * `{k: {$null: false}}` beside `{k: spec}`, `allowNull` writes + * `{$or: [{k: {$null: true}}, {k: spec}]}`, a total spec is left in place — + * carries `spec` through by reference, so the author's `undefined` always + * reaches this gate and always throws. Pinned in * `filter-normalizer-undefined-comparand.test.ts` as its own block: one case per - * rewrite path that can carry a SWEPT comparand (`requireValue`, `allowNull`, and - * the nested-relation recursion), plus the measured reason there is no third — - * `none` needs every operator to satisfy {@link operatorIsNullTotal}, which is - * false for an `undefined` comparand on every operator this gate sweeps, so the - * only field specs that reach it holding one are the `$null` / `$exists` flags it - * deliberately does not sweep. + * rewrite path that can carry a SWEPT comparand (`requireValue`, `allowNull`, + * and the nested-relation recursion). A spec is left unguarded only when every + * operator in it is already total for a missing value, which no operator this + * gate sweeps is when its comparand is `undefined`; the `$null` / `$exists` + * flags it deliberately does not sweep are the only specs that reach it that + * way. */ function assertDefinedComparands(field: string, spec: unknown): void { const root = `"${field}"`; @@ -1069,16 +1033,15 @@ function mixedFieldWrapperError(field: string, opKeys: string[], nonOpKeys: stri * `opKeys` only and returns, and the nested-relation flatten sits after that * early return — so with even one `$` key present, every non-`$` sibling was * simply never visited. Dropping a conjunct WIDENS (#3650), and inside a `$not` - * it did worse than widen by one conjunct: {@link nullGuardForFieldSpec} judged - * the wrapper while the sibling still existed (a non-`$` key never satisfies - * {@link operatorIsNullTotal}, so the disposition was `requireValue` or - * `allowNull`, never `none`), the sibling then vanished here, and for a - * null-predicate operator the surviving guard was CONTRADICTORY — + * it did worse than widen by one conjunct: the #5146 rewrite judged the wrapper + * while the sibling still existed (a non-`$` key is never total for a missing + * value, so the wrapper was always guarded), the sibling then vanished here, + * and for a null-predicate operator the surviving guard was CONTRADICTORY — * `{$not: {d: {$null: true, nested: 'x'}}}` compiled to `NOT(d set AND d - * notSet)`, which is TRUE for every row. That same never-`none` fact is what - * guarantees the #5146 rewrite carries a mixed wrapper to this gate by - * reference instead of swallowing it — pinned in - * `filter-normalizer-mixed-wrapper.test.ts`'s rewrite block. + * notSet)`, which is TRUE for every row. That same always-guarded fact is what + * guarantees the rewrite (the shared lowering's rule 3 since #5930 step 4) + * carries a mixed wrapper to this gate by reference instead of swallowing it — + * pinned in `filter-normalizer-mixed-wrapper.test.ts`'s rewrite block. * * ## Ordering against the neighbouring gates * @@ -1162,13 +1125,18 @@ function fieldLeaves(key: string, raw: unknown): NormalizedFilterNode[] { if (opKeys.length > 0) { for (const opKey of opKeys) { // `$between [min, max]` LOWERS to its two bounds rather than getting a - // `between` operator of its own. Both strategies already carry the - // calendar-day whole-day rule on their upper bound — NativeSQLStrategy - // compiles a bare-day `lte` half-open (#3777), ObjectQLStrategy hands - // `$lte` to the driver, which does the same — so a range's max - // inherits that rule by construction instead of needing a second - // implementation to keep in step. (The preview evaluator's `$between` - // gap was closed the same way, sharing its `$lte` helper.) + // `between` operator of its own: `gte` its minimum and `lte` its + // maximum, inclusive at both ends, as written. + // + // [ADR-0053 D-D1, amended — #5930 step 4] The whole-day rule is not + // applied here, and no compiler downstream applies it to the `lte` + // this produces: it is the shared lowering's (rule 1 splits a + // `$between` on a `datetime` column, or on one whose type the reader + // cannot name, and rule 2 widens its bare-day maximum), which + // {@link normalizeAnalyticsFilterTree} runs before this function. A + // `$between` that reaches this arm is on a column the reader declares + // something else (`date`, `time`, text, a number), so the split here is + // structural only — the comparison the typed drivers run for it. // // Before this, `$between` was simply absent from the operator map and // fell to the `continue` below: the predicate VANISHED from the WHERE @@ -1311,18 +1279,9 @@ function fieldLeaves(key: string, raw: unknown): NormalizedFilterNode[] { // it every field entry before any leaf exists — so no scalar leaf is // built from a list, and no compiler's `values[0]` read ever drops one. const values = Array.isArray(v) ? v.map(comparand) : [comparand(v)]; - // [#5298] The operators that carry their own negation are NULL-safe, - // here as everywhere else — see the module header's section on it. - if (nullValueSatisfiesOperator(opKey, v) && !operatorIsNullTotal(opKey, v)) { - out.push({ - kind: 'or', - children: [ - { kind: 'leaf', member: key, operator: 'notSet', values: [] }, - { kind: 'leaf', member: key, operator: cubeOp, values }, - ], - }); - continue; - } + // [#5298] A negative-polarity operator reaches this line already inside + // the NULL escape the shared lowering wrote around it (see the module + // header's section on it), so it compiles as written here. leaf(cubeOp, values); } return out; @@ -1443,12 +1402,12 @@ function buildNode(cond: Record): NormalizedFilterNode | null { `Dropping it would silently widen the query to rows the filter excludes.`, ); } - // NULL-safe negation (#5146): totalise the operand's leaves FIRST, so the - // negation can never be UNKNOWN and this path admits the same rows - // `driver-memory` / `formula` / `driver-sql` admit. The guard is added as - // STRUCTURE here, which is what makes it survive into the ObjectQL engine - // path too (see the module header). - const inner = buildNode(nullSafeNegationOperand(raw)); + // NULL-safe negation (#5146): the operand's leaves arrive TOTAL — the + // shared lowering guarded each one as structure before this function ran + // (see the module header) — so the negation can never be UNKNOWN and + // this path admits the rows `driver-memory` / `formula` / `driver-sql` + // admit, on the ObjectQL engine path too. + const inner = buildNode(raw); // `notOf` turns a TRUE operand into FALSE instead of nothing: `{$not: {}}` // is the zero-row filter, and emitting nothing for it charted every row. children.push(notOf(inner)); @@ -1468,269 +1427,6 @@ function buildNode(cond: Record): NormalizedFilterNode | null { return andOf(children); } -// ── [#5146 / #5325] NULL-safe `$not` ───────────────────────────────────────── - -/** - * What one field constraint needs so the leaves it produces are TOTAL — TRUE or - * FALSE for every row, never UNKNOWN. - * - * - `'none'` — already total (`set` / `notSet`, a boolean constant), or - * a shape this normalizer refuses, which must keep refusing. - * - `'requireValue'` — a NULL column does NOT satisfy it: `col IS NOT NULL AND (…)`. - * - `'allowNull'` — a NULL column DOES satisfy it: `col IS NULL OR (…)`. - */ -type NullGuard = 'none' | 'requireValue' | 'allowNull'; - -/** - * Does a NULL column satisfy this one operator, under the semantics the JS - * backends (`driver-memory`'s `match`, `formula`'s `matchesFilterCondition`) - * give it? They evaluate a missing value in ordinary two-valued JS — `undefined - * !== 'won'` is simply `true` — and #5146 ruled that answer canonical. - * - * This is `sql-driver.ts`'s and `read-scope-sql.ts`'s table, with the - * differences that come from THIS module's emitter rather than from a different - * reading of #5146 — each guard matches its own emitter, which is the invariant, - * not the literal table: - * - * - `$null` / `$exists` are read by IDENTITY (`=== true` / `=== false`) - * because {@link fieldLeaves} reads them that way, where `read-scope-sql` - * uses truthiness because its emitter does. Immaterial in practice: both - * compile to a null predicate, so they are total either way and never - * reach the polarity question. [#20040] And from the `where` door the - * flag is always a boolean here: {@link assertBooleanNullFlags} refuses - * any other value before the `$not` rewrite that consults this table runs. - * - `$between` exists in this vocabulary; it lowers to `gte` + `lte`, two - * positive comparisons, so it takes the same default they do. - * - * `$eq` / `$ne` DO carry `read-scope-sql`'s `value === null` arms — since #5332, - * and only since then. While {@link fieldLeaves} stringified a `null` comparand - * to `''`, these two arms had to describe THAT emitter: `{$eq: null}` was an - * ordinary value comparison here, the guard said so, and the TSDoc recorded the - * `''` comparand as a separate defect deliberately left undecided. #5332 decided - * it — the emitter now compiles the pair to `notSet` / `set` — so the arms moved - * with it, in the same commit. The invariant is not "copy the sibling table", it - * is "each guard matches its OWN emitter"; the two tables agreeing again is the - * consequence of the emitters agreeing, not the reason for the edit. - * - * The default is the large positive-comparison family (`$gt` / `$in` / - * `$contains` / …), every member of which answers `false` for a value that is - * not there. An operator this module does not support also lands here; it is - * guarded and then still THROWS from {@link fieldLeaves}, so fail-closed is - * preserved. - */ -function nullValueSatisfiesOperator(op: string, value: unknown): boolean { - switch (op) { - // [#5332] `$eq: null` IS the null predicate — a NULL column satisfies it, - // and no other comparand does. - case '$eq': return value === null; - // Mirror image: `$ne: null` compiles to `set` (`IS NOT NULL`), which a NULL - // column FAILS. Any other comparand is the two-valued JS `!==`, which an - // absent value passes — the arm this used to be for every comparand. - case '$ne': return value !== null; - case '$null': return value === true; - case '$exists': return value === false; - // [#20445] Null is empty on every row of the ruled table, so a NULL column - // satisfies `$empty: true` and fails its complement. - case '$empty': return value === true; - // Negative-polarity set / substring tests hold vacuously for an absent value. - case '$nin': return true; - // `$notContains` is the one operator where the two JS backends disagree for - // a null-valued field (`driver-memory` answers false, `formula` true). - // `formula` is followed because `driver-sql` and `read-scope-sql` follow it, - // so this module casts no vote on a disagreement that is filed elsewhere. - case '$notContains': return true; - default: return false; - } -} - -/** Is this operator's compiled leaf already total for a NULL column? */ -function operatorIsNullTotal(op: string, value: unknown): boolean { - // [#7598, maintainer ruling 2026-08-12] A `{ $field }` comparand on any of the - // six scalar comparison operators is TOTAL AT THE BACKEND, so this module must - // add no guard of its own — and MEASURED, adding one changes the answer. - // - // Every other entry in this switch is total because THIS module compiles the - // operator into a null predicate. This one is total because of where the leaf - // ends up: since the ruling, a `where` carrying a reference is declined by - // `NativeSQLStrategy.canHandle` and served on the engine path, where - // `driver-sql`'s `applyCrossFieldComparison` emits a predicate written total - // across NULLs by construction (it repeats both column expressions for exactly - // that reason — see `cross-field-conformance-cases.ts`, whose rows 4-6 carry - // every NULL arrangement a pair of columns can be in). `@objectstack/formula` - // resolves the reference and then compares in two-valued JS. The two agree, - // and the corpus's declared id lists are the third statement of it. - // - // ## What the guard did before this arm existed — measured on the wasm driver - // - // The `$ne` arm of {@link nullValueSatisfiesOperator} answers `true` for any - // non-null comparand, so a reference took the negative-polarity totalisation - // in {@link fieldLeaves} and `{ amount: { $ne: { $field: 'budget' } } }` - // lowered to `{$or: [{amount: null}, {amount: {$ne: ref}}]}`. That admitted - // fixture row 6 — BOTH columns NULL — where the corpus, both SQL drivers and - // the memory evaluator all EXCLUDE it, because row 6 satisfies the inner - // `$eq` and `$ne` is its exact complement. Six corpus cases moved: the three - // `$ne` class-pair cases, `a column differs from itself on no row`, and the - // two `$not`-of-`$eq` cases (which reach the same guard through - // {@link nullGuardForFieldSpec}). Widening a `$ne`, on a shape whose producer - // is an RLS rule, is the direction that matters. - // - // The guard is right for a LITERAL comparand and is untouched there: `{amount: - // {$ne: 5}}` must still admit a NULL `amount`, which is #5298's ruling and the - // JS backends' answer. What differs is only that a reference's NULL semantics - // are already decided by the referent, not by the target column alone — so - // there is nothing left for a guard to decide. - if (CROSS_FIELD_COMPARISON_OPERATORS.has(op) && isFieldReference(value)) return true; - switch (op) { - // Compile to `set` / `notSet` — `IS NULL` / `IS NOT NULL`, two-valued by - // construction, on every strategy that compiles this tree. - case '$null': - case '$exists': - return true; - // [#20445] `empty` / `notEmpty` spell their NULL case out on both SQL - // compilers (`col IS NULL OR …` / `col IS NOT NULL AND …`), and the engine - // answers the operator by its own arm, so the leaf is TOTAL: a guard would - // only restate what the predicate already says. - case '$empty': - return true; - // [#5332] A `null` comparand makes these null PREDICATES too — `notSet` / - // `set`, not comparisons — so they are total by construction and take NO - // guard. Left out, `{$not: {stage: {$eq: null}}}` wrapped `stage IS NOT NULL - // AND stage IS NULL` (an always-false conjunction) and negated it to EVERY - // row, for a filter meaning "stage is not empty". - case '$eq': - case '$ne': - return value === null; - // An EMPTY set compiles to a boolean CONSTANT (see `fieldLeaves`), and a - // constant is total. Wrapping a guard around it would only add a redundant - // conjunct to a predicate whose value is already decided. - case '$in': - case '$nin': - return Array.isArray(value) && value.length === 0; - default: - return false; - } -} - -/** - * The guard one field constraint needs. A constraint is the AND of its - * operators, so it is total when every operator is, and a NULL column satisfies - * it only when it satisfies all of them. - */ -function nullGuardForFieldSpec(spec: unknown): NullGuard { - // `{field: null}` compiles to `notSet` (`IS NULL`) — already total. - if (spec === null) return 'none'; - // [#19888] No bare-array arm: a list in the equality slot is refused by - // `assertNoListInEqualitySlot` before this rewrite runs. - // A scalar / Date is an implicit `=`; a NULL column fails it. - if (typeof spec !== 'object' || spec instanceof Date) return 'requireValue'; - const entries = Object.entries(spec as Record); - // `{field: {}}` is REFUSED by `fieldLeaves` (#5240). Passing it through - // unrewritten is what keeps that refusal reachable — a guard wrapped around it - // would only change which message the caller sees. - if (entries.length === 0) return 'none'; - let total = true; - let nullSatisfies = true; - for (const [op, value] of entries) { - if (!operatorIsNullTotal(op, value)) total = false; - if (!nullValueSatisfiesOperator(op, value)) nullSatisfies = false; - } - if (total) return 'none'; - return nullSatisfies ? 'allowNull' : 'requireValue'; -} - -/** - * Guard one `field: spec` entry, writing either the untouched entry into `out` - * or its guarded form into `guarded`. - * - * [#20887] A nested-relation condition (`{account: {region: 'NA'}}`) is written - * through untouched: it reaches the engine as written, and the engine guards - * what it lowers it to — the `$in` / `$contains` over the related ids — with - * the same NULL-safe rule, after reading the related object. A guard here would - * test the relation column before the engine knows which ids match. - */ -function guardFieldEntry( - key: string, - spec: unknown, - out: Record, - guarded: unknown[], -): void { - if (isNestedRelationCondition(spec)) { - out[key] = spec; - return; - } - - const guard = nullGuardForFieldSpec(spec); - if (guard === 'none') { - out[key] = spec; - } else if (guard === 'requireValue') { - // `col IS NOT NULL AND (…)` — both conjuncts of the enclosing node. - guarded.push({ [key]: { $null: false } }, { [key]: spec }); - } else { - // `col IS NULL OR (…)` — one conjunct, so the OR binds tighter than the AND - // this node's keys form. - guarded.push({ $or: [{ [key]: { $null: true } }, { [key]: spec }] }); - } -} - -/** - * [#5146] Rewrite the operand of a `$not` so every leaf compiles to a TOTAL - * predicate — which is what makes `NOT (…)` mean here what it means in - * `driver-memory`, `formula` and (since #5296) `driver-sql`. - * - * # Why the guard rides the LEAF, not the `NOT` - * - * For a flat operand `NOT (a IS NOT NULL AND a = ?)` and `NOT (a = ?) OR a IS - * NULL` are the same predicate. They stop being the same as soon as the operand - * nests: hoisting the guard above a `$not` whose operand is a `$or` re-admits - * rows the JS backends exclude — a NULL `a` would satisfy the whole negation - * even when the `$or`'s OTHER branch is satisfied. Totalising each leaf makes - * the rewrite compositional instead: De Morgan is sound over two-valued leaves, - * so `$and`, `$or` and a nested `$not` all stay correct with no special cases. - * - * # Why polarity is per operator - * - * A blanket `OR col IS NULL` would WIDEN the negative-polarity operators: - * `{$not: {a: {$ne: 5}}}` means "a is 5", and both JS backends exclude a NULL - * row from it. Adding an unconditional null escape there would hand back exactly - * the rows the filter excludes. So each leaf is guarded in the direction its own - * operator answers, per {@link nullValueSatisfiesOperator}. - * - * # Why it is a REWRITE of the condition, not of the tree - * - * The output is still a `FilterCondition`, so `buildNode` compiles it with no - * new cases and — the point of doing it here rather than in the SQL strategy — - * the guard reaches the ObjectQL engine as structure too. Running only inside a - * `$not` keeps every other comparison's shape untouched, and a NESTED `$not` is - * left alone on purpose: its own branch totalises its operand, and - * `NOT ` is itself total, so recursing would stack a redundant guard on - * the same column. - */ -function nullSafeNegationOperand(node: Record): Record { - const out: Record = {}; - const guarded: unknown[] = []; - for (const [key, value] of Object.entries(node)) { - if ((key === '$and' || key === '$or') && Array.isArray(value)) { - // A non-object element is passed through so `buildNode` still refuses it - // with its own message. - out[key] = value.map((element) => (isFilterObject(element) ? nullSafeNegationOperand(element) : element)); - continue; - } - if (key.startsWith('$')) { - // `$not` (handled by its own branch) and anything else `$`-prefixed keep - // whatever this module does with them today — the rewrite rules on NULL, - // not on the operator vocabulary, and an unknown one must still throw. - out[key] = value; - continue; - } - guardFieldEntry(key, value, out, guarded); - } - if (guarded.length > 0) { - const existing = Array.isArray(out.$and) ? out.$and : []; - out.$and = [...existing, ...guarded]; - } - return out; -} - // ── [#5334] The FilterArray door ───────────────────────────────────────────── /** @@ -2192,10 +1888,10 @@ function nonBooleanFlagError(op: BooleanFlagOperator, field: string, path: strin * * ## Why before any lowering, and not at the identity read in `fieldLeaves` * - * Two readers see the flag before that read does. {@link nullSafeNegationOperand} - * classifies every field spec under a `$not` through - * {@link nullValueSatisfiesOperator} and {@link operatorIsNullTotal}, both of - * which read the flag, and the draft preview evaluates the condition + * Two readers see the flag before that read does. The shared lowering + * (`lowerFilterCondition`, run by {@link normalizeAnalyticsFilterTree} before + * {@link buildNode}) classifies every field spec under a `$not` by its NULL + * polarity, which reads the flag, and the draft preview evaluates the condition * {@link normalizeWhereComparands} returns without ever reaching `fieldLeaves`. * A gate here answers all of them, and the preview then refuses this cell in * the published door's words. `read-scope-sql.ts` placed its twin at its one @@ -2469,8 +2165,9 @@ export function conjunctFieldKeys(condition: Record): string[] * `lowering` is the caller's column-type reader (item 7), and it is REQUIRED, * so no compile site can reach the tree without deciding it: a strategy passes * the member's declared type through its context's `declaredFieldType` hook - * ({@link declaredDatetimeLowering}), and a position that cannot or need not - * read types passes {@link NO_DATETIME_COLUMNS}. {@link lowerAnalyticsWhere} + * ({@link declaredDatetimeLowering}, which also states how that strategy reads + * a column the hook cannot name), and a position that only collects members + * passes {@link NO_DATETIME_COLUMNS}. {@link lowerAnalyticsWhere} * itself stays un-lowered: its other readers ask about the AUTHORED condition * (the keys an ad-hoc cube is minted from, the routing detectors), not about * the predicate that runs. @@ -2550,27 +2247,50 @@ function shieldNestedRelations(node: Record): Record false, }); /** - * [ADR-0053 D-D1, amended — #5930 step 3] A strategy's column-type reader for - * the shared lowering (item 7): a `where` member is a `datetime` column when + * Item 7's type-blind reading: the lowering is handed no reader, so its two + * type-scoped rules apply to every column. + */ +const TYPE_BLIND: FilterLoweringOptions = Object.freeze({}); + +/** + * How a strategy's lowering reads a column whose declared type its host cannot + * name (no `declaredFieldType` hook, or a hook answering no type for it). + * + * - `'type-blind'` — ADR-0053 D-D1 item 7's reading for a seam that cannot read + * the declaration: the whole-day rule and the `$between` split apply to that + * column (sound on `Field.date` text, where `< next-day` orders exactly as + * `<= day`). For a face that is the LAST seam before its statement runs, + * where nothing downstream reads the declaration. + * - `'as-written'` — leave that column's bounds as written, for a face whose + * filter is handed to a seam that does read the declaration: the ObjectQL + * engine's `where` seam, which lowers it again with the object's own field + * map (and itself applies item 7's type-blind reading to an object with no + * field map). + */ +export type UndeclaredColumnReading = 'type-blind' | 'as-written'; + +/** + * [ADR-0053 D-D1, amended — #5930 steps 3 and 4] A strategy's column-type reader + * for the shared lowering (item 7): a `where` member is a `datetime` column when * the host's declared-type hook says the column it binds against is one — * `type === 'datetime'`, the test `SqlDriver` indexes `datetimeFields` by and * the engine seam reads. `target` resolves a member to its (object, column) @@ -2578,23 +2298,58 @@ export const NO_DATETIME_COLUMNS: FilterLoweringOptions = Object.freeze({ * asked of the column the predicate will read. * * The hook is the context's optional `declaredFieldType` — the one - * `nonTextColumnResolver` asks — and a context without one gets - * {@link NO_DATETIME_COLUMNS}. + * `nonTextColumnResolver` asks; the production composition answers it from the + * engine's registry (`AnalyticsServiceConfig.sourceFieldMeta`). A column the + * hook declares is read by its declaration. A column it cannot name (no hook, + * or no type for that column) is read as `undeclared` says + * ({@link UndeclaredColumnReading}), so each strategy states which seam owns the + * column's type rather than inheriting one silent default. + * + * This reader is the whole of the whole-day rule on the strategies' `where`, + * measure-filter, dataset-scope and `dateRange` positions: no strategy keeps a + * whole-day copy of its own since #5930 step 4. */ export function declaredDatetimeLowering( ctx: StrategyContext, target: (member: string) => { object: string; field: string }, + undeclared: UndeclaredColumnReading, ): FilterLoweringOptions { const declared = (ctx as DatasetScopedStrategyContext).declaredFieldType; - if (typeof declared !== 'function') return NO_DATETIME_COLUMNS; + if (typeof declared !== 'function') return undeclared === 'type-blind' ? TYPE_BLIND : NO_DATETIME_COLUMNS; return { isDatetimeColumn: (member) => { const { object, field } = target(member); - return declared.call(ctx, object, field) === 'datetime'; + const type = declared.call(ctx, object, field); + if (typeof type !== 'string' || type === '') return undeclared === 'type-blind'; + return type === 'datetime'; }, }; } +/** + * [ADR-0053 D-D1 item 8, amended — #5930 step 4] A `timeDimensions[].dateRange` + * window as the tree the strategies compile: the `{ $gte, $lte }` pair (or the + * `{ $gte, $lt }` pair of a resolved preset that stops before its end) on the + * window's member, through the same shared lowering, with the same reader, as + * the strategy's `where`. + * + * So an explicit window's bare-day end takes the whole-day rule exactly where + * a `where` bound on the same member would — on a `datetime` column, or one + * whose type the reader cannot name — and nowhere else, and on the last + * supported day the end is dropped. A resolved preset's ends are instants, + * which the lowering never widens. The window's own door (the preset + * vocabulary, `explicitDateRangeWindow`) has already judged the bounds, so the + * `where` door's comparand faces do not run here: this changes what a window + * means on no input it accepts. + */ +export function normalizeDateRangeWindow( + member: string, + bounds: Record, + lowering: FilterLoweringOptions, +): NormalizedFilterNode | null { + return buildNode(lowerFilterCondition({ [member]: bounds }, lowering)); +} + /** * Every leaf in the tree, structure discarded. * 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 c6951af6c50..64d8ec0dac1 100644 --- a/packages/services/service-analytics/src/strategies/native-sql-strategy.ts +++ b/packages/services/service-analytics/src/strategies/native-sql-strategy.ts @@ -10,6 +10,7 @@ import { invalidFilterError, lowerAnalyticsWhere, normalizeAnalyticsFilterTree, + normalizeDateRangeWindow, toSqlBindValue, SQL_CONST_FALSE, SQL_CONST_TRUE, @@ -46,7 +47,7 @@ import { textMatchPredicateSql, sqlDialectFor, type AnalyticsSqlDialect } from ' import { whereContainsMembershipSql } from '../contains-membership-sql.js'; import { isJsonStoredShape } from '../contains-membership-sql.js'; import { expandEmptyOperator } from '@objectstack/spec/data'; -import { nextUtcCalendarDay, resolveAnalyticsDateRangeString, isUnboundedAbove } from '@objectstack/core'; +import { resolveAnalyticsDateRangeString } from '@objectstack/core'; // [#20889] What each aggregate function ANSWERS, and the `'number'` presenter — // the rule `driver-sql`'s own `aggregate()` applies, defined once in core. import { AGGREGATE_ANSWER_KIND, presentAsNumber } from '@objectstack/core'; @@ -1194,14 +1195,21 @@ export class NativeSQLStrategy implements AnalyticsStrategy { // dataset, which is why an inferred or manifest cube compiles unchanged. const datasetScope = (ctx as DatasetScopedStrategyContext).getDatasetScope?.(query.cube!); - // [ADR-0053 D-D1, amended — #5930 step 3] The column-type reader the `where` - // door's shared lowering applies at every filter position below — the - // measure filters, the `where` and the dataset's own scope (item 7): a - // member is `datetime` when the column it binds against is declared so, - // asked of the SAME target `compileFilterNode` coerces for. This face's own - // 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)); + // [ADR-0053 D-D1, amended — #5930 steps 3 and 4] The column-type reader the + // `where` door's shared lowering applies at every filter position below — + // the measure filters, the `where`, the dataset's own scope and the + // `dateRange` windows (items 7 and 8): a member is `datetime` when the + // column it binds against is declared so, asked of the SAME target + // `compileFilterNode` coerces for. It is the ONE source of the whole-day + // rule on this face: `buildFilterClause` compiles the bound it is handed. + // A column the host cannot name a type for is read type-blind (item 7): + // this face is the last seam before its statement runs, so nothing + // downstream reads the declaration. + const lowering = declaredDatetimeLowering( + ctx, + (member) => this.resolveStorageTarget(cube, member, tableName, joins.referenceOf), + 'type-blind', + ); // [#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}). @@ -1267,7 +1275,9 @@ export class NativeSQLStrategy implements AnalyticsStrategy { // Build time dimension filters if (query.timeDimensions && query.timeDimensions.length > 0) { for (const td of query.timeDimensions) { - const colExpr = this.resolveFieldSql(cube, td.dimension, tableName, joins); + // Resolved for every time dimension, window or not, as it always was: + // it registers the join a relationship-path member walks. + this.resolveFieldSql(cube, td.dimension, tableName, joins); if (td.dateRange) { // [#16322] The STRING arm is the CLOSED preset vocabulary (#16041), // lowered by the ONE shared resolver `driver-memory` and the ObjectQL @@ -1281,8 +1291,8 @@ export class NativeSQLStrategy implements AnalyticsStrategy { const resolved = Array.isArray(td.dateRange) ? null : resolveAnalyticsDateRangeString(td.dateRange, { timezone: query.timezone }); - const range = resolved - ? ([resolved.start, resolved.end] as [string, string]) + const [start, end] = resolved + ? [resolved.start, resolved.end] // [commit 86c505286] An oddly-sized array is REFUSED, by the one // `explicitDateRangeWindow` every face in this package calls. ⛔ What // this replaced was a silent `if (range.length === 2)` DROP: a @@ -1290,43 +1300,36 @@ export class NativeSQLStrategy implements AnalyticsStrategy { // ALL of history — "plot all of history" is the very failure #16322 // repaired for the string arm, and it was still live on this arm. : explicitDateRangeWindow(td.dateRange as readonly unknown[]); - // Same epoch-vs-text root cause as buildFilterClause: a dateRange on a - // SQLite `Field.datetime` column compares ISO TEXT against an INTEGER - // epoch and matches nothing. Coerce both bounds to the storage form — - // and normalise the column to that form too, because the column holds - // BOTH forms at once and coercing only the bounds still empties the - // half the writer stored the other way (#3912). - const td2 = this.resolveStorageTarget(cube, td.dimension, tableName, joins.referenceOf); - const column = this.temporalColumn(ctx, td2, colExpr); - // A bare-day window end means "through that whole day" (#3777). A - // BETWEEN's inclusive upper bound anchors a bare `YYYY-MM-DD` to - // midnight on a datetime column, dropping the final day's rows, so - // the window compiles half-open — `>= start AND < end+1day` — the - // same `[gte, lt)` the drill ranges emit. Equivalent to the old - // BETWEEN for a `date` column (plain `YYYY-MM-DD` ordering), which - // is what lets this path stay column-type-blind. + // [ADR-0053 D-D1 item 8, amended — #5930 step 4] The window is the + // `{ $gte, $lte }` pair the ObjectQL strategy hands the engine, lowered + // by the same reader as this statement's `where` + // ({@link normalizeDateRangeWindow}) and compiled by the same + // `compileFilterNode`, so its bounds take the storage-form coercion and + // the column normalisation every `where` bound takes (#3912). A bare-day + // explicit end means "through that whole day" (#3777): on a `datetime` + // column, or one whose type the host cannot name, the lowering rewrites + // it to `< end+1day`, the same `[gte, lt)` the drill ranges emit, and on + // the last supported day it drops the end (#20600); on a column declared + // anything else the end stays inclusive, the comparison the typed + // drivers run. This face kept its own type-blind copy of that rule + // until #5930 step 4. // - // [#16322] A RESOLVED window already states its own upper reading - // and is never a bare day, so it never takes the widening branch: - // the ten calendar presets stop BEFORE their end instant (`<`), the - // three rolling ones end at NOW and reach it (`<=`). ⛔ An explicit - // `[a, b]` a CALLER wrote keeps the inclusive reading it has always - // had — the #16179 separation, on this side too. - const nextDay = resolved ? null : nextUtcCalendarDay(range[1]); - params.push(this.coerceTemporal(ctx, td2, range[0])); - const lower = `${column} >= $${params.length}`; - // [#20600] A bare end on the last supported day has no next day to - // stop before: every value is inside it, so the window keeps its - // start alone. - if (isUnboundedAbove(nextDay)) { - whereClauses.push(`(${lower})`); - } else { - const upperExclusive = resolved ? resolved.endExclusive : nextDay != null; - params.push(this.coerceTemporal(ctx, td2, nextDay ?? range[1])); - whereClauses.push( - `(${lower} AND ${column} ${upperExclusive ? '<' : '<='} $${params.length})`, - ); - } + // [#16322] A RESOLVED window states its own upper reading, and its ends + // are instants the lowering never widens: the ten calendar presets stop + // BEFORE their end instant (`$lt`), the three rolling ones end at NOW + // and reach it (`$lte`). ⛔ An explicit `[a, b]` a CALLER wrote keeps the + // inclusive reading it has always had — the #16179 separation, on this + // side too. + const bounds = resolved?.endExclusive ? { $gte: start, $lt: end } : { $gte: start, $lte: end }; + const windowSql = this.compileFilterNode( + normalizeDateRangeWindow(td.dimension, bounds, lowering), + cube, + tableName, + joins, + params, + ctx, + ); + if (windowSql) whereClauses.push(windowSql); } } } @@ -1832,10 +1835,11 @@ export class NativeSQLStrategy implements AnalyticsStrategy { * through the combinators. `null` = no constraint. * * Leaves go through {@link buildFilterClause} exactly as they did when this - * was a flat loop, so the storage-form coercion and the calendar-day - * upper-bound rule (#3777) apply at every depth — including inside an `$or`, - * where a second, combinator-aware implementation would have been free to - * drift from the first. + * was a flat loop, so the storage-form coercion applies at every depth — + * including inside an `$or`, where a second, combinator-aware implementation + * would have been free to drift from the first. The calendar-day upper-bound + * rule (#3777) is not applied here at any depth: the tree arrives with it + * already applied by the shared lowering (#5930 step 4). * * Parenthesisation is explicit rather than left to SQL's precedence: `AND` * does bind tighter than `OR`, so `a AND b OR c` happens to be right, but @@ -2077,21 +2081,17 @@ export class NativeSQLStrategy implements AnalyticsStrategy { }); } - // A bare-day `lte` bound means "through that whole day" (#3777): compile - // half-open (`< day+1`) so a datetime column keeps the final day's rows. - // Equivalent to `<=` for a `date` column, so no column-type lookup needed. - if (operator === 'lte') { - const nextDay = nextUtcCalendarDay(values[0]); - // [#20600] On the last supported day there is no next day: every value is - // inside the bound, so what `lte` still asks is a value — the `set` arm's - // `IS NOT NULL`. - if (isUnboundedAbove(nextDay)) return `${rawCol} IS NOT NULL`; - if (nextDay != null) { - params.push(this.coerceTemporal(ctx, target, nextDay)); - return `${this.temporalColumn(ctx, target, rawCol)} < $${params.length}`; - } - } - + // [ADR-0053 D-D1, amended — #5930 step 4] An `lte` compiles the bound it is + // handed, like every other comparison. A bare-day upper bound means + // "through that whole day" (#3777) on a `datetime` column, and the shared + // lowering already rewrote such a bound to `lt` the next day (or, on the + // last supported day, to `set`) before this compiler saw the tree — with the + // column's declared type in hand ({@link compileClauses}' `lowering`). An + // `lte` that reaches this line is on a column declared something else + // (`date`, text, a number), where the comparison as written is the typed + // drivers' answer. This compiler kept a type-blind copy of the rule here, + // which widened a bare day on every column, until #5930 step 4. + // // Coerce so booleans/numbers bind as their native SQL types AND so a // relative-date / ISO-string comparand on a SQLite `Field.datetime` column // is converted to that column's storage form (#16737: the ONE statement of diff --git a/packages/services/service-analytics/src/strategies/objectql-strategy.ts b/packages/services/service-analytics/src/strategies/objectql-strategy.ts index e0a184a4ff6..9839b161945 100644 --- a/packages/services/service-analytics/src/strategies/objectql-strategy.ts +++ b/packages/services/service-analytics/src/strategies/objectql-strategy.ts @@ -12,6 +12,7 @@ import { lowerAnalyticsWhere, NO_DATETIME_COLUMNS, normalizeAnalyticsFilterTree, + normalizeDateRangeWindow, collectFilterLeaves, SQL_CONST_FALSE, SQL_CONST_TRUE, @@ -36,7 +37,7 @@ import { projectedDimensions } from '../order-key-door.js'; import { applyOrdering, applyWindow } from '../dataset-executor.js'; import { type LikeShape } from '../like-pattern.js'; import { textMatchPredicateSql, sqlDialectFor } from '../text-match-sql.js'; -import { nextUtcCalendarDay, resolveAnalyticsDateRangeString, isUnboundedAbove } from '@objectstack/core'; +import { resolveAnalyticsDateRangeString } from '@objectstack/core'; import { explicitDateRangeWindow } from '../date-range-array-arm.js'; import { rebucketCrossObject, @@ -192,12 +193,19 @@ export class ObjectQLStrategy implements AnalyticsStrategy { // inferred or manifest cube compiles unchanged. const datasetScope = (ctx as DatasetScopedStrategyContext).getDatasetScope?.(query.cube!); - // [ADR-0053 D-D1, amended — #5930 step 3] The column-type reader the `where` - // door's shared lowering applies at the three filter positions this path - // hands the engine (item 7): a member is `datetime` when the column it binds - // against is declared so. The engine seam lowers the same filter again with - // the same scope, and the lowering is idempotent. - const lowering = declaredDatetimeLowering(ctx, (member) => this.resolveStorageTarget(cube, member, objectName, relationshipReferenceOf(ctx))); + // [ADR-0053 D-D1, amended — #5930 steps 3 and 4] The column-type reader the + // `where` door's shared lowering applies at the three filter positions this + // path hands the engine (item 7): a member is `datetime` when the column it + // binds against is declared so. The engine seam lowers the same filter + // again with the object's own field map, and the lowering is idempotent. A + // column the host cannot name a type for is left as written: the engine's + // seam reads the declaration this one cannot (and applies item 7's + // type-blind reading itself to an object with no field map). + const lowering = declaredDatetimeLowering( + ctx, + (member) => this.resolveStorageTarget(cube, member, objectName, relationshipReferenceOf(ctx)), + 'as-written', + ); // Build aggregations from measures. // @@ -492,12 +500,15 @@ export class ObjectQLStrategy implements AnalyticsStrategy { // the same channel `execute()` reads it from, so the echo cannot drift // from what actually ran. const datasetScope = (ctx as DatasetScopedStrategyContext).getDatasetScope?.(query.cube!); - // [ADR-0053 D-D1, amended — #5930 step 3] The same column-type reader - // `execute()` hands the `where` door's shared lowering, so the echo prints - // the lowered bound the engine receives — a bare-day `$lte` on a `datetime` - // member reads `< next-day` here because that is what runs. - const echoLowering = declaredDatetimeLowering(ctx, (member) => - this.resolveStorageTarget(cube, member, this.extractObjectName(cube), relationshipReferenceOf(ctx)), + // [ADR-0053 D-D1, amended — #5930 steps 3 and 4] The same column-type + // reader `execute()` hands the `where` door's shared lowering, so the echo + // prints the lowered bound the engine receives — a bare-day `$lte` on a + // `datetime` member reads `< next-day` here because that is what runs, in + // the `where`, the scopes and the `dateRange` windows alike. + const echoLowering = declaredDatetimeLowering( + ctx, + (member) => this.resolveStorageTarget(cube, member, this.extractObjectName(cube), relationshipReferenceOf(ctx)), + 'as-written', ); const crossByDim = new Map((plan?.crossDims ?? []).map((cd) => [cd.outputName, cd])); const joinClauses: string[] = []; @@ -600,23 +611,20 @@ export class ObjectQLStrategy implements AnalyticsStrategy { } // Bounds bind as `$n` placeholders like every other comparand: this string // travels to the browser, and a window can carry tenant-derived dates. - // A bare-day upper bound renders half-open (`< day+1`) because that is - // what `execute()`'s driver actually runs for it on a datetime column - // (#3777) — rendering the BETWEEN would hand a debugger SQL that drops - // the final day's rows and cannot reproduce the result. - for (const { field, bounds } of this.dateRangeBounds(cube, query)) { - const nextDay = nextUtcCalendarDay(bounds.$lte); - // [#20600] A bare end on the last supported day renders no upper bound, - // because the driver compiles none for it. - if (isUnboundedAbove(nextDay)) { - params.push(bounds.$gte); - whereParts.push(`(${field} >= $${params.length})`); - continue; - } - params.push(bounds.$gte, nextDay ?? bounds.$lte); - whereParts.push( - `(${field} >= $${params.length - 1} AND ${field} ${nextDay ? '<' : '<='} $${params.length})`, - ); + // + // [ADR-0053 D-D1 item 8, amended — #5930 step 4] Each window renders as the + // `{ $gte, $lte }` pair `execute()` hands the engine, lowered by + // {@link normalizeDateRangeWindow} with the echo's reader and rendered by + // the same `renderFilterNodeSql` as the `where`. So a bare-day end on a + // `datetime` column renders half-open (`< day+1`), and on the last + // supported day as `IS NOT NULL` beside the start, because that is what + // the engine's seam runs for it (#3777, #20600); a `date` column renders + // the inclusive `<=` the engine runs there. This echo kept its own + // type-blind copy of the rule, which rendered `< day+1` on every column, + // until #5930 step 4. + for (const { member, bounds } of this.dateRangeBounds(cube, query)) { + const windowSql = this.renderFilterNodeSql(normalizeDateRangeWindow(member, bounds, echoLowering), cube, params, ctx); + if (windowSql) whereParts.push(windowSql); } // Read scope last, so it reads as the outermost constraint. Compiled by the // same fail-closed compiler `NativeSQLStrategy` uses — it throws rather than @@ -1840,11 +1848,12 @@ export class ObjectQLStrategy implements AnalyticsStrategy { * * An EXPLICIT `[a, b]` window is inclusive on both ends — logically "from day * X through day Y". The `$lte` end is left as the bare calendar day on - * purpose: the driver's filter compiler owns the calendar-day → instant - * translation, compiling a bare-day `$lte` on a `datetime` column into the - * half-open `< nextDay` (#3777) while a `date` column keeps the plain `<=`. - * `NativeSQLStrategy` performs the same half-open translation itself because - * it binds into raw SQL, so one dashboard reads the same on every driver. + * purpose: the shared lowering at the engine's `where` seam owns the + * calendar-day → instant translation, rewriting a bare-day `$lte` on a + * `datetime` column into the half-open `< nextDay` (#3777) while a `date` + * column keeps the plain `<=` (ADR-0053 D-D1, amended, items 7 and 8). + * `NativeSQLStrategy` runs the same lowering on the same pair because it + * binds into raw SQL, so one dashboard reads the same on every driver. * * [#16322] A window this face RESOLVED is a different question and carries * its own upper reading — see the string arm below. @@ -1901,8 +1910,8 @@ export class ObjectQLStrategy implements AnalyticsStrategy { private dateRangeBounds( cube: Cube, query: AnalyticsQuery, - ): Array<{ field: string; bounds: Record }> { - const out: Array<{ field: string; bounds: Record }> = []; + ): Array<{ member: string; field: string; bounds: Record }> { + const out: Array<{ member: string; field: string; bounds: Record }> = []; for (const td of query.timeDimensions ?? []) { if (!td.dateRange) continue; // [#16322] The STRING arm is the CLOSED preset vocabulary, resolved by @@ -1913,6 +1922,7 @@ export class ObjectQLStrategy implements AnalyticsStrategy { if (!Array.isArray(td.dateRange)) { const window = resolveAnalyticsDateRangeString(td.dateRange, { timezone: query.timezone }); out.push({ + member: td.dimension, field: this.resolveFieldName(cube, td.dimension, 'dimension'), // A window this path RESOLVED states its own upper reading: the ten // calendar presets stop BEFORE their end instant (`$lt`, so two @@ -1927,10 +1937,12 @@ export class ObjectQLStrategy implements AnalyticsStrategy { } // ⛔ The CALLER's explicit window is untouched, bound for bound: `$lte` // on a bound they wrote is the reading this face has published since it - // existed (#16179), and the driver's own bare-day widening still owns - // the calendar-day → instant translation for it. + // existed (#16179), and the shared lowering at the engine's `where` seam + // owns the calendar-day → instant translation for it (ADR-0053 D-D1 item + // 8; the echo renders the same lowering). const [start, end] = explicitDateRangeWindow(td.dateRange); out.push({ + member: td.dimension, field: this.resolveFieldName(cube, td.dimension, 'dimension'), bounds: { $gte: start, $lte: end }, });