Skip to content

Commit 1ca1eb0

Browse files
fix(service-analytics)!: the read scope and the draft preview compare a temporal comparand in the column storage form (ADR-0053 D-A1 / D-A2) (#21562)
Fixes #21505 Clause-②: yes (narrowing) ADR-0053 D-A1 binds every surface that puts a filter comparand into raw SQL to the driver's own temporal coercion, and D-A2 makes the comparand coercion and its column companion a pair. The analytics read scope bound a temporal comparand as written, and the draft preview compared temporal values as text. On a declared `datetime` column both faces then selected different rows from `engine.find` for the same filter, in both directions. Triage ruling 5964364198 (one coercion, the engine door's, no second copy) and claim revision 5964626028 set the shape built here. ## What changed **The read scope** (`read-scope-sql.ts`). `ReadScopeCompileOptions` gains two optional members, `coerceTemporalFilterValue(field, value)` and `coerceTemporalFilterColumn(field, columnSql)`. They mirror `StrategyContext.coerceTemporalFilterValue` / `coerceTemporalFilterColumn`, bound to the object the scope reads, and reach the driver's `temporalFilterValue` / `temporalFilterColumnSql`. After the shared lowering, every value comparison (implicit equality, `$eq`, `$ne`, the four orderings, `$in`, `$nin`, `$between`) binds its comparand through the first and reads its column through the second. Null tests, `$empty` and the text arms read the column as stored, as the native `where` face does. **An absent member is identity.** The order is ADR-0053 D-E3's by construction: the lowering widens a bare day first, then the arm converts the bound. **The two callers.** `NativeSQLStrategy.applyReadScope` binds the context's pair to the OBJECT, never the join alias. The `ObjectQLStrategy` read-scope echo binds it to its table. One call site each. **The draft preview** (`preview-evaluator.ts`). It 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 declared type, is applied to both sides of each value comparison: the comparand, and the drafted row's value, as `driver-memory` reads them. It covers the `where` and the window. A column the host names no type for stays as written. The declared type is the reader #21417 already hands the evaluator, so `analytics-service.ts` is untouched. No new export (the package index is unchanged), no `@objectstack/spec` edit, and no new dependency edge. **Changeset:** `@objectstack/service-analytics` minor, BREAKING, with an ADR-0087 `not-required (no-migration-prescription)` disposition. The answer moves in both directions onto the engine's rows: on some filters fewer rows than before, on others more. ## Measured before the change (at d2f452b; classes only, the cells live in the pins) - Read scope vs `engine.find`, over 16 cells (12 `datetime`, 4 `date`): 7 of 12 `datetime` cells differed on SQLite, and 9 of 12 on PostgreSQL 16.14 under a non-UTC server (America/New_York). Both dialects included the admitting direction. The `date` cells differed on 0 of 4. - Through the plugin's own composition with a host `getReadScope`, the native face differed on the same cells; the ObjectQL face (the engine path) differed on none. - The draft preview differed on 7 of 12 `datetime` cells and 0 of 4 `date` cells. A window end written shorter than the stored instant left out the row at that instant. - Core's `temporalStorageForm` equals the driver's coercion on SQLite and PostgreSQL. On MySQL the driver adds its own physical spelling, which is why the read scope takes the driver's pair rather than the core rule. ## Pins - `read-scope-temporal-coercion.test.ts` (new): - compiler shape: the hook sees the lowered comparand; every value comparison takes both halves; null, `$empty` and text arms are untouched; absent members equal identity members; - the 16 cells on SQLite and on live PostgreSQL, with a non-UTC server asserted: the read scope compiled with the driver pair equals `engine.find`; - end to end, the native face with a host `getReadScope` equals the engine on all 16; - the ObjectQL echo prints the coerced comparand; - the column half, on an uncertified SQLite `datetime` column (an external object, which the driver never backfills), against the driver's own `find`; - the draft preview through `queryDataset` `previewDrafts`: the 16 cells and two window ends written shorter than the stored instant equal `engine.find`, over drafted rows in the canonical spelling and over the same instants respelled. - `analytics-faces-one-lowering.test.ts`: the read-scope (F9) test's PostgreSQL skip is gone; it passes the driver pair and runs on both databases. ## Ablations (predicted first; mutated with `scripts/ablation-replace.mjs`, each restore verified blob == HEAD and `git diff HEAD` empty) - Read-scope comparand member bypassed: 10 red, exactly the predicted set. The first failing cell on each database is one of the card's cells. - Read-scope column member bypassed: 3 red, exactly the predicted set (two compiler-shape pins and the uncertified-column pin). Every certified-column cell stayed green. - Preview comparand side bypassed: 2 red (the preview pin in both database blocks). The first failure is on the canonical drafted rows. - Preview row side bypassed: 2 red. The first failure is on the respelled drafted rows; the canonical rows pass first. The read-scope pair was ablated at `508b0bea9d` and again at this head, with identical results. ## Tests and gates (at f8113c0) - `pnpm --filter @objectstack/service-analytics test`, with live PostgreSQL set: 175 files passed, 4403 tests passed, 2 skipped. - `pnpm --filter @objectstack/service-analytics typecheck`: green. - Downstream on the rebuilt dist, analytics files: rest 22 files / 291 passed, runtime 7 / 70, dogfood 8 / 70. - `dispatch-gates --commands`: 64 families derived, 64 run, every one exit 0. The `--ran` reconciliation reads 0 NOT-MEASURED (derived from recorded exit codes). - ESLint narrowed to the 6 changed code files: 0 errors, 0 warnings. The config enables no type-aware linting, so no untouched file's verdict can move. The full `pnpm lint` is CI's. ## Acceptance notes - CI's Temporal Conformance job runs this package's suite under a skewed process zone but sets no `OS_TEST_POSTGRES_URL` for it, so the PostgreSQL cells here (and #21417's in the same file) are named skips in CI. Each asserts a non-UTC server, so wiring the URL there would not pass vacuously. Noted, not filed. - `origin/main` took #21417 as a squash (81e69ca), so this branch's commit list still shows #21417's pre-squash commits beneath this card's. The file diff is this card's 7 files only. - A host that calls `compileScopedFilterToSql` directly keeps the as-written bind until it passes the pair from its driver: absent is identity. --- _Generated by [Claude Code](https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent bf36edd commit 1ca1eb0

7 files changed

Lines changed: 582 additions & 24 deletions

File tree

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
---
2+
"@objectstack/service-analytics": minor
3+
---
4+
5+
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)
6+
7+
Clause-②: yes (narrowing)
8+
9+
<!-- adr-0087: not-required (no-migration-prescription) a change of which rows the analytics read scope and the draft preview select for a value comparison on a declared datetime, date or time column, not of anything an author writes: no spec key, spelling or stored shape moves. AnalyticsQuerySchema, CubeSchema, DatasetSchema and every RLS policy parse and save as before, the package index exports the same names, and no stored row is read or rewritten. The two new options members are optional and identity when absent, so every existing caller of compileScopedFilterToSql compiles as before. What moves is the row set such a comparison selects, which now equals the engine's own answer for the same filter, so there is nothing for objectstack migrate meta to rewrite. The other categories are closed on facts: the package publishes (not unpublished); no ADR-0087 id covers a filter comparand's storage form and this diff adds none (not registered / already-registered); and the change is runtime behaviour, not a declaration alone (not runtime-interface-only / type-surface-only). -->
10+
11+
**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.
12+
13+
**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.
14+
15+
**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.
16+
17+
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.

