diff --git a/.changeset/21365-analytics-query-window.md b/.changeset/21365-analytics-query-window.md new file mode 100644 index 00000000000..f0fa8e5c1cf --- /dev/null +++ b/.changeset/21365-analytics-query-window.md @@ -0,0 +1,48 @@ +--- +'@objectstack/spec': minor +'@objectstack/service-analytics': patch +--- + +fix(spec)!: an analytics query's `limit` and `offset` are non-negative integers, and the native face runs an `offset` with no `limit` on SQLite + +Clause-②: yes (narrowing) + + + +**BREAKING** — an accept-set narrowing of a published request schema, shipped as `minor` under the repo's launch-window convention for accept-set narrowings. What reads it: the `/analytics` doors, which parse every body with `AnalyticsQueryRequestSchema` (`POST /analytics/query`, `POST /analytics/sql`) or `DatasetSelectionSchema` (`POST /analytics/dataset/query`), and answer `400 VALIDATION_FAILED` before any engine runs. + +**`@objectstack/spec`** + +- **`AnalyticsQuerySchema.limit` and `.offset`** were a bare `z.number()`. They are `z.number().int().nonnegative()` now. A negative number, a fraction, and an integer above `Number.MAX_SAFE_INTEGER` are refused at the member. `limit: 0` stays legal and answers no rows. +- **`DatasetSelectionSchema`** reads the same two declarations off `AnalyticsQuerySchema.shape`, so the dataset door holds the same accept set with no second copy. **`AnalyticsQueryRequestSchema`** extends the query, so it holds it too. +- The TypeScript types are unchanged (`number`). Only the parse narrows. + +Before, no refused value had one answer. Measured at `POST /api/v1/analytics/query` on SQLite and PostgreSQL 16.14, `order { note: 'asc' }` over four groups: + +| window | native SQLite | native PostgreSQL | ObjectQL face | +|:--|:--|:--|:--| +| `limit: -1` | every row | 500 | all but the last row | +| `limit: 1.5` | 500 | two rows | one row | +| `offset: -1` | 500 | 500 | every row | + +Each one now answers `400 VALIDATION_FAILED`, with `details.fields[].field` naming `limit` or `offset` (`selection.limit` / `selection.offset` at the dataset door), on both drivers and both faces. + +**`@objectstack/service-analytics`** + +- **An `offset` with no `limit`** is a valid window: every row after the offset. The native-SQL strategy wrote `OFFSET n` with no `LIMIT` in front of it, and SQLite's grammar has no `OFFSET` without a `LIMIT`, so the query answered `500` (`near "OFFSET": syntax error`) on SQLite, while PostgreSQL and the ObjectQL face answered rows. The statement now carries the executing driver's no-limit spelling, read off the `sqlDialect` hook: `LIMIT -1 OFFSET n` on SQLite, `OFFSET n` alone on PostgreSQL (unchanged bytes), and `LIMIT 9223372036854775807 OFFSET n` when the host names no dialect. The MySQL arm is `LIMIT 18446744073709551615`, asserted as text only (no MySQL server was available to run it). +- The echoed `sql` and `POST /analytics/sql` show the statement that ran, byte for byte, on this face. + +## FROM → TO + +| you wrote in an analytics query or dataset selection | write instead | +|:--|:--| +| `limit: -1` (meant: no limit) | omit `limit` | +| `limit: 1.5` | the integer page size you meant, for example `limit: 2` | +| `offset: -1` | omit `offset`, or `offset: 0` | +| `offset: 2.5` | the integer number of rows to skip, for example `offset: 2` | + +The one-line fix: write `limit` and `offset` as non-negative integers, or leave them out. + +## Who is affected, measured + +At `origin/main` `ee75aae1a`: no example, package fixture, document or published skill writes a negative or fractional analytics `limit` or `offset`. The one stored producer that lowers into a dataset selection, a dashboard widget's `limit`, is already declared a positive integer (`z.number().int().positive()`). The sibling console repository and deployed metadata were not measured. The service does not parse a query passed to it in-process, so a host that builds an `AnalyticsQuery` in code parses it with `AnalyticsQuerySchema` before handing it over. diff --git a/content/docs/references/api/analytics.mdx b/content/docs/references/api/analytics.mdx index 731f902a8eb..950d20d4103 100644 --- a/content/docs/references/api/analytics.mdx +++ b/content/docs/references/api/analytics.mdx @@ -97,8 +97,8 @@ const result = AnalyticsEndpoint.parse(data); | **where** | `any` | optional | Filtering criteria (canonical Query DSL FilterCondition). An authored `FilterArray` is lowered by `parseFilterAST` on the client before the wire; this field admits only the lowered `FilterCondition` (see `FilterArray` in `data/filter.zod.ts`). | | **timeDimensions** | `{ dimension: string; granularity?: Enum<'day' \| 'week' \| 'month' \| 'quarter' \| 'year'>; dateRange?: Enum<'today' \| 'yesterday' \| 'this_week' \| 'last_week' \| 'this_month' \| 'last_month' \| …> \| [string, string] }[]` | optional | Time-bucketed dimensions. Each entry names a dimension, an optional bucket `granularity`, and an optional `dateRange` — a preset name from the closed date-range vocabulary (e.g. `'last_7_days'`) or an explicit `[start, end]` window; an unrecognised string answers `400 ANALYTICS_DATE_RANGE_UNRECOGNIZED` instead of silently widening. | | **order** | `Record>` | optional | | -| **limit** | `number` | optional | | -| **offset** | `number` | optional | | +| **limit** | `integer` | optional | Maximum number of rows to return, applied after `order` — a non-negative integer (`0` returns no rows) | +| **offset** | `integer` | optional | Number of rows to skip before the first row returned, applied after `order` — a non-negative integer; an `offset` with no `limit` returns every row after it | | **timezone** | `string` | optional | | | **query** | `never` | optional | [REMOVED] `query` was removed from AnalyticsQueryRequest in @objectstack/spec 17.0.0. The `{ cube, query: {...} }` envelope was the dialect of the retired degraded analytics shim — the real engine never understood it. Move the query.* fields to the body top level: `{ cube, measures, dimensions?, where?, timeDimensions?, order?, limit?, offset?, timezone? }`. | | **format** | `never` | optional | [REMOVED] `format` was removed from AnalyticsQueryRequest in @objectstack/spec 17.0.0. It was never implemented — every response is the JSON envelope. Delete the key; for CSV/XLSX use the export surface instead. | @@ -226,8 +226,8 @@ const result = AnalyticsEndpoint.parse(data); | **timeDimensions** | `{ dimension: string; granularity?: Enum<'day' \| 'week' \| 'month' \| 'quarter' \| 'year'>; dateRange?: Enum<'today' \| 'yesterday' \| 'this_week' \| 'last_week' \| 'this_month' \| 'last_month' \| …> \| [string, string] }[]` | optional | Time-bucketed dimensions. Each entry names a dimension, an optional bucket `granularity`, and an optional `dateRange` — a preset name from the closed date-range vocabulary (e.g. `'last_7_days'`) or an explicit `[start, end]` window; an unrecognised string answers `400 ANALYTICS_DATE_RANGE_UNRECOGNIZED` instead of silently widening. | | **dateGranularity** | `Enum<'day' \| 'week' \| 'month' \| 'quarter' \| 'year'>` | optional | Presentation-scope date bucketing applied to every selected `date` dimension; an explicit `timeDimensions` entry wins over it, and the dataset dimension's own default is used when neither is set | | **order** | `Record>` | optional | | -| **limit** | `number` | optional | | -| **offset** | `number` | optional | | +| **limit** | `integer` | optional | Maximum number of rows to return, applied after `order` — a non-negative integer (`0` returns no rows) | +| **offset** | `integer` | optional | Number of rows to skip before the first row returned, applied after `order` — a non-negative integer; an `offset` with no `limit` returns every row after it | | **compareTo** | `{ kind: Enum<'previousPeriod' \| 'previousYear'>; dimension?: string }` | optional | Period-over-period comparison window (`{ kind, dimension? }`); attaches `__compare` columns | | **totals** | `{ groupings: string[][] }` | optional | Server-side marginal aggregates; each grouping is a dimension subset to additionally aggregate by, `[]` being the grand total | | **timezone** | `string` | optional | | diff --git a/content/docs/references/data/analytics.mdx b/content/docs/references/data/analytics.mdx index dd7ba809eb2..422775d08b2 100644 --- a/content/docs/references/data/analytics.mdx +++ b/content/docs/references/data/analytics.mdx @@ -101,8 +101,8 @@ Type: `[string, string]` | **where** | `any` | optional | Filtering criteria (canonical Query DSL FilterCondition). An authored `FilterArray` is lowered by `parseFilterAST` on the client before the wire; this field admits only the lowered `FilterCondition` (see `FilterArray` in `data/filter.zod.ts`). | | **timeDimensions** | `{ dimension: string; granularity?: Enum<'day' \| 'week' \| 'month' \| 'quarter' \| 'year'>; dateRange?: Enum<'today' \| 'yesterday' \| 'this_week' \| 'last_week' \| 'this_month' \| 'last_month' \| …> \| [string, string] }[]` | optional | Time-bucketed dimensions. Each entry names a dimension, an optional bucket `granularity`, and an optional `dateRange` — a preset name from the closed date-range vocabulary (e.g. `'last_7_days'`) or an explicit `[start, end]` window; an unrecognised string answers `400 ANALYTICS_DATE_RANGE_UNRECOGNIZED` instead of silently widening. | | **order** | `Record>` | optional | | -| **limit** | `number` | optional | | -| **offset** | `number` | optional | | +| **limit** | `integer` | optional | Maximum number of rows to return, applied after `order` — a non-negative integer (`0` returns no rows) | +| **offset** | `integer` | optional | Number of rows to skip before the first row returned, applied after `order` — a non-negative integer; an `offset` with no `limit` returns every row after it | | **timezone** | `string` | optional | | ### Nested Shape: `AnalyticsQuery.timeDimensions[number]` diff --git a/packages/runtime/src/analytics-query-window-validity.test.ts b/packages/runtime/src/analytics-query-window-validity.test.ts new file mode 100644 index 00000000000..e4725774767 --- /dev/null +++ b/packages/runtime/src/analytics-query-window-validity.test.ts @@ -0,0 +1,274 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#21365] An analytics query's row window has ONE answer at + * `POST /api/v1/analytics/query` (and its dry-run twin `/analytics/sql`), on + * both drivers and both faces: a window outside the non-negative integers is + * refused `400 VALIDATION_FAILED` at the door before any engine runs, and an + * `offset` with no `limit` answers the same rows everywhere. + * + * ## Measured on the base, through this door + * + * `main` `ee75aae1a`, four groups by `note` (w, x, y, z), `order { note: 'asc' }`: + * + * | window | native SQLite | native PostgreSQL 16.14 | ObjectQL face, both | + * |:--|:--|:--|:--| + * | `limit: -1` | 200, every row | 500 | 200, all but the last row | + * | `limit: 1.5` | 500 | 200, two rows | 200, one row | + * | `offset: -1` | 500 | 500 | 200, every row | + * | `offset: 1`, no `limit` | 500 (`near "OFFSET": syntax error`) | 200, x y z | 200, x y z | + * + * `/analytics/sql` answered 200 for every one of them, rendering the statement. + * + * ## The composition is the shipped one + * + * `AnalyticsServicePlugin` over a real `ObjectQL` engine as its `'data'` + * service: the default composition (native face, both auto-bridges live) and + * one narrowed to the engine aggregate (the ObjectQL face). The route is the + * real `dispatcher-plugin` mount. + * + * ## The dialect axis of THIS file + * + * The SQLite cell always runs. The PostgreSQL cell runs where + * `OS_TEST_POSTGRES_URL` is set and is a named skip otherwise. No CI step + * provisions that variable for this file, so the live cell is red-capable and + * un-run in CI; the PR that landed this file carries its local PostgreSQL 16 + * run. The live cell owns its table, dropped before and after. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { AnalyticsServicePlugin, type AnalyticsService } from '@objectstack/service-analytics'; + +import { createDispatcherPlugin } from './dispatcher-plugin.js'; + +// [#21061] The analytics domain refuses an anonymous caller first (ADR-0056 D2), +// so this harness signs its caller in: an `auth` slot in the shape +// `resolveExecutionContext` reads answers a session for every request. Only +// identity is stubbed; the route, the service and every expectation are +// unchanged. Anonymity is pinned in `domains/analytics-anonymous-deny.test.ts`. +const SIGNED_IN_AUTH = { api: { getSession: async () => ({ user: { id: 'usr_analytics_caller' } }) } }; + +const OBJECT = 'os21365_window_deal'; +const CUBE = 'os21365_window_cube'; + +const DEAL = { + name: OBJECT, + label: 'Window deal', + fields: { + note: { name: 'note', type: 'text' as const }, + amount: { name: 'amount', type: 'number' as const }, + }, +}; + +// Inserted out of note order, so an answer in note order is the ORDER BY's. +const ROWS = [ + { id: 'd1', note: 'y', amount: 20 }, + { id: 'd2', note: 'w', amount: 7 }, + { id: 'd3', note: 'z', amount: 1 }, + { id: 'd4', note: 'x', amount: 10 }, +]; + +const WINDOW_CUBE = { + name: CUBE, + title: 'Window cube', + sql: OBJECT, + public: true, + measures: { amount_sum: { type: 'sum' as const, sql: 'amount', label: 'Amount' } }, + dimensions: { note: { type: 'string' as const, sql: 'note', label: 'Note' } }, +}; + +const BASE = { cube: CUBE, measures: ['amount_sum'], dimensions: ['note'], order: { note: 'asc' } }; + +/** The card's rows 1 to 3, plus the fractional offset — the key the door names for each. */ +const REFUSED: ReadonlyArray<[string, Record, 'limit' | 'offset']> = [ + ['limit: -1', { limit: -1 }, 'limit'], + ['limit: 1.5', { limit: 1.5 }, 'limit'], + ['offset: -1', { offset: -1 }, 'offset'], + ['offset: 1.5', { offset: 1.5 }, 'offset'], +]; + +interface Cell { + id: 'sqlite' | 'pg'; + label: string; + env: string | null; + config: () => Record | null; +} + +const CELLS: readonly Cell[] = [ + { id: 'sqlite', label: 'sqlite', env: null, config: () => ({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }) }, + { + id: 'pg', + label: 'live postgres', + env: 'OS_TEST_POSTGRES_URL', + config: () => (process.env.OS_TEST_POSTGRES_URL ? { client: 'pg', connection: process.env.OS_TEST_POSTGRES_URL } : null), + }, +]; + +const FACES = ['native', 'objectql'] as const; +type Face = (typeof FACES)[number]; + +const quiet = { debug() {}, info() {}, warn() {}, error() {}, child() { return quiet; } }; + +type Handler = (req: unknown, res: unknown) => unknown; + +function makeRes() { + const res: any = { + statusCode: undefined as number | undefined, + body: undefined as any, + status(c: number) { res.statusCode = c; return res; }, + header() { return res; }, + json(b: unknown) { res.body = b; return res; }, + }; + return res; +} + +const notes = (body: any) => (body.data.rows as Array>).map((row) => row.note); + +for (const cell of CELLS) { + const config = cell.config(); + describe.skipIf(!config)( + `[#21365] the analytics query window at POST /api/v1/analytics/query — ${cell.label}${config ? '' : ` (skipped: set ${cell.env} to run this cell)`}`, + () => { + let driver: any; + let engine: ObjectQL; + /** Raw-SQL statements and engine aggregates that read THIS object. */ + const reads = { rawSql: [] as string[], aggregate: 0 }; + const handlers: Partial>> = {}; + + const dropTables = async () => { + if (cell.id === 'sqlite') return; + await driver?.execute(`drop table if exists ${OBJECT}`).catch(() => {}); + }; + + async function post(face: Face, sub: 'query' | 'sql', body: Record) { + const handler = handlers[face]![`POST /api/v1/analytics/${sub}`]; + expect(handler, `POST /api/v1/analytics/${sub} must be mounted`).toBeTypeOf('function'); + const res = makeRes(); + // What the wire carries: JSON. + await handler({ body: JSON.parse(JSON.stringify(body)), query: {} }, res); + return { status: res.statusCode ?? 200, body: res.body }; + } + + beforeAll(async () => { + driver = new SqlDriver(config as any); + await dropTables(); + engine = new ObjectQL({ logger: quiet } as any); + engine.registerDriver(driver, true); + await engine.init(); + engine.registry.registerObject(DEAL as any); + await engine.syncSchemas(); + for (const row of ROWS) await engine.insert(OBJECT, { ...row } as any); + + // Count what reaches the engine for THIS object, on both bridges. + const realExecute = (engine as any).execute.bind(engine); + (engine as any).execute = (sql: unknown, opts?: { object?: string }) => { + if (opts?.object === OBJECT) reads.rawSql.push(String(sql)); + return realExecute(sql, opts); + }; + const realAggregate = engine.aggregate.bind(engine); + (engine as any).aggregate = (object: string, ...rest: unknown[]) => { + if (object === OBJECT) reads.aggregate += 1; + return (realAggregate as any)(object, ...rest); + }; + + for (const [face, caps] of [ + ['native', undefined], + ['objectql', () => ({ nativeSql: false, objectqlAggregate: true, inMemory: false })], + ] as const) { + const registered: Record = {}; + await new AnalyticsServicePlugin({ cubes: [WINDOW_CUBE as any], debugSql: true, ...(caps ? { queryCapabilities: caps } : {}) } as any).init({ + getService: (name: string) => (name === 'data' ? engine : registered[name]), + registerService: (name: string, svc: unknown) => { registered[name] = svc; }, + replaceService: (name: string, svc: unknown) => { registered[name] = svc; }, + hook: () => {}, + logger: quiet, + } as never); + const analytics = registered.analytics as AnalyticsService; + + const routes: Record = {}; + const rec = (verb: string) => (path: string, handler: Handler) => { routes[`${verb} ${path}`] = handler; }; + const server = { get: rec('GET'), post: rec('POST'), put: rec('PUT'), delete: rec('DELETE'), patch: rec('PATCH') }; + const kernel = { + getService: (name: string) => (name === 'analytics' ? analytics : name === 'auth' ? SIGNED_IN_AUTH : undefined), + getServiceAsync: async (name: string) => (name === 'analytics' ? analytics : name === 'auth' ? SIGNED_IN_AUTH : undefined), + }; + const plugin = createDispatcherPlugin({ prefix: '/api/v1', securityHeaders: false }); + await plugin.start?.({ + getKernel: () => kernel, + getService: (name: string) => (name === 'http.server' ? server : undefined), + environmentId: undefined, + logger: quiet, + hook: () => {}, + on: () => {}, + } as any); + handlers[face] = routes; + } + }); + + afterAll(async () => { + await dropTables(); + try { await engine?.destroy(); } catch { /* noop */ } + }); + + for (const [label, window, key] of REFUSED) { + it(`${label} answers 400 VALIDATION_FAILED naming \`${key}\` at both routes, on both faces — no engine runs`, async () => { + for (const face of FACES) { + for (const sub of ['query', 'sql'] as const) { + const before = { rawSql: reads.rawSql.length, aggregate: reads.aggregate }; + const res = await post(face, sub, { ...BASE, ...window }); + expect(res.status, `${face} /${sub}: ${JSON.stringify(res.body)}`).toBe(400); + expect(res.body.error.code, `${face} /${sub}`).toBe('VALIDATION_FAILED'); + expect(res.body.error.httpStatus, `${face} /${sub}`).toBe(400); + const fields: Array<{ field: string }> = res.body.error.details.fields; + expect(fields.map((f) => f.field), `${face} /${sub}`).toEqual([key]); + expect( + { rawSql: reads.rawSql.length, aggregate: reads.aggregate }, + `${face} /${sub}: no raw SQL and no engine aggregate for the object`, + ).toEqual(before); + } + } + }); + } + + it("the card's row 4 — ordered, offset 1, no limit — answers the same rows on both faces", async () => { + for (const face of FACES) { + const res = await post(face, 'query', { ...BASE, offset: 1 }); + expect(res.status, `${face}: ${JSON.stringify(res.body)}`).toBe(200); + expect(notes(res.body), face).toEqual(['x', 'y', 'z']); + } + }); + + it('row 4 on the native face: the echoed `sql` and /analytics/sql are the statement that ran', async () => { + const before = reads.rawSql.length; + const res = await post('native', 'query', { ...BASE, offset: 1 }); + const ran = reads.rawSql.slice(before); + expect(ran, 'the native face ran ONE statement').toHaveLength(1); + // No bound parameter, so the raw-SQL bridge's `$N` → `?` rewrite + // leaves the statement byte-identical to the echo. + expect(res.body.data.sql).not.toContain('$'); + expect(res.body.data.sql).toBe(ran[0]); + const dryRun = await post('native', 'sql', { ...BASE, offset: 1 }); + expect(dryRun.status).toBe(200); + expect(dryRun.body.data.sql).toBe(ran[0]); + }); + + it('CONTROL: an integer window — limit 2, offset 1 — answers the same rows on both faces', async () => { + for (const face of FACES) { + const res = await post(face, 'query', { ...BASE, limit: 2, offset: 1 }); + expect(res.status, `${face}: ${JSON.stringify(res.body)}`).toBe(200); + expect(notes(res.body), face).toEqual(['x', 'y']); + } + }); + + it('CONTROL: limit 0 answers no rows on both faces', async () => { + for (const face of FACES) { + const res = await post(face, 'query', { ...BASE, limit: 0 }); + expect(res.status, `${face}: ${JSON.stringify(res.body)}`).toBe(200); + expect(res.body.data.rows, face).toEqual([]); + } + }); + }, + ); +} diff --git a/packages/services/service-analytics/src/__tests__/native-sql-offset-only-window.test.ts b/packages/services/service-analytics/src/__tests__/native-sql-offset-only-window.test.ts new file mode 100644 index 00000000000..28a17bdc58a --- /dev/null +++ b/packages/services/service-analytics/src/__tests__/native-sql-offset-only-window.test.ts @@ -0,0 +1,293 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#21365] An `offset` with no `limit` is a valid window — every row after the + * offset — and the native face renders it for the dialect that runs it. + * + * ## The shape this closes + * + * `NativeSQLStrategy.assembleStatement` wrote `OFFSET n` with no `LIMIT` in + * front of it. Measured at `POST /api/v1/analytics/query` on `main` + * `ee75aae1a`, `order { note: 'asc' }, offset: 1`: + * + * | | native SQLite | native PostgreSQL 16.14 | ObjectQL face, both | + * |:--|:--|:--|:--| + * | answer | 500, `near "OFFSET": syntax error` | rows 2..n | rows 2..n | + * + * SQLite's grammar has no `OFFSET` without a `LIMIT`; its "no upper bound" is + * a negative `LIMIT`. The statement now carries the dialect's no-limit + * spelling (`windowClauseSql`), read off the `sqlDialect` hook the plugin + * fills from the executing driver. + * + * ## The cells, and what each one's evidence is + * + * - **sqlite → EXECUTED** (better-sqlite3), every run: `LIMIT -1 OFFSET n`. + * - **postgres → EXECUTED** where `OS_TEST_POSTGRES_URL` is set, a named skip + * otherwise: `OFFSET n` alone, byte-identical to before. No CI step + * provisions that variable for this package, so the live cell is + * red-capable and un-run in CI; the PR that landed this file carries its + * local PostgreSQL 16 run. + * - **unknown (no `sqlDialect` hook) → EXECUTED** on both engines above, + * through a `NativeSQLStrategy` whose context names no dialect: + * `LIMIT 9223372036854775807 OFFSET n`, which every LIMIT dialect parses. + * - **mysql → NOT MEASURED.** Asserted as text only: no MySQL server is + * provisionable in this container. + * + * Each executed cell answers the same rows on both faces, and the native + * face's echoed `sql` and `generateSql` (the `/analytics/sql` body) are the + * statement that ran, byte for byte. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import type { Cube } from '@objectstack/spec/data'; +import type { AnalyticsService } from '../analytics-service.js'; +import { AnalyticsServicePlugin } from '../plugin.js'; +import { NativeSQLStrategy, windowClauseSql } from '../strategies/native-sql-strategy.js'; +import type { DatasetScopedStrategyContext } from '../strategies/types.js'; + +const DEAL = 'os21365_deal'; + +const DEAL_OBJECT = { + name: DEAL, + label: 'Offset window deal', + fields: { + note: { name: 'note', type: 'text' as const }, + amount: { name: 'amount', type: 'number' as const }, + }, +}; + +// Inserted out of note order, so an answer in note order is the ORDER BY's. +const DEALS = [ + { id: 'd1', note: 'y', amount: 20 }, + { id: 'd2', note: 'w', amount: 7 }, + { id: 'd3', note: 'z', amount: 1 }, + { id: 'd4', note: 'x', amount: 10 }, + { id: 'd5', note: 'x', amount: 5 }, +] as const; + +const CUBE = 'os21365_cube'; +const CUBES = [ + { + name: CUBE, + title: 'Offset window cube', + sql: DEAL, + public: true, + measures: { amount_sum: { type: 'sum', sql: 'amount', label: 'Amount' } }, + dimensions: { note: { type: 'string', sql: 'note', label: 'Note' } }, + }, +] as unknown as Cube[]; + +const DATASET = { + name: 'os21365_ds', + label: 'Offset window dataset', + object: DEAL, + dimensions: [{ name: 'note', field: 'note', type: 'string' }], + measures: [{ name: 'amount_sum', aggregate: 'sum', field: 'amount' }], +}; + +/** The card's row 4: ordered, an offset, no limit. */ +const OFFSET_ONLY = { cube: CUBE, measures: ['amount_sum'], dimensions: ['note'], order: { note: 'asc' }, offset: 1 }; +/** Every group after the first, in note order. */ +const AFTER_FIRST = [['x', 15], ['y', 20], ['z', 1]]; + +interface Cell { + id: 'sqlite' | 'pg'; + label: string; + env: string | null; + config: () => Record | null; + /** The no-limit spelling the plugin's dialect hook makes the native face write, `''` for none. */ + noLimit: string; +} + +const CELLS: readonly Cell[] = [ + { + id: 'sqlite', + label: 'sqlite', + env: null, + config: () => ({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }), + noLimit: ' LIMIT -1', + }, + { + id: 'pg', + label: 'live postgres', + env: 'OS_TEST_POSTGRES_URL', + config: () => (process.env.OS_TEST_POSTGRES_URL ? { client: 'pg', connection: process.env.OS_TEST_POSTGRES_URL } : null), + noLimit: '', + }, +]; + +const FACES = ['native', 'objectql'] as const; +type Face = (typeof FACES)[number]; + +const quiet = { debug() {}, info() {}, warn() {}, error() {}, child() { return quiet; } }; + +type Row = Record; + +/** Rows as tuples of the named columns, in arrival order; a numeric cell reads as a number on every dialect. */ +const tuples = (rows: unknown, columns: readonly string[]) => + (rows as Row[]).map((row) => columns.map((c) => (typeof row[c] === 'number' || /^-?\d+(\.\d+)?$/.test(String(row[c])) ? Number(row[c]) : row[c]))); + +describe('[#21365] windowClauseSql — the window clause per dialect', () => { + it('an offset with no limit takes the dialect\'s no-limit spelling in front of OFFSET', () => { + expect(windowClauseSql(undefined, 1, 'sqlite')).toBe(' LIMIT -1 OFFSET 1'); + expect(windowClauseSql(undefined, 1, 'postgres')).toBe(' OFFSET 1'); + expect(windowClauseSql(undefined, 1, 'unknown')).toBe(' LIMIT 9223372036854775807 OFFSET 1'); + // NOT MEASURED: text only — no MySQL server is provisionable here. + expect(windowClauseSql(undefined, 1, 'mysql')).toBe(' LIMIT 18446744073709551615 OFFSET 1'); + }); + + it('CONTROL: a limit is written as given, with or without an offset, on every dialect', () => { + for (const dialect of ['sqlite', 'postgres', 'mysql', 'unknown'] as const) { + expect(windowClauseSql(2, 1, dialect), dialect).toBe(' LIMIT 2 OFFSET 1'); + expect(windowClauseSql(0, undefined, dialect), dialect).toBe(' LIMIT 0'); + expect(windowClauseSql(undefined, undefined, dialect), dialect).toBe(''); + } + }); +}); + +for (const cell of CELLS) { + const config = cell.config(); + describe.skipIf(!config)( + `[#21365] an offset with no limit runs on the native face (${cell.label})${config ? '' : ` (skipped: set ${cell.env} to run this cell)`}`, + () => { + let driver: any; + let engine: ObjectQL; + /** Every raw statement the engine ran, in order. */ + const executed: string[] = []; + const reads = { aggregate: 0 }; + const services: Partial> = {}; + + const dropTables = async () => { + if (cell.id !== 'pg') return; + await driver?.execute(`drop table if exists ${DEAL}`).catch(() => {}); + }; + + /** One `query()` on one face, with the statements and aggregates it caused. */ + const ask = async (face: Face, query: Record) => { + const before = { statements: executed.length, aggregate: reads.aggregate }; + const res = await services[face]!.query(query as any); + return { res, statements: executed.slice(before.statements), aggregate: reads.aggregate - before.aggregate }; + }; + + beforeAll(async () => { + driver = new SqlDriver(config as any); + await dropTables(); + engine = new ObjectQL({ logger: quiet } as any); + engine.registerDriver(driver, true); + await engine.init(); + engine.registry.registerObject(DEAL_OBJECT as any); + await engine.syncSchemas(); + for (const row of DEALS) await engine.insert(DEAL, { ...row } as any); + + const realExecute = (engine as any).execute.bind(engine); + (engine as any).execute = (sql: unknown, opts?: unknown) => { + executed.push(String(sql)); + return realExecute(sql, opts); + }; + const realAggregate = engine.aggregate.bind(engine); + (engine as any).aggregate = (...args: unknown[]) => { + reads.aggregate += 1; + return (realAggregate as any)(...args); + }; + + for (const [face, caps] of [ + ['native', undefined], + ['objectql', () => ({ nativeSql: false, objectqlAggregate: true, inMemory: false })], + ] as const) { + const registered: Record = {}; + await new AnalyticsServicePlugin({ cubes: CUBES, debugSql: true, ...(caps ? { queryCapabilities: caps } : {}) } as any).init({ + getService: (name: string) => (name === 'data' ? engine : registered[name]), + registerService: (name: string, svc: unknown) => { registered[name] = svc; }, + replaceService: (name: string, svc: unknown) => { registered[name] = svc; }, + hook: () => {}, + logger: quiet, + } as never); + services[face] = registered.analytics as AnalyticsService; + } + }); + + afterAll(async () => { + await dropTables(); + try { await engine?.destroy(); } catch { /* noop */ } + }); + + it("the card's row: ordered, offset 1, no limit — both faces answer every row after the first", async () => { + const native = await ask('native', OFFSET_ONLY); + expect(native.statements, 'the native face ran ONE statement').toHaveLength(1); + expect(native.statements[0].endsWith(`ORDER BY "note" ASC${cell.noLimit} OFFSET 1`), native.statements[0]).toBe(true); + expect(tuples(native.res.rows, ['note', 'amount_sum']), 'native face').toEqual(AFTER_FIRST); + + const objectql = await ask('objectql', OFFSET_ONLY); + expect(objectql.statements, 'the ObjectQL face ran no statement').toEqual([]); + expect(objectql.aggregate, 'the ObjectQL face ran the engine aggregate').toBeGreaterThan(0); + expect(tuples(objectql.res.rows, ['note', 'amount_sum']), 'ObjectQL face').toEqual(AFTER_FIRST); + }); + + it('the echo is the statement that ran: the result\'s `sql` and `generateSql` (the /analytics/sql body) carry its bytes', async () => { + const { res, statements } = await ask('native', OFFSET_ONLY); + expect(statements).toHaveLength(1); + // The statement binds no parameter, so the raw-SQL bridge's `$N` → `?` + // rewrite leaves it byte-identical: the echo can be compared as is. + expect(res.sql).not.toContain('$'); + expect(res.sql).toBe(statements[0]); + const dryRun = await services.native!.generateSql!(OFFSET_ONLY as any); + expect(dryRun.sql).toBe(statements[0]); + }); + + it('an offset past every row answers no rows, and offset 0 answers every row', async () => { + const past = await ask('native', { ...OFFSET_ONLY, offset: 9 }); + expect(past.res.rows).toEqual([]); + const zero = await ask('native', { ...OFFSET_ONLY, offset: 0 }); + expect(zero.statements[0].endsWith(`${cell.noLimit} OFFSET 0`), zero.statements[0]).toBe(true); + expect(tuples(zero.res.rows, ['note', 'amount_sum'])).toEqual([['w', 7], ...AFTER_FIRST]); + }); + + it('CONTROL: an integer window keeps its bytes — `LIMIT 2 OFFSET 1` — and both faces agree', async () => { + const query = { ...OFFSET_ONLY, limit: 2 }; + const native = await ask('native', query); + expect(native.statements[0].endsWith('ORDER BY "note" ASC LIMIT 2 OFFSET 1'), native.statements[0]).toBe(true); + expect(tuples(native.res.rows, ['note', 'amount_sum'])).toEqual([['x', 15], ['y', 20]]); + const objectql = await ask('objectql', query); + expect(tuples(objectql.res.rows, ['note', 'amount_sum'])).toEqual([['x', 15], ['y', 20]]); + }); + + it('the dataset door pushes the offset-only window down, and both faces answer the same page', async () => { + for (const face of FACES) { + const before = executed.length; + const res = await services[face]!.queryDataset(DATASET as any, { + dimensions: ['note'], + measures: ['amount_sum'], + order: { note: 'asc' }, + offset: 1, + } as any); + expect(tuples(res.rows, ['note', 'amount_sum']), face).toEqual(AFTER_FIRST); + if (face === 'native') { + const ran = executed.slice(before); + expect(ran.some((sql) => sql.endsWith(`${cell.noLimit} OFFSET 1`)), ran.join('\n')).toBe(true); + } + } + }); + + it('a host that wires no sqlDialect hook (the `unknown` arm) runs the window too, and answers the same rows', async () => { + const strategy = new NativeSQLStrategy(); + // The plugin's own raw-SQL bridge, restated: `$N` → `?`, the object as + // the driver-selection key, `{ rows }` or an array back. No `sqlDialect`. + const ctx = { + getCube: (name: string) => (name === CUBE ? CUBES[0] : undefined), + queryCapabilities: () => ({ nativeSql: true, objectqlAggregate: false, inMemory: false }), + executeRawSql: async (object: string, sql: string, params: unknown[]) => { + const result = await (engine as any).execute(sql.replace(/\$(\d+)/g, '?'), { args: params, object }); + return Array.isArray(result) ? result : (result as { rows: Row[] }).rows; + }, + } as unknown as DatasetScopedStrategyContext; + const { sql } = await strategy.generateSql(OFFSET_ONLY as any, ctx); + expect(sql.endsWith('ORDER BY "note" ASC LIMIT 9223372036854775807 OFFSET 1'), sql).toBe(true); + const res = await strategy.execute(OFFSET_ONLY as any, ctx); + expect(res.sql).toBe(sql); + expect(tuples(res.rows, ['note', 'amount_sum'])).toEqual(AFTER_FIRST); + }); + }, + ); +} diff --git a/packages/services/service-analytics/src/strategies/native-sql-strategy.ts b/packages/services/service-analytics/src/strategies/native-sql-strategy.ts index d2c652842c9..d1ae690aef0 100644 --- a/packages/services/service-analytics/src/strategies/native-sql-strategy.ts +++ b/packages/services/service-analytics/src/strategies/native-sql-strategy.ts @@ -22,7 +22,7 @@ import { declaredValueShapeResolver, whereEmptyLeafSql } from '../empty-operator import { columnObjectOf, relationshipReferenceOf, resolvePathHops, type HopReference } from '../hop-object.js'; import { datasetInvalidError, invalidMemberError } from '../dataset-refusal.js'; import { type LikeShape } from '../like-pattern.js'; -import { textMatchPredicateSql, sqlDialectFor } from '../text-match-sql.js'; +import { textMatchPredicateSql, sqlDialectFor, type AnalyticsSqlDialect } from '../text-match-sql.js'; import { whereContainsMembershipSql } from '../contains-membership-sql.js'; import { isJsonStoredShape } from '../contains-membership-sql.js'; import { expandEmptyOperator } from '@objectstack/spec/data'; @@ -131,6 +131,68 @@ export const CONDITIONAL_AGGREGATE_SQL_KEYS = Object.keys(CONDITIONAL_AGGREGATE_ */ export const EXPRESSION_METRIC_TYPES = new Set(['number', 'string', 'boolean']); +/** + * [#21365] The `LIMIT` an offset-only window carries, per dialect — `null` + * where the dialect takes `OFFSET` with no `LIMIT` in front of it. + * + * An `offset` with no `limit` is a valid window (every row after the offset), + * and PostgreSQL runs it as written. SQLite and MySQL do not: their grammar + * has no `OFFSET` without a `LIMIT`, so `… ORDER BY "note" ASC OFFSET 1` + * answered `near "OFFSET": syntax error`, a 500, on SQLite — measured at + * `POST /analytics/query`, where PostgreSQL and the ObjectQL face both + * answered rows. The cell for a named dialect is that dialect's own "no upper + * bound", the spelling the driver's query compiler emits for the same window + * (knex 3.3.0: `limit -1` in the sqlite3 compiler, `limit + * 18446744073709551615` in the mysql one, nothing in the base compiler + * PostgreSQL uses), so the native statement and the engine agree on it. + * + * `unknown` is not a dialect: it is everything the `sqlDialect` hook could not + * name, SQLite among it (`text-match-sql.ts` lists the embedder compositions + * that reach it). Its cell is the largest `LIMIT` every LIMIT dialect parses — + * SQLite's signed 64-bit maximum, PostgreSQL's `bigint` maximum, inside + * MySQL's unsigned range — so an unnamed SQLite runs the window too, and an + * unnamed PostgreSQL answers the rows a bare `OFFSET` answers. + * + * ⚠️ The `mysql` cell is NOT MEASURED: no MySQL server is provisionable where + * this landed, the same declared skip as the `mysql` arm in + * `text-match-sql.ts`. + */ +const OFFSET_ONLY_LIMIT_SQL: Readonly> = { + sqlite: 'LIMIT -1', + mysql: 'LIMIT 18446744073709551615', + postgres: null, + unknown: 'LIMIT 9223372036854775807', +}; + +/** + * [#21365] The window clause a statement ends with, for `dialect`: ` LIMIT n`, + * ` OFFSET n`, both, or `''` for no window. + * + * `limit` and `offset` are non-negative integers by contract + * (`AnalyticsQuerySchema`), refused `400 VALIDATION_FAILED` at the + * `/analytics` door otherwise, so they are written verbatim. An offset with no + * limit takes the dialect's no-limit spelling ({@link OFFSET_ONLY_LIMIT_SQL}) + * in front of it. + * + * Exported so a face that echoes a statement for the same window can render + * it with the same bytes rather than a second spelling. + */ +export function windowClauseSql( + limit: number | undefined, + offset: number | undefined, + dialect: AnalyticsSqlDialect, +): string { + let sql = ''; + if (limit != null) { + sql += ` LIMIT ${limit}`; + } else if (offset != null) { + const noLimit = OFFSET_ONLY_LIMIT_SQL[dialect]; + if (noLimit) sql += ` ${noLimit}`; + } + if (offset != null) sql += ` OFFSET ${offset}`; + return sql; +} + /** * A dot-separated chain of bare identifiers — `amount`, `account.amount`, * `account.owner.region`. Distinguishes a relationship PATH, which @@ -1062,12 +1124,9 @@ export class NativeSQLStrategy implements AnalyticsStrategy { const orderClauses = Object.entries(query.order).map(([f, d]) => `"${f}" ${d.toUpperCase()}`); sql += ` ORDER BY ${orderClauses.join(', ')}`; } - if (query.limit != null) { - sql += ` LIMIT ${query.limit}`; - } - if (query.offset != null) { - sql += ` OFFSET ${query.offset}`; - } + // [#21365] The dialect of the driver `execute()` hands this statement to — + // the base object's, the one `executeRawSql` is called with. + sql += windowClauseSql(query.limit, query.offset, sqlDialectFor(ctx, this.extractObjectName(cube))); return { sql, params }; } diff --git a/packages/spec/src/data/analytics-query-window-integer.test.ts b/packages/spec/src/data/analytics-query-window-integer.test.ts new file mode 100644 index 00000000000..7d4927154b9 --- /dev/null +++ b/packages/spec/src/data/analytics-query-window-integer.test.ts @@ -0,0 +1,96 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#21365] An analytics query's row window — `limit` and `offset` — is a + * non-negative integer each, on every schema that carries it. + * + * ## The shape this closes + * + * Both were a bare `z.number()`, and every value outside the non-negative + * integers had no single answer. Measured at `POST /api/v1/analytics/query` + * (the real dispatcher route, `AnalyticsServicePlugin` over a real `ObjectQL` + * engine and `SqlDriver`) on `main` `ee75aae1a`, SQLite and PostgreSQL 16.14: + * + * | window | native SQLite | native PostgreSQL | ObjectQL face | + * |:--|:--|:--|:--| + * | `limit: -1` | every row | 500 | all but the last row | + * | `limit: 1.5` | 500 | 2 rows | 1 row | + * | `offset: -1` | 500 | 500 | every row | + * + * The schema now refuses each of them, and the route answers + * `400 VALIDATION_FAILED` before any engine runs (pinned at the route in + * `packages/runtime/src/analytics-query-window-validity.test.ts`). + * + * ## The three schemas, and why each is asserted + * + * `AnalyticsQuerySchema` declares the two fields. `AnalyticsQueryRequestSchema` + * (the `/analytics/query` and `/analytics/sql` body) extends it, and + * `DatasetSelectionSchema` (the `/analytics/dataset/query` selection) reads the + * two declarations off its `shape`. One declaration, three doors: the identity + * pin below is what makes "no second declaration" a fact rather than a claim. + * + * A refusal is asserted by the issue's path and code — the named subject and + * the kind — never by zod's message. An acceptance is a whole `safeParse` + * success, because this is a value verdict (an `unrecognized_keys`-free parse + * would say nothing about the value). + */ + +import { describe, it, expect } from 'vitest'; + +import { AnalyticsQuerySchema } from './analytics.zod'; +import { AnalyticsQueryRequestSchema, DatasetSelectionSchema } from '../api/analytics.zod'; + +type Parser = { safeParse: (v: unknown) => { success: boolean; error?: { issues: Array<{ path: PropertyKey[]; code: string }> } } }; + +/** Each door with the smallest body it accepts. */ +const DOORS: ReadonlyArray<[string, Parser, Record]> = [ + ['AnalyticsQuerySchema', AnalyticsQuerySchema as unknown as Parser, { measures: ['orders.count'] }], + ['AnalyticsQueryRequestSchema', AnalyticsQueryRequestSchema as unknown as Parser, { cube: 'orders', measures: ['count'] }], + ['DatasetSelectionSchema', DatasetSelectionSchema as unknown as Parser, { measures: ['revenue'] }], +]; + +/** The values with no single answer, the key each sits on, and the zod issue code it raises. */ +const REFUSED: ReadonlyArray<[string, Record, 'limit' | 'offset', string]> = [ + ['a negative limit', { limit: -1 }, 'limit', 'too_small'], + ['a fractional limit', { limit: 1.5 }, 'limit', 'invalid_type'], + ['a negative offset', { offset: -1 }, 'offset', 'too_small'], + ['a fractional offset', { offset: 1.5 }, 'offset', 'invalid_type'], +]; + +/** CONTROL: integer windows — the offset-only one included, which is valid and rendered per dialect, never refused. */ +const ACCEPTED: ReadonlyArray<[string, Record]> = [ + ['a limit and an offset', { limit: 2, offset: 1 }], + ['limit 0 (no rows, as LIMIT 0)', { limit: 0 }], + ['offset 0', { offset: 0 }], + ['an offset with no limit', { offset: 1 }], + ['no window at all', {}], +]; + +for (const [name, schema, base] of DOORS) { + describe(`[#21365] ${name} — the window is a non-negative integer`, () => { + for (const [label, window, key, code] of REFUSED) { + it(`refuses ${label} at \`${key}\``, () => { + const r = schema.safeParse({ ...base, ...window }); + expect(r.success, `expected a refusal of ${JSON.stringify(window)}`).toBe(false); + const issues = r.error!.issues; + expect(issues).toHaveLength(1); + expect(issues[0].path).toEqual([key]); + expect(issues[0].code).toBe(code); + }); + } + + for (const [label, window] of ACCEPTED) { + it(`CONTROL accepts ${label}`, () => { + const r = schema.safeParse({ ...base, ...window }); + expect(r.success, JSON.stringify(r.error?.issues)).toBe(true); + }); + } + }); +} + +describe('[#21365] the dataset selection holds the query\'s own declarations, not a copy', () => { + it('`limit` and `offset` on DatasetSelectionSchema are the AnalyticsQuerySchema instances', () => { + expect(DatasetSelectionSchema.shape.limit).toBe(AnalyticsQuerySchema.shape.limit); + expect(DatasetSelectionSchema.shape.offset).toBe(AnalyticsQuerySchema.shape.offset); + }); +}); diff --git a/packages/spec/src/data/analytics.zod.ts b/packages/spec/src/data/analytics.zod.ts index efde10ad72e..9663bfa5037 100644 --- a/packages/spec/src/data/analytics.zod.ts +++ b/packages/spec/src/data/analytics.zod.ts @@ -965,8 +965,32 @@ export const AnalyticsQuerySchema = lazySchema(() => strictObject( order: z.record(z.string(), z.enum(['asc', 'desc'])).optional(), - limit: z.number().optional(), - offset: z.number().optional(), + /** + * The row window, applied after `order`: a non-negative integer each. + * + * Both were a bare `z.number()` until #21365, and every value outside the + * non-negative integers answered differently per driver and per face — + * measured at `POST /analytics/query`: `limit: -1` returned every row on + * SQLite, a 500 on PostgreSQL and all but the last row on the ObjectQL face; + * `limit: 1.5` a 500, two rows and one row; `offset: -1` a 500 on both + * drivers and a slice on the ObjectQL face. No answer was one answer, so the + * schema refuses them (`400 VALIDATION_FAILED` at the door) rather than any + * engine guessing. `limit: 0` stays legal — it is `LIMIT 0`, no rows. + * + * An `offset` with no `limit` is a valid window (every row after the + * offset); each face renders it for its own dialect. + * + * ⛔ Declared once: `DatasetSelectionSchema` (`api/analytics.zod.ts`) reads + * these two declarations off this shape, so the dataset door holds the same + * accept set with no second copy. + */ + limit: z.number().int().nonnegative().optional().describe( + 'Maximum number of rows to return, applied after `order` — a non-negative integer (`0` returns no rows)', + ), + offset: z.number().int().nonnegative().optional().describe( + 'Number of rows to skip before the first row returned, applied after `order` — a non-negative ' + + 'integer; an `offset` with no `limit` returns every row after it', + ), /** * Reference timezone (IANA name) for date bucketing. OPTIONAL WITH NO diff --git a/packages/spec/src/migrations/entries/semantic/18.analytics-query-window-non-negative-integer.ts b/packages/spec/src/migrations/entries/semantic/18.analytics-query-window-non-negative-integer.ts new file mode 100644 index 00000000000..f94563430d9 --- /dev/null +++ b/packages/spec/src/migrations/entries/semantic/18.analytics-query-window-non-negative-integer.ts @@ -0,0 +1,50 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import type { SemanticMigration } from '../../types.js'; + +export const entry: SemanticMigration = { + id: 'analytics-query-window-non-negative-integer', + // No backticks in `surface` — build-upgrade-guide.ts renders it inside a + // code span already, and a nested backtick would close it. + surface: + 'the row window of an analytics query — limit and offset on AnalyticsQuerySchema, the ' + + 'POST /analytics/query and /analytics/sql bodies, and a dataset selection ' + + '(POST /analytics/dataset/query) — authored as a negative number (limit: -1, offset: -1), ' + + 'a fraction (limit: 1.5), or an integer above Number.MAX_SAFE_INTEGER', + replacement: + 'a non-negative integer, or no key at all: delete `limit` to return every row (a ' + + '`limit: -1` written to mean "no limit" is exactly that), write `limit: 0` only for no ' + + 'rows, and delete `offset` (or write `offset: 0`) to skip nothing; a fraction becomes the ' + + 'integer page size that was meant. An `offset` with no `limit` stays valid and returns ' + + 'every row after the offset, on SQLite and PostgreSQL alike', + reason: + 'Both members were a bare `z.number()`, and every value outside the non-negative integers ' + + 'answered differently per driver and per face. Measured at POST /api/v1/analytics/query on ' + + 'SQLite and PostgreSQL 16, through the real dispatcher route: `limit: -1` returned every row ' + + 'on the native SQLite face, a 500 on native PostgreSQL and all but the last row on the ' + + 'ObjectQL face; `limit: 1.5` answered a 500, two rows and one row; `offset: -1` a 500 on ' + + 'both native drivers and every row on the ObjectQL face. No value had one answer, so the ' + + 'contract refuses them instead of any engine guessing (contract first, ADR-0049): the two ' + + 'members are `z.number().int().nonnegative()` on `AnalyticsQuerySchema`, the dataset ' + + 'selection reads the same two declarations off its shape, and the runtime doors answer the ' + + 'ADR-0112 envelope `400 VALIDATION_FAILED` naming `limit` or `offset` before any engine ' + + 'runs. ⚠️ No D2 conversion and no stored-metadata rewrite: the window is a QUERY-time ' + + 'request field, not a `sys_metadata` shape, and the refused values meant different windows ' + + 'on different backends, so coercing one would be the platform guessing which the author ' + + 'meant. The one stored producer that lowers into a selection, a dashboard widget\'s ' + + '`limit`, is already declared a positive integer. Measured in this repository at the ' + + 'change: no example, fixture, document or published skill authors a negative or ' + + 'fractional analytics window. ADR-0049 / ADR-0112.', + acceptanceCriteria: + 'Grep every authored analytics `limit` and `offset` — saved analytics queries, SDK and MCP ' + + 'callers, dataset selections, and queries a host builds in-process — and rewrite each ' + + 'negative or fractional value as described. POST /analytics/query and /analytics/sql answer ' + + '`400 VALIDATION_FAILED` with `details.fields[].field` naming `limit` or `offset`, and ' + + 'POST /analytics/dataset/query names `selection.limit` or `selection.offset`, so a sweep is ' + + 'mechanical; `AnalyticsQuerySchema.safeParse` reports the same issue at the member. ' + + 'Non-negative integer windows parse byte-identically to before, `limit: 0` still answers ' + + 'no rows, and absence stays absence. The service does not parse a query passed to it ' + + 'in-process, so a host that builds one parses it with `AnalyticsQuerySchema` first. A ' + + 'query that carried a refused value was never returning one window, so re-check what the ' + + 'widget was meant to show rather than trusting the old result set.', +}; diff --git a/packages/spec/src/migrations/registry.ts b/packages/spec/src/migrations/registry.ts index 0eae471050e..87acefd17c3 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -6918,6 +6918,52 @@ const step18: MigrationStep = { + 'all of history on one backend and a single day on another. Re-check what each converted ' + 'widget was supposed to show against its two explicit bounds.', }, + { + id: 'analytics-query-window-non-negative-integer', + // No backticks in `surface` — build-upgrade-guide.ts renders it inside a + // code span already, and a nested backtick would close it. + surface: + 'the row window of an analytics query — limit and offset on AnalyticsQuerySchema, the ' + + 'POST /analytics/query and /analytics/sql bodies, and a dataset selection ' + + '(POST /analytics/dataset/query) — authored as a negative number (limit: -1, offset: -1), ' + + 'a fraction (limit: 1.5), or an integer above Number.MAX_SAFE_INTEGER', + replacement: + 'a non-negative integer, or no key at all: delete `limit` to return every row (a ' + + '`limit: -1` written to mean "no limit" is exactly that), write `limit: 0` only for no ' + + 'rows, and delete `offset` (or write `offset: 0`) to skip nothing; a fraction becomes the ' + + 'integer page size that was meant. An `offset` with no `limit` stays valid and returns ' + + 'every row after the offset, on SQLite and PostgreSQL alike', + reason: + 'Both members were a bare `z.number()`, and every value outside the non-negative integers ' + + 'answered differently per driver and per face. Measured at POST /api/v1/analytics/query on ' + + 'SQLite and PostgreSQL 16, through the real dispatcher route: `limit: -1` returned every row ' + + 'on the native SQLite face, a 500 on native PostgreSQL and all but the last row on the ' + + 'ObjectQL face; `limit: 1.5` answered a 500, two rows and one row; `offset: -1` a 500 on ' + + 'both native drivers and every row on the ObjectQL face. No value had one answer, so the ' + + 'contract refuses them instead of any engine guessing (contract first, ADR-0049): the two ' + + 'members are `z.number().int().nonnegative()` on `AnalyticsQuerySchema`, the dataset ' + + 'selection reads the same two declarations off its shape, and the runtime doors answer the ' + + 'ADR-0112 envelope `400 VALIDATION_FAILED` naming `limit` or `offset` before any engine ' + + 'runs. ⚠️ No D2 conversion and no stored-metadata rewrite: the window is a QUERY-time ' + + 'request field, not a `sys_metadata` shape, and the refused values meant different windows ' + + 'on different backends, so coercing one would be the platform guessing which the author ' + + 'meant. The one stored producer that lowers into a selection, a dashboard widget\'s ' + + '`limit`, is already declared a positive integer. Measured in this repository at the ' + + 'change: no example, fixture, document or published skill authors a negative or ' + + 'fractional analytics window. ADR-0049 / ADR-0112.', + acceptanceCriteria: + 'Grep every authored analytics `limit` and `offset` — saved analytics queries, SDK and MCP ' + + 'callers, dataset selections, and queries a host builds in-process — and rewrite each ' + + 'negative or fractional value as described. POST /analytics/query and /analytics/sql answer ' + + '`400 VALIDATION_FAILED` with `details.fields[].field` naming `limit` or `offset`, and ' + + 'POST /analytics/dataset/query names `selection.limit` or `selection.offset`, so a sweep is ' + + 'mechanical; `AnalyticsQuerySchema.safeParse` reports the same issue at the member. ' + + 'Non-negative integer windows parse byte-identically to before, `limit: 0` still answers ' + + 'no rows, and absence stays absence. The service does not parse a query passed to it ' + + 'in-process, so a host that builds one parses it with `AnalyticsQuerySchema` first. A ' + + 'query that carried a refused value was never returning one window, so re-check what the ' + + 'widget was meant to show rather than trusting the old result set.', + }, { id: 'analytics-time-dimension-date-range-vocabulary-closed', // No backticks in `surface` — build-upgrade-guide.ts renders it inside a