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
62 changes: 62 additions & 0 deletions .changeset/objectql-strategy-daterange.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
---
"@objectstack/service-analytics": patch
---

fix(analytics): ObjectQLStrategy applies `timeDimensions[].dateRange` — the predicate every date-bucketed chart was missing (#3650)

`ObjectQLStrategy.execute()` built its engine filter purely from
`normalizeAnalyticsFilters(query)`, which reads only `query.where`. But
`dateRange` is a **sibling** of `where`, never folded into it — so the window
was dropped on the floor. No error, no warning: the chart rendered, and the
numbers were for all of history.

This was not a "some drivers only" corner. `NativeSQLStrategy.canHandle`
declines any query carrying a `granularity`, so a **date-bucketed trend lands on
the ObjectQL path on every driver**, Postgres and SQLite included — and a
bucketed trend is precisely the shape that also carries a range ("last 12
months", "this quarter"). The other two paths always applied it
(`NativeSQLStrategy` as `BETWEEN`, `preview-evaluator` row-wise); only this one
did not.

**Two visible symptoms:**

- A trend chart with a time filter plotted **every row ever recorded** instead
of the selected window.
- `compareTo` (period-over-period) was **structurally dead**. `runCompare`
builds the comparison pass by shifting `dateRange` and changing nothing else,
so with the window ignored both passes issued a byte-identical aggregate:
every `<measure>__compare` column equalled its primary and the delta was a
flat 0%. And since `compareTo` requires a time dimension, it always took this
path.

The window now lowers to an inclusive `{$gte, $lte}` on the resolved field — the
same shape `NativeSQLStrategy` binds as `BETWEEN` and the memory driver builds
as a `$match` — so one dashboard reads the same on every driver. No storage
coercion is applied here on purpose: unlike the raw-SQL path (which had to learn
about SQLite's INTEGER epoch in #2034), this path goes through
`engine.aggregate()`, where the driver's own CRUD filter coercion already
handles a `where` bound on that same column.

**Same-field composition was fixed alongside it**, because the window makes it
routine. Operands merged into one field entry by spreading, which silently kept
whichever came last: a `where` bound and a window bound on `close_date` would
have had one erase the other, and a `where` that names one field twice through
`$and` (`{$and: [{stage: 'won'}, {stage: {$ne: 'lost'}}]}`) already lost its
first operand today. Operands that name **different** operators still share one
entry; colliding ones become their own `$and` conjunct, so the engine
intersects them instead of the strategy picking a winner.

`generateSql()` renders the window as a parameterised `BETWEEN` to match — its
comment previously explained why a `BETWEEN` was deliberately absent, which was
correct only while `execute()` dropped the window. Bounds bind as `$n`
placeholders, never inlined: the echoed statement travels to the browser.

A window on a **cross-object** time dimension is still rejected, and is now
reported as the bucketing error it is rather than as the "cross-object filter"
its lowered predicate would otherwise resemble. `execute()` and
`/analytics/sql` continue to accept and reject the same set.

Relative-phrase ranges ("Last 7 days") are still not resolved on this path, and
a bare-string `dateRange` degenerates to a single point — both matching
`NativeSQLStrategy` exactly, rather than inventing a second interpretation for
the driver-independent path.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* The load-bearing assumption behind #3650.
*
* `ObjectQLStrategy` lowers `timeDimensions[].dateRange` to `{$gte, $lte}` ISO
* strings and hands them to `engine.aggregate()` WITHOUT the storage coercion
* `NativeSQLStrategy` performs. The justification is that this path goes through
* the driver's own CRUD filter coercion, whereas the raw-SQL path binds straight
* into a statement and therefore had to learn about SQLite's INTEGER epoch
* itself (#2034).
*
* That is an assumption about the driver, not about the strategy — so it is
* pinned here against a real SQLite database. A `Field.datetime` column is
* stored as INTEGER epoch ms; an uncoerced ISO TEXT comparand matches NOTHING
* (TEXT sorts after every INTEGER), which would have made the #3650 fix a no-op
* on exactly the column type dashboards use most.
*
* `Field.date` (ISO TEXT storage) is covered alongside it, since a dataset may
* bucket on either.
*/

import { describe, it, expect, beforeEach, afterEach } from 'vitest';
import { SqlDriver } from '../src/index.js';

const TABLE = 'opportunity';

describe('SqlDriver.aggregate — ISO window over epoch-stored datetime (#3650)', () => {
let driver: SqlDriver;

beforeEach(async () => {
driver = new SqlDriver({
client: 'better-sqlite3',
connection: { filename: ':memory:' },
useNullAsDefault: true,
});

await driver.initObjects([
{
name: TABLE,
fields: {
closed_at: { type: 'datetime' }, // INTEGER epoch ms under better-sqlite3
closed_on: { type: 'date' }, // YYYY-MM-DD TEXT
amount: { type: 'number' },
},
},
]);

// Inserted as real Date objects, the path the seed loader takes.
const rows = [
['o1', '2025-11-15T09:00:00Z', '2025-11-15', 900], // before window
['o2', '2026-01-10T09:00:00Z', '2026-01-10', 100], // inside
['o3', '2026-01-20T09:00:00Z', '2026-01-20', 200], // inside
['o4', '2026-02-14T09:00:00Z', '2026-02-14', 30], // inside (Feb)
['o5', '2026-06-05T09:00:00Z', '2026-06-05', 700], // after window
] as const;
for (const [id, at, on, amount] of rows) {
await driver.create(
TABLE,
{ id, closed_at: new Date(at), closed_on: on, amount },
{ bypassTenantAudit: true },
);
}
});

afterEach(async () => {
await driver.disconnect();
});

/** Exactly what the strategy emits: an inclusive ISO range, uncoerced. */
const window = (col: string) => ({
[col]: { $gte: '2026-01-01', $lte: '2026-02-28' },
});

it('confines a datetime aggregate to the window (the epoch-affinity trap)', async () => {
const rows = await driver.aggregate(TABLE, {
aggregations: [
{ function: 'sum', field: 'amount', alias: 'total' },
{ function: 'count', alias: 'n' },
],
where: window('closed_at'),
} as any);

// 330 over 3 rows. An uncoerced TEXT-vs-INTEGER compare yields 0 rows;
// a dropped window yields 1930 over 5.
expect(Number(rows[0].total)).toBe(330);
expect(Number(rows[0].n)).toBe(3);
});

it('buckets a TEXT-stored date inside the window — the shape #3650 is about', async () => {
// A `granularity` is what makes NativeSQLStrategy decline, so window +
// bucketing together is the combination that reaches the ObjectQL path.
const rows = await driver.aggregate(TABLE, {
groupBy: [{ field: 'closed_on', dateGranularity: 'month' }],
aggregations: [{ function: 'sum', field: 'amount', alias: 'total' }],
where: window('closed_on'),
} as any);

const byMonth = Object.fromEntries(rows.map((r: any) => [r.closed_on, Number(r.total)]));
expect(byMonth).toEqual({ '2026-01': 300, '2026-02': 30 });
});

it('KNOWN GAP — bucketing an EPOCH-stored datetime collapses into one null bucket', async () => {
// Pre-existing, unrelated to #3650, and deliberately NOT fixed here — but
// it lands on the exact same query shape, so it is pinned rather than left
// to be rediscovered as "the dateRange fix did nothing".
//
// SQLite advertises `queryDateGranularity.month`, so `engine.aggregate`
// pushes the bucketing down to the driver — `engine.ts` only falls back to
// in-memory bucketing when a granularity is UNSUPPORTED or a non-UTC
// timezone is in play, neither of which applies here. The dialect
// expression is `strftime('%Y-%m', col)`, and SQLite reads a bare INTEGER
// as a Julian day number; an epoch-ms value is far outside the legal range,
// so every row buckets as NULL.
const rows = await driver.aggregate(TABLE, {
groupBy: [{ field: 'closed_at', dateGranularity: 'month' }],
aggregations: [{ function: 'sum', field: 'amount', alias: 'total' }],
where: window('closed_at'),
} as any);

// The WINDOW works — the total is the in-window 330, not the full 1930 —
// which is what #3650 is responsible for. The BUCKETS are what is broken.
// When that is fixed, this becomes `{ '2026-01': 300, '2026-02': 30 }`.
const byMonth = Object.fromEntries(rows.map((r: any) => [String(r.closed_at), Number(r.total)]));
expect(byMonth).toEqual({ null: 330 });
});

it('confines a date (TEXT-stored) aggregate to the same window', async () => {
const rows = await driver.aggregate(TABLE, {
aggregations: [{ function: 'sum', field: 'amount', alias: 'total' }],
where: window('closed_on'),
} as any);

expect(Number(rows[0].total)).toBe(330);
});

it('excludes the whole table when the window selects nothing', async () => {
const rows = await driver.aggregate(TABLE, {
aggregations: [{ function: 'count', alias: 'n' }],
where: { closed_at: { $gte: '2027-01-01', $lte: '2027-12-31' } },
} as any);

// An empty window must read as zero, not as "filter ignored → 5".
expect(Number(rows[0]?.n ?? 0)).toBe(0);
});
});
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* A FilterCondition may carry a `$and` array ALONGSIDE plain field keys, and
* the `$and` elements may nest further. Both are legal per
* `FilterConditionSchema` (`$and` recurses; the schema intersects a record with
* the logical-operator object), and `applyFilterCondition` handles them by
* iterating every key — but nothing exercised the combination end-to-end, so
* "the compiler happens to iterate all keys" was an implementation detail
* rather than a contract.
*
* It became a contract with #3650: ObjectQLStrategy now AND-composes predicates
* that cannot share one field entry (a `where` bound colliding with a
* `timeDimensions[].dateRange` window) into `filter.$and`, and `withReadScope`
* then wraps THAT in another `$and` with the tenant predicate. The exact
* two-level shape below is what the analytics path hands the driver. Silently
* dropping either level would widen the query — the analytics path's read scope
* lives in the outer one.
*/

import { describe, it, expect, beforeEach, afterEach } from 'vitest';
import { SqlDriver } from '../src/index.js';

/** One row survives all three predicates; each other row fails exactly one. */
const FIXTURE = [
{ id: 'a', close_date: '2026-01-05', organization_id: 'org_A', amount: 10 }, // fails inner-sibling $gte
{ id: 'b', close_date: '2026-01-20', organization_id: 'org_A', amount: 20 }, // survives
{ id: 'c', close_date: '2026-02-10', organization_id: 'org_A', amount: 40 }, // fails nested $lte
{ id: 'd', close_date: '2026-01-20', organization_id: 'org_B', amount: 80 }, // fails outer tenant
];

/**
* `{ $and: [ {field…, $and: [ … ]}, {tenant} ] }` — a field key and a nested
* `$and` inside one branch of an outer `$and`.
*/
const NESTED_AND = {
$and: [
{
close_date: { $gte: '2026-01-15' },
$and: [{ close_date: { $gte: '2026-01-01', $lte: '2026-01-31' } }],
},
{ organization_id: 'org_A' },
],
};

describe('SqlDriver — field key alongside a nested $and (#3650)', () => {
let driver: SqlDriver;
let knex: any;

beforeEach(async () => {
driver = new SqlDriver({
client: 'better-sqlite3',
connection: { filename: ':memory:' },
useNullAsDefault: true,
});
knex = (driver as any).knex;
await knex.schema.createTable('opportunity', (t: any) => {
t.string('id').primary();
t.string('close_date');
t.string('organization_id');
t.float('amount');
});
await knex('opportunity').insert(FIXTURE);
});

afterEach(async () => {
await knex.destroy();
});

it('intersects every level on find()', async () => {
const rows = await driver.find('opportunity', { where: NESTED_AND } as any);
expect(rows.map((r: any) => r.id)).toEqual(['b']);
});

it('intersects every level on aggregate() — the analytics path', async () => {
const rows = await driver.aggregate('opportunity', {
groupBy: ['organization_id'],
aggregations: [{ function: 'sum', field: 'amount', alias: 'total' }],
where: NESTED_AND,
} as any);

// 20 only. Dropping the nested `$and` would add row c (60); dropping the
// sibling field key would add row a (30); dropping the outer branch would
// add row d and split the grouping.
expect(rows).toEqual([{ organization_id: 'org_A', total: 20 }]);
});

it('keeps a bucketed aggregate scoped to the same intersection', async () => {
// The #3650 shape in its native habitat: a date-bucketed trend whose window
// and read scope both have to survive.
const rows = await driver.aggregate('opportunity', {
groupBy: [{ field: 'close_date', dateGranularity: 'month' }],
aggregations: [{ function: 'sum', field: 'amount', alias: 'total' }],
where: NESTED_AND,
} as any);

expect(rows).toEqual([{ close_date: '2026-01', total: 20 }]);
});
});
Loading
Loading