‎packages/services/service-analytics/src/__tests__/analytics-faces-one-lowering.test.ts‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -409,16 +409,22 @@ for (const cell of DB_CELLS) {
409409
});
410410
}
411411

412-
// F9 binds a comparand as written (no storage-form coercion), so its
413-
// datetime cells are pinned on SQLite, whose stored form is ISO text.
414-
// PostgreSQL reads a bare-day text bound in the session's zone; that is
415-
// the read scope's temporal-coercion question, not its lowering.
416-
it.skipIf(cell.id !== 'sqlite')('the read scope (F9), typed by its declared value shape, answers the same rows', async () => {
412+
// [#21505] F9 binds each comparand through the driver's coercion pair
413+
// (ADR-0053 D-A1 / D-A2), as both of its consumers wire it, so its
414+
// datetime cells hold on PostgreSQL too, where a bare-day text bound
415+
// would otherwise be read in the session's zone.
416+
it('the read scope (F9), typed by its declared value shape, answers the same rows', async () => {
417417
const declaredValueShape = (field: string) => (DECLARED[field] ? { type: DECLARED[field], multiple: false } : undefined);
418418
for (const [label, query, expected] of CELLS) {
419419
if (!query.where) continue;
420-
const { sql, params } = compileScopedFilterToSql(query.where as FilterCondition, OBJECT, { declaredValueShape, dialect: 'sqlite' });
421-
const rows = await (engine as any).execute(`select "id" from "${OBJECT}" where ${sql}`, { args: params, object: OBJECT });
420+
const { sql, params } = compileScopedFilterToSql(query.where as FilterCondition, OBJECT, {
421+
declaredValueShape,
422+
dialect: cell.id === 'pg' ? 'postgres' : 'sqlite',
423+
coerceTemporalFilterValue: (field, value) => driver.temporalFilterValue(OBJECT, field, value),
424+
coerceTemporalFilterColumn: (field, columnSql) => driver.temporalFilterColumnSql(OBJECT, field, columnSql),
425+
});
426+
const res = await (engine as any).execute(`select "id" from "${OBJECT}" where ${sql}`, { args: params, object: OBJECT });
427+
const rows = Array.isArray(res) ? res : (res as { rows: Array<Record<string, unknown>> }).rows;
422428
expect(ids(rows as Array<Record<string, unknown>>), label).toBe(expected);
423429
}
424430
});

0 commit comments

Comments
 (0)