Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .changeset/20148-aggregation-filter-where-doors.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
---
"@objectstack/objectql": minor
---
Expand All @@ -24,6 +24,6 @@

Not refused, but answering differently:

- **A `Date` bound is compared as an instant.** `{ opened_at: { $gt: new Date('2026-02-01') } }` against the ISO text a `datetime` column holds counted no row: JS compared the `Date` with the string by coercion. `$ne` and `$nin` counted every row, and a `$between` of two `Date`s counted every row. The same bound in a `where` counted 4 of 6 on both drivers. The comparison now reads the pair through `@objectstack/spec/data`'s `utcInstantMs`, the lift `@objectstack/formula`'s evaluator applies, whenever one side is a `Date` and both sides denote an instant. Every `datetime` row and a UTC-midnight `Date` on a `date` field now count what the same bound counts in a `where`. `having` shares this comparison, so a `Date` bound in `having` keeps the groups its ISO spelling keeps on a `datetime` column; on a `date`-class column (`min` / `max` of a `date` field) a UTC-midnight `Date` follows the calendar-day reading a `where` gives (`{ $gte: new Date('2026-02-01') }` keeps the group whose max is `2026-02-01`, and so does `$eq`), which the ISO-instant text, compared as text, did not. A `Date` is not JSON, so this reaches in-process callers only. Two readings still differ from a `where`, because the per-row comparison holds no declaration: a `Date` carrying a time of day against a `date` field, which a `where` reads as that UTC calendar day, and a `Date` against a `time` field, which is not an instant and is left as before.
- **A `Date` bound is compared as an instant.** `{ opened_at: { $gt: new Date('2026-02-01') } }` against the ISO text a `datetime` column holds counted no row: JS compared the `Date` with the string by coercion. `$ne` and `$nin` counted every row, and a `$between` of two `Date`s counted every row. The same bound in a `where` counted 4 of 6 on both drivers. The comparison now reads the pair through `@objectstack/spec/data`'s `utcInstantMs`, the lift `@objectstack/formula`'s evaluator applies, whenever one side is a `Date` and both sides denote an instant. Every `datetime` row and a UTC-midnight `Date` on a `date` field now count what the same bound counts in a `where`. `having` shares this comparison, so a `Date` bound in `having` keeps the groups its ISO spelling keeps on a `datetime` column; on a `date`-class column (`min` / `max` of a `date` field) a UTC-midnight `Date` follows the calendar-day reading a `where` gives (`{ $gte: new Date('2026-02-01') }` keeps the group whose max is `2026-02-01`, and so does `$eq`), which the ISO-instant text, compared as text, did not. A `Date` is not JSON, so this reaches in-process callers only. Two readings still differed from a `where` in this change alone, because the per-row comparison held no declaration (#20176 closes both in the same release, reading every temporal comparand at this position by the column's storage rule): a `Date` carrying a time of day against a `date` field, which a `where` reads as that UTC calendar day, and a `Date` against a `time` field, which is not an instant and is left as before.

Not changed, measured identical before and after on both drivers: every `where`, `groupBy` and `having` answer outside the `Date` shapes above, and a per-aggregation filter that uses implicit equality, ordering bounds, `$in`, `$between`, `$or`, `{}`, a `{ $field }` between two declared numeric fields, `addDays` between two `date` or two `datetime` fields (literal or read from a numeric column), and a reference to `created_at` or `id`.
35 changes: 35 additions & 0 deletions .changeset/20176-aggregation-temporal-storage-rule.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
---
"@objectstack/core": minor
"@objectstack/objectql": minor
"@objectstack/driver-sql": patch
"@objectstack/driver-memory": patch
---

fix(objectql,core): a per-aggregation `filter` and `having` on `engine.aggregate` read a temporal comparand by the column's storage rule, the rule `where` already applies — one function, `temporalStorageForm`, now exported by `@objectstack/core` and shared by both drivers (#20176)

A per-aggregation `filter` (`aggregations[i].filter`) and `having` are evaluated by the engine itself, over the rows (or aggregated rows) a driver returns. Both compared a temporal comparand exactly as written, while the same condition as a `where` is put into the column's storage form by the driver first. So they counted differently. Measured through `engine.aggregate` and through `POST /data/:object/query`, on `driver-memory` and `driver-sql`, over six rows:

| in `aggregations[i].filter` (or `having`) | before | now, and the `where` twin |
|:--|:--|:--|
| an ISO instant on a `date` field, `{ placed_on: { $gte: '2026-02-01T00:00:00.000Z' } }` | 1 | 3 |
| the same instant under `$eq` | 0 | 2 |
| a bare day as the upper bound of a `datetime`, `{ opened_at: { $lte: '2026-02-01' } }`, or as a `$between` max | 2 | 3 |
| an epoch-millisecond bound on a `datetime` | 0 | 3 |
| a `Date` carrying a time of day on a `date` field, `$gte` / `$lt` / `$eq` (in-process only) | 1 / 5 / 0 | 3 / 3 / 2 |
| a `Date` on a `time` field (in-process only) | 0 | 3 |
| `having` on `max` of a `date` field with an ISO-instant bound | kept one group | keeps the two groups whose day is on or after it |

The same holds for `$ne`, `$in` / `$nin` members, `$between` endpoints, implicit equality, an offset instant (`'…T18:00:00+08:00'`), an epoch-millisecond string, a zone-naive `'2026-02-01T10:00'`, and a short wall clock (`'11:00'`) or an ISO instant on a `time` field. On a `having` column, the class comes from the query, as the `addDays` rule already reads it: `min` / `max` take the class of the field they read, a `groupBy` projection takes its field's, and a `day` date bucket is a `date`.

What the rule does, now in one place:

- A comparand, and the row's value, are put into the column's storage form: canonical UTC ISO text for `datetime`, `YYYY-MM-DD` for `date`, and `HH:MM:SS` (`.fff` only when non-zero) for `time`.
- A bare `YYYY-MM-DD` used as the upper bound of a `datetime` (`$lte`, a `$between` max) means that whole day, as it does in a `where` (ADR-0053 D-D). On a `date` or `time` column it is not widened.
- A value the rule cannot read is compared as written, and so is every non-temporal column, presence tests (`$exists`, `$null`), the text operators and a `{ $field }` reference.
- An object whose declared fields the engine cannot see keeps the previous comparison.

`@objectstack/core` exports the rule as `temporalStorageForm(value, kind)`, `kind` being `'datetime' | 'date' | 'time'`. `driver-sql` (`canonicalUtcDatetime`, `toDateOnly`, `canonicalTimeOfDay`) and `driver-memory` (`coerceTemporalValue`) each carried a copy of it; both now call it. The copies agreed on every shape measured when they were lifted, so no `where`, write or read answer of either driver changes. MySQL still binds a `datetime` in its own literal spelling.

`@objectstack/objectql`'s `applyInMemoryAggregation(rows, ast, timezone?, fields?)` takes the object's declared field map as an optional fourth argument, and a per-aggregation `filter` reads a temporal comparand by the rule only when it is given. Called without it, the function answers as before.

Not changed, measured identical before and after on both drivers: every `where` answer, every refusal a per-aggregation `filter` or `having` gives, and every per-aggregation `filter` and `having` cell whose column is not temporal.
7 changes: 7 additions & 0 deletions packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,13 @@ export * from './utils/analytics-date-range-conformance.js';
// on each other.
export * from './utils/temporal-comparand.js';

// [#20176] …and the storage FORM that column's rule puts a value in — the one
// rule `driver-sql` and `driver-memory` apply to a `where` comparand (and on
// write), and `@objectstack/objectql` applies to a per-aggregation `filter` and
// `having`, which it evaluates itself. Here because the drivers and the engine
// do not depend on each other, and each driver used to carry its own copy.
export * from './utils/temporal-storage-form.js';

// [#12350 / ADR-0126 §4] THE activation-ledger row contract, parameterized by
// `metadata_type`. Same reason as the two entries above: its consumers —
// `@objectstack/objectql` (packaged actions) and
Expand Down
14 changes: 7 additions & 7 deletions packages/core/src/utils/temporal-comparand.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,9 @@
* ## Interpretability is defined by the DRIVERS' own totals, deliberately
*
* Each predicate below mirrors the total function that would receive the value
* if the door let it through — `storageDatetimeValue` / `storageDateValue` /
* `storageTimeValue` in `driver-memory`, and their `SqlDriver` twins. Those
* functions are total on purpose: an input they cannot interpret is returned
* if the door let it through — `temporalStorageForm` (`temporal-storage-form.ts`,
* the one rule both drivers read since #20176; before it, a copy in each).
* That function is total on purpose: an input it cannot interpret is returned
* UNCHANGED rather than becoming an invented instant. So "the driver would
* return it unchanged" IS the definition of uninterpretable, and defining it
* any other way would refuse comparands that work today.
Expand Down Expand Up @@ -87,7 +87,7 @@ export function temporalComparandKind(fieldType: unknown): TemporalComparandKind
}

/**
* `storageDatetimeValue`'s reading, as a yes/no.
* `temporalStorageForm`'s `datetime` reading, as a yes/no.
*
* Mirrors it step for step: a bare integer in either sign is epoch
* milliseconds; a bare `YYYY-MM-DD` is midnight UTC; a zone-naive
Expand All @@ -105,11 +105,11 @@ function readsAsInstant(s: string): boolean {
}

/**
* `storageDateValue`'s reading of a STRING: its leading `YYYY-MM-DD`, and
* `temporalStorageForm`'s `date` reading of a STRING: its leading `YYYY-MM-DD`, and
* nothing else.
*
* Narrower than {@link readsAsInstant} on purpose, because the rule it mirrors
* is narrower: `storageDateValue` collapses a leading calendar day and returns
* is narrower: the `date` rule collapses a leading calendar day and returns
* every other string untouched. `2026/07/15` is therefore uninterpretable for a
* `date` column even though `Date.parse` reads it — today it survives to the
* driver and compares as text against `YYYY-MM-DD` values, which is the silent
Expand All @@ -120,7 +120,7 @@ function readsAsCalendarDay(s: string): boolean {
}

/**
* `storageTimeValue`'s reading: a bare wall clock whose components are in
* `temporalStorageForm`'s `time` reading: a bare wall clock whose components are in
* range. Out-of-range (`25:00`) is uninterpretable — the rule it mirrors
* returns such a value untouched rather than wrapping it.
*/
Expand Down
89 changes: 89 additions & 0 deletions packages/core/src/utils/temporal-storage-form.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
//
// [#20176] `temporalStorageForm` — the ONE storage rule a temporal comparand is
// put in before it is compared: by `driver-sql` and `driver-memory` on `where`
// (and on write), and by `@objectstack/objectql` on a per-aggregation `filter`
// and `having`. Each driver carried a word-for-word copy until it was lifted
// here; the drivers' own suites pin that their `where` coercion IS this
// function (`memory-temporal-storage-form.test.ts`,
// `sql-driver-temporal-storage-form.test.ts`). This file pins the rule itself.

import { describe, it, expect } from 'vitest';
import { temporalStorageForm } from './temporal-storage-form.js';

describe('temporalStorageForm — datetime: canonical UTC ISO text', () => {
const cases: ReadonlyArray<readonly [string, unknown, unknown]> = [
['a Date', new Date('2026-02-01T10:00:00.000Z'), '2026-02-01T10:00:00.000Z'],
['epoch milliseconds', 1769940000000, '2026-02-01T10:00:00.000Z'],
['an epoch-millisecond string, padded', ' 1769940000000 ', '2026-02-01T10:00:00.000Z'],
['a negative epoch string', '-86400000', '1969-12-31T00:00:00.000Z'],
['a bare day — midnight UTC, never the host zone', '2026-02-01', '2026-02-01T00:00:00.000Z'],
['a zone-naive wall clock — read AS UTC', '2026-02-01 09:00', '2026-02-01T09:00:00.000Z'],
['a zone-naive T spelling with a long fraction', '2026-02-01T09:00:00.123456', '2026-02-01T09:00:00.123Z'],
['an ISO instant without milliseconds', '2026-02-01T10:00:00Z', '2026-02-01T10:00:00.000Z'],
['an offset instant — re-spelled in Z', '2026-02-01T18:00:00+08:00', '2026-02-01T10:00:00.000Z'],
];
for (const [name, input, expected] of cases) {
it(name, () => expect(temporalStorageForm(input, 'datetime')).toBe(expected));
}
});

describe('temporalStorageForm — date: the UTC calendar day', () => {
const cases: ReadonlyArray<readonly [string, unknown, unknown]> = [
['an ISO instant — its leading day', '2026-02-01T00:00:00.000Z', '2026-02-01'],
['a Date carrying a time of day — its UTC day', new Date('2026-02-01T23:30:00.000Z'), '2026-02-01'],
['a bare day, padded', ' 2026-02-01 ', '2026-02-01'],
['a zone-naive timestamp', '2026-02-01 23:59', '2026-02-01'],
];
for (const [name, input, expected] of cases) {
it(name, () => expect(temporalStorageForm(input, 'date')).toBe(expected));
}

it('epoch milliseconds are not a calendar day — returned as they came', () => {
expect(temporalStorageForm(1769940000000, 'date')).toBe(1769940000000);
});
});

describe('temporalStorageForm — time: the UTC wall clock, .fff only when non-zero', () => {
const cases: ReadonlyArray<readonly [string, unknown, unknown]> = [
['a short wall clock', '09:00', '09:00:00'],
['a zero-millisecond suffix is trimmed', '09:00:00.000', '09:00:00'],
['a fraction is kept to milliseconds', '09:00:00.5', '09:00:00.500'],
['a Date — its UTC time of day', new Date('2026-02-01T11:00:00.000Z'), '11:00:00'],
['an ISO instant — its UTC time of day', '2026-02-01T11:00:00.250Z', '11:00:00.250'],
['a bare day — its midnight', '2026-02-01', '00:00:00'],
];
for (const [name, input, expected] of cases) {
it(name, () => expect(temporalStorageForm(input, 'time')).toBe(expected));
}
});

describe('temporalStorageForm — total: what the rule cannot read comes back unchanged', () => {
const invalid = new Date('nope');
const untouched: ReadonlyArray<readonly [string, unknown]> = [
['null', null],
['undefined', undefined],
['the empty string', ''],
['whitespace', ' '],
['junk', 'not-a-date'],
['a preset name', 'last_30_days'],
['a placeholder', '{today}'],
['a boolean', true],
['an Invalid Date', invalid],
['NaN', Number.NaN],
];
for (const kind of ['datetime', 'date', 'time'] as const) {
for (const [name, input] of untouched) {
it(`${kind}: ${name}`, () => expect(temporalStorageForm(input, kind)).toBe(input));
}
}

it('time: an out-of-range wall clock is not wrapped', () => {
expect(temporalStorageForm('25:00', 'time')).toBe('25:00');
});

it('a list is NOT mapped — a caller comparing a list maps its members', () => {
const list = ['2026-02-01', '2026-02-02'];
expect(temporalStorageForm(list, 'datetime')).toBe(list);
});
});
Loading
Loading