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
15 changes: 15 additions & 0 deletions .changeset/21595-sqlite-week-bucket.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
---
'@objectstack/driver-sql': patch
'@objectstack/service-analytics': patch
---

On SQLite, a `week` date bucket is grouped in SQL, and the analytics SQL echo never prints a bucket statement that SQLite refuses (#21595).

Clause-②: no

- **What was wrong.** `driver-sql` grouped `day`, `month`, `quarter` and `year` in SQL on SQLite, but not `week`. Its `supports.queryDateGranularity` said `week: false`, so the engine bucketed weeks in memory, and the ObjectQL face of `POST /api/v1/analytics/query` and `POST /api/v1/analytics/sql` echoed the bucket as `date_trunc('week', col)`. SQLite has no `date_trunc`, so that echo could not run. A non-UTC `timezone` on SQLite gave the same echo for every granularity.
- **What it does now.**
- SQLite advertises all five granularities. `week` buckets as `YYYY-Www`, the ISO 8601 week that the PostgreSQL and MySQL arms answer. The expression does not use `strftime('%V')`, which needs SQLite 3.46: `@libsql/client` 0.18.0 bundles SQLite 3.45.1, where `%V` answers NULL. It runs on better-sqlite3, on libSQL (`driver-turso`) and on sql.js (`driver-sqlite-wasm`). A `Field.date` still buckets as its own calendar day.
- The echo prints that expression for a `week` bucket on SQLite, and the statement runs.
- With a non-UTC `timezone` on SQLite, `POST /api/v1/analytics/sql` refuses with `NOT_IMPLEMENTED` / 501, declared as a refusal so its message reaches the caller. `POST /api/v1/analytics/query` still answers the rows, and its answer carries no `sql`. The engine buckets on that zone's calendar in memory, and SQLite has no time-zone database, so no SQLite statement produces those keys.
- **Where it shows.** `aggregate()` with a `week` group on SQLite, `SqlDriver.dateBucketSql()`, and the analytics SQL echo. A query sent with `timezone: 'UTC'`, or with no `timezone`, still echoes the driver's own expression.
6 changes: 3 additions & 3 deletions content/docs/data-modeling/queries.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -677,9 +677,9 @@ A `groupBy` entry is either a bare field name or a structured node that buckets

`dateGranularity` accepts `day` | `week` | `month` | `quarter` | `year`. The engine
pushes bucketing down to the driver only when that driver advertises the granularity
via `supports.queryDateGranularity` — SQLite reports `week: false`, for instance, so a
weekly bucket falls back to fetching rows and bucketing in memory. The fallback is
transparent to the query — you get the same buckets either way.
via `supports.queryDateGranularity` — the Turso remote transport reports none, for
instance, so every bucket there falls back to fetching rows and bucketing in memory. The
fallback is transparent to the query — you get the same buckets either way.

<Callout type="warn">
Buckets are computed in **UTC**. A non-UTC reference timezone is only reachable through
Expand Down
16 changes: 8 additions & 8 deletions packages/drivers/driver-mongodb/src/mongodb-aggregation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,14 +57,14 @@ export interface AggregationInput {
* `mongodb-date-bucket-parity.test.ts` pins the identity so a future edit to one
* cannot silently miss the other.
*
* ## Why all five, when `driver-sql` on SQLite advertises four
*
* SQLite has no ISO-week format specifier, so `driver-sql` sets `week: false`
* and lets the engine bucket weeks in memory. MongoDB's `$dateToString` has both
* halves of the ISO-8601 week date — `%G` (ISO week-YEAR) and `%V` (ISO week
* number, zero-padded to 2) — which is exactly the label
* `bucketDateValue` computes by hand. The dialect difference is real, so the
* records differ.
* ## Why `week` too
*
* MongoDB's `$dateToString` has both halves of the ISO-8601 week date — `%G`
* (ISO week-YEAR) and `%V` (ISO week number, zero-padded to 2) — which is
* exactly the label `bucketDateValue` computes by hand. (`driver-sql` on
* SQLite advertises all five too, but cannot use `%V`: the SQLite libSQL
* bundles predates it, so its `week` arm computes the ISO week from the
* week's Thursday instead.)
*
* ⚠️ **Documentation-derived, not observed** (#5517). Every `$dateToString`
* format specifier and every `$convert`/`$concat`/`$switch` null rule this
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -294,8 +294,8 @@ describe('the published capability record and the lowering are the same set', ()

it('advertises exactly the five granularities @objectstack/spec declares', () => {
// MongoDB has both halves of the ISO-8601 week date (`%G`/`%V`), which is why
// this record carries `week: true` where `driver-sql` on SQLite carries
// `week: false` — the dialects genuinely differ, and the records say so.
// this record carries `week: true`. (`driver-sql` on SQLite carries it too,
// computing the ISO week without `%V`.)
expect(MONGODB_DATE_GRANULARITIES).toEqual({
day: true, week: true, month: true, quarter: true, year: true,
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,9 @@
* `DateGranularity.options` — the spec's own list — and a granularity the spec
* grows joins it without an edit here.
*
* ⚠️ A dialect that does not bucket a granularity natively (SQLite + `week`,
* which is capped in `dateGranularityCapabilities` because `%V` needs SQLite
* 3.46) is NOT a failure and is asserted as its own declared answer: the #6212
* ⚠️ A dialect that does not bucket a granularity natively (none of the three
* cells since SQLite gained `week` in #21595; a client `driver-sql` does not
* model declines every one) is NOT a failure and is asserted as its own declared answer: the #6212
* `NOT_IMPLEMENTED`/501 capability refusal, which `engine.aggregate` reads off
* `supports.queryDateGranularity` and serves in memory instead. The invariant
* that spans both answers is the one this card is about: no shape answers
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,11 @@ const GRANULARITIES: readonly Granularity[] = ['day', 'week', 'month', 'quarter'
/**
* The granularities each dialect buckets in SQL. Declared rather than read off
* the driver, and checked against it below, so the set the tests iterate cannot
* shrink without a red. SQLite buckets `week` in memory (#21595 owns that).
* shrink without a red. Every dialect buckets all five: SQLite's `week` arm
* landed with #21595.
*/
const BUCKETED_IN_SQL: Record<DialectId, readonly Granularity[]> = {
sqlite: ['day', 'month', 'quarter', 'year'],
sqlite: GRANULARITIES,
pg: GRANULARITIES,
mysql: GRANULARITIES,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,10 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest';
import { SqlDriver } from '../src/index.js';
import { LegacyStorageDriver } from '../src/legacy-datetime-storage.testkit.js';

type Granularity = 'day' | 'month' | 'quarter' | 'year';
type Granularity = 'day' | 'week' | 'month' | 'quarter' | 'year';

/** Every granularity SQLite advertises natively (week is bucketed in-memory). */
const GRANULARITIES: Granularity[] = ['day', 'month', 'quarter', 'year'];
/** Every granularity SQLite advertises natively: all five, `week` included since #21595. */
const GRANULARITIES: Granularity[] = ['day', 'week', 'month', 'quarter', 'year'];

/**
* ⚠️ Keep in sync with `packages/objectql/src/in-memory-aggregation.ts#bucketDateValue`.
Expand All @@ -54,6 +54,17 @@ function bucketDateValue(value: unknown, g: Granularity): string | null {
case 'quarter': return `${y}-Q${Math.floor((m - 1) / 3) + 1}`;
case 'month': return `${y}-${String(m).padStart(2, '0')}`;
case 'day': return `${y}-${String(m).padStart(2, '0')}-${String(d.getUTCDate()).padStart(2, '0')}`;
case 'week': {
// The same ISO-week arm as `sql-driver-date-bucket.test.ts`'s copy.
const target = new Date(Date.UTC(y, d.getUTCMonth(), d.getUTCDate()));
const dayNum = (target.getUTCDay() + 6) % 7;
target.setUTCDate(target.getUTCDate() - dayNum + 3);
const firstThursday = new Date(Date.UTC(target.getUTCFullYear(), 0, 4));
const weekNo = 1 + Math.round(
((target.getTime() - firstThursday.getTime()) / 86400000 - 3 + ((firstThursday.getUTCDay() + 6) % 7)) / 7,
);
return `${target.getUTCFullYear()}-W${String(weekNo).padStart(2, '0')}`;
}
}
}

Expand Down Expand Up @@ -243,6 +254,18 @@ describe('SqlDriver date bucketing over a MIXED-form datetime column (#3773)', (
expect(byMonth['1970-01']).toBeUndefined(); // TEXT never divided by 1000
});

it('[#21595] buckets each row by its own stored form at `week` too, where the repair is read twice', async () => {
// The `week` arm references the column twice, so the legacy repair is
// expanded twice and binds twice as many identifiers. A binding shifted
// between the two copies would put a row in another week, or in 1970.
expect(await bucketSums(driver, 'closed_at', 'week')).toEqual({
'2026-W02': 1, // INTEGER epoch ms, Saturday 2026-01-10
'2026-W07': 2, // ISO TEXT, Saturday 2026-02-14
'2026-W08': 4, // zone-naive TEXT, Friday 2026-02-20
[EMPTY]: 8,
});
});

it('leaves a NULL instant in its own bucket', async () => {
const byMonth = await bucketSums(driver, 'closed_at', 'month');
// SQL NULL propagates through the bucket expression, so the row lands in the
Expand Down
75 changes: 44 additions & 31 deletions packages/drivers/driver-sql/src/sql-driver-date-bucket.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,12 +107,13 @@ describe('SqlDriver date bucket (dateGranularity)', () => {
expect(caps.month).toBe(true);
expect(caps.quarter).toBe(true);
expect(caps.year).toBe(true);
// SQLite-specific: ISO week (%V) is not assumed.
expect(caps.week).toBe(false);
// [#21595] SQLite buckets the ISO week too: the Thursday rule through
// date modifiers, so no `%V` (which needs SQLite 3.46) is assumed.
expect(caps.week).toBe(true);
});
});

describe.each<Granularity>(['day', 'month', 'quarter', 'year'])(
describe.each<Granularity>(['day', 'week', 'month', 'quarter', 'year'])(
'granularity=%s — native SQL matches bucketDateValue',
(g) => {
it('produces the same label set as the in-memory reference', async () => {
Expand Down Expand Up @@ -141,35 +142,47 @@ describe('SqlDriver date bucket (dateGranularity)', () => {

describe('unsupported granularity', () => {
/**
* [#6212] The subject is unchanged — week is not bucketed in SQL on SQLite,
* so the engine must be pushed back to in-memory bucketing — but the refusal
* now carries a wire identity, so the assertion moved with it. It used to be
* `rejects.toThrow(/dateGranularity 'week' not supported/)`: a bare `Error`
* with `code`/`status` both `undefined`, which `mapDataError` served as an
* opaque 500 for a named capability gap. `code` and `status` are asserted
* here for the #6144 reason — the un-fixed driver threw for this input too,
* so a `toThrow()` alone was green before and after and could never see the
* defect. The remote face's twin, and the parity between them, live in
* driver-turso's `remote-transport-groupby-node.test.ts`.
* [#6212] A granularity a dialect has no bucket expression for is refused
* with a wire identity, so the engine is pushed back to in-memory
* bucketing. It used to be `rejects.toThrow(/dateGranularity 'week' not
* supported/)`: a bare `Error` with `code`/`status` both `undefined`, which
* `mapDataError` served as an opaque 500 for a named capability gap. `code`
* and `status` are asserted here for the #6144 reason — the un-fixed driver
* threw for this input too, so a `toThrow()` alone was green before and
* after and could never see the defect. The remote face's twin, and the
* parity between them, live in driver-turso's
* `remote-transport-groupby-node.test.ts`.
*
* [#21595] This used to be SQLite `week`. SQLite now buckets all five, so
* the pin moved to the population that still reaches the refusal: a client
* this driver does not model, whose capability row is empty. `mssql` is
* one (`tedious` is a declared dependency), and the refusal is raised while
* the statement is built, so no server is needed.
*/
it('refuses week on SQLite with NOT_IMPLEMENTED / 501 (so engine routes to in-memory)', async () => {
const err = await driver
.aggregate('events', {
groupBy: [{ field: 'ts', dateGranularity: 'week' }],
aggregations: [{ function: 'count', alias: 'n' }],
})
.then(
() => { throw new Error('expected the driver to refuse week on SQLite'); },
(e) => e as Error & { code?: string; status?: number },
);

expect(err.code).toBe('NOT_IMPLEMENTED');
expect(err.status).toBe(501);
expect(err.message.startsWith("Date bucketing by 'week' is not supported by this backend.")).toBe(true);
// The message names what this dialect DOES bucket, so a reader is told
// where the boundary is rather than only that they crossed it.
expect(err.message).toContain('Bucketed here: day, month, quarter, year');
expect(err.message).toContain('supports.queryDateGranularity');
it('refuses a granularity on a client with no bucket expression, with NOT_IMPLEMENTED / 501', async () => {
const unmodeled = new SqlDriver({ client: 'mssql' });
try {
expect(unmodeled.supports.queryDateGranularity).toEqual({});
const err = await unmodeled
.aggregate('events', {
groupBy: [{ field: 'ts', dateGranularity: 'week' }],
aggregations: [{ function: 'count', alias: 'n' }],
})
.then(
() => { throw new Error('expected the driver to refuse week on mssql'); },
(e) => e as Error & { code?: string; status?: number },
);

expect(err.code).toBe('NOT_IMPLEMENTED');
expect(err.status).toBe(501);
expect(err.message.startsWith("Date bucketing by 'week' is not supported by this backend.")).toBe(true);
// The message names what this dialect DOES bucket, so a reader is told
// where the boundary is rather than only that they crossed it.
expect(err.message).toContain("Bucketed here: none (dialect 'mssql')");
expect(err.message).toContain('supports.queryDateGranularity');
} finally {
await unmodeled.disconnect().catch(() => {});
}
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,9 @@ describe('temporalFilterValue dialect gating', () => {
* bucketing silently — the loud failure SQLite did not give us.
*/
describe('buildDateBucketExpr dialect gating (#3773)', () => {
const GRANULARITIES = ['day', 'month', 'quarter', 'year'] as const;
// [#21595] `week` joined when SQLite gained a `week` arm: every dialect now
// renders all five, so each gate below holds for the new expression too.
const GRANULARITIES = ['day', 'week', 'month', 'quarter', 'year'] as const;
const expr = (d: ProbeDriver, field: string, g: string, table?: string) =>
(d as any).buildDateBucketExpr(field, g, table) as { sql: string; bindings: any[] } | null;

Expand Down Expand Up @@ -190,6 +192,11 @@ describe('buildDateBucketExpr dialect gating (#3773)', () => {
expect(expr(d, 'anything', g, 't')!.sql).not.toContain('unixepoch');
// No table key at all (a caller outside the aggregate path) → plain form.
expect(expr(d, 'at', g)!.sql).not.toContain('unixepoch');
// [#21595] A `Field.date` is its calendar day on SQLite, as on the other
// dialects since #21485: no arm, `week` included, reads it through a zone.
for (const zoned of [`'localtime'`, `'utc'`]) {
expect(expr(d, 'on', g, 't')!.sql.toLowerCase(), `sqlite ${g}`).not.toContain(zoned);
}
}
});

Expand Down Expand Up @@ -231,7 +238,7 @@ describe('buildDateBucketExpr dialect gating (#3773)', () => {
for (const client of ['pg', 'mysql2']) {
const d = makeDriver(client);
d.seedDate('t', 'on');
for (const g of [...GRANULARITIES, 'week']) {
for (const g of GRANULARITIES) {
const sql = expr(d, 'on', g, 't')!.sql.toLowerCase();
for (const zoned of ['timestamptz', 'time zone', 'convert_tz', 'time_zone']) {
expect(sql, `${client} ${g}`).not.toContain(zoned);
Expand Down
35 changes: 27 additions & 8 deletions packages/drivers/driver-sql/src/sql-driver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6101,9 +6101,14 @@ export class SqlDriver implements IDataDriver {
return { day: true, month: true, quarter: true, year: true, week: true };
}
if (this.isSqlite) {
// SQLite's strftime gained ISO week (%V) in 3.46 (2024-05-23); play it safe
// and bucket week in-memory. Day/month/year/quarter are universally available.
return { day: true, month: true, quarter: true, year: true, week: false };
// [#21595] All five, `week` included. The ISO week is computed by the
// Thursday rule through date modifiers (the `week` arm of
// `buildDateBucketExpr`), never `strftime('%V')` / `'%G'`: those arrived
// in SQLite 3.46, and libSQL (`driver-turso`) bundles an older engine.
// Measured: better-sqlite3 13.0.3 runs 3.53.4, `@libsql/client` 0.18.0
// runs 3.45.1 (where `%V` answers NULL), sql.js 1.14.2
// (`driver-sqlite-wasm`) runs 3.49.1. The expression runs on all three.
return { day: true, month: true, quarter: true, year: true, week: true };
}
return {};
}
Expand Down Expand Up @@ -6185,7 +6190,22 @@ export class SqlDriver implements IDataDriver {
case 'month': return fmt('%Y-%m');
case 'day': return fmt('%Y-%m-%d');
case 'quarter': return { sql: `(strftime('%Y', ${arg}) || '-Q' || ((cast(strftime('%m', ${arg}) as integer) - 1) / 3 + 1))`, bindings: [...argBindings, ...argBindings] };
case 'week': return null; // see capabilities note
case 'week': {
// [#21595] The ISO 8601 week, `YYYY-Www`: the label the PostgreSQL
// arm's `IYYY"-W"IW` and the MySQL arm's `%x-W%v` answer. A week
// belongs to the year of its Thursday. `'-3 days', 'weekday 4'` moves
// any day to the Thursday of its Monday-start week (a Monday moves
// +3, a Sunday -3), so that Thursday's `%Y` is the week-numbering
// year and `(%j - 1) / 7 + 1` of it is the week number. Both
// modifiers predate every SQLite this platform ships, unlike `%V`
// (see `dateGranularityCapabilities`). No zone is read, so a
// `Field.date` stays its calendar day.
const thursday = (f: string) => `strftime('${f}', ${arg}, '-3 days', 'weekday 4')`;
return {
sql: `(${thursday('%Y')} || '-W' || printf('%02d', (cast(${thursday('%j')} as integer) - 1) / 7 + 1))`,
bindings: [...argBindings, ...argBindings],
};
}
}
}

Expand All @@ -6195,10 +6215,9 @@ export class SqlDriver implements IDataDriver {
/**
* [#21441] The date-bucket expression this dialect groups `field` by at
* `granularity` (the one {@link aggregate} runs), rendered as SQL text, or
* `null` where {@link buildDateBucketExpr} has none: a granularity this
* dialect buckets in memory (`week` on SQLite, see
* {@link dateGranularityCapabilities}), or a client this driver does not
* model.
* `null` where {@link buildDateBucketExpr} has none: a client this driver
* does not model, whose {@link dateGranularityCapabilities} row is empty.
* SQLite, PostgreSQL and MySQL render all five granularities.
*
* It is for callers that PRINT the statement an aggregate stands for rather
* than run it. `service-analytics`' ObjectQL face echoes a date-bucketed
Expand Down
Loading
Loading