diff --git a/.changeset/21630-echo-refuses-in-memory-bucket.md b/.changeset/21630-echo-refuses-in-memory-bucket.md new file mode 100644 index 00000000000..16af18660ba --- /dev/null +++ b/.changeset/21630-echo-refuses-in-memory-bucket.md @@ -0,0 +1,13 @@ +--- +'@objectstack/service-analytics': patch +--- + +With a non-UTC `timezone`, the analytics SQL echo of a date-bucketed dimension refuses on every dialect instead of printing `date_trunc` (#21630). + +Clause-②: no + +- **What was wrong.** With a non-UTC `timezone`, the engine buckets a date dimension in memory on that zone's calendar, on every driver. The ObjectQL face of `POST /api/v1/analytics/query` and `POST /api/v1/analytics/sql` still echoed the bucket as `date_trunc('month', col)` (or the asked granularity) on PostgreSQL and MySQL, a statement the engine never ran. On PostgreSQL that statement groups on the database session's calendar: measured on PostgreSQL 16.14 with the server at `Asia/Shanghai`, it answered timestamp keys such as `2025-12-31T16:00:00.000Z` where the query answered `2026-01`, and with `timezone: 'America/New_York'` it grouped the rows differently from the query. MySQL has no `date_trunc` at all. SQLite already refused this echo. +- **What it does now.** For a date-bucketed dimension with a non-UTC `timezone`, on every dialect: + - `POST /api/v1/analytics/sql` refuses with `NOT_IMPLEMENTED` / 501, declared as a refusal so its message reaches the caller. This is the answer SQLite already gave. + - `POST /api/v1/analytics/query` answers the same rows as before, and its answer carries no `sql`. +- **Unchanged.** A query sent with `timezone: 'UTC'`, or with no `timezone`, still echoes the expression the driver groups by: `to_char(…)` on PostgreSQL, `date_format(…)` on MySQL and `strftime(…)` on SQLite. 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 c82c7f201d8..eb6d3ab251e 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 @@ -34,11 +34,19 @@ * 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 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`. + * [#21630] A non-UTC `timezone` makes the engine bucket in memory on that + * zone's calendar, on every driver, so no statement the database runs groups + * by those keys. The echo refuses there on every dialect: `/analytics/sql` + * answers `NOT_IMPLEMENTED` / 501 as a declared refusal, and `/analytics/query` + * serves the same rows with no `sql`. It used to print `date_trunc`, which + * PostgreSQL 16.14 ran on the session zone's calendar (timestamp keys, and at + * `America/New_York` other groupings than the face's). The live cells pin it + * on SQLite and PostgreSQL, and the dialect matrix below pins it on SQLite, + * PostgreSQL and MySQL through each driver's own hooks, with no server. + * + * At UTC, where the hook answers nothing (a host that wires no hook), the + * bucket keeps the representative `date_trunc`. [#21595] Except on SQLite, + * which has no `date_trunc`: there the echo refuses too. */ import { describe, it, expect, beforeAll, afterAll } from 'vitest'; @@ -142,6 +150,23 @@ const tuples = (rows: unknown, columns: readonly string[]) => /** The bucket expression an echo selects for `dim`: everything between `SELECT ` and ` AS ""`. */ const selectedBucket = (sql: string, dim: string) => sql.slice('SELECT '.length, sql.indexOf(` AS "${dim}"`)); +/** + * [#21630] Non-UTC zones, each with the `closed_at` month rows the engine + * answers on that zone's calendar. d2 (20:00 UTC on 31 January) is February + * in Asia/Shanghai and January in America/New_York. + */ +const ZONED: ReadonlyArray]> = [ + ['Asia/Shanghai', [['2026-01', 20], ['2026-02', 8], ['2026-03', 10], ['2026-04', 5]]], + ['America/New_York', [['2026-01', 27], ['2026-02', 1], ['2026-03', 10], ['2026-04', 5]]], +]; + +/** The error `p` rejects with; a resolution fails the case. */ +const refusalOf = (p: Promise) => + p.then( + () => { throw new Error('expected the echo to refuse'); }, + (e) => e as Error & { code?: string; status?: number; refusal?: unknown }, + ); + for (const cell of CELLS) { const config = cell.config(); describe.skipIf(!config)( @@ -235,42 +260,92 @@ for (const cell of CELLS) { // 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(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(); - - // `/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 }, - ); + // [#21595, #21630] One rule on every cell. The PostgreSQL cell asserted + // `date_trunc('month', closed_at)` here, a statement the engine did not + // run: it buckets a non-UTC zone in memory. + it.each(ZONED)('[#21630] REFUSAL: timezone %s, which the engine buckets in memory: the dry run refuses, and the query serves its rows with no `sql`', async (timezone, rows) => { + const query = bucketed('closed_at', 'month', { timezone }); + const { res, ran } = await ask(query); + expect(ran.some((sql) => /group by/i.test(sql)), 'the driver grouped nothing').toBe(false); + // The rows are the engine's, on the zone's calendar; the refusal + // concerns the echoed statement only. + expect(tuples(res.rows, ['closed_at', 'amount_sum'])).toEqual(rows); + // `/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(); + + // `/analytics/sql`: the dry run refuses in the declared envelope. + const err = await refusalOf(analytics.generateSql(query as any)); + 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 own expression. + const utc = await analytics.generateSql({ ...query, timezone: 'UTC' } as any); + expect(selectedBucket(utc.sql, 'closed_at')).toBe(driver.dateBucketSql(DEAL, 'closed_at', 'month')); + }); + }, + ); +} + +/** + * [#21630] One rule on every dialect `driver-sql` models, MySQL included. Each + * driver is constructed and never connected: its `dialectName` and + * `dateBucketSql` answer from the client config alone, so this runs with no + * server. The context is wired with them as `AnalyticsServicePlugin` wires its + * `sqlDialect` and `dateBucketSql` hooks. The rows are not asked here: the + * engine buckets a non-UTC zone in memory whatever the dialect, and the live + * cells above pin them. + */ +describe('[#21630] the echo of a date bucket, per dialect: a non-UTC timezone refuses, and UTC keeps the driver\'s expression', () => { + const DIALECT_CONFIGS = [ + { dialect: 'sqlite', config: { client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true } }, + { dialect: 'postgres', config: { client: 'pg', connection: { host: '127.0.0.1', database: 'unconnected' }, pool: { min: 0 } } }, + { dialect: 'mysql', config: { client: 'mysql2', connection: { host: '127.0.0.1', database: 'unconnected' }, pool: { min: 0 } } }, + ] as const; + const drivers: SqlDriver[] = []; + + afterAll(async () => { + for (const d of drivers) await (d as any).knex?.destroy().catch(() => {}); + }); + + for (const { dialect, config } of DIALECT_CONFIGS) { + describe(dialect, () => { + const driver = new SqlDriver(config as any); + drivers.push(driver); + const ctx = { + getCube: (name: string) => (name === CUBE ? CUBES[0] : undefined), + sqlDialect: () => (driver as any).dialectName, + dateBucketSql: (object: string, field: string, granularity: string) => + driver.dateBucketSql(object, field, granularity as any) ?? undefined, + } as unknown as StrategyContext; + + it('names its dialect', () => { + expect((driver as any).dialectName).toBe(dialect); + }); + + for (const [timezone] of ZONED) { + it.each(['month', 'week'])(`timezone ${timezone}, %s: refuses`, async (granularity) => { + const err = await refusalOf(new ObjectQLStrategy().generateSql(bucketed('closed_at', granularity, { timezone }) as any, ctx)); 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)"); + } + + for (const timezone of ['UTC', undefined]) { + it.each(['month', 'week'])(`timezone ${timezone ?? 'unset'}, %s: the driver's own expression`, async (granularity) => { + const { sql } = await new ObjectQLStrategy().generateSql( + bucketed('closed_at', granularity, timezone === undefined ? {} : { timezone }) as any, + ctx, + ); + const expected = driver.dateBucketSql(DEAL, 'closed_at', granularity as any); + expect(expected, 'the driver renders this granularity').toBeTruthy(); + expect(selectedBucket(sql, 'closed_at')).toBe(expected); }); } - }, - ); -} + }); + } +}); describe('[#21441] FALLBACK: a host that wires no dateBucketSql hook', () => { it('echoes the bucket as `date_trunc`', async () => { @@ -279,6 +354,13 @@ describe('[#21441] FALLBACK: a host that wires no dateBucketSql hook', () => { expect(selectedBucket(sql, 'closed_on')).toBe("date_trunc('month', closed_on)"); }); + it('[#21630] REFUSAL: with a non-UTC timezone, it refuses: the engine buckets that in memory whatever the host wires', async () => { + const ctx = { getCube: (name: string) => (name === CUBE ? CUBES[0] : undefined) } as unknown as StrategyContext; + const err = await refusalOf(new ObjectQLStrategy().generateSql(bucketed('closed_on', 'month', { timezone: 'Asia/Shanghai' }) as any, ctx)); + expect([err.code, err.status, err.refusal]).toEqual(['NOT_IMPLEMENTED', 501, true]); + expect(declaredRefusalMessage(err)).toBe(err.message); + }); + 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), diff --git a/packages/services/service-analytics/src/analytics-service.ts b/packages/services/service-analytics/src/analytics-service.ts index b9fb64d62f2..9b3c9f09e04 100644 --- a/packages/services/service-analytics/src/analytics-service.ts +++ b/packages/services/service-analytics/src/analytics-service.ts @@ -1143,8 +1143,11 @@ export interface AnalyticsServiceConfig { * `undefined` when the host cannot answer. See * `DatasetScopedStrategyContext.dateBucketSql` (`strategies/types.ts`). * - * Answered by the plugin from the driver that executes the aggregate. A host - * that wires nothing keeps the echo's representative `date_trunc(…)`. + * Answered by the plugin from the driver that executes the aggregate. Asked + * only at a UTC or unset `timezone`: a non-UTC one is bucketed in memory, and + * the echo refuses it whatever is wired (#21630). At UTC, a host that wires + * nothing keeps the echo's representative `date_trunc(…)`, except on SQLite, + * where the echo refuses (#21595). */ dateBucketSql?: (object: string, field: string, granularity: string) => string | undefined; /** diff --git a/packages/services/service-analytics/src/strategies/objectql-strategy.ts b/packages/services/service-analytics/src/strategies/objectql-strategy.ts index ef7ba359d9c..1ebcf894298 100644 --- a/packages/services/service-analytics/src/strategies/objectql-strategy.ts +++ b/packages/services/service-analytics/src/strategies/objectql-strategy.ts @@ -142,17 +142,26 @@ interface CrossObjectPlan { } /** - * [#21595] The echo has no bucket expression SQLite runs, so it refuses rather - * than print a statement SQLite refuses. + * The echo of a date bucket that no statement the engine runs produces: it + * refuses rather than print one. * * `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. + * groups by (the `dateBucketSql` hook). Two cases have no such expression, and + * both refuse here: + * + * - [#21630] **A non-UTC `timezone`, on every dialect.** The engine then + * buckets in memory on that zone's calendar (ADR-0053 Phase 2, D2; + * `tzRequiresInMemory` in objectql's `engine.ts`) on every driver: the driver + * only fetches the rows, and no statement the database runs groups by those + * keys. The representative `date_trunc('', col)` this printed + * instead groups on the database SESSION's calendar where it runs at all. + * Measured on PostgreSQL 16.14 with the server at `Asia/Shanghai`, it + * answered timestamp keys such as `2025-12-31T16:00:00.000Z` where the face + * answers `2026-01`, and at `America/New_York` it grouped 20 and 8 where the + * face groups 27 and 1. SQLite and MySQL have no `date_trunc` at all. + * - [#21595] **SQLite, where no driver expression answers**: a host whose hook + * answers nothing for a SQLite datasource. `date_trunc` is not a SQLite + * function, so the representative text is a statement SQLite refuses. * * `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 @@ -165,19 +174,21 @@ interface CrossObjectPlan { * `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. + * + * `zone` is the non-UTC `timezone` of the first case; `undefined` is the + * SQLite case. */ -function sqliteBucketEchoRefused(dimension: string, granularity: string, zone: string | undefined): Error { - const zoned = !!zone && zone !== 'UTC'; +function bucketEchoRefused(dimension: string, granularity: string, zone: string | undefined): Error { 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.` : '.'), + `[analytics] cannot render display SQL for the "${granularity}" bucket of "${dimension}"` + + (zone !== undefined + ? ` with timezone "${zone}". The query itself is SERVED: the engine buckets it in memory on that zone's ` + + `calendar, so no statement the database runs groups by those bucket keys. Refusing rather than printing ` + + `one that groups on another calendar. Run the query itself (/analytics/query) to get its rows; with ` + + `timezone "UTC" or none, a SQL driver groups the bucket itself and this dry run renders its expression.` + : ` on SQLite. 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.`), ) as Error & { code?: string; status?: number; refusal?: true }; err.code = StandardErrorCode.enum.NOT_IMPLEMENTED; err.status = 501; @@ -465,13 +476,15 @@ export class ObjectQLStrategy implements AnalyticsStrategy { * * [#21441] A date bucket renders the expression the driver itself groups by * 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), or a non-UTC `timezone`, which the engine buckets in - * memory on that zone's calendar. There it prints + * and answers the face's bucket keys. [#21630] A non-UTC `timezone`, which + * the engine buckets in memory on that zone's calendar, refuses on every + * dialect ({@link bucketEchoRefused}): no statement the engine runs groups + * by those keys. At a UTC or unset `timezone`, the bucket stays + * REPRESENTATIVE where the hook answers nothing (no hook, or a driver with no + * bucket expression, such as a non-SQL driver) and 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. + * `date_trunc`: there the echo refuses too, 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 @@ -583,15 +596,17 @@ export class ObjectQLStrategy implements AnalyticsStrategy { // answers `2026-01`. // // Asked only for a UTC or unset `timezone`. The driver's expression is a - // 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`, except - // on SQLite, which has no `date_trunc`: there the echo refuses - // ({@link sqliteBucketEchoRefused}, #21595). + // UTC bucket. [#21630] A non-UTC zone makes the engine bucket in memory on + // that zone's calendar instead, on every driver (ADR-0053 Phase 2, D2; + // `tzRequiresInMemory` in objectql's `engine.ts`, the same test on the same + // `timezone`), so no statement the database runs groups by those keys, and + // the echo refuses on every dialect before the hook is asked + // ({@link bucketEchoRefused}). Where nothing answers at UTC, the bucket + // keeps the representative `date_trunc`, except on SQLite, which has no + // `date_trunc`: there the echo refuses too (#21595). const zone = query.timezone; + const inMemoryZone = zone && zone !== 'UTC' ? zone : undefined; const driverBucketSql = (col: string, granularity: string): string | undefined => { - if (zone && zone !== 'UTC') return undefined; const answered = (ctx as DatasetScopedStrategyContext).dateBucketSql?.(tableName, col, granularity); return typeof answered === 'string' && answered !== '' ? answered : undefined; }; @@ -606,9 +621,10 @@ export class ObjectQLStrategy implements AnalyticsStrategy { const col = this.resolveFieldName(cube, dim, 'dimension'); const gran = granByDim.get(dim); if (!gran) return col; + if (inMemoryZone !== undefined) throw bucketEchoRefused(dim, gran, inMemoryZone); const bucket = driverBucketSql(col, gran); if (bucket !== undefined) return bucket; - if (sqlDialectFor(ctx, tableName) === 'sqlite') throw sqliteBucketEchoRefused(dim, gran, zone); + if (sqlDialectFor(ctx, tableName) === 'sqlite') throw bucketEchoRefused(dim, gran, undefined); return `date_trunc('${gran}', ${col})`; };