Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions .changeset/28841-sql-dry-run-refuses-column-twice.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
---
'@objectstack/service-analytics': patch
---

fix(service-analytics)!: the `/analytics/sql` dry run refuses a selection asking one column twice, as `/analytics/query` does, instead of rendering a statement the query refuses

Clause-②: no (narrowing) — `/analytics/sql` (`AnalyticsService.generateSql`) now refuses `INVALID_QUERY` / 400 a selection that names one column twice in its groups (two granularities of one time dimension, the column raw beside its own bucket, or two spellings of it bucketed twice), with the refusal `/analytics/query` gives the same selection, field for field; it rendered a statement for each. Nothing is widened: no export, public member, answer member, error code or status is added.
Metadata-protocol: none

<!-- adr-0087: not-required (no-migration-prescription) No key of any schema is added, removed, renamed or re-shaped, and no export or type of the package root changes: a dry run that rendered a statement the query refuses is refused with the query's refusal, already on the wire. There is nothing for a migration to rewrite and no tombstone. -->

**BREAKING** accept-set narrowing on the `/analytics/sql` dry run (`@objectstack/service-analytics`). `/analytics/query` and `queryDataset` are unchanged.

- **Before.** Measured on real SQLite on `main` `7e60dc23` (native declines every bucketed selection, so the ObjectQL strategy's echo answers). `/analytics/query` refused each selection below, and the dry run did this instead:
- `timeDimensions` `closed_at` by month and by year: rendered one `year` bucket, the month entry dropped.
- `closed_at` raw beside `CUBE.closed_at` by month, or a second dimension over `closed_at` beside its bucket: rendered `GROUP BY closed_at, strftime('%Y-%m', …)`.
- `closed_at` by month beside `CUBE.closed_at` by year: rendered both buckets.
- Beside a cross-object `account.name`: rendered the join and one bucket.
- At a non-UTC `timezone`: answered `NOT_IMPLEMENTED` / 501, saying the query is served.
- **Now.** The echo judges the keys `execute()` hands the engine with the same dataset door (`dataset-group-column-door.ts`, #28798) before it renders a bucket, so it answers the query's `INVALID_QUERY` / 400 with the same message, `object`, `param`, `path` and `fields`. On a selection that routes to the cross-object path, those are that path's keys. The echo's one-granularity-per-dimension map is gone: it reads the granularities the query's key builders read.
- **Unchanged.** Every echo the query serves renders byte for byte (pinned): one bucket of a column, buckets of two columns, one granularity asked twice, raw columns, a cross-object dimension. The 501 for a bucket the engine groups in memory is unchanged when the selection names no column twice. No new refusal is added; the query's own now reaches the dry run.
- **Who sees it.** A caller of `/analytics/sql` sending one of those selections now gets the query's refusal, instead of SQL for a query that does not run.

**The remedy.** Ask each column once: drop the raw dimension or the bucket's granularity, or ask one granularity. The refusal names the members.
17 changes: 14 additions & 3 deletions packages/client/src/envelope-caller-census.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -525,6 +525,12 @@ const LEDGER: readonly LedgerRow[] = [
method: 'analytics.query', receiver: 'service', count: 2, verdict: 'NOT_SDK',
why: 'the real AnalyticsService that AnalyticsServicePlugin registers over an ObjectQL engine with the driver\'s data doors spied, called directly (no HTTP, no dispatcher envelope) to read whether the ObjectQL face\'s answer carries an echoed `sql`, and its rows',
},
// ── [#28841] the /analytics/sql dry run vs the query for one column asked twice: producer reads only ──
{
file: 'packages/services/service-analytics/src/__tests__/objectql-echo-column-twice.test.ts',
method: 'analytics.query', receiver: 'service', count: 3, verdict: 'NOT_SDK',
why: 'the real AnalyticsService that AnalyticsServicePlugin registers over a live engine, called directly (no HTTP, no dispatcher envelope) to compare the query\'s refusal with the dry run\'s, and to read the `sql` and rows the query serves',
},
{
file: 'packages/client/src/analytics-automation-json-erasure.test.ts',
method: 'analytics.meta', receiver: 'sdk', count: 2, verdict: 'PAYLOAD_DEPENDENT',
Expand Down Expand Up @@ -761,12 +767,16 @@ describe('#13079 §2 — positive controls on the matcher itself', () => {
// [#21647] A fifth, two sites: the bucket echo's enumeration pin calls
// the same plugin-registered AnalyticsService, over an engine with the
// driver's data doors spied, to read whether the answer carries `sql`.
expect(service.length, literalNote()).toBe(11);
// [#28841] A sixth, three sites: the dry-run parity pin calls the same
// plugin-registered AnalyticsService to compare the query's refusal
// and served `sql` with `generateSql`'s.
expect(service.length, literalNote()).toBe(14);
expect([...new Set(service.map((s) => s.file))].sort()).toEqual([
'packages/client/src/analytics-automation-json-erasure.test.ts',
'packages/drivers/driver-memory/src/memory-exists-non-boolean-refusal.test.ts',
'packages/rest/src/analytics-nested-relation-filter.test.ts',
'packages/services/service-analytics/src/__tests__/objectql-echo-bucket-enumeration.test.ts',
'packages/services/service-analytics/src/__tests__/objectql-echo-column-twice.test.ts',
'packages/services/service-analytics/src/__tests__/objectql-echo-date-bucket.test.ts',
]);
expect(service.every((s) => s.method === 'analytics.query')).toBe(true);
Expand Down Expand Up @@ -814,12 +824,13 @@ describe('#13079 §3 — every call site is classified', () => {
expect(production, literalNote()).toEqual([]);
});

it('records the split: 18 payload pins, 10 result-insensitive, 11 not-SDK', () => {
it('records the split: 18 payload pins, 10 result-insensitive, 14 not-SDK', () => {
expect(verdictTotal('PAYLOAD_DEPENDENT')).toBe(18);
expect(verdictTotal('RESULT_INSENSITIVE')).toBe(10);
// [#21441] 8 -> 9: the date-bucket echo pin's producer call (§2).
// [#21647] 9 -> 11: the bucket echo enumeration's two producer calls (§2).
expect(verdictTotal('NOT_SDK')).toBe(11);
// [#28841] 11 -> 14: the dry-run parity pin's three producer calls (§2).
expect(verdictTotal('NOT_SDK')).toBe(14);
// The three above are LEDGER sums and cannot move on a census reading;
// this one is census-derived, so it carries the note. [#13874]
expect(sdkSites.length, literalNote()).toBe(28);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,179 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* [#28841] The `/analytics/sql` dry run refuses what `/analytics/query`
* refuses for one column asked twice under one name, on real SQLite.
*
* ## The shape this closes
*
* A selection asking one column for two values (two granularities of it, the
* column raw beside its own bucket, or two spellings of it bucketed twice) is
* refused by the query: the ObjectQL strategy's dataset door
* (`dataset-group-column-door.ts`, #28798) judges the keys `execute()` hands
* the engine, `INVALID_QUERY` / 400. Native declines every bucketed selection
* to that strategy, so its `generateSql` is the dry run that answers. It built
* its GROUP BY from its own map of one granularity per dimension and ran no
* door, so on `main` `7e60dc23` it rendered a statement for each:
*
* | selection | the dry run rendered |
* |:--|:--|
* | `closed_at` month and year | one `strftime('%Y', …)` bucket (the month entry dropped) |
* | `closed_at` raw beside `CUBE.closed_at` month | `GROUP BY closed_at, strftime('%Y-%m', …)` |
* | `closed_day` (over `closed_at`) beside `closed_at` month | the same two columns |
* | `closed_at` month beside `CUBE.closed_at` year | both buckets |
* | `account.name` beside `closed_at` month and year | the join, one `year` bucket |
* | `closed_at` month and year at `America/New_York` | `NOT_IMPLEMENTED` / 501 saying the query is served |
*
* It now judges the same keys with the same door before it renders anything
* (the cross-object path's keys when the selection routes there), so it
* answers the query's refusal field for field. Every echo the query serves
* still renders byte for byte (pinned below).
*/

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 { installDefaultOrganization } from './default-organization.testkit.js';

const DEAL = 'os28841_deal';
const ACCOUNT = 'os28841_account';
const CUBE = 'os28841_cube';

const CUBES = [
{
name: CUBE,
title: 'Column twice cube',
sql: DEAL,
public: true,
measures: { amount_sum: { type: 'sum', sql: 'amount', label: 'Amount' } },
dimensions: {
region: { type: 'string', sql: 'region', label: 'Region' },
closed_at: { type: 'time', sql: 'closed_at', label: 'Closed at' },
closed_on: { type: 'time', sql: 'closed_on', label: 'Closed on' },
closed_day: { type: 'time', sql: 'closed_at', label: 'Closed day' },
},
},
] as unknown as Cube[];

const quiet = { debug() {}, info() {}, warn() {}, error() {}, child() { return quiet; } };

type Query = Record<string, unknown>;
const td = (dimension: string, granularity: string) => ({ dimension, granularity });
const onCube = (extra: Query): Query => ({ cube: CUBE, measures: ['amount_sum'], ...extra });
/** The inferred cube over the deal object, which serves a lookup path as a cross-object dimension. */
const onObject = (extra: Query): Query => ({ cube: DEAL, measures: ['amount_sum'], ...extra });

describe('[#28841] the /analytics/sql dry run refuses one column asked twice, as the query does', () => {
let engine: ObjectQL;
let analytics: AnalyticsService;

beforeAll(async () => {
const driver = new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true } as never);
engine = new ObjectQL({ logger: quiet } as never);
engine.registerDriver(driver as never, true);
await engine.init();
engine.registry.registerObject({ name: ACCOUNT, label: 'Account', fields: { name: { name: 'name', type: 'text' } } } as never);
engine.registry.registerObject({
name: DEAL,
label: 'Deal',
fields: {
region: { name: 'region', type: 'text' },
amount: { name: 'amount', type: 'number' },
closed_at: { name: 'closed_at', type: 'datetime' },
closed_on: { name: 'closed_on', type: 'date' },
account: { name: 'account', type: 'lookup', reference: ACCOUNT },
},
} as never);
await engine.syncSchemas();
await installDefaultOrganization(engine as never);
await engine.insert(ACCOUNT, { id: 'a1', name: 'Acme' } as never);
await engine.insert(DEAL, { id: 'd1', region: 'amer', amount: 10, closed_at: '2026-03-01T00:00:00.000Z', closed_on: '2026-03-02', account: 'a1' } as never);

// The default composition: no `queryCapabilities` override.
const registered: Record<string, unknown> = {};
await new AnalyticsServicePlugin({ cubes: CUBES, debugSql: true } as never).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);
analytics = registered.analytics as AnalyticsService;
});

afterAll(async () => {
try { await engine?.destroy(); } catch { /* noop */ }
});

/** The refusal `p` rejects with, as the keys a caller reads; a resolution fails the case. */
const refusalOf = async (face: string, p: Promise<unknown>) => {
const err = (await p.then(
() => { throw new Error(`${face}: expected a refusal, but it resolved`); },
(e: unknown) => e,
)) as Error & Record<string, unknown>;
return {
message: err.message, code: err.code, status: err.status, httpStatus: err.httpStatus,
object: err.object, param: err.param, path: err.path, fields: err.fields,
};
};

it.each<[string, Query, string]>([
['two granularities of one dimension', onCube({ timeDimensions: [td('closed_at', 'month'), td('closed_at', 'year')] }), 'groupBy.1.field'],
['a column raw beside its own bucket, spelled CUBE.closed_at', onCube({ dimensions: ['closed_at'], timeDimensions: [td(`${CUBE}.closed_at`, 'month')] }), 'groupBy.1.field'],
['a second dimension over the column, raw beside its bucket', onCube({ dimensions: ['closed_day'], timeDimensions: [td('closed_at', 'month')] }), 'groupBy.1.field'],
['two spellings of one column at two granularities', onCube({ timeDimensions: [td('closed_at', 'month'), td(`${CUBE}.closed_at`, 'year')] }), 'groupBy.1.field'],
['the column raw beside its bucket on the inferred cube', onObject({ dimensions: ['closed_at'], timeDimensions: [td(`${DEAL}.closed_at`, 'month')] }), 'groupBy.1.field'],
["two granularities beside a cross-object 'account.name' (its FK column first)", onObject({ dimensions: ['account.name'], timeDimensions: [td('closed_at', 'month'), td('closed_at', 'year')] }), 'groupBy.2.field'],
["two granularities beside two dimensions through one relationship (one FK key, so the later position)", onObject({ dimensions: ['account.id', 'account.name'], timeDimensions: [td('closed_at', 'month'), td('closed_at', 'year')] }), 'groupBy.2.field'],
['two granularities at a non-UTC zone, where the dry run said the query is served (501)', onCube({ timeDimensions: [td('closed_at', 'month'), td('closed_at', 'year')], timezone: 'America/New_York' }), 'groupBy.1.field'],
])('refuses %s with the query\'s refusal, field for field', async (_label, query, path) => {
const asQuery = await refusalOf('query', analytics.query({ ...query } as never));
const asSql = await refusalOf('sql', analytics.generateSql({ ...query } as never));
expect(asQuery).toMatchObject({ code: 'INVALID_QUERY', status: 400, httpStatus: 400, object: DEAL, param: 'groupBy', path, fields: ['closed_at'] });
expect(asSql, 'the dry run vs the query').toEqual(asQuery);
});

it.each<[string, Query, string]>([
[
'one bucket of one column',
onCube({ timeDimensions: [td('closed_at', 'month')] }),
"SELECT strftime('%Y-%m', `closed_at`) AS \"closed_at\", SUM(amount) AS \"amount_sum\" FROM \"os28841_deal\" "
+ "GROUP BY strftime('%Y-%m', `closed_at`)",
],
[
'buckets of two columns',
onCube({ timeDimensions: [td('closed_at', 'month'), td('closed_on', 'year')] }),
"SELECT strftime('%Y-%m', `closed_at`) AS \"closed_at\", strftime('%Y', `closed_on`) AS \"closed_on\", "
+ "SUM(amount) AS \"amount_sum\" FROM \"os28841_deal\" GROUP BY strftime('%Y-%m', `closed_at`), strftime('%Y', `closed_on`)",
],
[
'a dimension that is its own bucket, asked twice at one granularity, beside a raw column',
onCube({ dimensions: ['region', 'closed_at'], timeDimensions: [td('closed_at', 'month'), td('closed_at', 'month')] }),
"SELECT region AS \"region\", strftime('%Y-%m', `closed_at`) AS \"closed_at\", SUM(amount) AS \"amount_sum\" "
+ "FROM \"os28841_deal\" GROUP BY region, strftime('%Y-%m', `closed_at`)",
],
])('still renders %s byte for byte, the statement the query echoes', async (_label, query, sql) => {
const dryRun = await analytics.generateSql({ ...query } as never);
expect(dryRun).toEqual({ sql, params: [] });
expect((await analytics.query({ ...query } as never)).sql).toBe(sql);
});

it("still renders a cross-object 'account.name' beside one bucket byte for byte", async () => {
const dryRun = await analytics.generateSql(onObject({ dimensions: ['account.name'], timeDimensions: [td('closed_at', 'month')] }) as never);
expect(dryRun).toEqual({
sql: "SELECT \"os28841_account\".\"name\" AS \"account.name\", strftime('%Y-%m', `closed_at`) AS \"closed_at\", "
+ "SUM(amount) AS \"amount_sum\" FROM \"os28841_deal\" LEFT JOIN \"os28841_account\" ON \"os28841_deal\".\"account\" = "
+ "\"os28841_account\".\"id\" GROUP BY \"os28841_account\".\"name\", strftime('%Y-%m', `closed_at`)",
params: [],
});
});

it('still answers one bucket at a non-UTC zone with the echo refusal, while the query serves it', async () => {
const query = onCube({ timeDimensions: [td('closed_at', 'month')], timezone: 'America/New_York' });
expect(await refusalOf('sql', analytics.generateSql({ ...query } as never))).toMatchObject({ code: 'NOT_IMPLEMENTED', status: 501 });
expect((await analytics.query({ ...query } as never)).rows).toHaveLength(1);
});
});
Loading
Loading