diff --git a/.changeset/24618-date-bucket-calendar-day.md b/.changeset/24618-date-bucket-calendar-day.md new file mode 100644 index 00000000000..895e03056af --- /dev/null +++ b/.changeset/24618-date-bucket-calendar-day.md @@ -0,0 +1,11 @@ +--- +'@objectstack/objectql': patch +--- + +fix(objectql): a `date` dimension buckets on its own calendar day under every timezone on the in-memory path, as the SQL pushdown path already did + +Clause-②: no (no authorable key, export or signature moves; the in-memory answer for a `date` dimension under a non-UTC zone now equals the stored day, which is what the pushdown path answers) + +- **A `Field.date` is a calendar day, not an instant (ADR-0053).** When `engine.aggregate` buckets in memory (any non-UTC `timezone` forces that path), a `groupBy` item with a `dateGranularity` on a field the object declares a `date` is now labelled by that field's own `YYYY-MM-DD`, with no zone. Before, the value was read as that day's UTC midnight and placed on the requested zone's calendar, so in every zone west of UTC each row landed one day early: `2026-10-01` bucketed as `2026-09-30`, and month, quarter and year totals moved across their edges (`POST /api/v1/analytics/query` with `timezone: "America/New_York"` answered `{2026-09: 1, 2026-10: 5, 2026-11: 4}` for six October and four November deliveries). +- **A `datetime` keeps the zone.** An instant still lands on the requested zone's calendar day, unchanged. So does a field the declaration cannot type: `applyInMemoryAggregation` called without its `fields` argument, an undeclared field, or a `formula`. +- **The value is read by the `date` storage rule first** (`temporalStorageForm(value, 'date')` in `@objectstack/core`): a bare `YYYY-MM-DD` is itself, and a `Date` handed back by a driver is its UTC calendar day. diff --git a/packages/core/src/utils/datetime.ts b/packages/core/src/utils/datetime.ts index 674356907b5..323d3b5aa0a 100644 --- a/packages/core/src/utils/datetime.ts +++ b/packages/core/src/utils/datetime.ts @@ -290,6 +290,16 @@ export function isBucketGranularity(value: unknown): value is BucketGranularity * built from those parts — the parts already carry the zone shift, so the week * boundary lands correctly without re-applying any offset. * + * [#24618] `value` is an INSTANT, and the zone places it on a calendar. A + * `Field.date` value is not one: it is a timezone-naive calendar day (ADR-0053, + * "never converted to an instant and never timezone-shifted"), and handed in + * with a zone it is read as that day's UTC midnight and lands on the PREVIOUS + * day in every zone west of UTC. A caller bucketing a `date` therefore passes + * no `timezone` — after putting the value into the `date` storage form + * (`temporalStorageForm(value, 'date')`) — which is what + * `@objectstack/objectql`'s rows path does for a field its object declares a + * `date`. The field type is the caller's to read; this function never sees it. + * * A finite NUMBER is read as epoch milliseconds — the form SQLite stores a * `Field.datetime` in, and what any driver that hands back raw storage values * yields. `new Date(String(1767225600000))` is an Invalid Date, so without this diff --git a/packages/objectql/src/engine-aggregate-timezone.test.ts b/packages/objectql/src/engine-aggregate-timezone.test.ts index 322df73d4f1..cae834cd4bc 100644 --- a/packages/objectql/src/engine-aggregate-timezone.test.ts +++ b/packages/objectql/src/engine-aggregate-timezone.test.ts @@ -61,9 +61,14 @@ async function makeEngine(rows: any[]) { const engine = new ObjectQL(); engine.registerDriver(driver, true); await engine.init(); + // [#24618] `closed_at` holds INSTANTS, so it is declared a `datetime` — the + // type whose buckets follow the reference zone. Declared a `date` (as it + // was), the rows path buckets it by its calendar day under every zone + // (ADR-0053), which is what in-memory-aggregation-date-calendar-day.test.ts + // pins; the routing this file is about is the same for both types. engine.registry.registerObject({ name: 'deal', - fields: { closed_at: { type: 'date' }, amount: { type: 'number' } }, + fields: { closed_at: { type: 'datetime' }, amount: { type: 'number' } }, } as any); return { engine, nativeCalls }; } diff --git a/packages/objectql/src/in-memory-aggregation-date-calendar-day.test.ts b/packages/objectql/src/in-memory-aggregation-date-calendar-day.test.ts new file mode 100644 index 00000000000..32227c2c023 --- /dev/null +++ b/packages/objectql/src/in-memory-aggregation-date-calendar-day.test.ts @@ -0,0 +1,244 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#24618] A `Field.date` dimension buckets on its OWN calendar day under every + * reference timezone — on the in-memory rows path exactly as on the pushdown + * path. + * + * ADR-0053: a `date` is a timezone-naive calendar day, "never converted to an + * instant and never timezone-shifted"; a `datetime` keeps the requested zone. + * The rows path used to hand a `date` value to `bucketDateKey` WITH the zone, + * which read `'2026-10-01'` as the instant `2026-10-01T00:00Z` and placed it on + * that zone's calendar: `2026-09-30` in America/New_York. Measured at the + * public door (`POST /api/v1/analytics/query`, stock showcase): every day key + * one day early, month `{2026-09: 1, 2026-10: 5, 2026-11: 4}` for a stored + * `{2026-10: 6, 2026-11: 4}`, quarter `{2026-Q3: 1, 2026-Q4: 9}` for + * `{2026-Q4: 10}` — while the SQL path, taken at UTC, answered the stored days. + * + * The expected labels below are LITERALS, the calendar facts of each stored + * day — never computed by the labeller under test. + * + * Three parts: + * (a) the rows path, declared `date`, under a negative-offset, a zero and a + * positive-offset zone, at every granularity — and the `Date` value shape; + * (b) the control: a declared `datetime` still follows the zone; + * (c) `engine.aggregate`: the pushdown route (taken only at UTC) and the rows + * route (forced by any other zone) answer ONE set of labels for a `date`. + * The pushdown side here is a driver double answering the literal labels; + * the REAL SQL faces are held to the same labels at UTC by + * `checkDateBucketParity` (`@objectstack/verify`), so (c) + that check + * are the parity under a non-UTC zone. + */ + +import { describe, it, expect } from 'vitest'; +import type { EngineAggregateOptions } from '@objectstack/spec/data'; +import { ObjectQL } from './engine.js'; +import { applyInMemoryAggregation } from './in-memory-aggregation.js'; + +type Granularity = 'day' | 'week' | 'month' | 'quarter' | 'year'; +const GRANULARITIES: readonly Granularity[] = ['day', 'week', 'month', 'quarter', 'year']; + +/** + * Each stored day's label at each granularity — its own calendar, no zone. + * Chosen for the edges a one-day-early shift crosses: a quarter and month start + * (10-01), two ISO-week Mondays (10-05, 11-02), a month start on a Sunday + * (11-01) and a year start whose ISO week is the previous year's 53rd (2027-01-01). + */ +const STORED_DAY_LABELS: Readonly>>> = { + '2026-10-01': { day: '2026-10-01', week: '2026-W40', month: '2026-10', quarter: '2026-Q4', year: '2026' }, + '2026-10-05': { day: '2026-10-05', week: '2026-W41', month: '2026-10', quarter: '2026-Q4', year: '2026' }, + '2026-10-13': { day: '2026-10-13', week: '2026-W42', month: '2026-10', quarter: '2026-Q4', year: '2026' }, + '2026-10-21': { day: '2026-10-21', week: '2026-W43', month: '2026-10', quarter: '2026-Q4', year: '2026' }, + '2026-10-31': { day: '2026-10-31', week: '2026-W44', month: '2026-10', quarter: '2026-Q4', year: '2026' }, + '2026-11-01': { day: '2026-11-01', week: '2026-W44', month: '2026-11', quarter: '2026-Q4', year: '2026' }, + '2026-11-02': { day: '2026-11-02', week: '2026-W45', month: '2026-11', quarter: '2026-Q4', year: '2026' }, + '2026-11-16': { day: '2026-11-16', week: '2026-W47', month: '2026-11', quarter: '2026-Q4', year: '2026' }, + '2026-11-30': { day: '2026-11-30', week: '2026-W49', month: '2026-11', quarter: '2026-Q4', year: '2026' }, + '2027-01-01': { day: '2027-01-01', week: '2026-W53', month: '2027-01', quarter: '2027-Q1', year: '2027' }, +}; + +/** The empty bucket's label, chosen so no `String(value)` can produce it (#3839). */ +const EMPTY = '‹empty bucket›'; + +/** + * Ten deliveries in the shape of the door's fixture (six in October, four in + * November, so the stored month and quarter counts are the door's expected + * ones), plus a year-edge row and an unset day. + */ +const DUE_DATES: ReadonlyArray = [ + '2026-10-01', '2026-10-05', '2026-10-13', '2026-10-21', '2026-10-21', '2026-10-31', + '2026-11-01', '2026-11-02', '2026-11-16', '2026-11-30', + '2027-01-01', + null, +]; + +/** `{label: count}` per granularity, for the rows above, as literals. */ +const EXPECTED: Readonly>>> = { + day: { + '2026-10-01': 1, '2026-10-05': 1, '2026-10-13': 1, '2026-10-21': 2, '2026-10-31': 1, + '2026-11-01': 1, '2026-11-02': 1, '2026-11-16': 1, '2026-11-30': 1, '2027-01-01': 1, [EMPTY]: 1, + }, + week: { + '2026-W40': 1, '2026-W41': 1, '2026-W42': 1, '2026-W43': 2, '2026-W44': 2, + '2026-W45': 1, '2026-W47': 1, '2026-W49': 1, '2026-W53': 1, [EMPTY]: 1, + }, + month: { '2026-10': 6, '2026-11': 4, '2027-01': 1, [EMPTY]: 1 }, + quarter: { '2026-Q4': 10, '2027-Q1': 1, [EMPTY]: 1 }, + year: { '2026': 10, '2027': 1, [EMPTY]: 1 }, +}; + +const FIELDS = { + due_date: { type: 'date' }, + shipped_at: { type: 'datetime' }, + n: { type: 'number' }, +} as const; + +const ROWS = DUE_DATES.map((due_date, i) => ({ id: `d${i}`, due_date, n: 1 })); + +/** A negative-offset pair, the zero zone, and a positive-offset zone. */ +const ZONES = ['America/New_York', 'America/Los_Angeles', 'UTC', 'Asia/Tokyo'] as const; + +function bucketAst(field: string, granularity: Granularity): EngineAggregateOptions { + return { + groupBy: [{ field, dateGranularity: granularity }], + aggregations: [{ function: 'count', alias: 'n' }], + }; +} + +function labelCounts(rows: any[], field: string): Record { + const out: Record = {}; + for (const row of rows) { + const v = row?.[field]; + out[v == null ? EMPTY : String(v)] = Number(row?.n ?? 0); + } + return out; +} + +describe('(a) the rows path buckets a declared `date` by its stored day under every zone (#24618)', () => { + for (const tz of ZONES) { + for (const granularity of GRANULARITIES) { + it(`${granularity} under ${tz}`, () => { + const out = applyInMemoryAggregation(ROWS, bucketAst('due_date', granularity), tz, FIELDS); + expect(labelCounts(out, 'due_date')).toEqual(EXPECTED[granularity]); + }); + } + } + + it('the door\'s expected month and quarter counts, under America/New_York', () => { + const month = labelCounts( + applyInMemoryAggregation(ROWS.slice(0, 10), bucketAst('due_date', 'month'), 'America/New_York', FIELDS), + 'due_date', + ); + const quarter = labelCounts( + applyInMemoryAggregation(ROWS.slice(0, 10), bucketAst('due_date', 'quarter'), 'America/New_York', FIELDS), + 'due_date', + ); + expect(month).toEqual({ '2026-10': 6, '2026-11': 4 }); + expect(quarter).toEqual({ '2026-Q4': 10 }); + }); + + it('a `Date` value of a `date` field buckets by its UTC calendar day — the `date` storage rule', () => { + // Some drivers hand a `date` back as a `Date` at that day's UTC midnight; + // `temporalStorageForm(value, 'date')` reads it as its UTC calendar day. + const asDates = ROWS.map((r) => ({ ...r, due_date: r.due_date === null ? null : new Date(`${r.due_date}T00:00:00.000Z`) })); + for (const granularity of GRANULARITIES) { + const out = applyInMemoryAggregation(asDates, bucketAst('due_date', granularity), 'America/New_York', FIELDS); + expect(labelCounts(out, 'due_date')).toEqual(EXPECTED[granularity]); + } + }); + + it('an alias names the output column only — the declaration is read off `field`', () => { + const out = applyInMemoryAggregation( + ROWS, + { + groupBy: [{ field: 'due_date', dateGranularity: 'month', alias: 'due_month' }], + aggregations: [{ function: 'count', alias: 'n' }], + }, + 'America/Los_Angeles', + FIELDS, + ); + expect(labelCounts(out, 'due_month')).toEqual(EXPECTED.month); + }); +}); + +describe('(b) control: a declared `datetime` keeps the reference zone (#24618)', () => { + // Two instants either side of the New York midnight that opens 2026-10-01 + // (04:00Z, EDT −04), and one an hour into 2026-11-01 UTC. + const SHIPPED = [ + { id: 's1', shipped_at: '2026-10-01T02:00:00.000Z', n: 1 }, // NY 2026-09-30 22:00 + { id: 's2', shipped_at: '2026-10-01T15:00:00.000Z', n: 1 }, // NY 2026-10-01 11:00 + { id: 's3', shipped_at: '2026-11-01T01:00:00.000Z', n: 1 }, // NY 2026-10-31 21:00 + ]; + + it('under America/New_York an instant lands on the New York calendar day', () => { + const day = applyInMemoryAggregation(SHIPPED, bucketAst('shipped_at', 'day'), 'America/New_York', FIELDS); + expect(labelCounts(day, 'shipped_at')).toEqual({ '2026-09-30': 1, '2026-10-01': 1, '2026-10-31': 1 }); + const quarter = applyInMemoryAggregation(SHIPPED, bucketAst('shipped_at', 'quarter'), 'America/New_York', FIELDS); + expect(labelCounts(quarter, 'shipped_at')).toEqual({ '2026-Q3': 1, '2026-Q4': 2 }); + }); + + it('under UTC the same instants land on the UTC calendar day', () => { + const day = applyInMemoryAggregation(SHIPPED, bucketAst('shipped_at', 'day'), 'UTC', FIELDS); + expect(labelCounts(day, 'shipped_at')).toEqual({ '2026-10-01': 2, '2026-11-01': 1 }); + }); +}); + +describe('(c) engine.aggregate answers one set of labels for a `date` on both routes (#24618)', () => { + /** + * A driver advertising native bucketing at every granularity. Its native + * `aggregate` is the pushdown route: it labels each row by the literal table + * above — what a SQL face answers for a `date` column, where no zone is ever + * applied — and counts how often it ran, so each case can assert the route. + */ + function makeEngine() { + let nativeCalls = 0; + const driver: any = { + name: 'date-bucket-pushdown-double', + version: '0.0.0', + supports: { queryDateGranularity: { day: true, week: true, month: true, quarter: true, year: true } }, + async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; }, + async find() { return ROWS.map((r) => ({ ...r })); }, + async findOne() { return null; }, + async create(_o: string, d: any) { return d; }, + async update(_o: string, _id: string, d: any) { return d; }, + async delete() { return true; }, + async count() { return ROWS.length; }, + async bulkCreate(_o: string, r: any[]) { return r; }, + async bulkUpdate() { return []; }, async bulkDelete() {}, + async beginTransaction() { return { __trx: true, commit: async () => {}, rollback: async () => {} }; }, + async commit() {}, async rollback() {}, + async aggregate(_object: string, ast: any) { + nativeCalls += 1; + const { field, dateGranularity } = ast.groupBy[0]; + const counts = new Map(); + for (const r of ROWS) { + const v = (r as Record)[field] as string | null; + const label = v == null ? null : STORED_DAY_LABELS[v][dateGranularity as Granularity]; + counts.set(label, (counts.get(label) ?? 0) + 1); + } + return Array.from(counts, ([label, n]) => ({ [field]: label, n })); + }, + }; + const engine = new ObjectQL(); + engine.registerDriver(driver, true); + return { engine, driver, nativeCalls: () => nativeCalls }; + } + + for (const granularity of GRANULARITIES) { + it(`${granularity}: the rows route under every non-UTC zone equals the pushdown route at UTC`, async () => { + const { engine, nativeCalls } = makeEngine(); + await engine.init(); + engine.registry.registerObject({ name: 'delivery', fields: FIELDS } as any); + + const pushedDown = await engine.aggregate('delivery', { ...bucketAst('due_date', granularity), timezone: 'UTC' }); + expect(nativeCalls()).toBe(1); // the pushdown route + expect(labelCounts(pushedDown, 'due_date')).toEqual(EXPECTED[granularity]); + + for (const tz of ZONES.filter((z) => z !== 'UTC')) { + const inMemory = await engine.aggregate('delivery', { ...bucketAst('due_date', granularity), timezone: tz }); + expect(nativeCalls()).toBe(1); // a non-UTC zone forced the rows route + expect(labelCounts(inMemory, 'due_date')).toEqual(labelCounts(pushedDown, 'due_date')); + } + }); + } +}); diff --git a/packages/objectql/src/in-memory-aggregation.ts b/packages/objectql/src/in-memory-aggregation.ts index c0101c667a0..62caf1e3d8e 100644 --- a/packages/objectql/src/in-memory-aggregation.ts +++ b/packages/objectql/src/in-memory-aggregation.ts @@ -84,7 +84,7 @@ // every entry: labels fell back to raw ids, and cross-object rebucketing filed // every row under `'(restricted)'` while the grand total still reconciled. -import { bucketDateKey, compensatedSum } from '@objectstack/core'; +import { bucketDateKey, compensatedSum, temporalStorageForm } from '@objectstack/core'; import type { QueryAST, GroupByNode, AggregationNode, DateGranularityValue } from '@objectstack/spec/data'; import { aggregationFilterClause, @@ -104,6 +104,14 @@ import { * It is only consulted by `groupBy` items carrying a `dateGranularity`; an * unset or `'UTC'` value keeps the historical UTC bucketing. * + * [#24618] …and only for a field holding an INSTANT. A `groupBy` field that + * `fields` declares a `date` (`Field.date`, a timezone-naive calendar day under + * ADR-0053, "never converted to an instant and never timezone-shifted") is + * bucketed by its own calendar day under every `timezone` + * (`bucketCalendarDay` below) — the label the pushed-down SQL gives the same + * column. A `datetime`, and a field the declaration cannot type (absent + * `fields`, an undeclared field, a `formula`), keep the reference zone. + * * [#20176] `fields` is the object's declared field map. A per-aggregation * `filter` reads a temporal comparand by the column's storage rule when it is * handed one — the rule the driver applies to the same comparand in a `where` @@ -173,13 +181,16 @@ export function applyInMemoryAggregation( return [aggregateBucket(rows, aggregations, filterClasses, filterJsonStored)]; } + // [#24618] Read once per call, off the declaration, never off a row. + const calendarDayFields = calendarDayGroupFields(groupBy, fields); + const buckets = new Map; rows: any[] }>(); for (const row of rows) { const key: Record = {}; const parts: string[] = []; for (const g of groupBy) { const fieldName = typeof g === 'string' ? g : (g.alias ?? g.field); - const value = projectGroupValue(row, g, timezone); + const value = projectGroupValue(row, g, timezone, calendarDayFields); key[fieldName] = value; // Type-preserving, because the key no longer is: `1` and `'1'`, `true` and // `'true'`, `null` and `'null'` are all distinct groups and must not merge @@ -216,11 +227,65 @@ function bucketIdPart(v: unknown): string { return JSON.stringify(v) ?? `${typeof v}:${String(v)}`; // undefined / symbol / function } -function projectGroupValue(row: any, g: GroupByNode, timezone?: string): unknown { +/** + * [#24618] The date-bucketed `groupBy` fields the object DECLARES a calendar + * day — a `CALENDAR_DATE_TYPES` type, read through the same classification the + * per-aggregation `filter` reads a comparand by ({@link declaredFieldClasses}). + * No usable field map (a registry-less host) ⇒ none, and every date bucket + * keeps the reference zone, as before. + */ +function calendarDayGroupFields( + groupBy: GroupByNode[], + fields: Record | undefined, +): ReadonlySet { + const out = new Set(); + if (!fields) return out; + let classes: ReturnType | undefined; + for (const g of groupBy) { + if (typeof g === 'string' || !g?.dateGranularity) continue; + classes ??= declaredFieldClasses(fields); + if (classes.get(g.field) === 'date') out.add(g.field); + } + return out; +} + +/** + * [#24618] The bucket key of a `Field.date` value: its own calendar day's, with + * NO reference timezone. + * + * ADR-0053 makes a `date` a timezone-naive calendar day, "never converted to an + * instant and never timezone-shifted"; a reference zone decides which calendar + * day an INSTANT falls on, and a calendar day has no instant to place. The + * value is first put into the `date` storage form by `temporalStorageForm` — + * THE storage rule, the one the drivers write the column with — so a bare + * `YYYY-MM-DD` stays itself and a `Date` (a driver that hands one back) becomes + * its UTC calendar day. `bucketDateKey` then labels that day with no zone: it + * reads a bare `YYYY-MM-DD` as that day's UTC midnight (the ECMA-262 date-only + * form is UTC on every host) and takes the UTC parts, which are the day itself. + * + * That is the label the pushed-down SQL gives the same column, where no zone is + * applied to a `date` at all. Before, the value went to `bucketDateKey` WITH + * the zone, which read `'2026-10-01'` as the instant `2026-10-01T00:00Z` and + * placed it on that zone's calendar — `2026-09-30` in every zone west of UTC — + * so the two paths disagreed on every row of a `date` dimension, and month / + * quarter / year totals moved across their edges. + */ +function bucketCalendarDay(value: unknown, granularity: DateGranularityValue): string | null { + return bucketDateKey(temporalStorageForm(value, 'date'), granularity); +} + +function projectGroupValue( + row: any, + g: GroupByNode, + timezone: string | undefined, + calendarDayFields: ReadonlySet, +): unknown { const field = typeof g === 'string' ? g : g.field; const v = row?.[field]; if (typeof g !== 'string' && g.dateGranularity) { - return bucketDateValue(v, g.dateGranularity, timezone); + return calendarDayFields.has(field) + ? bucketCalendarDay(v, g.dateGranularity) + : bucketDateValue(v, g.dateGranularity, timezone); } // The value as `driver.find()` presented it — these rows ARE find() output, so // passing it through is what makes the bucket key equal the column's own read