diff --git a/.changeset/20661-memory-analytics-lte-whole-day-first.md b/.changeset/20661-memory-analytics-lte-whole-day-first.md new file mode 100644 index 00000000000..1b46707f74b --- /dev/null +++ b/.changeset/20661-memory-analytics-lte-whole-day-first.md @@ -0,0 +1,13 @@ +--- +'@objectstack/driver-memory': patch +--- + +fix(driver-memory): a cube `where` `$lte` on a bare day keeps the whole day on a declared `datetime` field (#20661) + +Clause-②: no + +`MemoryAnalyticsService` put a `$lte` comparand into the field's storage form before it applied the whole-day rule for a bare-day upper bound. On a field declared `datetime` (through `syncSchema`) the storage form of `'2026-07-28'` is the instant `'2026-07-28T00:00:00.000Z'`, and the whole-day rule does not widen an instant. So `where: { created_at: { $lte: '2026-07-28' } }` compiled an inclusive bound at that midnight and dropped every row later in the named day, while `find()` with the same filter kept them. `generateSql()` echoed the same narrowed bound. + +Both exits now follow ADR-0053's order: the bare day is widened first, and only the resulting bound is converted to the storage form. On a declared `datetime` field the example compiles `created_at < '2026-07-29T00:00:00.000Z'` and answers the same rows as `find()`. On `9999-12-31`, the last supported day, a declared `datetime` field now asks only for a value (`IS NOT NULL` in the echo), as an undeclared field already did. + +Unchanged: an undeclared field, a declared `date` field, a full timestamp or `Date` comparand (inclusive, as written), and a `timeDimensions[].dateRange` end, which already widened the day before building its bounds. `$between` stays refused on this face (`INVALID_FILTER`, 400). Nothing is removed or renamed, and there is nothing to migrate. diff --git a/packages/drivers/driver-memory/src/memory-analytics-20661-lte-whole-day-first.test.ts b/packages/drivers/driver-memory/src/memory-analytics-20661-lte-whole-day-first.test.ts new file mode 100644 index 00000000000..86b7b989278 --- /dev/null +++ b/packages/drivers/driver-memory/src/memory-analytics-20661-lte-whole-day-first.test.ts @@ -0,0 +1,165 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#20661] The analytics (cube) face widens a bare-day `lte` BEFORE it puts the + * bound into the field's storage form — ADR-0053 D-E3's order, the one `find()` + * already follows (`memory-driver.ts` widens `val`, then converts `nextDay`). + * + * Both `lte` rows (the mingo `$match` and the SQL echo) used to ask + * `nextUtcCalendarDay` about the comparand AFTER the conversion. On an + * undeclared field the storage form is the authored string, so the day + * widened. On a DECLARED `datetime` field it is the instant + * `2026-07-28T00:00:00.000Z`, which the helper correctly refuses to widen, so + * the cube compiled `<= '2026-07-28T00:00:00.000Z'` and dropped the rest of the + * named day — one row where `find()` on the same filter answers two. + * + * Every case drives both exits and `find()` on the same data, for an + * undeclared field and for the same field declared through `syncSchema`. + */ + +import { describe, it, expect } from 'vitest'; +import type { Cube, FilterCondition } from '@objectstack/spec/data'; +import { InMemoryDriver } from './memory-driver.js'; +import { MemoryAnalyticsService } from './memory-analytics.js'; + +const ids = (rows: ReadonlyArray>) => rows.map((r) => String(r.id)).sort(); + +/** The card's two rows: the named day's morning, and the day before. */ +const CARD_ROWS = [ + { id: 'r28', created_at: '2026-07-28T10:00:00.000Z', made_on: '2026-07-28' }, + { id: 'r27', created_at: '2026-07-27T10:00:00.000Z', made_on: '2026-07-27' }, +] as const; + +const CUBE = { + name: 'tasks', + title: 'Tasks', + sql: 'task', + measures: { count: { label: 'Count', type: 'count', sql: 'id' } }, + dimensions: { + id: { label: 'Id', type: 'string', sql: 'id' }, + created_at: { label: 'Created', type: 'time', sql: 'created_at' }, + made_on: { label: 'Made on', type: 'time', sql: 'made_on' }, + }, +} as unknown as Cube; + +type Declaration = 'undeclared' | 'declared'; + +async function setup(declaration: Declaration, rows: ReadonlyArray> = CARD_ROWS) { + const driver = new InMemoryDriver({}); + await driver.connect(); + if (declaration === 'declared') { + await driver.syncSchema('task', { + name: 'task', + fields: { created_at: { type: 'datetime' }, made_on: { type: 'date' } }, + }); + } + for (const row of rows) await driver.create('task', { ...row }); + const service = new MemoryAnalyticsService({ driver, cubes: [CUBE] }); + return { driver, service }; +} + +const cubeQuery = (where: FilterCondition) => + ({ cube: 'tasks', measures: ['count'], dimensions: ['id'], where }) as any; + +/** The echo's WHERE clause alone. */ +const whereOf = (sql: string) => /WHERE (.*?)(?: GROUP BY|$)/.exec(sql)?.[1]; + +/** + * One `where` on all three readings: `find()`, the cube's rows, and the echo. + * The ids are asserted twice on purpose — equal to `find()` (the invariant) and + * equal to the literal list (so the two cannot be wrong together). + */ +async function answer(declaration: Declaration, where: FilterCondition, rows?: ReadonlyArray>) { + const { driver, service } = await setup(declaration, rows); + return { + find: ids(await driver.find('task', { where })), + cube: ids((await service.query(cubeQuery(where))).rows), + echo: whereOf((await service.generateSql(cubeQuery(where))).sql), + }; +} + +describe('[#20661] cube `lte` on a bare day widens the authored day, then converts the bound', () => { + const where: FilterCondition = { created_at: { $lte: '2026-07-28' } }; + + it('undeclared `created_at`: both rows, the half-open bound at the next day', async () => { + const got = await answer('undeclared', where); + expect(got.cube).toEqual(got.find); + expect(got.cube).toEqual(['r27', 'r28']); + expect(got.echo).toBe("created_at < '2026-07-29'"); + }); + + it('declared `datetime` `created_at`: the same rows as find(), the bound in storage form', async () => { + const got = await answer('declared', where); + expect(got.cube).toEqual(got.find); + // Before the fix: ['r27'], echoed as `created_at <= '2026-07-28T00:00:00.000Z'`. + expect(got.cube).toEqual(['r27', 'r28']); + expect(got.echo).toBe("created_at < '2026-07-29T00:00:00.000Z'"); + }); + + it('a full timestamp keeps instant semantics: inclusive, in storage form, not widened', async () => { + const instant: FilterCondition = { created_at: { $lte: '2026-07-28T10:00:00Z' } }; + const got = await answer('declared', instant); + expect(got.cube).toEqual(got.find); + expect(got.cube).toEqual(['r27', 'r28']); + expect(got.echo).toBe("created_at <= '2026-07-28T10:00:00.000Z'"); + }); + + it('declared `date` control: a bare day is its own storage form, so nothing moves', async () => { + const dateWhere: FilterCondition = { made_on: { $lte: '2026-07-28' } }; + for (const declaration of ['undeclared', 'declared'] as const) { + const got = await answer(declaration, dateWhere); + expect(got.cube, declaration).toEqual(got.find); + expect(got.cube, declaration).toEqual(['r27', 'r28']); + expect(got.echo, declaration).toBe("made_on < '2026-07-29'"); + } + }); + + it('declared `datetime` on the last supported day asks only for a value; the day before is a bound', async () => { + const rows = [ + { id: 'c26', created_at: '2026-07-15T14:00:00.000Z' }, + { id: 'prev', created_at: '9999-12-30T10:00:00.000Z' }, + { id: 'mid', created_at: '9999-12-31T10:00:00.000Z' }, + { id: 'last', created_at: '9999-12-31T23:59:59.999Z' }, + ]; + const lastDay = await answer('declared', { created_at: { $lte: '9999-12-31' } }, rows); + expect(lastDay.cube).toEqual(lastDay.find); + // Before the fix: ['c26', 'prev'], echoed as `created_at <= '9999-12-31T00:00:00.000Z'`. + expect(lastDay.cube).toEqual(['c26', 'last', 'mid', 'prev']); + expect(lastDay.echo).toBe('created_at IS NOT NULL'); + + const dayBefore = await answer('declared', { created_at: { $lte: '9999-12-30' } }, rows); + expect(dayBefore.cube).toEqual(dayBefore.find); + expect(dayBefore.cube).toEqual(['c26', 'prev']); + expect(dayBefore.echo).toBe("created_at < '9999-12-31T00:00:00.000Z'"); + }); +}); + +describe('[#20661] the siblings on this face', () => { + it('a `dateRange` bare-day end already widens the authored end: declared `datetime` keeps the whole day', async () => { + const { driver, service } = await setup('declared'); + const result = await service.query({ + cube: 'tasks', + measures: ['count'], + dimensions: ['id'], + timeDimensions: [{ dimension: 'created_at', dateRange: ['2026-07-01', '2026-07-28'] }], + } as any); + const found = ids(await driver.find('task', { where: { created_at: { $gte: '2026-07-01', $lte: '2026-07-28' } } })); + expect(ids(result.rows)).toEqual(found); + expect(ids(result.rows)).toEqual(['r27', 'r28']); + }); + + it('`$between` is refused on both exits, so its maximum never reaches a bound here', async () => { + // Not a lowering this face owns: widening `MONGO_TO_CUBE_OPERATOR` to take + // `$between` turns this red, and its maximum then owes the same whole-day + // order as the `lte` rows (`lteUpperBound`). + const { service } = await setup('declared'); + const between = cubeQuery({ created_at: { $between: ['2026-07-01', '2026-07-28'] } }); + for (const exit of [() => service.query(between), () => service.generateSql(between)]) { + const err = await exit().then( + () => undefined, + (e: unknown) => e as { code?: unknown; status?: unknown }, + ); + expect(err).toMatchObject({ code: 'INVALID_FILTER', status: 400 }); + } + }); +}); diff --git a/packages/drivers/driver-memory/src/memory-analytics.ts b/packages/drivers/driver-memory/src/memory-analytics.ts index f0f5ffb6613..014497594cc 100644 --- a/packages/drivers/driver-memory/src/memory-analytics.ts +++ b/packages/drivers/driver-memory/src/memory-analytics.ts @@ -154,7 +154,7 @@ interface NormalizedCubeFilter { operator: CubeOperator; /** * The comparands, as authored. Temporal values are put into the field's - * storage form at the exits ({@link MemoryAnalyticsService.comparandsFor}), + * storage form at the exits ({@link MemoryAnalyticsService.storageFormFor}), * never here — that rule needs the resolved field path, which only an exit has. */ values: unknown[]; @@ -204,10 +204,65 @@ interface MongoPredicateInput { * Unicode range and would answer `CAFÉ` for `café`. */ readonly asciiSubstring: (value: unknown) => RegExp; + /** + * [#20661] One value put into the storage form of the field this entry + * constrains — the SAME conversion `comparands` came out of + * ({@link MemoryAnalyticsService.storageFormFor}), handed over for the one + * value that is not an authored operand: a bound the builder DERIVES. + * See {@link lteUpperBound}. + */ + readonly storageForm: (value: unknown) => unknown; } type MongoPredicateBuilder = (input: MongoPredicateInput) => Record; +/** + * [#20661] What an `lte` bound compiles to, decided ONCE for both exits — the + * mingo `$match` ({@link CUBE_OPERATOR_TO_MONGO_PREDICATE}) and the SQL echo + * ({@link CUBE_OPERATOR_TO_SQL_PREDICATE}) only render it, so the rows a chart + * is drawn from and the statement shown beside it cannot disagree on it. + * + * - `before` — a bare `YYYY-MM-DD` means the WHOLE day (#4042; the SQL twin is + * #3777): the bound is the next day's midnight, exclusive. + * - `unbounded` — the same on `9999-12-31`, which has no next day (#20600): + * every value is inside the bound, so what is left to ask is a value. + * - `through` — anything else keeps instant semantics, inclusive, compared + * against the authored comparand in its storage form. + * + * ## ⛔ The order is the fix: widen the AUTHORED string, then convert the bound + * + * ADR-0053 D-E3: the calendar-day rewrite is a *calendar* operation and runs + * on the bare-day STRING first; only the resulting bound is converted to the + * storage form. Both rows used to ask {@link nextUtcCalendarDay} about + * `comparands[0]`, which is ALREADY in storage form — on a declared `datetime` + * field that is the instant `2026-07-28T00:00:00.000Z`, which the helper + * correctly refuses to widen, so `$lte: '2026-07-28'` compiled an inclusive + * bound at that midnight and dropped the rest of the day, while `find()` on the + * same filter (which widens `val` and converts `nextDay`, `memory-driver.ts`) + * kept it. On an undeclared field the storage form IS the authored string, + * which is why only the declared case was wrong. ⛔ Never teach + * `nextUtcCalendarDay` to widen an instant instead: it refuses one because an + * instant already says where it stops. + * + * `comparand` is the authored value's storage form, passed in rather than + * recomputed: it already exists, and the `through` arm is exactly it. + */ +type LteUpperBound = + | { readonly kind: 'before'; readonly bound: unknown } + | { readonly kind: 'unbounded' } + | { readonly kind: 'through'; readonly bound: unknown }; + +function lteUpperBound( + authored: unknown, + comparand: unknown, + storageForm: (value: unknown) => unknown, +): LteUpperBound { + const nextDay = nextUtcCalendarDay(authored); + if (isUnboundedAbove(nextDay)) return { kind: 'unbounded' }; + if (nextDay != null) return { kind: 'before', bound: storageForm(nextDay) }; + return { kind: 'through', bound: comparand }; +} + /** * [#5374] How each cube operator becomes a mingo field predicate — the whole * `{$op: …}` object, not the name of an operator. @@ -269,10 +324,13 @@ const CUBE_OPERATOR_TO_MONGO_PREDICATE: Readonly { - const nextDay = nextUtcCalendarDay(comparands[0]); - if (isUnboundedAbove(nextDay)) return { $ne: null }; - return nextDay != null ? { $lt: nextDay } : { $lte: comparands[0] }; + // [#20661] Decided from the AUTHORED value by {@link lteUpperBound}, which + // the SQL twin shares — widening `comparands[0]` read an instant on a + // declared `datetime` field and never widened at all. + lte: ({ raw, comparands, storageForm }) => { + const upper = lteUpperBound(raw[0], comparands[0], storageForm); + if (upper.kind === 'unbounded') return { $ne: null }; + return upper.kind === 'before' ? { $lt: upper.bound } : { $lte: upper.bound }; }, // The list operators take the WHOLE list. An empty one is a real predicate — // `$in: []` selects nothing, `$nin: []` selects everything — and saying so @@ -326,6 +384,11 @@ interface SqlPredicateInput { * See {@link globSubstringPattern} for why GLOB and not LIKE. */ readonly globSubstring: (value: unknown) => string; + /** + * [#20661] The storage-form conversion `comparands` came out of, for a bound + * the builder derives — the twin of {@link MongoPredicateInput.storageForm}. + */ + readonly storageForm: (value: unknown) => unknown; } type SqlPredicateBuilder = (input: SqlPredicateInput) => string; @@ -464,12 +527,15 @@ const CUBE_OPERATOR_TO_SQL_PREDICATE: Readonly { - const nextDay = nextUtcCalendarDay(comparands[0]); - if (isUnboundedAbove(nextDay)) return `${column} IS NOT NULL`; - return nextDay != null - ? `${column} < ${literal(nextDay)}` - : `${column} <= ${literal(comparands[0])}`; + // [#20661] The same {@link lteUpperBound} decision the mingo row renders, so + // on a declared `datetime` field the echo reads `< '2026-07-29T00:00:00.000Z'` + // where it used to read `<= '2026-07-28T00:00:00.000Z'`. + lte: ({ column, raw, comparands, storageForm, literal }) => { + const upper = lteUpperBound(raw[0], comparands[0], storageForm); + if (upper.kind === 'unbounded') return `${column} IS NOT NULL`; + return upper.kind === 'before' + ? `${column} < ${literal(upper.bound)}` + : `${column} <= ${literal(upper.bound)}`; }, // The list operators take the WHOLE list, and an EMPTY one is a real // predicate on this side too — `$in: []` selects nothing, `$nin: []` @@ -628,7 +694,7 @@ function numericAggregandExpr(path: string): Record { * * Every other value on this path already renders faithfully, measured rather * than assumed: a `Date` comparand is canonicalized to an ISO string by - * {@link MemoryAnalyticsService.comparandsFor} before it reaches here, and + * {@link MemoryAnalyticsService.storageFormFor} before it reaches here, and * `toJSON` runs BEFORE a replacer in any case, so dates are unchanged. A * `BigInt` comparand does throw — but out of mingo's own `Query.compile` during * EXECUTION, before this dump is ever built, so no replacer here reaches it. @@ -882,9 +948,11 @@ export class MemoryAnalyticsService implements IAnalyticsService { // FROM: a boolean reaches mingo as a boolean and `null` as `null`, so a // predicate over `is_active` or `closed_at` selects the same rows // `find()` selects instead of none / all of them. + const storageForm = this.storageFormFor(cube, filter.member); const predicate = this.mongoPredicateBuilder(filter.operator)({ - comparands: this.comparandsFor(cube, filter.member, filter.values), + comparands: filter.values.map(storageForm), raw: filter.values, + storageForm, substring: (value) => this.driver.filterSubstringPattern(value), // [#6520] `$icontains`' fold, from the spec's shared definition rather // than from the driver's Unicode-folding `filterSubstringPattern`. @@ -1362,10 +1430,12 @@ export class MemoryAnalyticsService implements IAnalyticsService { const normalizedFilters = this.normalizeFilters(query); for (const filter of normalizedFilters) { const fieldPath = this.resolveFieldPath(cube, filter.member); + const storageForm = this.storageFormFor(cube, filter.member); whereClauses.push(this.sqlPredicateBuilder(filter.operator)({ column: fieldPath, - comparands: this.comparandsFor(cube, filter.member, filter.values), + comparands: filter.values.map(storageForm), raw: filter.values, + storageForm, literal: (value) => this.toSqlLiteral(value), globSubstring: (value) => this.toSqlLiteral(globSubstringPattern(value)), })); @@ -1537,9 +1607,13 @@ export class MemoryAnalyticsService implements IAnalyticsService { } /** - * [#5373] The comparands of one lowered entry, in the storage form of the - * field they are compared against — the ONE place either exit converts a - * value, so the two exits cannot drift apart. + * [#5373] The conversion that puts a value into the storage form of the field + * one lowered entry is compared against — the ONE place either exit converts + * a value, so the two exits cannot drift apart. Each exit maps the entry's + * comparands through it, and hands the same function to its predicate builder + * for the one value a builder derives rather than receives: the whole-day + * bound of an `lte` (#20661, {@link lteUpperBound}), which has to be widened + * from the AUTHORED day before it is converted (ADR-0053 D-E3). * * The only conversion left is the temporal one (#4047): a `datetime` column * holds canonical UTC ISO text, so a `Date` comparand has to become that text @@ -1552,10 +1626,10 @@ export class MemoryAnalyticsService implements IAnalyticsService { * boolean stays a boolean, `null` stays `null`, and a text column's `'100'` * stays the string `'100'` instead of becoming the number `100`. */ - private comparandsFor(cube: Cube, member: string, values: unknown[]): unknown[] { + private storageFormFor(cube: Cube, member: string): (value: unknown) => unknown { const table = this.extractTableName(cube.sql); const fieldPath = this.resolveFieldPath(cube, member); - return values.map(v => this.driver.filterComparandStorageForm(table, fieldPath, v)); + return (value) => this.driver.filterComparandStorageForm(table, fieldPath, value); } /**