From 7a8bd5e5340e54fd0fd0672b201e0c1696125efb Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 3 Oct 2026 18:24:23 +0000 Subject: [PATCH 1/4] fix(driver-sql): bucket the ISO week natively on SQLite, by the Thursday rule SQLite's capability row now advertises `week`, and `buildDateBucketExpr`'s SQLite arm renders `YYYY-Www` through the `'-3 days', 'weekday 4'` date modifiers instead of `%V`, which libSQL's bundled SQLite 3.45.1 lacks. Claude-Session: https://claude.ai/code/session_017ErfyP2Rx7XWHJA27QjyUi Co-authored-by: Claude --- ...-aggregate-alias-single-identifier.test.ts | 6 +- ...ver-21485-date-bucket-calendar-day.test.ts | 5 +- .../sql-driver-date-bucket-storage.test.ts | 29 ++++++- .../src/sql-driver-date-bucket.test.ts | 75 +++++++++++-------- .../src/sql-driver-temporal-dialect.test.ts | 11 ++- packages/drivers/driver-sql/src/sql-driver.ts | 35 +++++++-- 6 files changed, 112 insertions(+), 49 deletions(-) diff --git a/packages/drivers/driver-sql/src/sql-driver-13714-aggregate-alias-single-identifier.test.ts b/packages/drivers/driver-sql/src/sql-driver-13714-aggregate-alias-single-identifier.test.ts index 96ff0e50f09..ef19f1527a1 100644 --- a/packages/drivers/driver-sql/src/sql-driver-13714-aggregate-alias-single-identifier.test.ts +++ b/packages/drivers/driver-sql/src/sql-driver-13714-aggregate-alias-single-identifier.test.ts @@ -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 diff --git a/packages/drivers/driver-sql/src/sql-driver-21485-date-bucket-calendar-day.test.ts b/packages/drivers/driver-sql/src/sql-driver-21485-date-bucket-calendar-day.test.ts index 604cb310d8a..fe911fd94f9 100644 --- a/packages/drivers/driver-sql/src/sql-driver-21485-date-bucket-calendar-day.test.ts +++ b/packages/drivers/driver-sql/src/sql-driver-21485-date-bucket-calendar-day.test.ts @@ -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 = { - sqlite: ['day', 'month', 'quarter', 'year'], + sqlite: GRANULARITIES, pg: GRANULARITIES, mysql: GRANULARITIES, }; diff --git a/packages/drivers/driver-sql/src/sql-driver-date-bucket-storage.test.ts b/packages/drivers/driver-sql/src/sql-driver-date-bucket-storage.test.ts index 42770a03d23..62bf7935723 100644 --- a/packages/drivers/driver-sql/src/sql-driver-date-bucket-storage.test.ts +++ b/packages/drivers/driver-sql/src/sql-driver-date-bucket-storage.test.ts @@ -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`. @@ -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')}`; + } } } @@ -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 diff --git a/packages/drivers/driver-sql/src/sql-driver-date-bucket.test.ts b/packages/drivers/driver-sql/src/sql-driver-date-bucket.test.ts index fbf079191ba..84231c88b8f 100644 --- a/packages/drivers/driver-sql/src/sql-driver-date-bucket.test.ts +++ b/packages/drivers/driver-sql/src/sql-driver-date-bucket.test.ts @@ -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(['day', 'month', 'quarter', 'year'])( + describe.each(['day', 'week', 'month', 'quarter', 'year'])( 'granularity=%s — native SQL matches bucketDateValue', (g) => { it('produces the same label set as the in-memory reference', async () => { @@ -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', connection: { server: '127.0.0.1' } }); + 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(() => {}); + } }); }); diff --git a/packages/drivers/driver-sql/src/sql-driver-temporal-dialect.test.ts b/packages/drivers/driver-sql/src/sql-driver-temporal-dialect.test.ts index 4fdfff96856..7b68d84eeff 100644 --- a/packages/drivers/driver-sql/src/sql-driver-temporal-dialect.test.ts +++ b/packages/drivers/driver-sql/src/sql-driver-temporal-dialect.test.ts @@ -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; @@ -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); + } } }); @@ -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); diff --git a/packages/drivers/driver-sql/src/sql-driver.ts b/packages/drivers/driver-sql/src/sql-driver.ts index be60fd4699d..5f6b143dbe1 100644 --- a/packages/drivers/driver-sql/src/sql-driver.ts +++ b/packages/drivers/driver-sql/src/sql-driver.ts @@ -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 {}; } @@ -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], + }; + } } } @@ -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 From d55f4fd20f0fffc5575a7db0a13feb12fa1e4476 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 3 Oct 2026 18:47:55 +0000 Subject: [PATCH 2/4] test(driver-turso,driver-sqlite-wasm,dogfood): run SQLite's week bucket on libSQL and sql.js R2: the remote face's inherited `dateBucketSql` row is now pinned by running the rendered expression on a libSQL `file:` client beside the local face. The pins that held SQLite's `week` refusal move to a client driver-sql does not model, which still declines every granularity. Claude-Session: https://claude.ai/code/session_017ErfyP2Rx7XWHJA27QjyUi Co-authored-by: Claude --- .../sqlite-wasm-driver-date-bucket.test.ts | 35 +++------ .../src/remote-transport-groupby-node.test.ts | 61 ++++++++-------- .../turso-remote-inherited-members.test.ts | 72 +++++++++++++++++++ .../test/date-bucket-parity-turso.test.ts | 20 ++++-- 4 files changed, 124 insertions(+), 64 deletions(-) diff --git a/packages/drivers/driver-sqlite-wasm/src/sqlite-wasm-driver-date-bucket.test.ts b/packages/drivers/driver-sqlite-wasm/src/sqlite-wasm-driver-date-bucket.test.ts index 48f73e2f655..aee1e9f4612 100644 --- a/packages/drivers/driver-sqlite-wasm/src/sqlite-wasm-driver-date-bucket.test.ts +++ b/packages/drivers/driver-sqlite-wasm/src/sqlite-wasm-driver-date-bucket.test.ts @@ -103,12 +103,13 @@ describe('SqliteWasmDriver 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] The ISO week too, inherited from SqlDriver's SQLite arm. It + // uses no `%V`, so it runs on the SQLite sql.js bundles (3.49.1). + expect(caps.week).toBe(true); }); }); - describe.each(['day', 'month', 'quarter', 'year'])( + describe.each(['day', 'week', 'month', 'quarter', 'year'])( 'granularity=%s — native SQL matches bucketDateValue', (g) => { it('produces the same label set as the in-memory reference', async () => { @@ -171,30 +172,10 @@ describe('SqliteWasmDriver date bucket (dateGranularity)', () => { }); }); - describe('unsupported granularity', () => { - /** - * [#6212] The twin of `driver-sql`'s case, moved for the same reason and - * kept here rather than dropped: this driver inherits `SqlDriver.aggregate` - * but reports a DIFFERENT dialect name in the refusal's tail, which is the - * one part of the message a shared implementation cannot prove. - */ - 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); - expect(err.message).toContain('Bucketed here: day, month, quarter, year'); - }); - }); + // [#21595] The `unsupported granularity` twin of driver-sql's #6212 refusal + // case used to live here, on `week`. This driver now buckets all five, so no + // granularity reaches that refusal on it; driver-sql pins the refusal on a + // client it does not model, and `week` runs above with the other four. describe('mixed groupBy', () => { it('combines a plain field with a structured dateGranularity item', async () => { diff --git a/packages/drivers/driver-turso/src/remote-transport-groupby-node.test.ts b/packages/drivers/driver-turso/src/remote-transport-groupby-node.test.ts index dfe00dbfa50..f6ba8f5740b 100644 --- a/packages/drivers/driver-turso/src/remote-transport-groupby-node.test.ts +++ b/packages/drivers/driver-turso/src/remote-transport-groupby-node.test.ts @@ -235,6 +235,17 @@ async function localDriver(): Promise { return d; } +/** + * [#21595] A `SqlDriver` over a client it does not model, whose capability row + * is empty: since SQLite buckets all five granularities, the one local face + * that still declines one. `mssql`'s driver (`tedious`) is a driver-sql + * dependency, and the refusal is raised while the statement is built, so no + * server is needed. + */ +function unmodeledDriver(): SqlDriver { + return new SqlDriver({ client: 'mssql', connection: { server: '127.0.0.1' } }); +} + describe('[#6212] RemoteTransport compiles the GroupByNode union', () => { describe('a plain field name — the half that never moved', () => { it('compiles a string entry to GROUP BY, as it always did', async () => { @@ -433,10 +444,10 @@ describe('[#6212] RemoteTransport compiles the GroupByNode union', () => { */ describe('local/remote parity (#5240 — one condition, one wording)', () => { it("answers 'week' identically on both faces", async () => { - // `week` is the granularity BOTH faces decline: SQLite buckets - // day/month/quarter/year and leaves week to the in-memory path (strftime - // %V landed only in SQLite 3.46), and this transport buckets nothing. So - // it is the one input on which the two messages are comparable at all. + // The condition needs a `SqlDriver` face that declines the granularity, + // and this transport, which buckets nothing. [#21595] SQLite buckets all + // five since it gained a `week` arm, so the local side is a client + // driver-sql does not model, which declines every one. const query = { groupBy: [{ field: 'closed_at', dateGranularity: 'week' as const }], aggregations: [{ function: 'count' as const, alias: 'n' }], @@ -448,30 +459,35 @@ describe('[#6212] RemoteTransport compiles the GroupByNode union', () => { (e) => e as WireBearingError, ); - const d = await localDriver(); + const d = unmodeledDriver(); const local = await d.aggregate('deal', query).then( () => { throw new Error('expected the local driver to refuse'); }, (e) => e as WireBearingError, ); + await d.disconnect().catch(() => {}); expect(remote.code).toBe(local.code); expect(remote.status).toBe(local.status); // First sentence is the contract; the tails differ on purpose — each face - // reports what IT buckets, which is the whole content of the asymmetry - // below. + // reports what IT buckets and names itself. expect(remote.message.split('. ')[0]).toBe(local.message.split('. ')[0]); expect(remote.message.split('. ')[0]).toBe(REFUSAL_SENTENCE('week').replace(/\.$/, '')); + expect(local.message).toContain("Bucketed here: none (dialect 'mssql')"); }); - it("'month' is declined here and compiled there — a DECLARED asymmetry, not a fork", async () => { + // [#21595] `week` joined `month` here when SQLite gained a `week` arm. The + // case that pinned the SQLite face's refusal tail for `week` ("Bucketed + // here: day, month, quarter, year") went with it: no granularity reaches + // that refusal on the SQLite face any more. + it.each(['month', 'week'] as const)("'%s' is declined here and compiled there — a DECLARED asymmetry, not a fork", async (g) => { // The two faces publish different `supports.queryDateGranularity`, and the - // engine reads it. `month` on the local face must therefore compile, not - // refuse: a parity test that demanded identical behaviour here would be - // demanding the capability bit mean nothing. + // engine reads it. The granularity on the local face must therefore + // compile, not refuse: a parity test that demanded identical behaviour + // here would be demanding the capability bit mean nothing. const d = await localDriver(); await expect( d.aggregate('deal', { - groupBy: [{ field: 'closed_at', dateGranularity: 'month' }], + groupBy: [{ field: 'closed_at', dateGranularity: g }], aggregations: [{ function: 'count', alias: 'n' }], }), ).resolves.toBeDefined(); @@ -479,27 +495,10 @@ describe('[#6212] RemoteTransport compiles the GroupByNode union', () => { const { t } = transportWithCapturingClient(); await expect( t.aggregate('deal', { - groupBy: [{ field: 'closed_at', dateGranularity: 'month' }], + groupBy: [{ field: 'closed_at', dateGranularity: g }], aggregations: [{ function: 'count', alias: 'n' }], }), - ).rejects.toThrow(REFUSAL_SENTENCE('month')); - }); - - it('the local face lists the granularities it DOES bucket', async () => { - const d = await localDriver(); - const err = await d - .aggregate('deal', { - groupBy: [{ field: 'closed_at', dateGranularity: 'week' }], - aggregations: [{ function: 'count', alias: 'n' }], - }) - .then( - () => { throw new Error('expected the local driver to refuse'); }, - (e) => e as WireBearingError, - ); - expect(err.code).toBe('NOT_IMPLEMENTED'); - expect(err.status).toBe(501); - expect(err.message).toContain('Bucketed here: day, month, quarter, year'); - expect(err.message).toContain("dialect 'better-sqlite3'"); + ).rejects.toThrow(REFUSAL_SENTENCE(g)); }); }); diff --git a/packages/drivers/driver-turso/src/turso-remote-inherited-members.test.ts b/packages/drivers/driver-turso/src/turso-remote-inherited-members.test.ts index 3d474776d54..c84f5bbdec9 100644 --- a/packages/drivers/driver-turso/src/turso-remote-inherited-members.test.ts +++ b/packages/drivers/driver-turso/src/turso-remote-inherited-members.test.ts @@ -80,6 +80,20 @@ const TENANT_ROWS = [ { id: 't2', name: 'B', organization_id: 'org_b' }, { id: 't3', name: 'P', organization_id: null }, ]; +/** [#21595] A date and a datetime on ISO-week boundary days, and the empty bucket. */ +const BUCKET = { name: 'probe_bucket', fields: { on: { type: 'date' }, at: { type: 'datetime' } } }; +const BUCKET_ROWS = [ + { id: 'w1', on: '2020-12-31', at: '2020-12-31T23:30:00.000Z' }, // a Thursday + { id: 'w2', on: '2021-01-03', at: '2021-01-03T00:30:00.000Z' }, // the Sunday after it + { id: 'w3', on: '2021-01-04', at: '2021-01-04T12:00:00.000Z' }, // the Monday after that + { id: 'w4', on: '2024-12-30', at: '2024-12-30T12:00:00.000Z' }, // a Monday in December + { id: 'w5', on: '2026-12-28', at: '2026-12-28T12:00:00.000Z' }, // the Monday of a 53rd week + { id: 'w6', on: null, at: null }, +]; +/** The ISO week each row falls in, written out rather than computed. */ +const ISO_WEEK_OF: Record = { + w1: '2020-W53', w2: '2020-W53', w3: '2021-W01', w4: '2025-W01', w5: '2026-W53', w6: null, +}; let seq = 0; const nextFile = () => join(scratch, `db-${++seq}.db`); @@ -425,3 +439,61 @@ describe('members inherited unchanged, and true on the remote face', () => { ); }); }); + +/** + * [#21595] `dateBucketSql` is an `inherited` row: the remote face renders + * `SqlDriver`'s SQLite bucket expression with Knex's compiler, which needs no + * connection, and libSQL runs it. The row was measured once with a harness + * that no longer exists, so until this block it was held only structurally. + * + * Here the expression each face renders goes through that face's own + * `execute()`: libSQL behind the remote face, better-sqlite3 behind the local + * one. Measured: `@libsql/client` 0.18.0 bundles SQLite 3.45.1, which has no + * `strftime('%V')`, so a `week` arm built on `%V` answers NULL on the remote + * face while the local face answers the week. + */ +describe('dateBucketSql: the inherited expression runs on libSQL, as on the local face', () => { + const GRANULARITIES = ['day', 'week', 'month', 'quarter', 'year'] as const; + + /** A driver of the given face over a fresh SQLite file, holding the bucket rows. */ + async function withBuckets(face: Face): Promise { + const file = nextFile(); + const client = face === 'remote' ? createClient({ url: `file:${file}` }) : null; + const driver = + face === 'remote' + ? new TursoDriver({ url: 'libsql://probe.example.turso.io', client: client! }) + : new TursoDriver({ url: `file:${file}` }); + open.push({ driver, client }); + await driver.connect(); + await driver.initObjects([BUCKET]); + await driver.bulkCreate(BUCKET.name, BUCKET_ROWS.map((row) => ({ ...row }))); + return driver; + } + + /** `id -> label`: the face's rendered expression, run through the face's own `execute()`. */ + async function labels(driver: TursoDriver, field: 'on' | 'at', g: (typeof GRANULARITIES)[number]) { + const expr = driver.dateBucketSql(BUCKET.name, field, g); + expect(expr, `${field} @ ${g}: an expression is rendered`).toBeTypeOf('string'); + const rows = (await driver.execute(`select id, ${expr} as b from ${BUCKET.name} order by id`)) as Array<{ id: string; b: unknown }>; + return Object.fromEntries(rows.map((row) => [row.id, row.b ?? null])); + } + + it('the row is still `inherited`', () => { + expect(REMOTE_FACE_ANSWERS.dateBucketSql).toBe('inherited'); + }); + + it.each(GRANULARITIES)('%s: libSQL answers the labels the local face answers, on both columns', async (g) => { + const remote = await withBuckets('remote'); + const local = await withBuckets('local'); + for (const field of ['on', 'at'] as const) { + expect(await labels(remote, field, g), `${field} @ ${g}`).toEqual(await labels(local, field, g)); + } + }); + + it('week: libSQL answers the ISO week of each boundary day, on both columns', async () => { + const remote = await withBuckets('remote'); + for (const field of ['on', 'at'] as const) { + expect(await labels(remote, field, 'week'), field).toEqual(ISO_WEEK_OF); + } + }); +}); diff --git a/packages/qa/dogfood/test/date-bucket-parity-turso.test.ts b/packages/qa/dogfood/test/date-bucket-parity-turso.test.ts index 419ff3c107f..a21221bfda0 100644 --- a/packages/qa/dogfood/test/date-bucket-parity-turso.test.ts +++ b/packages/qa/dogfood/test/date-bucket-parity-turso.test.ts @@ -149,19 +149,27 @@ describe('TursoDriver date-bucket parity (framework#3773)', () => { it('the checker WOULD catch a driver that advertises a granularity it cannot run', async () => { // The tripwire the vacuous pass above is worth having. Rather than mock a - // remote transport into life, this drives a real local driver and makes it - // advertise `week` — which SqlDriver deliberately does NOT implement on - // SQLite (`%V` needs 3.46+), so `aggregate()` throws exactly as - // RemoteTransport would on a structured groupBy. + // remote transport into life, this drives a real local driver whose + // capability row advertises a granularity its bucket expression cannot + // render, so `aggregate()` throws exactly as RemoteTransport would on a + // structured groupBy. // // So: if someone deletes remote's `queryDateGranularity: {}` override // believing SqlDriver handles it, this is the shape of failure they get — // named, not silent. + // + // [#21595] It used to make the driver advertise `week`, which SqlDriver + // did not implement on SQLite. SQLite buckets `week` now, so the driver + // keeps its real row (which advertises `week`) and its `week` arm answers + // `null`, as it did before #21595: the incoherent pair a revert of the + // arm alone would ship. const driver = new TursoDriver({ url: ':memory:' }); - Object.defineProperty(driver, 'supports', { - get: () => ({ queryDateGranularity: { week: true } }), + const inherited = (driver as any).buildDateBucketExpr.bind(driver); + Object.defineProperty(driver, 'buildDateBucketExpr', { + value: (field: string, g: string, table?: string) => (g === 'week' ? null : inherited(field, g, table)), configurable: true, }); + expect(driver.supports.queryDateGranularity?.week).toBe(true); const problems = await checkDateBucketParity(driver, { createOptions: { bypassTenantAudit: true }, From 59a751f4ad7cb0ebcba40bd4e2781601f078abad Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 3 Oct 2026 18:57:18 +0000 Subject: [PATCH 3/4] fix(service-analytics): the SQL echo refuses a bucket it has no SQLite statement for On a SQLite datasource the ObjectQL face's echo no longer prints `date_trunc(...)`, which SQLite refuses. With a non-UTC timezone the dry run answers NOT_IMPLEMENTED / 501 as a declared refusal, and the query answer carries no `sql`. Comments that quoted SQLite `week` as bucketed in memory, or PostgreSQL's datetime bucket text as its only bucket, are corrected (objectql engine.ts, driver-mongodb, the analytics strategy types and the data-modeling queries page). Claude-Session: https://claude.ai/code/session_017ErfyP2Rx7XWHJA27QjyUi Co-authored-by: Claude --- .changeset/21595-sqlite-week-bucket.md | 15 ++++ content/docs/data-modeling/queries.mdx | 6 +- .../driver-mongodb/src/mongodb-aggregation.ts | 16 ++-- .../src/mongodb-date-bucket-parity.test.ts | 4 +- packages/objectql/src/engine.ts | 8 +- .../objectql-echo-date-bucket.test.ts | 79 ++++++++++++++----- .../src/strategies/objectql-strategy.ts | 73 ++++++++++++++--- .../service-analytics/src/strategies/types.ts | 9 ++- 8 files changed, 161 insertions(+), 49 deletions(-) create mode 100644 .changeset/21595-sqlite-week-bucket.md diff --git a/.changeset/21595-sqlite-week-bucket.md b/.changeset/21595-sqlite-week-bucket.md new file mode 100644 index 00000000000..d3fcfb5b6ce --- /dev/null +++ b/.changeset/21595-sqlite-week-bucket.md @@ -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. diff --git a/content/docs/data-modeling/queries.mdx b/content/docs/data-modeling/queries.mdx index 5f9003db36f..74e3ae3dc06 100644 --- a/content/docs/data-modeling/queries.mdx +++ b/content/docs/data-modeling/queries.mdx @@ -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. Buckets are computed in **UTC**. A non-UTC reference timezone is only reachable through diff --git a/packages/drivers/driver-mongodb/src/mongodb-aggregation.ts b/packages/drivers/driver-mongodb/src/mongodb-aggregation.ts index 87170e38e24..aaf51e578cb 100644 --- a/packages/drivers/driver-mongodb/src/mongodb-aggregation.ts +++ b/packages/drivers/driver-mongodb/src/mongodb-aggregation.ts @@ -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 diff --git a/packages/drivers/driver-mongodb/src/mongodb-date-bucket-parity.test.ts b/packages/drivers/driver-mongodb/src/mongodb-date-bucket-parity.test.ts index 91c4cd6cf64..9216f02905f 100644 --- a/packages/drivers/driver-mongodb/src/mongodb-date-bucket-parity.test.ts +++ b/packages/drivers/driver-mongodb/src/mongodb-date-bucket-parity.test.ts @@ -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, }); diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index aadf4018201..12317a87d39 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -17553,7 +17553,8 @@ export class ObjectQL implements IObjectQLEngine { // `supports.queryDateGranularity[g]`. If every structured item is // supported we can push the aggregate down to the driver; otherwise // we fall back to driver.find() + in-memory bucketing so the result - // remains correct on partial-support dialects (e.g. SQLite + week). + // remains correct on a face that buckets fewer (e.g. the Turso remote + // transport, which advertises none). const groupByItems = Array.isArray(query.groupBy) ? query.groupBy : []; const granularityCaps: Record | undefined = drv?.supports?.queryDateGranularity; @@ -17562,8 +17563,9 @@ export class ObjectQL implements IObjectQLEngine { if (!g?.dateGranularity) return true; // plain {field} object is fine return granularityCaps?.[g.dateGranularity] === true; }); - // ADR-0053 Phase 2 (D2): native driver date bucketing (`date_trunc`) is - // UTC-only — SQLite has no tz database and MySQL needs tz tables loaded, + // ADR-0053 Phase 2 (D2): native driver date bucketing (driver-sql's + // `to_char` / `date_format` / `strftime`) is UTC-only — SQLite has no + // tz database and MySQL needs tz tables loaded, // so pushing tz-aware bucketing down splits boundaries per dialect. When // a non-UTC reference timezone is in play we therefore force the // in-memory path: the date-range `where` still goes to the driver (only diff --git a/packages/services/service-analytics/src/__tests__/objectql-echo-date-bucket.test.ts b/packages/services/service-analytics/src/__tests__/objectql-echo-date-bucket.test.ts index c0eb312b251..c82c7f201d8 100644 --- a/packages/services/service-analytics/src/__tests__/objectql-echo-date-bucket.test.ts +++ b/packages/services/service-analytics/src/__tests__/objectql-echo-date-bucket.test.ts @@ -18,22 +18,27 @@ * | SQLite, quarter | `(strftime('%Y', …) \|\| '-Q' \|\| …)` | `no such function: date_trunc` | * | PostgreSQL 16.14, month | `to_char((…)::timestamptz AT TIME ZONE 'UTC', 'YYYY-MM')` | `2026-01-01T00:00:00.000Z` where the face answers `2026-01` | * + * (The PostgreSQL row is the `datetime` expression. Since #21485 a `date` + * buckets as `to_char((…)::date::timestamp, …)`, its calendar day.) + * * The rows were right. The echo now reads the bucket from the `dateBucketSql` * hook, which the plugin fills from `SqlDriver.dateBucketSql`: the driver's * own `buildDateBucketExpr`, rendered. No second bucketing table. * * ## The cells * - * - **sqlite** (better-sqlite3), every run. Month and quarter are grouped by - * the driver; week is bucketed in memory (the driver declares no `week`). + * - **sqlite** (better-sqlite3), every run. Month, quarter and week are + * grouped by the driver (SQLite's `week` arm landed with #21595). * - **postgres** where `OS_TEST_POSTGRES_URL` is set, a named skip otherwise. * Month, quarter and week are grouped by the driver. No CI step provisions * that variable for this package, so the live cell is red-capable and * un-run in CI. * - * Where the hook answers nothing, the bucket keeps `date_trunc`: a granularity - * the driver buckets in memory, a non-UTC `timezone` (the engine buckets in - * memory on that zone's calendar), and a host that wires no hook. + * Where the hook answers nothing, the bucket keeps `date_trunc`: a non-UTC + * `timezone` (the engine buckets in memory on that zone's calendar), and a host + * that wires no hook. [#21595] On SQLite, which has no `date_trunc`, the echo + * refuses there instead: `/analytics/sql` answers `NOT_IMPLEMENTED` / 501 as a + * declared refusal, and `/analytics/query` serves the rows with no `sql`. */ import { describe, it, expect, beforeAll, afterAll } from 'vitest'; @@ -45,6 +50,7 @@ import type { AnalyticsService } from '../analytics-service.js'; import { AnalyticsServicePlugin } from '../plugin.js'; import { ObjectQLStrategy } from '../strategies/objectql-strategy.js'; import type { StrategyContext } from '../strategies/types.js'; +import { declaredRefusalMessage } from '@objectstack/types'; const DEAL = 'os21441_bucket_deal'; @@ -114,7 +120,7 @@ const CELLS: readonly Cell[] = [ label: 'sqlite', env: null, config: () => ({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }), - driverGrouped: ['month', 'quarter'], + driverGrouped: ['month', 'quarter', 'week'], }, { id: 'pg', @@ -225,21 +231,43 @@ for (const cell of CELLS) { expect(tuples(await run(dryRun.sql, dryRun.params), ['closed_on', 'big_sum'])).toEqual(tuples(res.rows, ['closed_on', 'big_sum'])); }); - it.each(['month', 'quarter', 'week'].filter((g) => !cell.driverGrouped.includes(g)))( - 'FALLBACK: %s, which the driver buckets in memory, keeps `date_trunc`', - async (granularity) => { - const { res, ran } = await ask(bucketed('closed_on', granularity)); + // [#21595] A FALLBACK case for "a granularity the driver buckets in + // memory" lived here. Its one input was SQLite `week`, and both cells now + // group all three, so it had none left: `week` runs in the case above. + + if (cell.id === 'sqlite') { + it('[#21595] REFUSAL: a non-UTC timezone on SQLite: the dry run refuses, and the query serves its rows with no `sql`', async () => { + const query = bucketed('closed_at', 'month', { timezone: 'Asia/Shanghai' }); + const { res, ran } = await ask(query); expect(ran.some((sql) => /group by/i.test(sql)), 'the driver grouped nothing').toBe(false); - expect(selectedBucket(res.sql!, 'closed_on')).toBe(`date_trunc('${granularity}', closed_on)`); - }, - ); + expect(tuples(res.rows, ['closed_at', 'amount_sum'])).toEqual([['2026-01', 20], ['2026-02', 8], ['2026-03', 10], ['2026-04', 5]]); + // `/analytics/query`: the echo is a debugging aid, so its refusal + // leaves the answer without one rather than failing the query. + expect(res.sql).toBeUndefined(); - it('FALLBACK: a non-UTC timezone buckets in memory on that zone\'s calendar, and the echo keeps `date_trunc`', async () => { - const { res, ran } = await ask(bucketed('closed_at', 'month', { timezone: 'Asia/Shanghai' })); - expect(ran.some((sql) => /group by/i.test(sql)), 'the driver grouped nothing').toBe(false); - expect(tuples(res.rows, ['closed_at', 'amount_sum'])).toEqual([['2026-01', 20], ['2026-02', 8], ['2026-03', 10], ['2026-04', 5]]); - expect(selectedBucket(res.sql!, 'closed_at')).toBe("date_trunc('month', closed_at)"); - }); + // `/analytics/sql`: the dry run refuses in the declared envelope. + const err = await analytics.generateSql(query as any).then( + () => { throw new Error('expected the dry run to refuse'); }, + (e) => e as Error & { code?: string; status?: number; refusal?: unknown }, + ); + expect([err.code, err.status, err.refusal]).toEqual(['NOT_IMPLEMENTED', 501, true]); + // The read every withhold arm makes: the prose reaches the caller. + expect(declaredRefusalMessage(err)).toBe(err.message); + + // The control: the same query at UTC renders the driver's expression. + const utc = await analytics.generateSql({ ...query, timezone: 'UTC' } as any); + expect(selectedBucket(utc.sql, 'closed_at')).not.toContain('date_trunc'); + }); + } else { + // Not this card: whether these keys match the face's on PostgreSQL is + // the follow-up #21595's triage foresaw. + it('FALLBACK: a non-UTC timezone buckets in memory on that zone\'s calendar, and the echo keeps `date_trunc`', async () => { + const { res, ran } = await ask(bucketed('closed_at', 'month', { timezone: 'Asia/Shanghai' })); + expect(ran.some((sql) => /group by/i.test(sql)), 'the driver grouped nothing').toBe(false); + expect(tuples(res.rows, ['closed_at', 'amount_sum'])).toEqual([['2026-01', 20], ['2026-02', 8], ['2026-03', 10], ['2026-04', 5]]); + expect(selectedBucket(res.sql!, 'closed_at')).toBe("date_trunc('month', closed_at)"); + }); + } }, ); } @@ -250,4 +278,17 @@ describe('[#21441] FALLBACK: a host that wires no dateBucketSql hook', () => { const { sql } = await new ObjectQLStrategy().generateSql(bucketed('closed_on', 'month') as any, ctx); expect(selectedBucket(sql, 'closed_on')).toBe("date_trunc('month', closed_on)"); }); + + it('[#21595] REFUSAL: on a SQLite datasource, it refuses rather than echo `date_trunc`', async () => { + const ctx = { + getCube: (name: string) => (name === CUBE ? CUBES[0] : undefined), + sqlDialect: () => 'sqlite', + } as unknown as StrategyContext; + const err = await new ObjectQLStrategy().generateSql(bucketed('closed_on', 'month') as any, ctx).then( + () => { throw new Error('expected the echo to refuse'); }, + (e) => e as Error & { code?: string; status?: number; refusal?: unknown }, + ); + expect([err.code, err.status, err.refusal]).toEqual(['NOT_IMPLEMENTED', 501, true]); + expect(declaredRefusalMessage(err)).toBe(err.message); + }); }); diff --git a/packages/services/service-analytics/src/strategies/objectql-strategy.ts b/packages/services/service-analytics/src/strategies/objectql-strategy.ts index 3769d2062c9..ef7ba359d9c 100644 --- a/packages/services/service-analytics/src/strategies/objectql-strategy.ts +++ b/packages/services/service-analytics/src/strategies/objectql-strategy.ts @@ -5,6 +5,7 @@ import type { AggregationFunction, Cube } from '@objectstack/spec/data'; // [#8220] The read-scope provenance mark: `withReadScope` below is one of the // two merge boundaries that stamp it. import { markFilterSubtreeProvenance } from '@objectstack/spec/data'; +import { StandardErrorCode } from '@objectstack/spec/api'; import type { AnalyticsStrategy, StrategyContext, DatasetScopedStrategyContext } from './types.js'; import { declaredDatetimeLowering, @@ -140,6 +141,50 @@ interface CrossObjectPlan { crossDims: CrossObjectPlanDim[]; } +/** + * [#21595] The echo has no bucket expression SQLite runs, so it refuses rather + * than print a statement SQLite refuses. + * + * `generateSql` prints a date-bucketed dimension in the expression the driver + * groups by (the `dateBucketSql` hook). Where nothing answers, it prints the + * representative `date_trunc('', col)`, and SQLite has no + * `date_trunc`. On the shipped composition one case reaches that on SQLite: a + * non-UTC `timezone`. The engine then buckets in memory on that zone's + * calendar (ADR-0053 Phase 2, D2), and SQLite has no time-zone database, so no + * SQLite expression produces those keys. The other branch is a host whose hook + * answers nothing for a SQLite datasource. + * + * `NOT_IMPLEMENTED` / 501, for the reason `driver-sql`'s own bucket refusal + * gives: the query is spelled correctly and served, and the gap is the + * backend's. `refusal: true` declares the 501 a deliberate refusal addressed + * to the caller (`ApiErrorSchema.refusal`), so the boundaries keep its message + * instead of withholding it as a fault. + * + * The reach is the cross-field decline's: `execute()` calls `generateSql` + * inside a `try`, so `/analytics/query` still serves the rows and carries no + * `sql`, and the dry run (`/analytics/sql`) refuses. Both are within the + * declared response contracts: `sql` is optional on the query answer, and the + * dry run answers in the error envelope. + */ +function sqliteBucketEchoRefused(dimension: string, granularity: string, zone: string | undefined): Error { + const zoned = !!zone && zone !== 'UTC'; + const err = new Error( + `[analytics] cannot render display SQL for the "${granularity}" bucket of "${dimension}" on SQLite` + + (zoned ? ` with timezone "${zone}". ` : '. ') + + (zoned + ? `The query itself is SERVED: the engine buckets it in memory on that zone's calendar, and SQLite has ` + + `no time-zone database, so no SQLite expression produces those bucket keys. ` + : `The query itself is SERVED, but the driver behind this datasource renders no bucket expression for it. `) + + `Refusing rather than printing date_trunc(...), which is not a SQLite function. ` + + `Run the query itself (/analytics/query) to get its rows` + + (zoned ? `; with timezone "UTC" or none, this dry run renders the expression the driver groups by.` : '.'), + ) as Error & { code?: string; status?: number; refusal?: true }; + err.code = StandardErrorCode.enum.NOT_IMPLEMENTED; + err.status = 501; + err.refusal = true; + return err; +} + /** * ObjectQLStrategy — Priority 2 * @@ -422,10 +467,11 @@ export class ObjectQLStrategy implements AnalyticsStrategy { * for its dialect, through the `dateBucketSql` hook, so the echo runs there * and answers the face's bucket keys. The bucket stays REPRESENTATIVE where * the hook answers nothing: no hook, a driver with no bucket expression (a - * non-SQL driver), a granularity the driver buckets in memory (`week` on - * SQLite), or a non-UTC `timezone`, which the engine buckets in memory on - * that zone's calendar. There it prints `date_trunc('', col)`, - * which SQLite refuses. + * non-SQL driver), or a non-UTC `timezone`, which the engine buckets in + * memory on that zone's calendar. There it prints + * `date_trunc('', col)`. [#21595] Except on SQLite, which has no + * `date_trunc`: there the echo refuses ({@link sqliteBucketEchoRefused}), so + * it never prints a statement SQLite refuses. * * Filter VALUES are rendered as `$n` placeholders and returned in `params`, * never inlined: the echoed statement travels to the browser, and a filter @@ -527,10 +573,12 @@ export class ObjectQLStrategy implements AnalyticsStrategy { const joinClauses: string[] = []; // [#21441] The bucket expression the driver itself groups by for its // dialect, read from the `dateBucketSql` hook: `strftime('%Y-%m', …)` on - // SQLite, `to_char(… AT TIME ZONE 'UTC', 'YYYY-MM')` on PostgreSQL. The - // echo then runs there and answers the face's bucket keys. It used to - // print `date_trunc('', col)` on every dialect, calling that - // the driver's own bucketing: no driver buckets with it, SQLite refuses it + // SQLite, `to_char(…, 'YYYY-MM')` on PostgreSQL, whose argument is + // `(col)::timestamptz AT TIME ZONE 'UTC'` for a `datetime` and, since + // #21485, `(col)::date::timestamp` for a `date`. The echo then runs there + // and answers the face's bucket keys. It used to print + // `date_trunc('', col)` on every dialect, calling that the + // driver's own bucketing: no driver buckets with it, SQLite refuses it // (`no such function`), and PostgreSQL answers a timestamp where the face // answers `2026-01`. // @@ -538,7 +586,9 @@ export class ObjectQLStrategy implements AnalyticsStrategy { // UTC bucket, and a non-UTC zone makes the engine bucket in memory on that // zone's calendar instead (ADR-0053 Phase 2, D2; `tzRequiresInMemory` in // objectql's `engine.ts`), which no driver expression describes. Where - // nothing answers, the bucket keeps the representative `date_trunc`. + // nothing answers, the bucket keeps the representative `date_trunc`, except + // on SQLite, which has no `date_trunc`: there the echo refuses + // ({@link sqliteBucketEchoRefused}, #21595). const zone = query.timezone; const driverBucketSql = (col: string, granularity: string): string | undefined => { if (zone && zone !== 'UTC') return undefined; @@ -556,7 +606,10 @@ export class ObjectQLStrategy implements AnalyticsStrategy { const col = this.resolveFieldName(cube, dim, 'dimension'); const gran = granByDim.get(dim); if (!gran) return col; - return driverBucketSql(col, gran) ?? `date_trunc('${gran}', ${col})`; + const bucket = driverBucketSql(col, gran); + if (bucket !== undefined) return bucket; + if (sqlDialectFor(ctx, tableName) === 'sqlite') throw sqliteBucketEchoRefused(dim, gran, zone); + return `date_trunc('${gran}', ${col})`; }; if (query.dimensions) { diff --git a/packages/services/service-analytics/src/strategies/types.ts b/packages/services/service-analytics/src/strategies/types.ts index 43163d14108..8a840020c65 100644 --- a/packages/services/service-analytics/src/strategies/types.ts +++ b/packages/services/service-analytics/src/strategies/types.ts @@ -163,10 +163,11 @@ export interface DatasetScopedStrategyContext extends StrategyContext { /** * [#21441] The date-bucket expression the driver backing `objectName` * groups `field` by at `granularity`, as SQL text in that driver's dialect: - * `strftime('%Y-%m', …)` on SQLite, `to_char(… AT TIME ZONE 'UTC', - * 'YYYY-MM')` on PostgreSQL. `undefined` when the host cannot answer: no - * hook wired, a driver with no bucket expression (a non-SQL driver), or a - * granularity the driver buckets in memory (`week` on SQLite). + * `strftime('%Y-%m', …)` on SQLite, `to_char(…, 'YYYY-MM')` on PostgreSQL + * (over `(col)::timestamptz AT TIME ZONE 'UTC'` for a `datetime`, and + * `(col)::date::timestamp` for a `date` since #21485). `undefined` when the + * host cannot answer: no hook wired, a driver with no bucket expression (a + * non-SQL driver), or a SQL client that driver does not model. * * `ObjectQLStrategy.generateSql` prints a date-bucketed dimension in this * expression, so its echo runs on that dialect and answers the face's From 9328c3c6cc90554e1027f7d1850ed4d0eb725518 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 3 Oct 2026 19:29:28 +0000 Subject: [PATCH 4/4] test(driver-sql,driver-turso): build the unmodeled mssql driver with no connection The refusal is raised while the statement is built, so no connection is needed, and a `{ server }` object is not a connection config knex types. Claude-Session: https://claude.ai/code/session_017ErfyP2Rx7XWHJA27QjyUi Co-authored-by: Claude --- packages/drivers/driver-sql/src/sql-driver-date-bucket.test.ts | 2 +- .../driver-turso/src/remote-transport-groupby-node.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/drivers/driver-sql/src/sql-driver-date-bucket.test.ts b/packages/drivers/driver-sql/src/sql-driver-date-bucket.test.ts index 84231c88b8f..67a4497546e 100644 --- a/packages/drivers/driver-sql/src/sql-driver-date-bucket.test.ts +++ b/packages/drivers/driver-sql/src/sql-driver-date-bucket.test.ts @@ -160,7 +160,7 @@ describe('SqlDriver date bucket (dateGranularity)', () => { * the statement is built, so no server is needed. */ it('refuses a granularity on a client with no bucket expression, with NOT_IMPLEMENTED / 501', async () => { - const unmodeled = new SqlDriver({ client: 'mssql', connection: { server: '127.0.0.1' } }); + const unmodeled = new SqlDriver({ client: 'mssql' }); try { expect(unmodeled.supports.queryDateGranularity).toEqual({}); const err = await unmodeled diff --git a/packages/drivers/driver-turso/src/remote-transport-groupby-node.test.ts b/packages/drivers/driver-turso/src/remote-transport-groupby-node.test.ts index f6ba8f5740b..eb22209aca9 100644 --- a/packages/drivers/driver-turso/src/remote-transport-groupby-node.test.ts +++ b/packages/drivers/driver-turso/src/remote-transport-groupby-node.test.ts @@ -243,7 +243,7 @@ async function localDriver(): Promise { * server is needed. */ function unmodeledDriver(): SqlDriver { - return new SqlDriver({ client: 'mssql', connection: { server: '127.0.0.1' } }); + return new SqlDriver({ client: 'mssql' }); } describe('[#6212] RemoteTransport compiles the GroupByNode union', () => {