From 03a31643e4f6b414ef4d21e3abebc8a93b3885ba Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 15 Aug 2026 02:30:24 +0000 Subject: [PATCH 1/3] fix(objectql): refuse an uninterpretable temporal filter comparand at the engine door (#8690) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A bare string a temporal field cannot interpret — `last_30_days`, `not-a-date-at-all` — was bound as written, compared false for every row, and answered 200 with an empty result and no diagnostic, while an unknown `{placeholder}` was refused loudly one branch over. Refuse it at the ObjectQL engine's single filter collection point, per the maintainer ruling of 2026-08-15 (option B): `lowerWhereFilterArray` is the one seam holding the caller's comparand and the field's declared type at the same moment, on every verb and through both doors. - `@objectstack/core`: the value-half predicate, shared so the rule cannot exist twice; interpretability is defined by the drivers' own totals. - `@objectstack/objectql`: the door, `INVALID_FILTER` / 400. - `@objectstack/service-analytics`: `NativeSQLStrategy.canHandle` declines an uninterpretable temporal comparand so raw-SQL paths fall through to the door instead of binding it directly. Scoped to non-empty strings by ruling: the empty-string cell stays its own card, and `{placeholder}` strings keep their existing loud refusal. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XeQRiAa7vYRVX5Fog7Zby8 --- packages/core/src/index.ts | 6 + packages/core/src/utils/temporal-comparand.ts | 161 ++++++++++ .../engine-temporal-comparand-door.test.ts | 265 ++++++++++++++++ packages/objectql/src/engine.ts | 15 + .../objectql/src/temporal-comparand-door.ts | 287 ++++++++++++++++++ .../service-analytics/src/comparand-shape.ts | 118 +++++++ .../src/strategies/native-sql-strategy.ts | 76 ++++- 7 files changed, 927 insertions(+), 1 deletion(-) create mode 100644 packages/core/src/utils/temporal-comparand.ts create mode 100644 packages/objectql/src/engine-temporal-comparand-door.test.ts create mode 100644 packages/objectql/src/temporal-comparand-door.ts diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 9773b97da2..9dd9714187 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -52,6 +52,12 @@ export * from './utils/migration-journal.js'; // Export the runtime filter-placeholder resolver (framework#3582) export * from './utils/filter-tokens.js'; +// [#8690] Can a temporal column's storage rule read this comparand? The VALUE +// half of the field-typed judgement behind the engine's temporal-comparand door +// and the analytics raw-SQL decline — one rule, two packages that do not depend +// on each other. +export * from './utils/temporal-comparand.js'; + // Export the shared single-record 404 (#4435/#5138, moved down here in #7867) — // the one `RECORD_NOT_FOUND` envelope `protocol.updateData`/`deleteData`, // `callData`'s ObjectQL fallback and the engine's own by-id write gate answer diff --git a/packages/core/src/utils/temporal-comparand.ts b/packages/core/src/utils/temporal-comparand.ts new file mode 100644 index 0000000000..3ecfc133ca --- /dev/null +++ b/packages/core/src/utils/temporal-comparand.ts @@ -0,0 +1,161 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#8690] Can a temporal field's storage rule READ this comparand at all? + * + * The one predicate behind the engine's temporal-comparand door + * (`@objectstack/objectql`, `temporal-comparand-door.ts`) and the analytics + * raw-SQL decline (`@objectstack/service-analytics`, + * `NativeSQLStrategy.canHandle`). It lives in `core` because those two packages + * do not depend on each other and a rule that exists twice is a rule that will + * disagree with itself — the shape the 2026-08-12 ruling named by name. + * + * ## The defect it exists to close + * + * A `datetime` field filtered with a bare string the API cannot take literally + * — `last_30_days`, `not-a-date-at-all` — was bound AS-IS all the way to the + * driver, where the comparison is false for every row. Measured end to end on + * `InMemoryDriver` with 51 rows seeded / 38 in-window: + * + * ``` + * $gte "last_30_days" HTTP 200 count=0 <- silent zero (the defect) + * $gte "not-a-date-at-all" HTTP 200 count=0 <- silent zero + * $gte "{30_days_ago}" HTTP 200 count=38 <- the positive control + * $gte "{TODAY}" REFUSED FILTER_TOKEN_UNKNOWN / 400 + * ``` + * + * A `{placeholder}` the resolver does not know is refused loudly; a bare string + * that is not a date was not validated at all. An empty chart is the hardest + * failure to debug — it is indistinguishable from "there is genuinely no data". + * + * ## Why the KIND is an argument, and not something this file works out + * + * "Uninterpretable comparand" is a field-TYPED judgement, and `packages/spec` + * says so outright (`filter.zod.ts`): a filter schema "is field-AGNOSTIC … it + * never sees which column the operator is applied to". This module therefore + * answers only the VALUE half — given a kind, can that kind's storage rule read + * this value — and the caller, which owns object metadata, supplies the kind. + * That split is what lets the rule sit below both consumers without dragging + * field metadata down with it. + * + * ## Interpretability is defined by the DRIVERS' own totals, deliberately + * + * Each predicate below mirrors the total function that would receive the value + * if the door let it through — `storageDatetimeValue` / `storageDateValue` / + * `storageTimeValue` in `driver-memory`, and their `SqlDriver` twins. Those + * functions are total on purpose: an input they cannot interpret is returned + * UNCHANGED rather than becoming an invented instant. So "the driver would + * return it unchanged" IS the definition of uninterpretable, and defining it + * any other way would refuse comparands that work today. + * + * That is why this is not `utcInstantMs` (`@objectstack/spec/data`), which is + * the stricter canonical reader: it rejects a bare epoch-millisecond string and + * every non-ISO spelling `Date.parse` accepts, both of which the drivers read + * correctly today. Refusing those would be a narrowing this card did not ask + * for and no measurement supports. + * + * ## Two things it deliberately does NOT judge + * + * - **Non-string comparands.** A number is epoch milliseconds, a `Date` is an + * instant, `null` is a null test. The refusal scopes to strings by ruling. + * - **The EMPTY string.** Measured, `$gte ""` binds as `''` and every canonical + * UTC text sorts at or above it, so it returns every non-null row — a third + * behaviour again, and one the maintainer ruled stays its own card: "B and C + * scope to non-empty strings and must not decide it in passing". A + * whitespace-only string is the same cell (the drivers `trim()` before they + * test), so it is left alone too. + */ + +import { classifyFilterToken } from '@objectstack/spec/data'; + +/** Which temporal storage rule a declared field takes. */ +export type TemporalComparandKind = 'datetime' | 'date' | 'time'; + +/** + * The kind a declared field's `type` takes, or `null` for every non-temporal + * field. + * + * The same three-way split `driver-memory`'s `indexTemporalFields` and + * `SqlDriver.temporalFieldKind` make, so the door and the drivers cannot + * disagree about which fields are temporal at all. + */ +export function temporalComparandKind(fieldType: unknown): TemporalComparandKind | null { + if (fieldType === 'datetime') return 'datetime'; + if (fieldType === 'date') return 'date'; + if (fieldType === 'time') return 'time'; + return null; +} + +/** + * `storageDatetimeValue`'s reading, as a yes/no. + * + * Mirrors it step for step: a bare integer in either sign is epoch + * milliseconds; a bare `YYYY-MM-DD` is midnight UTC; a zone-naive + * `YYYY-MM-DD[ T]HH:MM[:SS[.fff]]` has its wall clock read AS UTC; anything + * else is handed to `Date.parse` exactly as the drivers hand it over. + */ +function readsAsInstant(s: string): boolean { + if (/^-?\d+$/.test(s)) return Number.isFinite(new Date(Number(s)).getTime()); + const iso = /^\d{4}-\d{2}-\d{2}$/.test(s) + ? `${s}T00:00:00.000Z` + : /^\d{4}-\d{2}-\d{2}[ T]\d{2}:\d{2}(:\d{2}(\.\d+)?)?$/.test(s) + ? `${s.replace(' ', 'T')}Z` + : s; + return Number.isFinite(Date.parse(iso)); +} + +/** + * `storageDateValue`'s reading of a STRING: its leading `YYYY-MM-DD`, and + * nothing else. + * + * Narrower than {@link readsAsInstant} on purpose, because the rule it mirrors + * is narrower: `storageDateValue` collapses a leading calendar day and returns + * every other string untouched. `2026/07/15` is therefore uninterpretable for a + * `date` column even though `Date.parse` reads it — today it survives to the + * driver and compares as text against `YYYY-MM-DD` values, which is the silent + * wrong answer this door exists to stop. + */ +function readsAsCalendarDay(s: string): boolean { + return /^\d{4}-\d{2}-\d{2}/.test(s); +} + +/** + * `storageTimeValue`'s reading: a bare wall clock whose components are in + * range. Out-of-range (`25:00`) is uninterpretable — the rule it mirrors + * returns such a value untouched rather than wrapping it. + */ +function readsAsWallClock(s: string): boolean { + const m = /^(\d{2}):(\d{2})(?::(\d{2})(?:\.(\d+))?)?$/.exec(s); + if (!m) return false; + return Number(m[1]) <= 23 && Number(m[2]) <= 59 && Number(m[3] ?? '0') <= 59; +} + +/** + * Is `value` a comparand that a `kind` column's storage rule cannot read? + * + * `true` ONLY for a non-empty, non-placeholder STRING that the kind's rule + * would hand back unchanged. Everything else — a number, a `Date`, `null`, a + * `{ $field }` reference, filter structure, the empty string, a `{token}` — + * answers `false`, each for a reason recorded in the module note or below. + * + * A `{placeholder}` is stepped around rather than judged because it is another + * layer's vocabulary and that layer already refuses the unknown ones loudly + * (`FILTER_TOKEN_UNKNOWN` / 400, with the resolvable tokens listed). Both doors + * that call this run BEFORE token resolution, so judging a placeholder here + * would refuse `{30_days_ago}` — the platform's own correct spelling, and the + * positive control this fix is pinned against. + */ +export function isUninterpretableTemporalComparand( + kind: TemporalComparandKind, + value: unknown, +): boolean { + if (typeof value !== 'string') return false; + const s = value.trim(); + // The empty-string cell is its own card — see the module note. + if (s === '') return false; + // Another layer's vocabulary, and it has its own loud refusal. + if (classifyFilterToken(value) !== null) return false; + if (kind === 'datetime') return !readsAsInstant(s); + if (kind === 'date') return !readsAsCalendarDay(s); + return !(readsAsWallClock(s) || readsAsInstant(s)); +} diff --git a/packages/objectql/src/engine-temporal-comparand-door.test.ts b/packages/objectql/src/engine-temporal-comparand-door.test.ts new file mode 100644 index 0000000000..d14bd8fa4a --- /dev/null +++ b/packages/objectql/src/engine-temporal-comparand-door.test.ts @@ -0,0 +1,265 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#8690] The temporal-comparand door at the engine's filter collection point. + * + * The card's whole table is the fixture: 51 rows seeded, 38 inside a + * start-of-day 30-days-ago floor, on a declared `datetime` field. Every + * assertion below is one of its cells, so the suite reads as the defect it + * closes rather than as an abstraction of it. + * + * The refusal pin and the POSITIVE CONTROL live in one `it()` by ruling: a + * refusal pin with no positive control cannot show the gate is discriminating + * rather than refusing everything, and the two drifting apart into separate + * cases is how that guarantee gets lost. + */ + +import { describe, it, expect, beforeEach } from 'vitest'; +import { ObjectQL } from './engine.js'; + +/** Days back from `now`, as the canonical UTC instant the store holds. */ +function daysAgoIso(now: Date, days: number): string { + return new Date(now.getTime() - days * 86_400_000).toISOString(); +} + +const support_case = { + name: 'support_case', + label: 'Support Case', + fields: { + id: { name: 'id', type: 'text' as const, primaryKey: true }, + subject: { name: 'subject', type: 'text' as const }, + created_date: { name: 'created_date', type: 'datetime' as const }, + due_on: { name: 'due_on', type: 'date' as const }, + opens_at: { name: 'opens_at', type: 'time' as const }, + }, +}; + +interface SeenRead { ast: any } + +/** + * A recording driver whose comparison is the LEXICOGRAPHIC one every temporal + * backend performs on canonical UTC text — which is exactly why an + * uninterpretable comparand answers zero rows instead of erroring: `'…Z' >= + * 'last_30_days'` is simply false for every row. + */ +function makeRecordingDriver() { + const rows = new Map>(); + const reads: SeenRead[] = []; + const matches = (row: any, where: any): boolean => { + if (where == null) return true; + for (const [k, v] of Object.entries(where)) { + if (k === '$and') { if (!(v as any[]).every((w) => matches(row, w))) return false; continue; } + if (k === '$or') { if (!(v as any[]).some((w) => matches(row, w))) return false; continue; } + if (v && typeof v === 'object' && !Array.isArray(v)) { + const ops = v as Record; + if ('$eq' in ops && row[k] !== ops.$eq) return false; + if ('$gte' in ops && !(String(row[k]) >= String(ops.$gte))) return false; + if ('$lte' in ops && !(String(row[k]) <= String(ops.$lte))) return false; + if ('$in' in ops && !(ops.$in as unknown[]).includes(row[k])) return false; + continue; + } + if (row[k] !== v) return false; + } + return true; + }; + const run = (ast: any) => [...rows.values()].filter((r) => matches(r, ast?.where)); + const driver: any = { + name: 'recording', version: '0.0.0', supports: {}, + async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; }, + async find(_o: string, ast: any) { reads.push({ ast }); return run(ast); }, + async findOne(_o: string, ast: any) { reads.push({ ast }); return run(ast)[0] ?? null; }, + async count(_o: string, ast: any) { reads.push({ ast }); return run(ast).length; }, + async aggregate(_o: string, ast: any) { reads.push({ ast }); return run(ast); }, + async create(_o: string, data: Record) { + const id = (data.id as string) ?? `r_${rows.size + 1}`; + const row = { ...data, id }; rows.set(id, row); return row; + }, + async update(_o: string, id: string, data: Record) { + const cur = rows.get(id) ?? {}; + const up = { ...cur, ...data, id }; rows.set(id, up); return up; + }, + async updateMany(_o: string, ast: any, data: Record) { + const hit = run(ast); + for (const r of hit) rows.set(r.id as string, { ...r, ...data }); + return hit.length; + }, + async delete(_o: string, id: string) { return rows.delete(id); }, + async deleteMany(_o: string, ast: any) { + const hit = run(ast); + for (const r of hit) rows.delete(r.id as string); + return hit.length; + }, + async bulkCreate(o: string, batch: Record[]) { + return Promise.all(batch.map((r) => this.create(o, r))); + }, + async beginTransaction() { return { commit: async () => {}, rollback: async () => {} }; }, + async commit() {}, async rollback() {}, + }; + return { driver, reads }; +} + +describe('[#8690] the temporal-comparand door at the engine collection point', () => { + let engine: ObjectQL; + let reads: SeenRead[]; + const now = new Date('2026-08-15T09:00:00.000Z'); + + beforeEach(async () => { + const rec = makeRecordingDriver(); + reads = rec.reads; + engine = new ObjectQL(); + engine.registerDriver(rec.driver, true); + await engine.init(); + engine.registry.registerObject(support_case, 'test'); + // The card's dataset shape: 51 rows, 38 of them inside a start-of-day + // 30-days-ago floor. `{30_days_ago}` resolves to that floor's `YYYY-MM-DD`, + // and canonical UTC text sorts chronologically against it. + for (let i = 0; i < 38; i++) { + await engine.insert('support_case', { + id: `in_${i}`, subject: `in ${i}`, created_date: daysAgoIso(now, i % 29), + }); + } + for (let i = 0; i < 13; i++) { + await engine.insert('support_case', { + id: `out_${i}`, subject: `out ${i}`, created_date: daysAgoIso(now, 40 + i), + }); + } + reads.length = 0; + }); + + const refusalOf = async (p: Promise) => + p.then(() => null, (e: any) => e as Error & { code?: string; status?: number }); + + it('refuses the card\'s comparands with code AND status, while the positive control still returns 38', async () => { + // ── the defect's own cells ────────────────────────────────────────────── + for (const comparand of ['last_30_days', 'not-a-date-at-all', 'last_7_days', 'last_90_days']) { + const err = await refusalOf( + engine.find('support_case', { where: { created_date: { $gte: comparand } } }, { context: { now } as never }), + ); + expect(err, `${comparand} must be refused, not answered with an empty chart`).not.toBeNull(); + // The reverse-verification requirement: BOTH halves of the envelope. + expect(err!.code).toBe('INVALID_FILTER'); + expect(err!.status).toBe(400); + // The caller learns which field, which value, and that nothing ran. + expect(err!.message).toContain('created_date'); + expect(err!.message).toContain(comparand); + expect(err!.message).toMatch(/NOT applied/); + // And no driver read happened — the refusal precedes the driver. + expect(reads).toHaveLength(0); + } + + // ── the POSITIVE CONTROL, in this same test by ruling ─────────────────── + // Without it the four refusals above are equally consistent with a gate + // that refuses everything. + const floor = new Date(now.getTime() - 30 * 86_400_000).toISOString().slice(0, 10); + const inWindow = await engine.find( + 'support_case', + { where: { created_date: { $gte: '{30_days_ago}' } } }, + { context: { now } as never }, + ); + expect(inWindow).toHaveLength(38); + // The token really resolved — the door let the platform's own spelling + // through untouched rather than judging it as an uninterpretable string. + expect(reads.at(-1)!.ast.where.created_date.$gte).toBe(floor); + }); + + it('refuses on both doors — the lowered object form and the authored array sugar', async () => { + const object = await refusalOf( + engine.find('support_case', { where: { created_date: { $gte: 'last_30_days' } } }), + ); + const array = await refusalOf( + engine.find('support_case', { where: [['created_date', '>=', 'last_30_days']] as never }), + ); + for (const err of [object, array]) { + expect(err).not.toBeNull(); + expect(err).toMatchObject({ code: 'INVALID_FILTER', status: 400 }); + } + expect(reads).toHaveLength(0); + }); + + it('covers every verb that collects a filter, read and write sides', async () => { + const where = { created_date: { $gte: 'last_30_days' } }; + for (const call of [ + () => engine.find('support_case', { where }), + () => engine.findOne('support_case', { where }), + () => engine.count('support_case', { where }), + () => engine.aggregate('support_case', { where, groupBy: ['subject'] } as never), + () => engine.update('support_case', { subject: 'x' }, { where, multi: true }), + () => engine.delete('support_case', { where, multi: true }), + ]) { + const err = await refusalOf(call()); + expect(err).not.toBeNull(); + expect(err).toMatchObject({ code: 'INVALID_FILTER', status: 400 }); + } + expect(reads).toHaveLength(0); + }); + + it('judges the other two temporal kinds by their own storage rule', async () => { + // `date` reads a leading `YYYY-MM-DD` and nothing else, so a slash-spelled + // day is uninterpretable there even though `Date.parse` reads it — today it + // survives to the driver and compares as text against `YYYY-MM-DD` values. + const badDate = await refusalOf(engine.find('support_case', { where: { due_on: { $gte: '2026/07/15' } } })); + expect(badDate).toMatchObject({ code: 'INVALID_FILTER', status: 400 }); + // …and the spelling that rule DOES read is not refused. A VERDICT, not a + // row count: these two columns are unset on the fixture, so which rows come + // back is the recording driver's business and not this door's. + await expect(engine.find('support_case', { where: { due_on: { $gte: '2026-07-15' } } })).resolves.toBeDefined(); + + // `time` reads a wall clock whose components are in range. + const badTime = await refusalOf(engine.find('support_case', { where: { opens_at: { $gte: '25:00' } } })); + expect(badTime).toMatchObject({ code: 'INVALID_FILTER', status: 400 }); + await expect(engine.find('support_case', { where: { opens_at: { $gte: '09:30' } } })).resolves.toBeDefined(); + }); + + it('judges every comparand position a temporal field can carry', async () => { + for (const where of [ + { created_date: 'last_30_days' }, // implicit equality + { created_date: { $in: ['2026-07-15', 'last_30_days'] } }, // a list MEMBER + { created_date: { $between: ['2026-07-15', 'last_30_days'] } }, // a range bound + { $and: [{ subject: 'x' }, { created_date: { $lte: 'last_30_days' } }] }, + { $or: [{ subject: 'x' }, { $not: { created_date: { $gte: 'last_30_days' } } }] }, + ]) { + const err = await refusalOf(engine.find('support_case', { where: where as never })); + expect(err, JSON.stringify(where)).not.toBeNull(); + expect(err).toMatchObject({ code: 'INVALID_FILTER', status: 400 }); + } + expect(reads).toHaveLength(0); + }); + + it('leaves alone everything the ruling scoped out', async () => { + // A NON-temporal field: `$gte 'last_30_days'` on text is a legitimate + // lexicographic bound and the door has no opinion about it. + await expect(engine.find('support_case', { where: { subject: { $gte: 'last_30_days' } } })).resolves.toBeDefined(); + // The EMPTY-string cell stays its own card — measured, it binds as `''` and + // returns every non-null row (51 of 51). Unchanged here, deliberately. + await expect(engine.find('support_case', { where: { created_date: { $gte: '' } } })).resolves.toHaveLength(51); + // Non-strings: epoch milliseconds and a real `Date` are read correctly today. + await expect(engine.find('support_case', { where: { created_date: { $gte: now.getTime() } } })).resolves.toBeDefined(); + await expect(engine.find('support_case', { where: { created_date: { $gte: now } } })).resolves.toBeDefined(); + // A `null` comparand is a null test, not a temporal value. + await expect(engine.find('support_case', { where: { created_date: null } })).resolves.toBeDefined(); + // A `{ $field }` reference is not a literal. + await expect( + engine.find('support_case', { where: { created_date: { $gte: { $field: 'due_on' } } } as never }), + ).resolves.toBeDefined(); + }); + + it('leaves an UNKNOWN placeholder to the token resolver, which still refuses it loudly', async () => { + // The door runs BEFORE token resolution, so stepping around placeholders is + // what keeps `{30_days_ago}` working. The unknown ones keep their own + // refusal — a different code on purpose, because nothing here is a token. + for (const token of ['{TODAY}', '{not_a_token}']) { + const err = await refusalOf(engine.find('support_case', { where: { created_date: { $gte: token } } })); + expect(err).not.toBeNull(); + expect(err!.code).toBe('FILTER_TOKEN_UNKNOWN'); + expect(err!.status).toBe(400); + } + }); + + it('invents no verdict on an object whose field map it cannot see', async () => { + // A registry-less host must not refuse a filter on a field it cannot type — + // the same early return the neighbouring gates make. + await expect( + engine.find('unregistered_object', { where: { created_date: { $gte: 'last_30_days' } } }), + ).resolves.toBeDefined(); + }); +}); diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index eac1ff8e33..191e787edf 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -40,6 +40,7 @@ import { // both verbs enforce one definition; the engine raises, the contract decides. import { MAX_BULK_PER_ROW_HOOK_ROWS, resolveBulkPerRowHookBudget } from '@objectstack/spec/data'; import { assertListComparandShapes, assertFilterIsMaterializable } from './filter-comparand-shape.js'; +import { assertTemporalComparandsInterpretable } from './temporal-comparand-door.js'; // Seek pagination for the walks that must read EVERY row — the autonumber seed // scan is one (#6249). Shared with `summary-backfill` rather than re-rolled: // the cursor merge is the part that is easy to get subtly wrong. @@ -658,6 +659,15 @@ function lowerWhereFilterArray( // run it against" — and because this seam is the one place EVERY // caller-supplied `where` passes through, whichever verb it arrived by. assertFilterIsMaterializable(object, operation, schema, where); + // [#8690] The TEMPORAL-comparand door, third on the same seam and third + // question about the same predicate: the shape gate asks "can this + // comparand run", the materializable gate asks "is there a column to run it + // against", and this asks "can that column's storage rule READ this value". + // It must run BEFORE `resolveWhereTokens` (which is downstream of every + // caller of this function) because the refusal has to precede the driver — + // hence the door steps around `{placeholder}` strings rather than judging + // them; the token resolver refuses the unknown ones a moment later, loudly. + assertTemporalComparandsInterpretable(object, operation, schema, where); // [#7872] The comparand-type door, on the OBJECT form. `parseFilterAST` // runs the same walk on everything it lowers or passes through, but // NEITHER door routes an object-form filter through it — Door 1 gates on @@ -718,6 +728,11 @@ function lowerWhereFilterArray( // the array sugar (`[['is_open','=',true]]`) names fields too, and a gate on // one branch would answer one mistake two ways depending on the spelling. assertFilterIsMaterializable(object, operation, schema, condition); + // [#8690] Same door as the object branch, on the LOWERED condition — the + // array sugar (`[['at','>=','last_30_days']]`) names temporal fields too, and + // a gate on one branch would answer one mistake two ways depending on the + // spelling. + assertTemporalComparandsInterpretable(object, operation, schema, condition); lowered.where = condition; return lowered as T; } diff --git a/packages/objectql/src/temporal-comparand-door.ts b/packages/objectql/src/temporal-comparand-door.ts new file mode 100644 index 0000000000..aae29a6a75 --- /dev/null +++ b/packages/objectql/src/temporal-comparand-door.ts @@ -0,0 +1,287 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#8690] The TEMPORAL-comparand door, at the engine's single filter collection + * point — the third gate on the seam that already carries the #5869 shape gate + * and the #8296 unmaterializable-field gate, answering a third question about + * the same predicate: *can the column's own storage rule read this value at + * all.* + * + * ## The defect + * + * A `datetime` field filtered with a bare string the API cannot take literally + * was bound as-is, compared false for every row, and answered `HTTP 200` with + * an empty result set and no diagnostic. Measured end to end on a real driver + * with a declared `datetime` field, 51 rows seeded / 38 in-window: + * + * ``` + * $gte "last_30_days" HTTP 200 count=0 <- silent zero (the defect) + * $gte "not-a-date-at-all" HTTP 200 count=0 <- silent zero + * $gte "{30_days_ago}" HTTP 200 count=38 <- positive control + * $gte "{TODAY}" REFUSED FILTER_TOKEN_UNKNOWN / 400 + * $gte "{not_a_token}" REFUSED FILTER_TOKEN_UNKNOWN / 400 + * ``` + * + * The asymmetry is the whole card: an unknown `{placeholder}` is refused + * loudly, with the resolvable tokens listed — while a bare string that is not a + * date is not validated anywhere. And `last_7_days` / `last_30_days` / + * `last_90_days` are REAL declared preset names in the dashboard schema. The + * shipped console lowers them to `{N_days_ago}` macros before they reach the + * API, so the console path is safe; a saved report, an integration, an MCP + * client or an AI-authored query sends the preset name itself and gets a silent + * zero. An empty chart is the hardest failure to debug — indistinguishable from + * "there is genuinely no data", and it cost one downstream project a + * workaround, a CI guard and three re-measurements over three weeks. + * + * ## Why HERE — the ruling, and the two seams that measurement ruled out + * + * Maintainer ruling, 2026-08-15 (delegated adjudication) — option B, with C + * shipped alongside, explicitly not A: + * + * > refuse the uninterpretable temporal comparand at the ObjectQL engine's + * > single filter collection point, per the #7872 precedent and the 2026-08-12 + * > Q1=B ruling ("the door refuses or narrows every comparand BEFORE the driver + * > runs"). + * + * Refusing "a comparand a temporal field cannot interpret" requires holding the + * comparand and the field's declared TYPE at the same moment, and two earlier + * seams were measured and cannot: + * + * - `packages/core`'s `resolveFilterTokens` is field-AGNOSTIC by construction — + * its context is `now` / `timezone` / `userId` / `orgId`, it never sees an + * object or a field, and it returns the tree by reference for every + * non-placeholder string. + * - `packages/rest` binds no comparands at all (zero hits for + * `temporalFilterValue` / `coerceFilterValue` / `storageDatetimeValue` under + * its `src`, against 8 files under `packages/drivers` — the reverse-check that + * makes the zero non-vacuous). + * + * The only other seam holding both facts is the driver layer — four packages + * each mirroring one function, under the #5499 investment freeze, where the + * pass-through is a DELIBERATE contract with counter-pins asserting it + * (`sql-driver-temporal-dialect.test.ts` asserts + * `temporalFilterValue('t','at','not-a-date') === 'not-a-date'` on purpose) and + * where `storageDatetimeValue` is shared with the WRITE path and the legacy + * read-repair, so refusing there would also reject ingest of pre-convention + * data. That option was rejected by name. + * + * This seam has what neither of the others has: `lowerWhereFilterArray` is + * handed `this._registry.getObject(object)` — the declared field map — at the + * moment it sees the caller's `where`, on every verb (`find` / `findOne` / + * `count` / `aggregate` / `update` / `delete`), through both doors (the array + * sugar and the already-lowered `FilterCondition` object the protocol face + * hands over). One gate, four backends inherit one answer. + * + * ## Envelope + * + * `INVALID_FILTER` / 400 — this package's VALUE-shape envelope (#5869 / #7047), + * reused rather than minted, because the verdict is about the comparand's + * value. Its neighbour {@link assertFilterIsMaterializable} answers + * `INVALID_FIELD` for the deliberately different fact that the NAME has no + * column. Not `FILTER_TOKEN_UNKNOWN` either: nothing here is a token, and + * borrowing that code would send a caller looking for a placeholder they never + * wrote. + * + * ## Scope — three boundaries, each ruled rather than chosen here + * + * - **Non-empty strings only.** The empty-string cell stays its own card by + * ruling ("B and C scope to non-empty strings and must not decide it in + * passing"); measured, `$gte ""` binds as `''` and returns every non-null row + * — 51 of 51, not the 38 the card's table records, which is a transcription + * error its own prose corrects. + * - **`{placeholder}` strings are stepped around**, not judged. This gate runs + * BEFORE `resolveWhereTokens` (which is where it must run — the refusal has + * to precede the driver), so judging one would refuse `{30_days_ago}`, the + * platform's own correct spelling. Unknown tokens keep their existing loud + * refusal one layer down. + * - **Non-string comparands are not judged.** A number is epoch milliseconds + * and a `Date` is an instant; both are read correctly today. + * + * @see `@objectstack/core`'s `temporal-comparand.ts` — the value-half predicate, + * shared with the analytics raw-SQL decline so one rule cannot exist twice. + * @see https://github.com/objectstack-ai/objectstack/issues/8690 + */ + +import { + isUninterpretableTemporalComparand, + temporalComparandKind, + type TemporalComparandKind, +} from '@objectstack/core'; +import { invalidFilterError } from './filter-comparand-shape.js'; + +/** What the door found, for the message and for the analytics-side decline. */ +export interface UninterpretableTemporalComparand { + field: string; + kind: TemporalComparandKind; + value: string; + /** The `where.…` key path the offending comparand sits at. */ + path: string; +} + +/** + * A plain object — filter STRUCTURE rather than a comparand. Same + * classification the #5869 gate and `driver-memory`'s own gate make: a `Date` + * is a comparand even though `typeof` calls it an object. + */ +function isFilterNode(value: unknown): value is Record { + return ( + typeof value === 'object' + && value !== null + && !Array.isArray(value) + && !(value instanceof Date) + ); +} + +/** A `{ $field: 'other_column' }` reference is not a literal — never judged. */ +function isFieldReference(value: unknown): boolean { + return isFilterNode(value) && typeof (value as { $field?: unknown }).$field === 'string'; +} + +/** + * Walk one `FilterCondition` and return the FIRST comparand a declared temporal + * field's storage rule cannot read, or `null`. + * + * Exported because the same walk answers the analytics strategy's routing + * question ("would the engine door refuse this?") — though that consumer reaches + * the value-half predicate directly, having no field map of its own. + * + * Structure discarded the same three conservative ways the sibling gates + * discard it: `$and` / `$or` / `$not` are descended, any OTHER `$` key at node + * level is skipped WITHOUT descending (an unrecognised combinator leaves the + * fields beneath it ungated — a hole, not a false 400, which is the right + * failure direction for a gate that exists to stop wrong answers), and a dotted + * key names a field of a DIFFERENT object whose map this door has not resolved. + */ +export function findUninterpretableTemporalComparand( + schema: unknown, + where: unknown, + path = 'where', + depth = 0, +): UninterpretableTemporalComparand | null { + // A registry-less host must not invent a verdict about a field map it cannot + // see — the same early return `assertFilterIsMaterializable` makes. + const fields = (schema as { fields?: Record } | undefined)?.fields; + if (!fields || typeof fields !== 'object') return null; + if (depth > 32) return null; + if (!isFilterNode(where)) return null; + + for (const [key, value] of Object.entries(where)) { + const here = `${path}.${key}`; + if (key === '$and' || key === '$or') { + if (Array.isArray(value)) { + for (const [index, arm] of value.entries()) { + const hit = findUninterpretableTemporalComparand(schema, arm, `${here}[${index}]`, depth + 1); + if (hit) return hit; + } + } + continue; + } + if (key === '$not') { + const hit = findUninterpretableTemporalComparand(schema, value, here, depth + 1); + if (hit) return hit; + continue; + } + if (key.startsWith('$')) continue; + if (key.includes('.')) continue; + const kind = temporalComparandKind((fields[key] as { type?: unknown } | undefined)?.type); + if (!kind) continue; + const hit = judgeFieldComparands(kind, key, value, here); + if (hit) return hit; + } + return null; +} + +/** One temporal field's constraint: `{ at: }`. */ +function judgeFieldComparands( + kind: TemporalComparandKind, + field: string, + spec: unknown, + path: string, +): UninterpretableTemporalComparand | null { + // Not filter structure → an implicit-equality comparand, judged at this path. + if (!isFilterNode(spec)) return judgeComparand(kind, field, spec, path); + // A field spec with no `$` key is a deep-equality / nested-relation condition; + // the #5869 gate records why descending into one would invent a contract no + // backend agrees with. + const keys = Object.keys(spec); + if (!keys.some((k) => k.startsWith('$'))) return null; + if (isFieldReference(spec)) return null; + for (const op of keys) { + if (!op.startsWith('$')) continue; + const comparand = spec[op]; + // Every MEMBER of a list operator is a comparand in its own right — the + // same split the #7872 type door makes at the shared compile face. + if (Array.isArray(comparand)) { + for (const [index, member] of comparand.entries()) { + const hit = judgeComparand(kind, field, member, `${path}.${op}[${index}]`); + if (hit) return hit; + } + continue; + } + const hit = judgeComparand(kind, field, comparand, `${path}.${op}`); + if (hit) return hit; + } + return null; +} + +function judgeComparand( + kind: TemporalComparandKind, + field: string, + value: unknown, + path: string, +): UninterpretableTemporalComparand | null { + if (isFieldReference(value)) return null; + if (!isUninterpretableTemporalComparand(kind, value)) return null; + return { field, kind, value: value as string, path }; +} + +/** A short, bounded rendering — the comparand came off the wire (#5869's bound). */ +function preview(value: string): string { + const text = JSON.stringify(value); + return text.length > 60 ? `${text.slice(0, 59)}…` : text; +} + +/** + * The remedy sentence, per kind. Deliberately names the platform's OWN + * relative-date spelling first: the measured caller is one holding a declared + * preset name (`last_30_days`) that the console would have lowered to + * `{30_days_ago}`, so the fix a caller needs is almost always "wrap it as the + * token the resolver knows", not "compute an instant yourself". + */ +const REMEDY: Record = { + datetime: + 'Write an ISO-8601 instant ("2026-07-15T00:00:00.000Z"), a bare "YYYY-MM-DD" ' + + '(read as midnight UTC), epoch milliseconds, or a relative-date placeholder the ' + + 'resolver knows, e.g. "{30_days_ago}" / "{current_month_start}".', + date: + 'Write a "YYYY-MM-DD" calendar day, or a relative-date placeholder the resolver ' + + 'knows, e.g. "{30_days_ago}" / "{current_month_start}".', + time: + 'Write an "HH:MM" / "HH:MM:SS" wall clock (timezone-naive, ADR-0053 D-C1).', +}; + +/** + * Refuse every comparand a declared temporal field's storage rule cannot read. + * + * Runs on the CALLER's own `where`, before the middleware chain composes + * RLS / sharing / tenant predicates onto the AST — deliberately, and for the + * reason its neighbour records: an injected read filter is the platform's own, + * not a declaration the caller can fix, and refusing one would turn a policy + * into a 400 nobody can act on. + */ +export function assertTemporalComparandsInterpretable( + object: string, + operation: string, + schema: unknown, + where: unknown, +): void { + const hit = findUninterpretableTemporalComparand(schema, where); + if (!hit) return; + throw invalidFilterError( + `${operation}('${object}'): filter on '${hit.field}' compares a declared ${hit.kind} ` + + `field against ${preview(hit.value)} at ${hit.path}, which is not a ${hit.kind} value ` + + 'this platform can interpret. It would reach the driver as written, compare false for ' + + 'EVERY row, and return 200 with an empty result — indistinguishable from "there is no ' + + `data". The filter was NOT applied. ${REMEDY[hit.kind]}`, + ); +} diff --git a/packages/services/service-analytics/src/comparand-shape.ts b/packages/services/service-analytics/src/comparand-shape.ts index db18f5147d..218f87c68a 100644 --- a/packages/services/service-analytics/src/comparand-shape.ts +++ b/packages/services/service-analytics/src/comparand-shape.ts @@ -1,5 +1,10 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +import { + isUninterpretableTemporalComparand, + type TemporalComparandKind, +} from '@objectstack/core'; + /** * Which comparand SHAPES this package's filter compilers can express (#5234). * @@ -268,6 +273,119 @@ function findIn( return null; } +/** + * [#8690, maintainer ruling 2026-08-15] The first comparand a declared TEMPORAL + * member's storage rule cannot read — or `null`. + * + * ## What it is FOR: the raw-SQL bypass named in the ruling + * + * The refusal itself lives at the ObjectQL engine's filter collection point + * (`@objectstack/objectql`, `temporal-comparand-door.ts`), which is the one + * seam that holds a comparand and the field's declared type at the same moment. + * `NativeSQLStrategy` never reaches it: it compiles its own + * `SELECT … WHERE col >= $N` and binds the comparand directly, so a raw-SQL + * deployment would keep answering the silent zero the engine door now refuses. + * The ruling closes that by name: + * + * > `NativeSQLStrategy.canHandle` must **decline** an uninterpretable temporal + * > comparand so raw-SQL paths fall through to the engine door. + * + * So this answers a ROUTING question, exactly as {@link findCrossFieldComparand} + * does one seam over: not "is this filter legal" but "does serving it correctly + * need the path that judges it". Declining sends the query to the ObjectQL + * strategy, whose `engine.aggregate` passes through that door — one refusal, + * one wording, one place, whichever strategy the deployment's driver selects. + * + * ## Why the KIND is supplied by the caller + * + * This package holds no field map — it depends on `core`, `spec` and `types`, + * and on no driver. The temporal fact therefore has to arrive with the query, + * and it already does: a cube DIMENSION declares `type: 'time'` (compiled from + * the dataset's `type: 'date'`), and `resolveStorageTarget`/`lookupMember` + * already map a filter member to it. `kindOf` is that lookup, passed in, so + * this walk stays a pure function of the filter and the caller's classification. + * + * ⚠️ Consequence, recorded rather than hidden: a temporal column filtered + * WITHOUT being declared as a time dimension on the cube is not classified + * here, so it is not declined and keeps today's behaviour on the raw-SQL path. + * That is a strictly smaller hole than "every raw-SQL query bypasses the door", + * it fails in the safe direction (a missed decline degrades to today's + * behaviour, never to a NEW wrong answer), and closing it fully would take a + * field map this package deliberately does not have. + * + * The walk is structural and total for the same reason its sibling's is: a + * comparand three combinators deep still needs the engine path. + */ +export function findUninterpretableTemporalMember( + filter: unknown, + kindOf: (member: string) => TemporalComparandKind | null, +): { field: string; kind: TemporalComparandKind; value: string } | null { + return findUninterpretableIn(filter, '', kindOf); +} + +function findUninterpretableIn( + node: unknown, + field: string, + kindOf: (member: string) => TemporalComparandKind | null, +): { field: string; kind: TemporalComparandKind; value: string } | null { + if (!node || typeof node !== 'object') return null; + if (Array.isArray(node)) { + for (const child of node) { + const hit = findUninterpretableIn(child, field, kindOf); + if (hit) return hit; + } + return null; + } + if (node instanceof Date || ArrayBuffer.isView(node)) return null; + // A reference is not a literal — the same position this file's sibling walk + // routes on, and never a value any storage rule reads. + if (isFieldReference(node)) return null; + for (const [key, value] of Object.entries(node as Record)) { + // `$`-prefixed keys are operators and combinators: the field in scope does + // not change. Anything else names a member and becomes the new scope. + const scope = key.startsWith('$') ? field : key; + const kind = scope ? kindOf(scope) : null; + if (kind) { + const hit = judgeTemporalLiterals(value, scope, kind); + if (hit) return hit; + continue; + } + const hit = findUninterpretableIn(value, scope, kindOf); + if (hit) return hit; + } + return null; +} + +/** + * Every literal reachable in one classified member's value position — the + * comparand itself, an operator bag's comparands, and each MEMBER of a list + * operator's array, which is a comparand in its own right. + */ +function judgeTemporalLiterals( + value: unknown, + field: string, + kind: TemporalComparandKind, +): { field: string; kind: TemporalComparandKind; value: string } | null { + if (Array.isArray(value)) { + for (const member of value) { + const hit = judgeTemporalLiterals(member, field, kind); + if (hit) return hit; + } + return null; + } + if (value && typeof value === 'object') { + if (value instanceof Date || ArrayBuffer.isView(value) || isFieldReference(value)) return null; + for (const nested of Object.values(value as Record)) { + const hit = judgeTemporalLiterals(nested, field, kind); + if (hit) return hit; + } + return null; + } + return isUninterpretableTemporalComparand(kind, value) + ? { field, kind, value: value as string } + : null; +} + /** * The Filter Protocol operators whose comparand becomes the text of a `LIKE` * pattern — the ones every compiler in this package routes through 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 0ece11e08c..dd98857dc0 100644 --- a/packages/services/service-analytics/src/strategies/native-sql-strategy.ts +++ b/packages/services/service-analytics/src/strategies/native-sql-strategy.ts @@ -11,7 +11,7 @@ import { SQL_CONST_TRUE, type NormalizedFilterNode, } from './filter-normalizer.js'; -import { findCrossFieldComparand } from '../comparand-shape.js'; +import { findCrossFieldComparand, findUninterpretableTemporalMember } from '../comparand-shape.js'; import { compileScopedFilterToSql } from '../read-scope-sql.js'; import { datasetInvalidError, invalidMemberError } from '../dataset-refusal.js'; import { likePattern, LIKE_ESCAPE_CHAR, asciiLowerSqlExpr, type LikeShape } from '../like-pattern.js'; @@ -170,10 +170,84 @@ export class NativeSQLStrategy implements AnalyticsStrategy { // and envelope it has always had — so it is caught and read as "no // reference found". if (this.carriesCrossFieldComparison(query, ctx)) return false; + // ── [#8690] DECLINE an uninterpretable TEMPORAL comparand ─────────────── + // + // ## The maintainer ruling this implements (2026-08-15, option B) + // + // > refuse the uninterpretable temporal comparand at the ObjectQL engine's + // > single filter collection point … Includes the measured gap: + // > `NativeSQLStrategy.canHandle` must **decline** an uninterpretable + // > temporal comparand so raw-SQL paths fall through to the engine door. + // + // The refusal itself is NOT here and must not be: judging "can this column + // read this comparand" needs the field's declared TYPE, which only the + // engine's filter collection point holds (this package depends on no + // driver and carries no field map). What is here is the ROUTING half — + // without it a raw-SQL deployment binds `WHERE col >= 'last_30_days'` + // directly, never reaches the door, and keeps answering 200 with zero rows. + // + // Same mechanism, same direction, as the two declines above and the #7598 + // one below it: when this strategy cannot serve something CORRECTLY, + // routing to the lower-priority ObjectQL path beats compiling it anyway. + // Content-based rather than shape-based, which #7598's ruling already + // named as new-but-accepted behaviour for `canHandle`. + // + // ⚠️ Deliberately NO fail-closed backstop at the emitter, unlike #7598's. + // There the routing gate's failure mode was a NEW wrong answer (a bound + // `{"$field":…}` object); here a missed decline degrades to exactly + // today's behaviour, and a throw at the emitter would answer 500 for a + // filter the engine door answers 400 for — two envelopes for one mistake, + // which is the drift this card exists to remove. + if (this.carriesUninterpretableTemporalComparand(query, ctx)) return false; const caps = ctx.queryCapabilities(query.cube); return caps.nativeSql && typeof ctx.executeRawSql === 'function'; } + /** + * [#8690] Does the query's `where` compare a declared TIME dimension against + * a value no temporal storage rule can read? See the ruling at + * {@link canHandle}. + * + * The classification comes from the CUBE, the only metadata this package has: + * a dimension declares `type: 'time'` (compiled from the dataset dimension's + * `type: 'date'`), and {@link lookupMember} is the same resolution every other + * member lookup in this strategy uses, so "the member the gate classified" + * and "the member the compiler emits" cannot drift apart. + * + * A `time` dimension is read with the DATETIME rule — the permissive one of + * the three. That is the right direction because this is a routing decision, + * not a verdict: the engine door re-judges with the field's real declared + * type and has the final say, so under-classifying an exotic spelling merely + * leaves today's behaviour, while over-classifying would silently move a + * working dashboard off the fast path. The comparands this card measured + * (`last_30_days`, `not-a-date-at-all`) are unreadable under all three rules, + * so the decline fires for them whichever backing type the dimension has. + * + * `lowerAnalyticsWhere` rather than `query.where` raw, so the authored ARRAY + * sugar is seen after `parseFilterAST` has lowered it; a THROW from that + * lowering is not this gate's to answer — the filter is malformed either way + * and `normalizeAnalyticsFilterTree` refuses it a moment later with the + * message and envelope it has always had. + */ + private carriesUninterpretableTemporalComparand( + query: AnalyticsQuery, + ctx: StrategyContext, + ): boolean { + const cube = query.cube ? ctx.getCube(query.cube) : undefined; + if (!cube) return false; + let where: unknown = null; + try { + where = lowerAnalyticsWhere(query); + } catch { + return false; + } + if (!where) return false; + return findUninterpretableTemporalMember( + where, + (member) => (this.lookupMember(cube, member, 'dimension')?.type === 'time' ? 'datetime' : null), + ) !== null; + } + /** * [#7598] Does serving this query require the cross-field capability this * strategy declines? See the ruling recorded at {@link canHandle}. From cff9972e6b80c30127afd04a47f3d7e92df25607 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 15 Aug 2026 02:55:32 +0000 Subject: [PATCH 2/3] test(analytics): pin the raw-SQL temporal decline; changeset (#8690) Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XeQRiAa7vYRVX5Fog7Zby8 --- ...filter-comparand-refused-at-engine-door.md | 47 ++++++ ...ive-sql-temporal-comparand-decline.test.ts | 135 ++++++++++++++++++ 2 files changed, 182 insertions(+) create mode 100644 .changeset/temporal-filter-comparand-refused-at-engine-door.md create mode 100644 packages/services/service-analytics/src/__tests__/native-sql-temporal-comparand-decline.test.ts diff --git a/.changeset/temporal-filter-comparand-refused-at-engine-door.md b/.changeset/temporal-filter-comparand-refused-at-engine-door.md new file mode 100644 index 0000000000..9345ea652a --- /dev/null +++ b/.changeset/temporal-filter-comparand-refused-at-engine-door.md @@ -0,0 +1,47 @@ +--- +"@objectstack/core": patch +"@objectstack/objectql": patch +"@objectstack/service-analytics": patch +--- + +fix(objectql): a temporal filter comparand the platform cannot interpret is refused at the engine door instead of answering 200 with zero rows (#8690) + + + +A `datetime` / `date` / `time` field filtered with a bare string the platform +cannot read — `last_30_days`, `not-a-date-at-all` — was bound **as written** +all the way to the driver, where the comparison is false for every row. The +caller received `HTTP 200`, an empty result set, and nothing to indicate the +filter was meaningless. An unknown `{placeholder}` in the same position was +already refused loudly (`FILTER_TOKEN_UNKNOWN` / 400, listing the resolvable +tokens), so one API answered two shapes of unusable comparand two different +ways. + +It is concretely reachable rather than theoretical: `last_7_days` / +`last_30_days` / `last_90_days` are **declared preset names** in the dashboard +schema. The shipped console lowers them to `{N_days_ago}` macros before they +reach the API, so the console path was always safe — but a saved report, an +integration, an MCP client or an AI-authored query sends the preset name itself +and got a silent zero. An empty chart is the hardest failure to debug: it is +indistinguishable from "there is genuinely no data". + +Such a comparand is now refused at the ObjectQL engine's single filter +collection point, with `code: 'INVALID_FILTER'` and `status: 400`, naming the +field, the value, the key path and the spellings that would work. That seam is +the one place holding the caller's comparand and the field's **declared type** +at the same moment, and every verb (`find` / `findOne` / `count` / `aggregate` +/ `update` / `delete`) and both filter spellings (the array sugar and the +lowered condition) pass through it, so all four backends inherit one answer +rather than four. `NativeSQLStrategy` additionally **declines** such a query so +the raw-SQL analytics path falls through to that door instead of binding the +value into its own statement. + +Deliberately unchanged, each by ruling: a `{placeholder}` keeps its existing +refusal one layer down (the door runs before token resolution and steps around +them, so `{30_days_ago}` still resolves normally); non-string comparands are +untouched (a number is epoch milliseconds, a `Date` is an instant); and the +**empty string** keeps today's behaviour exactly — it binds as `''` and matches +every non-null row, which is a separate question that remains its own card. diff --git a/packages/services/service-analytics/src/__tests__/native-sql-temporal-comparand-decline.test.ts b/packages/services/service-analytics/src/__tests__/native-sql-temporal-comparand-decline.test.ts new file mode 100644 index 0000000000..758ebfd706 --- /dev/null +++ b/packages/services/service-analytics/src/__tests__/native-sql-temporal-comparand-decline.test.ts @@ -0,0 +1,135 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// [#8690, maintainer ruling 2026-08-15 option B] The raw-SQL bypass the ruling +// names by name: +// +// > `NativeSQLStrategy.canHandle` must **decline** an uninterpretable +// > temporal comparand so raw-SQL paths fall through to the engine door. +// +// The refusal itself lives at the ObjectQL engine's filter collection point — +// the only seam holding a comparand and the field's declared type at once. +// `NativeSQLStrategy` never reaches it: it compiles its own `WHERE col >= $N` +// and binds the comparand directly, so without this decline a raw-SQL +// deployment keeps answering the card's silent zero while the same query on an +// engine-path deployment is refused 400. One filter, two answers, decided by +// which driver the deployment happens to run — which is the whole shape this +// card exists to remove. +// +// This suite pins the ROUTING, which is all this package decides. That the +// engine then refuses (`INVALID_FILTER` / 400, with the `{30_days_ago}` +// positive control in the same test) is pinned in `@objectstack/objectql`'s +// `engine-temporal-comparand-door.test.ts`. + +import { describe, it, expect } from 'vitest'; +import { DatasetSchema } from '@objectstack/spec/ui'; +import { AnalyticsService } from '../analytics-service.js'; + +const dataset = DatasetSchema.parse({ + name: 'support_cases', + label: 'Support Cases', + object: 'support_case', + dimensions: [ + { name: 'status', field: 'status', type: 'string' }, + // `type: 'date'` compiles to a cube dimension of `type: 'time'` — the + // declaration this decline classifies on. + { name: 'created_date', field: 'created_date', type: 'date' }, + ], + measures: [{ name: 'case_count', aggregate: 'count' }], +}); + +/** A real SQL deployment: native SQL AND the objectql aggregate path both up. */ +const PROD_CAPS = { nativeSql: true, objectqlAggregate: true, inMemory: true }; + +function buildService() { + const rawSqlCalls: Array<{ sql: string; params: unknown[] }> = []; + const aggregateCalls: Array<{ filter?: unknown }> = []; + const svc = new AnalyticsService({ + queryCapabilities: () => PROD_CAPS, + executeRawSql: async (_object, sql, params) => { + rawSqlCalls.push({ sql, params }); + return [{ case_count: 0 }]; + }, + executeAggregate: async (_object, options) => { + aggregateCalls.push({ filter: options.filter }); + return [{ case_count: 38 }]; + }, + }); + return { svc, rawSqlCalls, aggregateCalls }; +} + +describe('[#8690] NativeSQLStrategy declines an uninterpretable temporal comparand', () => { + // The card's own reachable vocabulary: declared preset names in the dashboard + // schema. The console lowers them to `{N_days_ago}` macros, so the console + // path is safe — a saved report, an integration or an AI-authored query sends + // the preset name itself and used to get a silent zero. + it.each(['last_30_days', 'last_7_days', 'last_90_days', 'not-a-date-at-all'])( + 'routes `%s` on a time dimension to the engine path, never to raw SQL', + async (comparand) => { + const { svc, rawSqlCalls, aggregateCalls } = buildService(); + + await svc.queryDataset!(dataset, { + measures: ['case_count'], + runtimeFilter: { created_date: { $gte: comparand } }, + }); + + // The bypass is closed: nothing was bound into a raw statement. + expect(rawSqlCalls).toHaveLength(0); + // …and the query went to the path that passes through the engine door. + expect(aggregateCalls).toHaveLength(1); + }, + ); + + it('does NOT over-decline: an interpretable comparand keeps the raw-SQL fast path', async () => { + // The control this decline is worthless without — a gate that declined + // everything would pass the assertions above while destroying the P1 path + // for every dashboard in the deployment. + for (const comparand of ['2026-07-15', '2026-07-15T00:00:00.000Z', '{30_days_ago}']) { + const { svc, rawSqlCalls, aggregateCalls } = buildService(); + await svc.queryDataset!(dataset, { + measures: ['case_count'], + runtimeFilter: { created_date: { $gte: comparand } }, + }); + expect(rawSqlCalls, comparand).toHaveLength(1); + expect(aggregateCalls, comparand).toHaveLength(0); + } + }); + + it('does NOT decline on a NON-temporal dimension', async () => { + // `$gte 'last_30_days'` on a string dimension is a legitimate + // lexicographic bound. Declining it would be an unforced routing loss. + const { svc, rawSqlCalls, aggregateCalls } = buildService(); + await svc.queryDataset!(dataset, { + measures: ['case_count'], + runtimeFilter: { status: { $gte: 'last_30_days' } }, + }); + expect(rawSqlCalls).toHaveLength(1); + expect(aggregateCalls).toHaveLength(0); + }); + + it('leaves the empty-string cell exactly as it is — that cell is its own card', async () => { + // Measured: `$gte ""` binds as `''` and every canonical UTC text sorts at + // or above it, so it returns every non-null row. The ruling scopes B and C + // to non-empty strings and forbids deciding this one in passing, so it must + // keep its current routing, not acquire a decline. + const { svc, rawSqlCalls } = buildService(); + await svc.queryDataset!(dataset, { + measures: ['case_count'], + runtimeFilter: { created_date: { $gte: '' } }, + }); + expect(rawSqlCalls).toHaveLength(1); + }); + + it('finds the comparand wherever it sits — nested combinators and list members', async () => { + for (const runtimeFilter of [ + { $and: [{ status: 'open' }, { created_date: { $lte: 'last_30_days' } }] }, + { $or: [{ status: 'open' }, { created_date: { $gte: 'last_30_days' } }] }, + { created_date: { $in: ['2026-07-15', 'last_30_days'] } }, + { created_date: 'last_30_days' }, + ]) { + const { svc, rawSqlCalls, aggregateCalls } = buildService(); + await svc.queryDataset!(dataset, { measures: ['case_count'], runtimeFilter }); + expect(rawSqlCalls, JSON.stringify(runtimeFilter)).toHaveLength(0); + expect(aggregateCalls, JSON.stringify(runtimeFilter)).toHaveLength(1); + } + }); +}); From fc1bdb651f44e51fbbee85b58162c9aa028b211e Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 15 Aug 2026 04:03:25 +0000 Subject: [PATCH 3/3] fix(objectql): index the recorded read instead of Array.prototype.at in the temporal-door pin (#8690) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `reads.at(-1)` is TS2550 under this package's lib target, and objectql's tsconfig hides `**/*.test.ts` from its own `typecheck` script — so the error was invisible to `tsc --noEmit` and surfaced only in the TEST_DEBT re-measure, pushing the shrink-only ledger 355 -> 356. Indexed access instead. Same assertion, same read, ledger back at 355. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XeQRiAa7vYRVX5Fog7Zby8 --- packages/objectql/src/engine-temporal-comparand-door.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/objectql/src/engine-temporal-comparand-door.test.ts b/packages/objectql/src/engine-temporal-comparand-door.test.ts index d14bd8fa4a..a6a78687cd 100644 --- a/packages/objectql/src/engine-temporal-comparand-door.test.ts +++ b/packages/objectql/src/engine-temporal-comparand-door.test.ts @@ -159,7 +159,9 @@ describe('[#8690] the temporal-comparand door at the engine collection point', ( expect(inWindow).toHaveLength(38); // The token really resolved — the door let the platform's own spelling // through untouched rather than judging it as an uninterpretable string. - expect(reads.at(-1)!.ast.where.created_date.$gte).toBe(floor); + // Indexed rather than `.at(-1)`: this package's tsconfig targets a lib + // older than ES2022, so `Array.prototype.at` is not declared for it. + expect(reads[reads.length - 1].ast.where.created_date.$gte).toBe(floor); }); it('refuses on both doors — the lowered object form and the authored array sugar', async () => {