From 2fb4ce3645c69a0c79f600b8841c3340496c7a4d Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 23:41:32 +0000 Subject: [PATCH 01/12] refactor(service-analytics): F9 and F10 compile the lowered filter; their whole-day and NULL-polarity copies are deleted #5930 step 4 (domain:services), faces F9 and F10. The shared lowering (lowerFilterCondition) is now the one source of the whole-day bound, the $between split and the NULL-polarity guards on the analytics read scope and the where tree: - native-sql-strategy: buildFilterClause's bare-day lte arm is deleted; the dateRange window is the { $gte, $lte } pair, lowered by the same reader as the where (ADR-0053 D-D1 item 8). The reader reads a column the host cannot name type-blind (item 7). - objectql-strategy: the /analytics/sql echo renders the window through the same lowering; the reader leaves an undeclared column as written, for the engine seam to read. - filter-normalizer: the $not-operand rewrite and the #5298 leaf wrap, with their polarity tables, are deleted. - read-scope-sql: the $not-operand rewrite, its three tables and the IS NULL OR wrap are deleted. Claude-Session: https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ Co-authored-by: Claude --- .../service-analytics/src/read-scope-sql.ts | 334 +++-------- .../src/strategies/filter-normalizer.ts | 547 +++++------------- .../src/strategies/native-sql-strategy.ts | 134 ++--- .../src/strategies/objectql-strategy.ts | 90 +-- 4 files changed, 334 insertions(+), 771 deletions(-) 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 492d5ce73f1..3cd0c0b642d 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, @@ -968,26 +934,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}"`; @@ -1061,16 +1025,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 * @@ -1154,13 +1117,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 @@ -1298,18 +1266,9 @@ function fieldLeaves(key: string, raw: unknown): NormalizedFilterNode[] { // emitters downstream of it. assertCompilableComparand(opKey, key, v); 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; @@ -1430,12 +1389,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)); @@ -1455,269 +1414,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 ───────────────────────────────────────────── /** @@ -2176,10 +1872,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 @@ -2450,8 +2146,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. @@ -2531,27 +2228,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) @@ -2559,23 +2279,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 }, }); From 224209c93bf73b023f533993104525f6431d4489 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 23:54:59 +0000 Subject: [PATCH 02/12] test(service-analytics): the shape pins read one guard from the shared lowering; the echo window renders the declared column's bound #5930 step 4. Every pin that recorded a face's own copy of the NULL guard stacked inside the shared lowering's (the step-3 rows marked "until the copy's deletion card") now reads the single guard. No row answer moved: every id-set assertion in these files is unchanged. The /analytics/sql window pins wire the declared type the plugin relays (sourceFieldMeta, close_date a datetime), and two controls pin the render on a declared date and where the host names no type (the bound execute() hands the engine, as written), and a preset that stops before its end. Claude-Session: https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ Co-authored-by: Claude --- .../cross-field-reference-refusal.test.ts | 15 +--- .../filter-normalizer-mixed-wrapper.test.ts | 25 +++--- .../filter-normalizer-not-null-safe.test.ts | 81 +++++++++---------- ...ter-normalizer-undefined-comparand.test.ts | 11 ++- ...jectql-contains-canonical-operator.test.ts | 8 +- .../src/__tests__/objectql-daterange.test.ts | 73 +++++++++++++---- .../read-scope-boolean-flag-comparand.test.ts | 21 ++--- .../read-scope-not-null-safe.test.ts | 16 ++-- ...read-scope-placeholder-three-faces.test.ts | 10 +-- .../read-scope-undefined-comparand.test.ts | 11 +-- .../__tests__/text-match-sqlite-nul.test.ts | 8 +- .../text-operator-case-exactness.test.ts | 8 +- .../text-operator-non-text-column.test.ts | 22 ++--- .../where-boolean-flag-refusal.test.ts | 26 +++--- .../where-door-shared-lowering-seam.test.ts | 22 +++-- 15 files changed, 201 insertions(+), 156 deletions(-) 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__/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__/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 7bebe8b9927..7a4134d81bc 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 @@ -228,7 +228,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; @@ -246,14 +246,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'], }, }; @@ -266,13 +267,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..2ac14781ed6 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,8 +24,11 @@ * 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 @@ -98,18 +101,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'])], }); }); From df2906102c12be2bf4dbb6b7493f42fae7b84029 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 23:59:26 +0000 Subject: [PATCH 03/12] docs(service-analytics): two notes name the shared lowering as the NULL rule's source #5930 step 4: the $empty and non-text-column notes named the deleted $not rewrite and its operatorIsNullTotal table. Claude-Session: https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ Co-authored-by: Claude --- .../services/service-analytics/src/empty-operator-sql.ts | 5 +++-- packages/services/service-analytics/src/non-text-column.ts | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-) 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`. From a6e3488e0a6cb6937d7ce7edd1f0e165149c5654 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 3 Oct 2026 00:10:26 +0000 Subject: [PATCH 04/12] test(service-analytics): pin the shared lowering as the one source on F9 and F10, and the bare day on every face #5930 step 4. analytics-faces-one-lowering.test.ts holds: - the enumeration: no analytics source file holds a whole-day helper but the draft preview (F11, pending its reader), and none holds a NULL-polarity copy; a positive control proves the scan reads the faces; - one source: the native compiler and the echo emit the bound the lowering hands them (a function of the declared type alone), and every null predicate F9 and F10 emit is one the lowering wrote; - the typed drivers' answer on every face over a real engine (SQLite, and PostgreSQL where OS_TEST_POSTGRES_URL is set): datetime, date and text columns, $lte, $between, $not and dateRange windows, the carrier-note text cell included; - a host with no typed reader: the native face reads type-blind, and the ObjectQL face hands the engine the bound as written; - TEMPORAL_CASES on the native and ObjectQL faces of the plugin's composition and through the read scope. native-sql-temporal-conformance.test.ts runs its matrix with and without the declared-type hook. Claude-Session: https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ Co-authored-by: Claude --- .../analytics-faces-one-lowering.test.ts | 536 ++++++++++++++++++ .../native-sql-temporal-conformance.test.ts | 89 +-- 2 files changed, 593 insertions(+), 32 deletions(-) create mode 100644 packages/services/service-analytics/src/__tests__/analytics-faces-one-lowering.test.ts 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..08de9276403 --- /dev/null +++ b/packages/services/service-analytics/src/__tests__/analytics-faces-one-lowering.test.ts @@ -0,0 +1,536 @@ +// 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**, keeps its own bound copy: see the enumeration + * pin below. + * + * ## 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 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 type { 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(); +} + +/** The file's code with its comments removed: a prose mention of a helper is not a copy of it. */ +function codeOf(file: string): string { + return readFileSync(join(SRC, file), 'utf8') + .replace(/\/\*[\s\S]*?\*\//g, '') + .replace(/(^|[^:'"`])\/\/.*$/gm, '$1'); +} + +/** Which of `names` each file's code holds, keyed by file; files holding none are left out. */ +function holders(names: readonly string[]): Record { + const out: Record = {}; + for (const file of sourceFiles()) { + const code = codeOf(file); + const held = names.filter((name) => new RegExp(`\\b${name}\\b`).test(code)); + 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 but the draft preview (F11) keeps a whole-day helper of its own', () => { + // F11 keeps `lteBound` and its window arm until its typed reader is wired: + // drafted rows reach it with no declared type, and the reader its host + // holds (`sourceFieldMeta`) is passed to it from `analytics-service.ts`. + // This entry only ever shrinks. + expect(holders(WHOLE_DAY_HELPERS)).toEqual({ + 'preview-evaluator.ts': ['nextUtcCalendarDay', 'isUnboundedAbove', 'lteBound'], + }); + }); + + 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') => ({ timeDimensions: [{ dimension, dateRange: ['2026-07-28', end] }] }); + 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 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__/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()); + }); + } + } + }); } }); From 464d6b8acad18210952e3576b9c187a75451d8c2 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 3 Oct 2026 00:15:08 +0000 Subject: [PATCH 05/12] =?UTF-8?q?docs(changeset):=20service-analytics=20mi?= =?UTF-8?q?nor,=20Clause-=E2=91=A1=20no=20(narrowing):=20the=20native=20fa?= =?UTF-8?q?ce's=20bare-day=20bound=20on=20a=20non-temporal=20column?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #5930 step 4. The measured answer move, the reader-less hosts, the /analytics/sql echo changes and the ADR-0087 disposition. Claude-Session: https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ Co-authored-by: Claude --- .../21417-analytics-faces-one-lowering.md | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 .changeset/21417-analytics-faces-one-lowering.md diff --git a/.changeset/21417-analytics-faces-one-lowering.md b/.changeset/21417-analytics-faces-one-lowering.md new file mode 100644 index 00000000000..c2c28eaafe4 --- /dev/null +++ b/.changeset/21417-analytics-faces-one-lowering.md @@ -0,0 +1,27 @@ +--- +"@objectstack/service-analytics": minor +--- + +fix(service-analytics)!: the analytics read scope and the `where` tree compile 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 selects 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. + +**Unchanged.** Every answer on a declared `datetime` or `date` column, on the native and the ObjectQL strategy; every answer of the ObjectQL strategy; every answer of the read scope; and the draft preview, which keeps its own bound until its typed reader is wired. From 1ccd3e07f4bf8e26d3e13af77488e99e46f371c7 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 3 Oct 2026 00:21:26 +0000 Subject: [PATCH 06/12] test(service-analytics): type the window helper of the one-lowering pin as a query The dateRange pair is a tuple in AnalyticsQuery; tsc refused the widened string[]. Claude-Session: https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ Co-authored-by: Claude --- .../src/__tests__/analytics-faces-one-lowering.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 index 08de9276403..5091b50619d 100644 --- 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 @@ -227,7 +227,8 @@ describe('[#5930 step 4] one source: the native compiler and the echo compile th }); it('a dateRange window: the same pair, through the same reader (item 8)', async () => { - const window = (dimension: string, end = '2026-07-28') => ({ timeDimensions: [{ dimension, dateRange: ['2026-07-28', end] }] }); + 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)'); From d2f452b889022e9da431264797ff23c9774cdfe0 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 3 Oct 2026 00:27:58 +0000 Subject: [PATCH 07/12] test(service-analytics): the enumeration pin reads what a file uses, with no private comment stripper check:comment-mask-adoption refused the pin's regex comment stripper. A file holds a helper when it imports, declares or calls it; a prose mention (a backticked name, a {@link}) is none of those, so no comment stripping is needed. Claude-Session: https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ Co-authored-by: Claude --- .../analytics-faces-one-lowering.test.ts | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) 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 index 5091b50619d..80b97f7f630 100644 --- 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 @@ -82,19 +82,25 @@ function sourceFiles(dir = SRC): string[] { return out.sort(); } -/** The file's code with its comments removed: a prose mention of a helper is not a copy of it. */ -function codeOf(file: string): string { - return readFileSync(join(SRC, file), 'utf8') - .replace(/\/\*[\s\S]*?\*\//g, '') - .replace(/(^|[^:'"`])\/\/.*$/gm, '$1'); +/** + * 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's code holds, keyed by file; files holding none are left out. */ +/** 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 code = codeOf(file); - const held = names.filter((name) => new RegExp(`\\b${name}\\b`).test(code)); + const source = readFileSync(join(SRC, file), 'utf8'); + const held = names.filter((name) => uses(source, name)); if (held.length > 0) out[file] = held; } return out; From 5c0079397ab07f80c8dc698f54e212b4da2e5ecc Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 3 Oct 2026 02:38:12 +0000 Subject: [PATCH 08/12] fix(service-analytics): the read scope binds a temporal comparand through the driver's coercion pair compileScopedFilterToSql takes two optional members, coerceTemporalFilterValue and coerceTemporalFilterColumn (the driver's ADR-0053 D-A2 pair, bound to the object), and applies them after the shared lowering to every value comparison's comparand and column. Absent is identity. The native read-scope merge and the ObjectQL echo pass the context's pair. Claude-Session: https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ Co-authored-by: Claude --- .../service-analytics/src/read-scope-sql.ts | 94 ++++++++++++++++--- .../src/strategies/native-sql-strategy.ts | 10 ++ .../src/strategies/objectql-strategy.ts | 8 ++ 3 files changed, 100 insertions(+), 12 deletions(-) diff --git a/packages/services/service-analytics/src/read-scope-sql.ts b/packages/services/service-analytics/src/read-scope-sql.ts index 012f458156f..fb1352e2305 100644 --- a/packages/services/service-analytics/src/read-scope-sql.ts +++ b/packages/services/service-analytics/src/read-scope-sql.ts @@ -642,6 +642,28 @@ import { * has no arm for: still `READ_SCOPE_COMPILE_FAILED` / 500, withheld, per the * #5367 section above. The note at {@link compileOperator}'s `default:` arm * records why a 400 would be the wrong class here. + * + * ## A temporal comparand binds in the column's storage form (#21505, ADR-0053 D-A1 / D-A2) + * + * D-A1 binds every surface that puts a filter comparand into raw SQL to the + * driver's dialect-aware temporal coercion. This compiler bound the comparand + * as written, so the database read it by its own rules instead of the + * engine's: PostgreSQL cast a bare day in the SESSION's zone, and SQLite + * compared it as text against the canonical instant. The read scope and the + * engine then admitted different rows for one policy, on some cells in the + * admitting direction. + * + * The caller now hands the driver's pair, bound to the object + * ({@link ReadScopeCompileOptions.coerceTemporalFilterValue} and + * {@link ReadScopeCompileOptions.coerceTemporalFilterColumn}): the engine + * door's own functions, never a second copy of the storage rule. Every value + * comparison in {@link compileOperator} and {@link compileField} binds its + * comparand through the first and reads its column through the second. The + * order is D-E3's by construction: the shared lowering at the entry widens a + * bare day first, and the arms convert the bound they are handed. The null + * tests, `$empty` and the text arms read the column as stored, as the native + * `where` face does. An absent member is identity, the contract's own reading + * for a driver whose storage form is the wire form. */ const IDENT = /^[a-z_][a-z0-9_]*$/i; @@ -746,6 +768,29 @@ export interface ReadScopeCompileOptions { * consumers fill it from the context's `declaredValueShape` hook. */ declaredValueShape?: (field: string) => ValueShapeFieldDef | undefined; + /** + * [#21505, ADR-0053 D-A1] The comparand half of the driver's temporal + * coercion, bound to the object this scope reads: `field`'s comparand in + * the form the column is STORED in. It is the engine door's own function + * (`IDataDriver.temporalFilterValue`, which `StrategyContext.coerceTemporalFilterValue` + * reaches), so a read scope and the engine compare one value. Applied after + * the shared lowering, to every comparand a value comparison binds + * (equality, `$ne`, the four orderings, `$in`, `$nin`, `$between`), never to + * a text pattern or a null test. Absent is identity: the comparand binds as + * written. Both of this compiler's consumers fill it from the context. + */ + coerceTemporalFilterValue?: (field: string, value: unknown) => unknown; + /** + * [#21505, ADR-0053 D-A2] The column half of the same coercion, and its + * required pair: given the column reference a value comparison was going + * to emit, the expression it must emit instead so the column reads in the + * form {@link ReadScopeCompileOptions.coerceTemporalFilterValue} put the + * comparand in (`IDataDriver.temporalFilterColumnSql`). It answers the + * reference unchanged for every column that needs no repair. The + * expression binds nothing: the consumers renumber every `?` in this + * compiler's output. Absent is identity: the column reads as written. + */ + coerceTemporalFilterColumn?: (field: string, columnSql: string) => string; } /** A node the compiler can walk: a plain object, not `null` and not an array. */ @@ -1310,11 +1355,11 @@ function compileField(field: string, value: unknown, qAlias: string, params: unk // "not a field reference". assertNoFieldReferenceComparand(field, value); - // Scalar / null → implicit equality. + // Scalar / null → implicit equality. [#21505] A value is compared in the + // column's storage form, like `$eq` below; the null test reads it as stored. if (value === null) return `${col} IS NULL`; if (typeof value !== 'object' || value instanceof Date) { - params.push(value); - return `${col} = ?`; + return `${comparisonColumn(col, field, opts)} = ${bindComparand(params, field, value, opts)}`; } // The implicit spelling of the equality slot {@link assertNoListInEqualitySlot} // guards under `$eq` — the shape a CEL `field == ` lowers to. @@ -1361,6 +1406,25 @@ function bind(params: unknown[], v: unknown): string { return '?'; } +/** + * [#21505] Bind a value comparison's comparand in the column's storage form, + * through the caller's {@link ReadScopeCompileOptions.coerceTemporalFilterValue} + * (identity when absent). See the module header's #21505 section. + */ +function bindComparand(params: unknown[], field: string, v: unknown, opts: ReadScopeCompileOptions): string { + return bind(params, opts.coerceTemporalFilterValue ? opts.coerceTemporalFilterValue(field, v) : v); +} + +/** + * [#21505] A value comparison's column, read in the form its comparand was + * coerced into, through {@link ReadScopeCompileOptions.coerceTemporalFilterColumn} + * (identity when absent). Null tests, `$empty` and the text arms read the + * column as stored, as the native `where` face does. + */ +function comparisonColumn(col: string, field: string, opts: ReadScopeCompileOptions): string { + return opts.coerceTemporalFilterColumn ? opts.coerceTemporalFilterColumn(field, col) : col; +} + /** * [#15684] Compile one case-EXACT text predicate for the dialect that will run * this scope — `text-match-sql.ts` picks the construct, this wrapper supplies @@ -2014,24 +2078,30 @@ function compileOperator( params: unknown[], opts: ReadScopeCompileOptions, ): string { + // [#21505] The value comparisons below compare in the column's STORAGE form: + // each comparand through {@link bindComparand}, the column through + // {@link comparisonColumn} — the driver's pair, identity when the caller + // passes none. The null tests and the text arms read the column as stored. + const vcol = (): string => comparisonColumn(col, field, opts); + const vbind = (v: unknown): string => bindComparand(params, field, v, opts); switch (op) { // [#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)}`; + case '$eq': return val === null ? `${col} IS NULL` : `${vcol()} = ${vbind(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)}`; - case '$lte': return `${col} <= ${bind(params, val)}`; + case '$ne': return val === null ? `${col} IS NOT NULL` : `${vcol()} <> ${vbind(val)}`; + case '$gt': return `${vcol()} > ${vbind(val)}`; + case '$gte': return `${vcol()} >= ${vbind(val)}`; + case '$lt': return `${vcol()} < ${vbind(val)}`; + case '$lte': return `${vcol()} <= ${vbind(val)}`; case '$in': { if (!Array.isArray(val)) throw readScopeCompileError(`[read-scope-sql] $in for "${field}" needs an array (fail-closed).`); if (val.length === 0) return FALSE_CLAUSE; // IN () matches nothing — safe assertCompilableMembers(op, field, val); - return `${col} IN (${val.map((v) => bind(params, v)).join(', ')})`; + return `${vcol()} IN (${val.map(vbind).join(', ')})`; } case '$nin': { if (!Array.isArray(val)) throw readScopeCompileError(`[read-scope-sql] $nin for "${field}" needs an array (fail-closed).`); @@ -2046,12 +2116,12 @@ function compileOperator( assertCompilableMembers(op, field, val); // [#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(', ')})`; + return `${vcol()} NOT IN (${val.map(vbind).join(', ')})`; } case '$between': { if (!Array.isArray(val) || val.length !== 2) throw readScopeCompileError(`[read-scope-sql] $between for "${field}" needs [min,max] (fail-closed).`); assertCompilableMembers(op, field, val); - return `${col} BETWEEN ${bind(params, val[0])} AND ${bind(params, val[1])}`; + return `${vcol()} BETWEEN ${vbind(val[0])} AND ${vbind(val[1])}`; } // [#5567] The comparand is a LITERAL, so it is escaped and the escape // character is bound with it. See {@link textMatch}. 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 64d8ec0dac1..0c4022fd163 100644 --- a/packages/services/service-analytics/src/strategies/native-sql-strategy.ts +++ b/packages/services/service-analytics/src/strategies/native-sql-strategy.ts @@ -1451,11 +1451,21 @@ export class NativeSQLStrategy implements AnalyticsStrategy { // text; one it cannot resolve is refused in the read-scope envelope. // [#20445] …and so does the declared value shape, so a policy's `$empty` // is answered by the field's row of the ruled per-type table. + // [#21505] …and so does the driver's temporal coercion pair (ADR-0053 + // D-A1 / D-A2), bound to the OBJECT (never the alias), so a temporal + // comparand binds in the column's storage form and the scope admits the + // rows the engine admits. const { sql, params: scopeParams } = compileScopedFilterToSql(filter, alias, { nonTextColumn: nonTextColumnResolver(ctx, objectName), dialect: sqlDialectFor(ctx, objectName), context: ctx.context, declaredValueShape: declaredValueShapeResolver(ctx, objectName), + coerceTemporalFilterValue: ctx.coerceTemporalFilterValue + ? (field, value) => ctx.coerceTemporalFilterValue!(objectName, field, value) + : undefined, + coerceTemporalFilterColumn: ctx.coerceTemporalFilterColumn + ? (field, columnSql) => ctx.coerceTemporalFilterColumn!(objectName, field, columnSql) + : undefined, }); // [#13926] The #13640 door guard, at THIS strategy's merge site. This is // not an echo: `execute()` runs this method's output through diff --git a/packages/services/service-analytics/src/strategies/objectql-strategy.ts b/packages/services/service-analytics/src/strategies/objectql-strategy.ts index 9839b161945..239144e623c 100644 --- a/packages/services/service-analytics/src/strategies/objectql-strategy.ts +++ b/packages/services/service-analytics/src/strategies/objectql-strategy.ts @@ -643,11 +643,19 @@ export class ObjectQLStrategy implements AnalyticsStrategy { // to, and refuses one it cannot resolve, as `execute()` does. // [#20445] …and the same declared value shape, so the echoed scope // prints the `$empty` arm the executed native statement runs. + // [#21505] …and the same temporal coercion pair, so the echo prints + // the storage-form comparand and column the executed statement binds. const { sql: scopeSql, params: scopeParams } = compileScopedFilterToSql(scope, tableName, { nonTextColumn: nonTextColumnResolver(ctx, tableName), dialect: sqlDialectFor(ctx, tableName), context: ctx.context, declaredValueShape: declaredValueShapeResolver(ctx, tableName), + coerceTemporalFilterValue: ctx.coerceTemporalFilterValue + ? (field, value) => ctx.coerceTemporalFilterValue!(tableName, field, value) + : undefined, + coerceTemporalFilterColumn: ctx.coerceTemporalFilterColumn + ? (field, columnSql) => ctx.coerceTemporalFilterColumn!(tableName, field, columnSql) + : undefined, }); // [#13926] The same door guard `execute()` trusts (`withReadScope`, // #13640), at the ECHO's own merge — so one read scope gets ONE verdict From 508b0bea9d2ffa11622b73cadbc0b559f6572467 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 3 Oct 2026 02:42:15 +0000 Subject: [PATCH 09/12] test(service-analytics): pin the read scope's temporal coercion against the engine on SQLite and PostgreSQL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sixteen cells (twelve datetime, four date as the control) answer the rows engine.find answers, compiled with the driver's pair and end to end through the native face with a host getReadScope; the ObjectQL echo prints the coerced comparand; an uncertified SQLite datetime column pins the column half. The F9 read-scope test in analytics-faces-one-lowering now runs on PostgreSQL too. Changeset: minor, Clause-② yes (widening). Claude-Session: https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ Co-authored-by: Claude --- .../21505-read-scope-temporal-coercion.md | 13 + .../analytics-faces-one-lowering.test.ts | 20 +- .../read-scope-temporal-coercion.test.ts | 322 ++++++++++++++++++ 3 files changed, 348 insertions(+), 7 deletions(-) create mode 100644 .changeset/21505-read-scope-temporal-coercion.md create mode 100644 packages/services/service-analytics/src/__tests__/read-scope-temporal-coercion.test.ts diff --git a/.changeset/21505-read-scope-temporal-coercion.md b/.changeset/21505-read-scope-temporal-coercion.md new file mode 100644 index 00000000000..abcbd181ddb --- /dev/null +++ b/.changeset/21505-read-scope-temporal-coercion.md @@ -0,0 +1,13 @@ +--- +"@objectstack/service-analytics": minor +--- + +fix(service-analytics): the analytics read scope binds a temporal comparand in the column's storage form, through the driver's own coercion pair (ADR-0053 D-A1 / D-A2) (#21505) + +Clause-②: yes (widening) + +`compileScopedFilterToSql` takes two new optional members in its options, `coerceTemporalFilterValue(field, value)` and `coerceTemporalFilterColumn(field, columnSql)`. Together they are the driver's `temporalFilterValue` / `temporalFilterColumnSql` pair, bound to the object the scope reads. After the shared lowering, every value comparison binds its comparand through the first and reads its column through the second: equality, `$ne`, the four orderings, `$in`, `$nin` and `$between`. Null tests, `$empty` and the text operators read the column as stored. An absent member is identity: the comparand and the column stay as written, which is what a host that passes neither got before. + +`NativeSQLStrategy` (the read scope merged into the native statement) and the `ObjectQLStrategy` echo (`/analytics/sql`) pass the context's `coerceTemporalFilterValue` / `coerceTemporalFilterColumn`, which `AnalyticsServicePlugin` wires to the driver. On those faces, a read scope that compares a `datetime` column with a temporal comparand now admits the rows `engine.find` admits for the same filter, on SQLite and on PostgreSQL whatever the server's time zone. Before, the comparand was bound as written and the database read it by its own rules, so the two disagreed: on some such scopes the native face admitted fewer rows than the engine, and on others more. A `date` column answered the engine's rows before and still does. + +No export is added or removed, no `@objectstack/spec` contract changes, and no dependency edge is added. A host that calls `compileScopedFilterToSql` directly gets the coercion by passing the pair from its driver. 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 index 80b97f7f630..52142157d63 100644 --- 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 @@ -405,16 +405,22 @@ for (const cell of DB_CELLS) { }); } - // 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 () => { + // [#21505] F9 binds each comparand through the driver's coercion pair + // (ADR-0053 D-A1 / D-A2), as both of its consumers wire it, so its + // datetime cells hold on PostgreSQL too, where a bare-day text bound + // would otherwise be read in the session's zone. + it('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 }); + const { sql, params } = compileScopedFilterToSql(query.where as FilterCondition, OBJECT, { + declaredValueShape, + dialect: cell.id === 'pg' ? 'postgres' : 'sqlite', + coerceTemporalFilterValue: (field, value) => driver.temporalFilterValue(OBJECT, field, value), + coerceTemporalFilterColumn: (field, columnSql) => driver.temporalFilterColumnSql(OBJECT, field, columnSql), + }); + const res = await (engine as any).execute(`select "id" from "${OBJECT}" where ${sql}`, { args: params, object: OBJECT }); + const rows = Array.isArray(res) ? res : (res as { rows: Array> }).rows; expect(ids(rows as Array>), label).toBe(expected); } }); diff --git a/packages/services/service-analytics/src/__tests__/read-scope-temporal-coercion.test.ts b/packages/services/service-analytics/src/__tests__/read-scope-temporal-coercion.test.ts new file mode 100644 index 00000000000..2ae223747cf --- /dev/null +++ b/packages/services/service-analytics/src/__tests__/read-scope-temporal-coercion.test.ts @@ -0,0 +1,322 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#21505, ADR-0053 D-A1 / D-A2] The read scope binds a temporal comparand in + * the column's storage form, through the driver's own coercion pair. + * + * `compileScopedFilterToSql` takes `coerceTemporalFilterValue` and + * `coerceTemporalFilterColumn`, the engine door's pair + * (`IDataDriver.temporalFilterValue` / `temporalFilterColumnSql`) bound to the + * object, and applies them after the shared lowering to every value + * comparison. Both of its consumers pass the context's pair. Absent members are + * identity. + * + * Measured at `d2f452b88`, before this change, with the comparand bound as + * written: the read scope differed from `engine.find` on 7 of the 12 + * `datetime` cells below on SQLite and 9 of 12 on PostgreSQL 16 under a + * non-UTC server, and on 0 of the 4 `date` cells. The native face, which runs + * the read scope, differed on the same cells end to end; the ObjectQL face + * (the engine) on none. + * + * The PostgreSQL cells run where `OS_TEST_POSTGRES_URL` is set, and are a + * named skip otherwise. Each asserts its server is not on UTC, because a UTC + * server reads a bare day as UTC midnight and would pass with no coercion at + * all. Each owns its table, dropped before and after. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { lowerFilterCondition, type Cube, type FilterCondition } from '@objectstack/spec/data'; +import type { AnalyticsService } from '../analytics-service.js'; +import { AnalyticsServicePlugin } from '../plugin.js'; +import { compileScopedFilterToSql, type ReadScopeCompileOptions } from '../read-scope-sql.js'; + +// ── Where the pair applies: after the lowering, on the value comparisons ───── + +const DECLARED: Record = { signed_at: 'datetime', due_on: 'date', note: 'text' }; +const declaredValueShape = (field: string) => (DECLARED[field] ? { type: DECLARED[field], multiple: false } : undefined); + +describe('[#21505] the compiler applies the coercion pair after the lowering, to every value comparison', () => { + const seen: Array<[string, unknown]> = []; + const RECORDING: ReadScopeCompileOptions = { + declaredValueShape, + dialect: 'sqlite', + coerceTemporalFilterValue: (field, value) => { seen.push([field, value]); return `C(${String(value)})`; }, + coerceTemporalFilterColumn: (_field, columnSql) => `COL(${columnSql})`, + }; + const compile = (scope: FilterCondition, opts: ReadScopeCompileOptions = RECORDING) => compileScopedFilterToSql(scope, 't', opts); + + it('the comparand the hook receives is the lowered one (ADR-0053 D-E3: widen first, convert second)', () => { + seen.length = 0; + expect(compile({ signed_at: { $lte: '2026-07-28' } })).toEqual({ sql: 'COL("t"."signed_at") < ?', params: ['C(2026-07-29)'] }); + expect(seen).toEqual([['signed_at', '2026-07-29']]); + expect(compile({ signed_at: { $between: ['2026-07-28', '2026-07-28'] } })).toEqual({ + sql: '(COL("t"."signed_at") >= ? AND COL("t"."signed_at") < ?)', + params: ['C(2026-07-28)', 'C(2026-07-29)'], + }); + }); + + it('every value comparison takes both halves', () => { + const cells: Array<[FilterCondition, string, unknown[]]> = [ + [{ due_on: '2026-07-28' }, 'COL("t"."due_on") = ?', ['C(2026-07-28)']], + [{ due_on: { $eq: '2026-07-28' } }, 'COL("t"."due_on") = ?', ['C(2026-07-28)']], + [{ due_on: { $ne: '2026-07-28' } }, '(("t"."due_on" IS NULL OR COL("t"."due_on") <> ?))', ['C(2026-07-28)']], + [ + { due_on: { $gt: 'a', $gte: 'b', $lt: 'c', $lte: 'd' } }, + '(COL("t"."due_on") > ? AND COL("t"."due_on") >= ? AND COL("t"."due_on") < ? AND COL("t"."due_on") <= ?)', + ['C(a)', 'C(b)', 'C(c)', 'C(d)'], + ], + [{ due_on: { $in: ['a', 'b'] } }, 'COL("t"."due_on") IN (?, ?)', ['C(a)', 'C(b)']], + [{ due_on: { $nin: ['a'] } }, '(("t"."due_on" IS NULL OR COL("t"."due_on") NOT IN (?)))', ['C(a)']], + [{ due_on: { $between: ['a', 'b'] } }, 'COL("t"."due_on") BETWEEN ? AND ?', ['C(a)', 'C(b)']], + ]; + for (const [scope, sql, params] of cells) expect(compile(scope), JSON.stringify(scope)).toEqual({ sql, params }); + }); + + it('a null test, `$empty` and a text arm read the column as stored and bind no coerced value', () => { + seen.length = 0; + expect(compile({ due_on: null })).toEqual({ sql: '"t"."due_on" IS NULL', params: [] }); + expect(compile({ note: { $null: true } })).toEqual({ sql: '"t"."note" IS NULL', params: [] }); + expect(compile({ note: { $empty: true } })).toEqual({ sql: '("t"."note" IS NULL OR "t"."note" = ?)', params: [''] }); + expect(compile({ note: { $contains: 'x' } }).sql).not.toContain('COL('); + expect(seen).toEqual([]); + }); + + it('absent members are identity: the same SQL and binds as identity members', () => { + const identity: ReadScopeCompileOptions = { + declaredValueShape, + dialect: 'sqlite', + coerceTemporalFilterValue: (_f, v) => v, + coerceTemporalFilterColumn: (_f, c) => c, + }; + const scopes: FilterCondition[] = [ + { signed_at: { $lte: '2026-07-28' } }, + { signed_at: { $ne: '2026-07-28' }, due_on: { $in: ['2026-07-28'] } }, + { $or: [{ note: 'n' }, { $not: { signed_at: { $between: ['2026-07-28', '9999-12-31'] } } }] }, + ]; + for (const scope of scopes) { + expect(compile(scope, { declaredValueShape, dialect: 'sqlite' }), JSON.stringify(scope)).toEqual(compile(scope, identity)); + } + }); +}); + +// ── The engine's rows, on a real engine, on SQLite and on PostgreSQL ────────── + +const OBJECT = 'os21505_coercion'; +const LEDGER = { + name: OBJECT, + label: 'Coercion', + 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 = { + name: 'os21505_cube', + title: 'Coercion', + sql: OBJECT, + public: true, + measures: { n: { type: 'count', sql: '*', label: 'n' } }, + dimensions: { id: { type: 'string', sql: 'id', label: 'Id' } }, +} as unknown as Cube; + +/** Each cell: a label, the scope, and the rows `engine.find` answers for it. */ +const CELLS: ReadonlyArray = [ + ['datetime $between one day', { signed_at: { $between: ['2026-07-28', '2026-07-28'] } }, 'r2,r3'], + ['datetime $between to the last day', { signed_at: { $between: ['2026-07-28', '9999-12-31'] } }, 'r2,r3,r4'], + ['datetime $ne a day', { signed_at: { $ne: '2026-07-28' } }, 'r1,r3,r4,r5'], + ['datetime equality on a day', { signed_at: '2026-07-28' }, 'r2'], + ['datetime $gte a day', { signed_at: { $gte: '2026-07-28' } }, 'r2,r3,r4'], + ['datetime $lte a day', { signed_at: { $lte: '2026-07-28' } }, 'r1,r2,r3'], + ['datetime $gt a day', { signed_at: { $gt: '2026-07-28' } }, 'r3,r4'], + ['datetime $lt a day', { signed_at: { $lt: '2026-07-28' } }, 'r1'], + ['datetime $in [a day]', { signed_at: { $in: ['2026-07-28'] } }, 'r2'], + ['datetime $nin [a day]', { signed_at: { $nin: ['2026-07-28'] } }, 'r1,r3,r4,r5'], + ['datetime $gte a zone-naive time', { signed_at: { $gte: '2026-07-28 05:00' } }, 'r3,r4'], + ['datetime $ne under $not under $or', { $or: [{ note: 'n' }, { $not: { signed_at: { $ne: '2026-07-28' } } }] }, 'r2,r4'], + ['date $between one day (control)', { due_on: { $between: ['2026-07-28', '2026-07-28'] } }, 'r2,r3'], + ['date $ne a day (control)', { due_on: { $ne: '2026-07-28' } }, 'r1,r4,r5'], + ['date $lte a day (control)', { due_on: { $lte: '2026-07-28' } }, 'r1,r2,r3'], + ['date equality on a day (control)', { due_on: '2026-07-28' }, 'r2,r3'], +]; + +const quiet = { debug() {}, info() {}, warn() {}, error() {}, child() { return quiet; } }; +/** The ids of a result, sorted: a row array, or a raw `pg` result's `rows`. */ +const ids = (res: unknown): string => + ((Array.isArray(res) ? res : (res as { rows: unknown[] }).rows) as Array>) + .map((r) => String(r.id)).sort().join(','); + +interface DbCell { id: 'sqlite' | 'pg'; dialect: string; config: () => Record | null } +const DB_CELLS: readonly DbCell[] = [ + { id: 'sqlite', dialect: 'sqlite', config: () => ({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }) }, + { + id: 'pg', + dialect: 'postgres', + config: () => (process.env.OS_TEST_POSTGRES_URL ? { client: 'pg', connection: process.env.OS_TEST_POSTGRES_URL } : null), + }, +]; + +for (const cell of DB_CELLS) { + const config = cell.config(); + describe.skipIf(!config)( + `[#21505] the read scope admits the engine's rows (${cell.id})${config ? '' : ' (skipped: set OS_TEST_POSTGRES_URL to run this cell)'}`, + () => { + let driver: SqlDriver; + let engine: ObjectQL; + let faces: { native: AnalyticsService; objectql: AnalyticsService }; + let scope: FilterCondition | null = null; + 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); + }; + // The plugin's own composition, with a host read scope: the native + // face, and the same narrowed to the engine aggregate (whose echo is + // the `/analytics/sql` statement). + const composed: Record = {}; + for (const [face, caps] of [ + ['native', undefined], + ['objectql', () => ({ nativeSql: false, objectqlAggregate: true, inMemory: false })], + ] as const) { + const registered: Record = {}; + await new AnalyticsServicePlugin({ cubes: [CUBE], getReadScope: () => scope, ...(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); + composed[face] = registered.analytics as AnalyticsService; + } + faces = composed as typeof faces; + }); + afterAll(async () => { + await drop(); + try { await engine?.destroy(); } catch { /* noop */ } + }); + + it.skipIf(cell.id !== 'pg')('the server is not on UTC (the premise of the PostgreSQL cells)', async () => { + const res = await (driver as any).execute('show timezone'); + const zone = String(Object.values((res as { rows: Array> }).rows[0])[0]); + expect(['UTC', 'Etc/UTC', 'GMT', 'Etc/GMT', 'UCT', 'Zulu']).not.toContain(zone); + }); + + it('compiled with the driver\'s pair, every cell admits the engine\'s rows, and the date control too', async () => { + const options: ReadScopeCompileOptions = { + declaredValueShape, + dialect: cell.dialect, + coerceTemporalFilterValue: (field, value) => driver.temporalFilterValue(OBJECT, field, value), + coerceTemporalFilterColumn: (field, columnSql) => driver.temporalFilterColumnSql(OBJECT, field, columnSql), + }; + for (const [label, where, expected] of CELLS) { + expect(ids(await engine.find(OBJECT, { where, fields: ['id'] } as never)), `${label}: engine.find`).toBe(expected); + const { sql, params } = compileScopedFilterToSql(where, OBJECT, options); + const rows = await (engine as any).execute(`select "id" from "${OBJECT}" where ${sql}`, { args: params, object: OBJECT }); + expect(ids(rows), `${label}: the read scope`).toBe(expected); + } + }); + + it('end to end, the native face scoped by a host getReadScope answers the engine\'s rows', async () => { + for (const [label, where, expected] of CELLS) { + scope = where; + const before = rawStatements; + const res = await faces.native.query({ cube: CUBE.name, measures: ['n'], dimensions: ['id'] } as never); + expect(ids(res.rows), `${label}: the native face`).toBe(expected); + expect(rawStatements - before, `${label}: the native strategy answered`).toBeGreaterThanOrEqual(1); + } + scope = null; + }); + + it('the ObjectQL echo prints the comparand the executed statement binds', async () => { + scope = { signed_at: { $ne: '2026-07-28' } }; + const echo = await faces.objectql.generateSql({ cube: CUBE.name, measures: ['n'], dimensions: ['id'] } as never); + scope = null; + expect(echo.params).toContain(driver.temporalFilterValue(OBJECT, 'signed_at', '2026-07-28')); + expect(echo.params).not.toContain('2026-07-28'); + }); + }, + ); +} + +// ── The column half: a SQLite datetime column the driver has not certified ──── + +/** + * An external object (ADR-0015) is registered with no backfill, so its SQLite + * `datetime` column is never certified canonical and may hold the forms written + * before the canonical convention. The driver reads such a column through its + * repair expression, and `temporalFilterColumnSql` hands that expression to a + * raw-SQL caller. Coercing the comparand alone keeps half the defect (D-A2). + */ +describe('[#21505] the column half reads an uncertified SQLite datetime column as the driver does', () => { + const LEGACY = 'os21505_legacy'; + let driver: SqlDriver; + + beforeAll(async () => { + driver = new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true } as never); + const knex = (driver as any).knex; + await knex.schema.createTable(LEGACY, (t: any) => { + t.string('id').primary(); + t.specificType('at', 'datetime'); + }); + await knex(LEGACY).insert([ + { id: 'l1', at: Date.UTC(2026, 6, 27, 10) }, + { id: 'l2', at: '2026-07-28T00:00:00.000Z' }, + { id: 'l3', at: '2026-07-28 10:00:00' }, + { id: 'l4', at: '2026-07-29T18:00:00+08:00' }, + { id: 'l5', at: null }, + ]); + driver.registerExternalObject({ name: LEGACY, fields: { at: { name: 'at', type: 'datetime' } } }); + }); + afterAll(async () => { + try { await driver?.disconnect(); } catch { /* noop */ } + }); + + it('the fixture is the uncertified state: the driver wraps this column', () => { + expect(driver.temporalFilterColumnSql(LEGACY, 'at', '"c"')).not.toBe('"c"'); + }); + + it('with the pair, the read scope admits the driver\'s own rows', async () => { + const options: ReadScopeCompileOptions = { + declaredValueShape: (field) => (field === 'at' ? { type: 'datetime', multiple: false } : undefined), + dialect: 'sqlite', + coerceTemporalFilterValue: (field, value) => driver.temporalFilterValue(LEGACY, field, value), + coerceTemporalFilterColumn: (field, columnSql) => driver.temporalFilterColumnSql(LEGACY, field, columnSql), + }; + const cells: Array<[FilterCondition, string]> = [ + [{ at: { $gte: '2026-07-28' } }, 'l2,l3,l4'], + [{ at: { $lt: '2026-07-28' } }, 'l1'], + [{ at: { $ne: '2026-07-28' } }, 'l1,l3,l4,l5'], + [{ at: { $in: ['2026-07-28 10:00'] } }, 'l3'], + ]; + for (const [where, expected] of cells) { + const lowered = lowerFilterCondition(where, { isDatetimeColumn: (field) => field === 'at' }); + expect(ids(await driver.find(LEGACY, { where: lowered, fields: ['id'] } as never)), `${JSON.stringify(where)}: the driver`).toBe(expected); + const { sql, params } = compileScopedFilterToSql(where, LEGACY, options); + const rows = await (driver as any).knex.raw(`select "id" from "${LEGACY}" where ${sql}`, params); + expect(ids(rows), `${JSON.stringify(where)}: the read scope`).toBe(expected); + } + }); +}); From 207ce2dbf9f3f8c6ffd899da655e94569a6aba8b Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 3 Oct 2026 07:11:29 +0000 Subject: [PATCH 10/12] fix(service-analytics): the draft preview compares a temporal value in its storage form on both sides The preview has no driver, so its counterpart of the engine door is the rule that door applies: @objectstack/core's temporalStorageForm, for the kind temporalComparandKind gives the column's declared type. Each value comparison's comparand and every declared temporal field of a drafted row take that form before the match, as driver-memory reads them. A column the host names no type for stays as written. Claude-Session: https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ Co-authored-by: Claude --- .../src/preview-evaluator.ts | 88 +++++++++++++++++-- 1 file changed, 83 insertions(+), 5 deletions(-) diff --git a/packages/services/service-analytics/src/preview-evaluator.ts b/packages/services/service-analytics/src/preview-evaluator.ts index 89cc78eb510..4a8656492a5 100644 --- a/packages/services/service-analytics/src/preview-evaluator.ts +++ b/packages/services/service-analytics/src/preview-evaluator.ts @@ -35,7 +35,10 @@ import { resolveAnalyticsDateRangeString, utcInstantMs, compensatedSum, + temporalComparandKind, + temporalStorageForm, type BucketGranularity, + type TemporalComparandKind, } from '@objectstack/core'; import { explicitDateRangeWindow } from './date-range-array-arm.js'; // [#19810] The `where` door's refusal envelope — `INVALID_FILTER` / 400, @@ -655,6 +658,77 @@ export function declaredPreviewLowering(declaredType?: (field: string) => string }; } +/** + * [#21505, ADR-0053 D-A1] The draft preview compares a temporal value in its + * STORAGE form, on both sides, as `driver-memory` does. The preview has no + * driver, so its counterpart of the engine door is the rule that door + * applies, `@objectstack/core`'s `temporalStorageForm`, for the kind + * `temporalComparandKind` gives the column's declared type. A drafted row + * keeps whatever spelling its author wrote, so the row value is put in that + * form too, the way a driver puts it on write. + * + * Without it a drafted chart compared the two spellings as text. An instant + * against a bare day, or a window end written shorter than the stored instant, + * then counted rows the published chart does not, or missed rows it counts. + * + * A column the host names no type for stays as written on both sides. + */ +type PreviewTemporalKind = (field: string) => TemporalComparandKind | null; + +function previewTemporalKind(declaredType?: (field: string) => string | undefined): PreviewTemporalKind { + const kinds = new Map(); + return (field) => { + if (!kinds.has(field)) kinds.set(field, temporalComparandKind(declaredType?.(field))); + return kinds.get(field)!; + }; +} + +/** The value comparisons whose comparand takes the storage form: `driver-memory`'s arm set. */ +const STORAGE_FORM_OPERATORS = new Set(['$eq', '$ne', '$gt', '$gte', '$lt', '$lte', '$in', '$nin', '$between']); + +/** `value` in the storage form of `kind`; a list maps its members, as the drivers do. */ +function storageForm(value: unknown, kind: TemporalComparandKind): unknown { + return Array.isArray(value) ? value.map((v) => temporalStorageForm(v, kind)) : temporalStorageForm(value, kind); +} + +/** The condition with each value comparison's comparand on a temporal column in its storage form. */ +function previewStorageComparands(node: Row | undefined, kindOf: PreviewTemporalKind): Row | undefined { + if (!node) return node; + const out: Row = {}; + for (const [key, cond] of Object.entries(node)) { + const kind = key.startsWith('$') ? null : kindOf(key); + if ((key === '$and' || key === '$or') && Array.isArray(cond)) { + out[key] = cond.map((arm) => previewStorageComparands(arm as Row, kindOf)); + } else if (key === '$not' && cond !== null && typeof cond === 'object' && !Array.isArray(cond)) { + out[key] = previewStorageComparands(cond as Row, kindOf); + } else if (!kind || cond == null || Array.isArray(cond)) { + out[key] = cond; + } else if (typeof cond !== 'object' || cond instanceof Date) { + out[key] = temporalStorageForm(cond, kind); // implicit equality + } else { + out[key] = Object.fromEntries(Object.entries(cond as Row).map(([op, expected]) => [ + op, + STORAGE_FORM_OPERATORS.has(op) && expected != null ? storageForm(expected, kind) : expected, + ])); + } + } + return out; +} + +/** The row with every declared temporal field in its storage form; the same row when none moved. */ +function previewStorageRow(row: Row, kindOf: PreviewTemporalKind): Row { + let out: Row | undefined; + for (const [field, value] of Object.entries(row)) { + const kind = kindOf(field); + if (!kind) continue; + const stored = storageForm(value, kind); + if (stored === value) continue; + out ??= { ...row }; + out[field] = stored; + } + return out ?? row; +} + /** * Evaluate `query` over `rows` using the cube's measure/dimension specs. * Mirrors the engine strategies' output contract: rows keyed by bare @@ -703,9 +777,13 @@ export function evaluateAnalyticsQueryOverRows( // 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 lowered = lowerFilterCondition(normalizeWhereComparands(query.where), lowering); + assertPreviewCanEvaluate(lowered); + // [#21505] …then each value comparison in the temporal STORAGE form, the + // comparand here and the row value at the match (see {@link previewTemporalKind}). + const kindOf = previewTemporalKind(declaredType); + const where = previewStorageComparands(lowered, kindOf); + let filtered = rows.filter((r) => matchesWhere(previewStorageRow(r, kindOf), where)); const timeDims = query.timeDimensions ?? []; for (const td of timeDims) { const dim = cube.dimensions?.[td.dimension]; @@ -727,8 +805,8 @@ export function evaluateAnalyticsQueryOverRows( // 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)); + const window = previewStorageComparands(lowerFilterCondition({ [field]: bounds }, lowering), kindOf); + filtered = filtered.filter((r) => matchesWhere(previewStorageRow(r, kindOf), window)); } // 2. Grouping keys: each selected dimension (time dims bucketed). From baffb08e61468b6a20002c7eff53f6f9a85eec1b Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 3 Oct 2026 07:14:52 +0000 Subject: [PATCH 11/12] test(service-analytics): pin the draft preview's temporal storage form against the engine The sixteen cells and two window ends written shorter than the stored instant answer engine.find's rows through queryDataset previewDrafts, over drafted rows in the canonical spelling and over the same instants respelled (a Date, no milliseconds, zone-naive, an offset). Claude-Session: https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ Co-authored-by: Claude --- .../read-scope-temporal-coercion.test.ts | 53 +++++++++++++++++-- 1 file changed, 50 insertions(+), 3 deletions(-) diff --git a/packages/services/service-analytics/src/__tests__/read-scope-temporal-coercion.test.ts b/packages/services/service-analytics/src/__tests__/read-scope-temporal-coercion.test.ts index 2ae223747cf..94821df1885 100644 --- a/packages/services/service-analytics/src/__tests__/read-scope-temporal-coercion.test.ts +++ b/packages/services/service-analytics/src/__tests__/read-scope-temporal-coercion.test.ts @@ -9,14 +9,17 @@ * (`IDataDriver.temporalFilterValue` / `temporalFilterColumnSql`) bound to the * object, and applies them after the shared lowering to every value * comparison. Both of its consumers pass the context's pair. Absent members are - * identity. + * identity. The draft preview, which has no driver, puts both sides of each + * value comparison in the same storage form with `@objectstack/core`'s + * `temporalStorageForm`, by the column's declared type. * * Measured at `d2f452b88`, before this change, with the comparand bound as * written: the read scope differed from `engine.find` on 7 of the 12 * `datetime` cells below on SQLite and 9 of 12 on PostgreSQL 16 under a * non-UTC server, and on 0 of the 4 `date` cells. The native face, which runs * the read scope, differed on the same cells end to end; the ObjectQL face - * (the engine) on none. + * (the engine) on none. The preview differed on 7 of the 12 `datetime` cells + * and on 0 of the 4 `date` cells. * * The PostgreSQL cells run where `OS_TEST_POSTGRES_URL` is set, and are a * named skip otherwise. Each asserts its server is not on UTC, because a UTC @@ -28,7 +31,8 @@ import { describe, it, expect, beforeAll, afterAll } from 'vitest'; import { ObjectQL } from '@objectstack/objectql'; import { SqlDriver } from '@objectstack/driver-sql'; import { lowerFilterCondition, type Cube, type FilterCondition } from '@objectstack/spec/data'; -import type { AnalyticsService } from '../analytics-service.js'; +import { DatasetSchema } from '@objectstack/spec/ui'; +import { AnalyticsService } from '../analytics-service.js'; import { AnalyticsServicePlugin } from '../plugin.js'; import { compileScopedFilterToSql, type ReadScopeCompileOptions } from '../read-scope-sql.js'; @@ -120,6 +124,14 @@ const ROWS = [ { 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 }, ]; +/** The same instants and days as {@link ROWS}, drafted in other spellings an author can write. */ +const RESPELLED = [ + { id: 'r1', signed_at: new Date(Date.UTC(2026, 6, 27, 10)), due_on: '2026-07-27T23:00:00Z', note: '2026-07-27' }, + { id: 'r2', signed_at: '2026-07-28T00:00:00Z', due_on: '2026-07-28T00:00:00.000Z', note: '2026-07-28' }, + { id: 'r3', signed_at: '2026-07-28 10:00:00', due_on: '2026-07-28', note: '2026-07-28 late' }, + { id: 'r4', signed_at: '2026-07-29T18:00:00+08:00', due_on: '2026-07-29', note: 'n' }, + { id: 'r5', signed_at: null, due_on: null, note: null }, +]; const CUBE = { name: 'os21505_cube', title: 'Coercion', @@ -257,6 +269,41 @@ for (const cell of DB_CELLS) { expect(echo.params).toContain(driver.temporalFilterValue(OBJECT, 'signed_at', '2026-07-28')); expect(echo.params).not.toContain('2026-07-28'); }); + + it('the draft preview (queryDataset previewDrafts) answers the engine\'s rows over drafted rows in either spelling', async () => { + // Window ends written shorter than the stored instant, beside the cells. + const windows: Array<[string, [string, string]]> = [ + ['datetime window to a zone-naive minute', ['2026-07-28', '2026-07-28T10:00']], + ['datetime window to a zone-naive second', ['2026-07-28', '2026-07-28T10:00:00']], + ]; + for (const [label, [start, end]] of windows) { + expect(ids(await engine.find(OBJECT, { where: { signed_at: { $gte: start, $lte: end } }, fields: ['id'] } as never)), `${label}: engine.find`).toBe('r2,r3'); + } + const dataset = DatasetSchema.parse({ + name: 'os21505_preview', + label: 'Coercion preview', + object: OBJECT, + dimensions: [{ name: 'id', field: 'id', type: 'string' }, { name: 'signed_at', field: 'signed_at', type: 'date' }], + measures: [{ name: 'row_count', aggregate: 'count' }], + }); + for (const [spelling, drafted] of [['canonical', ROWS], ['respelled', RESPELLED]] as const) { + // The live path is not wired, so an answer can only come from the preview. + const svc = new AnalyticsService({ + sourceFieldMeta: (object: string, field: string) => (object === OBJECT && DECLARED[field] ? { type: DECLARED[field] } : undefined), + 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 ? drafted.map((r) => ({ ...r })) : null), + } as never); + const preview = async (selection: Record) => + ids((await svc.queryDataset(dataset as never, { dimensions: ['id'], measures: ['row_count'], ...selection } as never, { tenantId: 'org_A' } as never, { previewDrafts: true })).rows); + for (const [label, where, expected] of CELLS) { + expect(await preview({ runtimeFilter: where }), `${spelling} rows, ${label}: the preview`).toBe(expected); + } + for (const [label, dateRange] of windows) { + expect(await preview({ timeDimensions: [{ dimension: 'signed_at', dateRange }] }), `${spelling} rows, ${label}: the preview`).toBe('r2,r3'); + } + } + }); }, ); } From f8113c0a093dbd9ccc77c9346dabf65ba53d7690 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 3 Oct 2026 07:16:59 +0000 Subject: [PATCH 12/12] =?UTF-8?q?docs(changeset):=20service-analytics=20mi?= =?UTF-8?q?nor,=20Clause-=E2=91=A1=20yes=20(narrowing):=20the=20read=20sco?= =?UTF-8?q?pe=20and=20the=20draft=20preview=20answer=20the=20engine's=20ro?= =?UTF-8?q?ws?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The answer moves in both directions onto engine.find's rows, so the changeset carries a BREAKING banner and an ADR-0087 not-required (no-migration-prescription) disposition, as #21417 declared for its answer change. It names the two new optional members, their identity default, and the preview's storage form on both sides. Claude-Session: https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ Co-authored-by: Claude --- .changeset/21505-read-scope-temporal-coercion.md | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/.changeset/21505-read-scope-temporal-coercion.md b/.changeset/21505-read-scope-temporal-coercion.md index abcbd181ddb..1dd56307b33 100644 --- a/.changeset/21505-read-scope-temporal-coercion.md +++ b/.changeset/21505-read-scope-temporal-coercion.md @@ -2,12 +2,16 @@ "@objectstack/service-analytics": minor --- -fix(service-analytics): the analytics read scope binds a temporal comparand in the column's storage form, through the driver's own coercion pair (ADR-0053 D-A1 / D-A2) (#21505) +fix(service-analytics)!: the analytics read scope and the draft preview compare a temporal comparand in the column's storage form, as the engine does (ADR-0053 D-A1 / D-A2) (#21505) -Clause-②: yes (widening) +Clause-②: yes (narrowing) -`compileScopedFilterToSql` takes two new optional members in its options, `coerceTemporalFilterValue(field, value)` and `coerceTemporalFilterColumn(field, columnSql)`. Together they are the driver's `temporalFilterValue` / `temporalFilterColumnSql` pair, bound to the object the scope reads. After the shared lowering, every value comparison binds its comparand through the first and reads its column through the second: equality, `$ne`, the four orderings, `$in`, `$nin` and `$between`. Null tests, `$empty` and the text operators read the column as stored. An absent member is identity: the comparand and the column stay as written, which is what a host that passes neither got before. + -`NativeSQLStrategy` (the read scope merged into the native statement) and the `ObjectQLStrategy` echo (`/analytics/sql`) pass the context's `coerceTemporalFilterValue` / `coerceTemporalFilterColumn`, which `AnalyticsServicePlugin` wires to the driver. On those faces, a read scope that compares a `datetime` column with a temporal comparand now admits the rows `engine.find` admits for the same filter, on SQLite and on PostgreSQL whatever the server's time zone. Before, the comparand was bound as written and the database read it by its own rules, so the two disagreed: on some such scopes the native face admitted fewer rows than the engine, and on others more. A `date` column answered the engine's rows before and still does. +**BREAKING**: this changes the rows two analytics faces select for a value comparison on a declared temporal column, in both directions, onto the rows `engine.find` selects for the same filter: on some filters fewer rows than before, on others more. The faces are the row-level read scope compiled into the native statement, and the draft preview (`queryDataset` with `previewDrafts`). It ships as `minor` under the launch-window convention for answer changes. No export is removed, no accepted input is refused and no error code changes. -No export is added or removed, no `@objectstack/spec` contract changes, and no dependency edge is added. A host that calls `compileScopedFilterToSql` directly gets the coercion by passing the pair from its driver. +**The read scope.** `compileScopedFilterToSql` takes two new optional members in its options, `coerceTemporalFilterValue(field, value)` and `coerceTemporalFilterColumn(field, columnSql)`. Together they are the driver's `temporalFilterValue` / `temporalFilterColumnSql` pair, bound to the object the scope reads. After the shared lowering, every value comparison binds its comparand through the first and reads its column through the second: equality, `$ne`, the four orderings, `$in`, `$nin` and `$between`. Null tests, `$empty` and the text operators read the column as stored. An absent member is identity: the comparand and the column stay as written, which is what a host that passes neither got before. `NativeSQLStrategy` (the read scope merged into the native statement) and the `ObjectQLStrategy` echo (`/analytics/sql`) pass the context's pair, which `AnalyticsServicePlugin` wires to the driver. Before, the comparand was bound as written and the database read it by its own rules, on SQLite and on PostgreSQL whatever the server's time zone. + +**The draft preview.** It has no driver, so each value comparison on a column the host declares `datetime`, `date` or `time` now puts both sides in the storage form `@objectstack/core`'s `temporalStorageForm` gives: the comparand, and the drafted row's value, as `driver-memory` reads them. Before, it compared the two spellings as text. A column the host names no type for is compared as written, as before. + +A `date` column answered the engine's rows on both faces before and still does when both sides are spelled as days. No `@objectstack/spec` contract changes and no dependency edge is added. A host that calls `compileScopedFilterToSql` directly gets the coercion by passing the pair from its driver.