Skip to content

Commit d7d5b4f

Browse files
fix(service-analytics): the ObjectQL face echoes an offset with no limit as a statement the dialect runs (#21440)
Fixes #21365 Clause-②: no ## What this changes This is the remainder of #21365 after PR #21399 (`6d67ad5ec`, which read `Part of`). That PR narrowed the window contract and made the native face render an offset-only window with the dialect's no-limit spelling. The ObjectQL face's echo still wrote its own window. `ObjectQLStrategy.generateSql` (`packages/services/service-analytics/src/strategies/objectql-strategy.ts`) wrote `LIMIT n` when a limit was set, then `OFFSET n` when an offset was. An `offset` with no `limit` therefore echoed a bare `OFFSET`, and SQLite's grammar has no bare `OFFSET`. Those two lines are now one call: - `windowClauseSql(query.limit, query.offset, sqlDialectFor(ctx, tableName))` - `windowClauseSql` is the function the native face runs, already exported from `native-sql-strategy.ts`. That file is not edited, and there is no second spelling table. The ObjectQL face's echoed `sql` and the `POST /api/v1/analytics/sql` body (both are `generateSql`) now carry the same window clause the native face executes on the same driver. ## Measured at the real route The harness is a scratch copy of `packages/runtime/src/analytics-query-window-validity.test.ts`. It uses the real `dispatcher-plugin` mount over `AnalyticsServicePlugin` and a real `ObjectQL` engine with `SqlDriver` (better-sqlite3). It boots the default composition and one narrowed to the engine aggregate (the ObjectQL face). The query is `order: { note: 'asc' }, offset: 1`, with no limit. Each echoed statement was then run on the same SQLite database through knex, to read SQLite's own diagnostic. The harness was deleted and never committed. | | `main` `bdd3654f2` | this branch | |:--|:--|:--| | ObjectQL face, `/query` rows | x y z | x y z | | ObjectQL face, echoed `sql` = `/sql` body | `… ORDER BY "note" ASC OFFSET 1` | `… ORDER BY "note" ASC LIMIT -1 OFFSET 1` | | that statement on SQLite | `near "OFFSET": syntax error` | x y z | | native face, executed = echoed = `/sql` | `… ASC LIMIT -1 OFFSET 1`, x y z | unchanged | | CONTROL `limit: 2, offset: 1`, ObjectQL echo | `… LIMIT 2 OFFSET 1`, runs | unchanged | On this query the ObjectQL echo now equals, byte for byte, the statement the native face runs on SQLite. ## The dialect source (dispatch, Zone 2 item 2) `generateSql` already has the strategy context in scope, and it already reads `sqlDialectFor(ctx, tableName)` for its read-scope compile. That is the same `sqlDialect` hook, read the same way, that `NativeSQLStrategy.generateSql` uses with `sqlDialectFor(ctx, this.extractObjectName(cube))`. `tableName` here is `this.extractObjectName(cube)` too. `AnalyticsServicePlugin` wires that hook from the data engine's driver whatever the query capabilities are, so the narrowed composition above reads `sqlite`. No new hook. ## Pin `packages/services/service-analytics/src/__tests__/objectql-echo-offset-only-window.test.ts` has three tests per cell: - **The card's row.** The ObjectQL face runs no raw statement and does run the engine aggregate. It answers x y z. Its echo ends with the dialect's window. `generateSql` (the `/analytics/sql` body) equals the echo, with no params. From `ORDER BY` on, the echo equals the statement the native face executes on the same driver. Run through the engine's raw-SQL bridge, the echo answers the face's rows. - **The `unknown` arm.** An `ObjectQLStrategy` whose context wires no `sqlDialect` hook echoes `LIMIT 9223372036854775807 OFFSET 1`, and that statement runs. - **CONTROL.** `limit: 2, offset: 1` keeps its bytes, and the echo runs. The cells: - **sqlite**: executed on every run. - **live postgres**: executed where `OS_TEST_POSTGRES_URL` is set, and a named skip otherwise. No CI step provisions that variable for this package, so CI runs only the SQLite cell. I ran it locally against PostgreSQL 16.14. There the echo keeps `OFFSET 1` alone, byte-identical to before. The comparison with the native statement starts at `ORDER BY` because on PostgreSQL the native face also casts the summed column. **Ablation** at committed `2cfcc44ee`, through `node scripts/ablation-replace.mjs`. The anchor hit once, and the blob changed `9ed31b50c50b` to `e8b910c3c13f`. The mutation restored the two old window lines. The pin reads `src/` with no `dist/` leg (objectql is aliased to source, and the strategy is imported from `src`). Result, with live PostgreSQL: **3 failed, 3 passed**. - Red: the SQLite card row (the echo ends with a bare `OFFSET 1`), and the `unknown` arm on both engines. - Green: both controls, and the PostgreSQL card row, whose bytes this change does not move. I predicted that direction before the run. Restore was proven: the blob after restore equals HEAD `9ed31b50c50b`, and `git diff HEAD` is empty. ## Tests and gates All of these ran at head `b9456bff3`, which is this branch with `origin/main` `8b123c0ae` merged in. That merge brought PR #21424's `native-sql-strategy.ts` change. `windowClauseSql` is unchanged by it. - `pnpm --filter @objectstack/service-analytics test`, with `OS_TEST_POSTGRES_URL` set to a local PostgreSQL 16.14: **Test Files 170 passed (170), Tests 3957 passed (3957)**. `os-verify-lock` printed `VERDICT command-exit 0`. - `pnpm --filter @objectstack/service-analytics typecheck`: `VERDICT command-exit 0`. `tsc --noEmit --listFiles` lists the new pin once. The package `tsconfig` includes `src` and does not exclude tests. - Gates: `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands` (no paths) derived 63 commands. All 63 ran at `b9456bff3`. 62 exited 0 on the first run. `pnpm check:dual-build-cjs-loads` first answered `PREREQUISITE NOT MET` (exit 3), because the workspace was not fully built. After a full `turbo run build` (72 tasks, exit 0), it exited 0. `--ran` reconciles **63 derived, 63 run, 0 NOT-MEASURED, 0 UNRUN**. The list includes `check:nul-bytes`, `check:changeset-gate-self-tests`, `check:adr-0087-registration`, `check:changeset-no-major` and `check:test-source-alias`. No gate touched `AGENTS.md`, and the tree stayed clean. - eslint: `eslint --no-inline-config --format json` on the 2 touched `.ts` files read 2 files, 0 errors, 0 warnings, none ignored. `eslint.config.mjs` enables no type-aware linting (no `parserOptions.project`), so an untouched file's verdict cannot move. The repo-wide `pnpm lint` is left to CI. ## Docs I grepped `content/docs/**` outside `releases/` for the analytics echo, `/analytics/sql` and window rendering. `api/data-api.mdx` describes `/analytics/sql` as a dry run returning `{ sql, params }`, and `api/client-sdk.mdx` shows `client.analytics.explain`. Neither says how a window renders. No sentence is made false, so there is no docs edit. ## Acceptance notes - **The bucketed echo is a separate truth (dispatch, Zone 2 item 3), not fixed here.** A month-bucketed dimension echoes `date_trunc('month', closed_on)`. That holds on both faces, and in the default composition too, because the native face declines granularity. SQLite refuses it with `no such function: date_trunc`, measured on this branch after the window fix. On `main` the same statement failed earlier, at the bare `OFFSET`. The driver runs `strftime('%Y-%m', …)` there (`driver-sql` `sql-driver.ts`). The comment over `dimExpr` says the echo renders "the SQL shape the driver's own bucketing implements", and on SQLite that is not so. This is `dimExpr`, not the two window lines, so it is reported to the seat rather than fixed here. - **`mysql`.** The window cell is `windowClauseSql`'s own (`LIMIT 18446744073709551615`). It is NOT MEASURED on either face here, because no MySQL server is available. That is the same declared skip PR #21399 carries. - **Unknown dialect on PostgreSQL.** A host whose `sqlDialect` hook names no dialect now echoes `LIMIT 9223372036854775807 OFFSET n` where it echoed `OFFSET n`. The native face already runs that statement for such a host. It answers the same rows on PostgreSQL and SQLite, as the pin's `unknown` arm measures. --- _Generated by [Claude Code](https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent b206403 commit d7d5b4f

3 files changed

Lines changed: 263 additions & 3 deletions

File tree

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
"@objectstack/service-analytics": patch
3+
---
4+
5+
fix(service-analytics): the ObjectQL strategy's echoed `sql` renders an offset with no limit in the dialect's own spelling, so SQLite runs the statement it prints
6+
7+
Clause-②: no
8+
9+
**Before**, the ObjectQL strategy wrote its own row window into the statement it echoes: `LIMIT n` when a limit was set, then `OFFSET n` when an offset was. An `offset` with no `limit` therefore echoed a bare `OFFSET`, which SQLite's grammar does not have. Measured through `POST /api/v1/analytics/query` and `POST /api/v1/analytics/sql` on SQLite, for a composition served by the engine aggregate, with `order: { note: 'asc' }` and `offset: 1`: the rows were right (every group after the first), but the echoed `sql` and the `/analytics/sql` body both ended `ORDER BY "note" ASC OFFSET 1`, and SQLite refuses that statement with `near "OFFSET": syntax error`.
10+
11+
**Now** the statement ends with the same window clause the native-SQL strategy runs, for the dialect of the driver that serves the object: `LIMIT -1 OFFSET 1` on SQLite, which runs and answers the same rows. One function renders the window for both strategies.
12+
13+
**Unchanged.** The rows either strategy answers. A window with a `limit` keeps its bytes (`LIMIT 2 OFFSET 1`) on every dialect, and on PostgreSQL an offset with no limit still echoes `OFFSET 1` alone. A host that wires no `sqlDialect` hook gets the native strategy's dialect-neutral spelling, `LIMIT 9223372036854775807 OFFSET 1`. A date-bucketed dimension still echoes as `date_trunc(…)`, which SQLite does not run; this change touches only the window.
Lines changed: 240 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,240 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#21365] The ObjectQL face echoes an offset-only window as a statement the
5+
* dialect runs: the same window clause the native face executes.
6+
*
7+
* ## The shape this closes
8+
*
9+
* `ObjectQLStrategy.generateSql` wrote its own window: `LIMIT n` when a limit
10+
* was set, then `OFFSET n` when an offset was. An offset with no limit
11+
* therefore echoed a bare `OFFSET`. Measured at `POST /api/v1/analytics/query`
12+
* and `POST /api/v1/analytics/sql` on `main` `bdd3654f2`, SQLite, a
13+
* composition narrowed to the engine aggregate, `order { note: 'asc' }`,
14+
* `offset: 1`:
15+
*
16+
* | | rows | echoed `sql` and `/analytics/sql` | that statement on SQLite |
17+
* |:--|:--|:--|:--|
18+
* | ObjectQL face | x y z | `… ORDER BY "note" ASC OFFSET 1` | `near "OFFSET": syntax error` |
19+
* | native face | x y z | `… ORDER BY "note" ASC LIMIT -1 OFFSET 1` (what ran) | x y z |
20+
*
21+
* The rows were right; the statement the face printed for them was one SQLite
22+
* refuses. `generateSql` now ends with `windowClauseSql` (exported from
23+
* `native-sql-strategy.ts`, not a second spelling table) for the dialect the
24+
* `sqlDialect` hook names for the base object, so both faces print one window.
25+
*
26+
* ## The cells
27+
*
28+
* - **sqlite → EXECUTED** (better-sqlite3), every run.
29+
* - **postgres → EXECUTED** where `OS_TEST_POSTGRES_URL` is set, a named skip
30+
* otherwise: `OFFSET n` alone, byte-identical to before. No CI step
31+
* provisions that variable for this package, so the live cell is
32+
* red-capable and un-run in CI.
33+
* - **unknown (no `sqlDialect` hook) → EXECUTED** on both engines above,
34+
* through an `ObjectQLStrategy` whose context names no dialect:
35+
* `LIMIT 9223372036854775807 OFFSET n`, the native face's spelling for it.
36+
*
37+
* In each cell the echo's window is compared with the statement the native
38+
* face runs for the same query on the same driver, and the echo is then run
39+
* itself through the engine's raw-SQL bridge, where it must answer the face's
40+
* rows.
41+
*/
42+
43+
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
44+
import { ObjectQL } from '@objectstack/objectql';
45+
import { SqlDriver } from '@objectstack/driver-sql';
46+
import type { Cube } from '@objectstack/spec/data';
47+
import type { AnalyticsService } from '../analytics-service.js';
48+
import { AnalyticsServicePlugin } from '../plugin.js';
49+
import { ObjectQLStrategy } from '../strategies/objectql-strategy.js';
50+
import type { StrategyContext } from '../strategies/types.js';
51+
52+
const DEAL = 'os21365_echo_deal';
53+
54+
const DEAL_OBJECT = {
55+
name: DEAL,
56+
label: 'Offset echo deal',
57+
fields: {
58+
note: { name: 'note', type: 'text' as const },
59+
amount: { name: 'amount', type: 'number' as const },
60+
},
61+
};
62+
63+
// Inserted out of note order, so an answer in note order is the ORDER BY's.
64+
const DEALS = [
65+
{ id: 'd1', note: 'y', amount: 20 },
66+
{ id: 'd2', note: 'w', amount: 7 },
67+
{ id: 'd3', note: 'z', amount: 1 },
68+
{ id: 'd4', note: 'x', amount: 10 },
69+
{ id: 'd5', note: 'x', amount: 5 },
70+
] as const;
71+
72+
const CUBE = 'os21365_echo_cube';
73+
const CUBES = [
74+
{
75+
name: CUBE,
76+
title: 'Offset echo cube',
77+
sql: DEAL,
78+
public: true,
79+
measures: { amount_sum: { type: 'sum', sql: 'amount', label: 'Amount' } },
80+
dimensions: { note: { type: 'string', sql: 'note', label: 'Note' } },
81+
},
82+
] as unknown as Cube[];
83+
84+
/** The card's row 4: ordered, an offset, no limit. */
85+
const OFFSET_ONLY = { cube: CUBE, measures: ['amount_sum'], dimensions: ['note'], order: { note: 'asc' }, offset: 1 };
86+
/** Every group after the first, in note order. */
87+
const AFTER_FIRST = [['x', 15], ['y', 20], ['z', 1]];
88+
89+
interface Cell {
90+
id: 'sqlite' | 'pg';
91+
label: string;
92+
env: string | null;
93+
config: () => Record<string, unknown> | null;
94+
/** The no-limit spelling the dialect takes in front of `OFFSET`, `''` for none. */
95+
noLimit: string;
96+
}
97+
98+
const CELLS: readonly Cell[] = [
99+
{
100+
id: 'sqlite',
101+
label: 'sqlite',
102+
env: null,
103+
config: () => ({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }),
104+
noLimit: ' LIMIT -1',
105+
},
106+
{
107+
id: 'pg',
108+
label: 'live postgres',
109+
env: 'OS_TEST_POSTGRES_URL',
110+
config: () => (process.env.OS_TEST_POSTGRES_URL ? { client: 'pg', connection: process.env.OS_TEST_POSTGRES_URL } : null),
111+
noLimit: '',
112+
},
113+
];
114+
115+
const FACES = ['native', 'objectql'] as const;
116+
type Face = (typeof FACES)[number];
117+
118+
const quiet = { debug() {}, info() {}, warn() {}, error() {}, child() { return quiet; } };
119+
120+
type Row = Record<string, unknown>;
121+
122+
/** Rows as tuples of the named columns, in arrival order; a numeric cell reads as a number on every dialect. */
123+
const tuples = (rows: unknown, columns: readonly string[]) =>
124+
(rows as Row[]).map((row) => columns.map((c) => (typeof row[c] === 'number' || /^-?\d+(\.\d+)?$/.test(String(row[c])) ? Number(row[c]) : row[c])));
125+
126+
for (const cell of CELLS) {
127+
const config = cell.config();
128+
describe.skipIf(!config)(
129+
`[#21365] the ObjectQL face echoes an offset-only window the dialect runs (${cell.label})${config ? '' : ` (skipped: set ${cell.env} to run this cell)`}`,
130+
() => {
131+
let driver: any;
132+
let engine: ObjectQL;
133+
/** Every raw statement the engine ran, in order. */
134+
const executed: string[] = [];
135+
const reads = { aggregate: 0 };
136+
const services: Partial<Record<Face, AnalyticsService>> = {};
137+
138+
const dropTables = async () => {
139+
if (cell.id !== 'pg') return;
140+
await driver?.execute(`drop table if exists ${DEAL}`).catch(() => {});
141+
};
142+
143+
/** One `query()` on one face, with the statements and aggregates it caused. */
144+
const ask = async (face: Face, query: Record<string, unknown>) => {
145+
const before = { statements: executed.length, aggregate: reads.aggregate };
146+
const res = await services[face]!.query(query as any);
147+
return { res, statements: executed.slice(before.statements), aggregate: reads.aggregate - before.aggregate };
148+
};
149+
150+
/** Run a statement through the engine's raw-SQL bridge, as the native face does. */
151+
const run = async (sql: string) => {
152+
const result = await (engine as any).execute(sql, { object: DEAL });
153+
return Array.isArray(result) ? result : (result as { rows: Row[] }).rows;
154+
};
155+
156+
beforeAll(async () => {
157+
driver = new SqlDriver(config as any);
158+
await dropTables();
159+
engine = new ObjectQL({ logger: quiet } as any);
160+
engine.registerDriver(driver, true);
161+
await engine.init();
162+
engine.registry.registerObject(DEAL_OBJECT as any);
163+
await engine.syncSchemas();
164+
for (const row of DEALS) await engine.insert(DEAL, { ...row } as any);
165+
166+
const realExecute = (engine as any).execute.bind(engine);
167+
(engine as any).execute = (sql: unknown, opts?: unknown) => {
168+
executed.push(String(sql));
169+
return realExecute(sql, opts);
170+
};
171+
const realAggregate = engine.aggregate.bind(engine);
172+
(engine as any).aggregate = (...args: unknown[]) => {
173+
reads.aggregate += 1;
174+
return (realAggregate as any)(...args);
175+
};
176+
177+
for (const [face, caps] of [
178+
['native', undefined],
179+
['objectql', () => ({ nativeSql: false, objectqlAggregate: true, inMemory: false })],
180+
] as const) {
181+
const registered: Record<string, unknown> = {};
182+
await new AnalyticsServicePlugin({ cubes: CUBES, debugSql: true, ...(caps ? { queryCapabilities: caps } : {}) } as any).init({
183+
getService: (name: string) => (name === 'data' ? engine : registered[name]),
184+
registerService: (name: string, svc: unknown) => { registered[name] = svc; },
185+
replaceService: (name: string, svc: unknown) => { registered[name] = svc; },
186+
hook: () => {},
187+
logger: quiet,
188+
} as never);
189+
services[face] = registered.analytics as AnalyticsService;
190+
}
191+
});
192+
193+
afterAll(async () => {
194+
await dropTables();
195+
try { await engine?.destroy(); } catch { /* noop */ }
196+
});
197+
198+
it("the card's row — ordered, offset 1, no limit: the echo and /analytics/sql carry the dialect's window, and it runs", async () => {
199+
const objectql = await ask('objectql', OFFSET_ONLY);
200+
expect(objectql.statements, 'the ObjectQL face ran no raw statement').toEqual([]);
201+
expect(objectql.aggregate, 'the ObjectQL face ran the engine aggregate').toBeGreaterThan(0);
202+
expect(tuples(objectql.res.rows, ['note', 'amount_sum']), 'ObjectQL face').toEqual(AFTER_FIRST);
203+
204+
const echo = objectql.res.sql!;
205+
expect(echo.endsWith(`ORDER BY "note" ASC${cell.noLimit} OFFSET 1`), echo).toBe(true);
206+
// `generateSql` is the body `POST /analytics/sql` answers with.
207+
const dryRun = await services.objectql!.generateSql!(OFFSET_ONLY as any);
208+
expect(dryRun.sql).toBe(echo);
209+
210+
// The window the native face runs for the same query on the same driver.
211+
// Compared from `ORDER BY` on: on PostgreSQL the native face also casts
212+
// the summed column, so the two statements differ before it.
213+
const native = await ask('native', OFFSET_ONLY);
214+
expect(native.statements, 'the native face ran ONE statement').toHaveLength(1);
215+
const fromOrderBy = (sql: string) => sql.slice(sql.indexOf(' ORDER BY '));
216+
expect(fromOrderBy(echo)).toBe(fromOrderBy(native.statements[0]));
217+
218+
// The echo binds no parameter, so it runs as printed — and answers the face's rows.
219+
expect(dryRun.params).toEqual([]);
220+
expect(tuples(await run(echo), ['note', 'amount_sum'])).toEqual(AFTER_FIRST);
221+
});
222+
223+
it('a host that wires no sqlDialect hook (the `unknown` arm) echoes the dialect-neutral window, and it runs', async () => {
224+
const ctx = { getCube: (name: string) => (name === CUBE ? CUBES[0] : undefined) } as unknown as StrategyContext;
225+
const { sql, params } = await new ObjectQLStrategy().generateSql(OFFSET_ONLY as any, ctx);
226+
expect(sql.endsWith('ORDER BY "note" ASC LIMIT 9223372036854775807 OFFSET 1'), sql).toBe(true);
227+
expect(params).toEqual([]);
228+
expect(tuples(await run(sql), ['note', 'amount_sum'])).toEqual(AFTER_FIRST);
229+
});
230+
231+
it('CONTROL: an integer window keeps its bytes — `LIMIT 2 OFFSET 1` — and the echo runs', async () => {
232+
const query = { ...OFFSET_ONLY, limit: 2 };
233+
const { res } = await ask('objectql', query);
234+
expect(res.sql!.endsWith('ORDER BY "note" ASC LIMIT 2 OFFSET 1'), res.sql).toBe(true);
235+
expect(tuples(res.rows, ['note', 'amount_sum'])).toEqual([['x', 15], ['y', 20]]);
236+
expect(tuples(await run(res.sql!), ['note', 'amount_sum'])).toEqual([['x', 15], ['y', 20]]);
237+
});
238+
},
239+
);
240+
}

‎packages/services/service-analytics/src/strategies/objectql-strategy.ts‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,9 @@ import {
4848
// [commit 017130a09] The custom-SQL half of the `AggregationMetricType` partition, ONE
4949
// source shared with `NativeSQLStrategy` and pinned against the spec enum by
5050
// `metric-type-coverage.test.ts` — a second literal set here would drift.
51-
import { EXPRESSION_METRIC_TYPES } from './native-sql-strategy.js';
51+
// [#21365] `windowClauseSql` is the same rule for the row window: one spelling
52+
// of an offset-only window per dialect, shared with the native face.
53+
import { EXPRESSION_METRIC_TYPES, windowClauseSql } from './native-sql-strategy.js';
5254

5355
/**
5456
* [#10861 / commit 399ecad58] Where a member in the cross-object envelope's inventory
@@ -673,8 +675,13 @@ export class ObjectQLStrategy implements AnalyticsStrategy {
673675
const orderClauses = Object.entries(query.order).map(([f, d]) => `"${f}" ${d.toUpperCase()}`);
674676
sql += ` ORDER BY ${orderClauses.join(', ')}`;
675677
}
676-
if (query.limit != null) sql += ` LIMIT ${query.limit}`;
677-
if (query.offset != null) sql += ` OFFSET ${query.offset}`;
678+
// [#21365] The window renders through the native face's own
679+
// `windowClauseSql`, for the dialect of the driver the engine aggregate
680+
// runs on — the same `sqlDialect` read the read scope above makes. An
681+
// offset with no limit then carries that dialect's no-limit spelling
682+
// (`LIMIT -1 OFFSET n` on SQLite, whose grammar has no bare `OFFSET`), so
683+
// the echoed `sql` and `/analytics/sql` print a window the dialect runs.
684+
sql += windowClauseSql(query.limit, query.offset, sqlDialectFor(ctx, tableName));
678685

679686
return { sql, params };
680687
}

0 commit comments

Comments
 (0)