From 4020cd4261d9834533d73094b54d64ad032f660b Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 16:06:31 +0000 Subject: [PATCH 1/7] feat(spec)!: retire the cube metric types number / string / boolean (WIP) AggregationMetricType keeps the six aggregates; the three custom-SQL-expression members are refused at parse by name with a prescription (enumWithRetiredValues). D3 entry cube-metric-expression-types-retired and its step-18 rationale fragment. Claude-Session: https://claude.ai/code/session_01UtnxvdiN376GF3sgXwAw4d Co-authored-by: Claude --- packages/spec/src/data/analytics.test.ts | 5 +- packages/spec/src/data/analytics.zod.ts | 82 +++++-- .../cube-member-sql-column-reference.test.ts | 6 +- ...metric-expression-types-retirement.test.ts | 217 ++++++++++++++++++ ...18.cube-metric-expression-types-retired.ts | 49 ++++ packages/spec/src/migrations/registry.ts | 59 +++++ 6 files changed, 394 insertions(+), 24 deletions(-) create mode 100644 packages/spec/src/data/cube-metric-expression-types-retirement.test.ts create mode 100644 packages/spec/src/migrations/entries/semantic/18.cube-metric-expression-types-retired.ts diff --git a/packages/spec/src/data/analytics.test.ts b/packages/spec/src/data/analytics.test.ts index 02d79533301..b7ca10e93a6 100644 --- a/packages/spec/src/data/analytics.test.ts +++ b/packages/spec/src/data/analytics.test.ts @@ -16,10 +16,13 @@ import { applyConversions, collectConversionNotices } from '../conversions/apply describe('AggregationMetricType', () => { it('should accept all valid metric types', () => { - const types = ['count', 'sum', 'avg', 'min', 'max', 'count_distinct', 'number', 'string', 'boolean']; + const types = ['count', 'sum', 'avg', 'min', 'max', 'count_distinct']; for (const t of types) { expect(() => AggregationMetricType.parse(t)).not.toThrow(); } + // The custom-SQL-expression members were retired (#21000); their refusal + // is pinned in `cube-metric-expression-types-retirement.test.ts`. + expect([...AggregationMetricType.options]).toEqual(types); }); it('should reject invalid metric type', () => { diff --git a/packages/spec/src/data/analytics.zod.ts b/packages/spec/src/data/analytics.zod.ts index 9663bfa5037..fb19a850767 100644 --- a/packages/spec/src/data/analytics.zod.ts +++ b/packages/spec/src/data/analytics.zod.ts @@ -15,26 +15,68 @@ import { DateGranularity } from './query.zod'; * "Business Data" (Metrics/Dimensions). */ -/** - * Aggregation Metric Type - * The mathematical operation to perform on a metric. - */ import { lazySchema } from '../shared/lazy-schema'; import { strictObject } from '../shared/strict-object'; -import { retiredKey } from '../shared/retired-key'; +import { enumWithRetiredValues, retiredKey } from '../shared/retired-key'; import { ANALYTICS_COLUMN_REFERENCE } from './analytics-column-reference'; import { MetadataProtectionFields } from '../kernel/metadata-protection.zod'; -export const AggregationMetricType = z.enum([ - 'count', - 'sum', - 'avg', - 'min', - 'max', - 'count_distinct', - 'number', // Custom SQL expression returning a number - 'string', // Custom SQL expression returning a string - 'boolean' // Custom SQL expression returning a boolean -]); + +// ── Retired metric types (ADR-0049 enforce-or-remove) ─────────────────────── +// +// #21000. `number`, `string` and `boolean` declared "a custom SQL expression +// returning a number / string / boolean": the measure's `sql` WAS the whole +// computation, and the type only named what it returned. Ruling D on #20943 +// made a cube member's `sql` a column reference (`CUBE_MEMBER_SQL` below), so +// the three were left with nothing to declare. Measured through +// `AnalyticsService` on both strategies before this retirement, with a column +// `sql`: the raw-SQL path emitted the column UNAGGREGATED +// (`SELECT status AS "status", amount AS "m" … GROUP BY status` — a bare +// column in a grouped statement, which PostgreSQL refuses and SQLite answers +// with an arbitrary row's value), and the ObjectQL path refused the measure. +// +// A VALUE-level retirement (`enumWithRetiredValues`, shared/retired-key.ts): +// the members left the enum, so `tsc` refuses them, and the parse answers each +// with the prescription below instead of zod's anonymous enum message. No D2 +// conversion — no rewrite can say which aggregate the author meant — so the +// D3 entry `cube-metric-expression-types-retired` carries that judgement. A +// stored cube carrying one is REFUSED, never stood down: every door that +// parses a cube refuses it here, and both analytics strategies refuse a cube +// that reached them unparsed with this same text, read off this enum. +// +// Module-private and written with `//`, never `/** */`: prose an enum's error +// map consumes, not documented surface — an export with no reader is a +// published surface the next narrowing must keep. +const METRIC_TYPE_EXPRESSION_FIX = + 'Name the aggregate the measure means — `sum`, `avg`, `min` or `max` over the column, `count` ' + + '(over `\'*\'` for a row count, or over a column for its non-null values), or `count_distinct`. ' + + 'A value computed per row has no expression form in the cube layer: keep it as a field of the ' + + 'object (a stored or formula field) and aggregate that field here; a ratio or other value ' + + 'derived from measures is `derived: { op, of: [...] }` on an ADR-0021 dataset.'; + +const metricTypeExpressionRetired = (member: 'number' | 'string' | 'boolean') => + `\`${member}\` was removed from \`AggregationMetricType\` (a cube measure's \`measures..type\`) ` + + 'in @objectstack/spec 17.7.0 (ADR-0049 enforce-or-remove) — it declared a custom SQL expression ' + + `returning a ${member}, and a measure's \`sql\` is a column reference, so the type had nothing left ` + + 'to compute: the raw-SQL path returned the column unaggregated and the ObjectQL path refused the ' + + `measure. ${METRIC_TYPE_EXPRESSION_FIX}`; + +/** + * Aggregation Metric Type + * + * The aggregate a cube measure applies to its column: the six aggregation + * functions, the same six an ADR-0021 dataset measure's `aggregate` names. + * The custom-SQL-expression members `number`, `string` and `boolean` were + * retired (ADR-0049) — a measure's `sql` is a column reference, so they had + * nothing left to compute — and are answered at parse with their prescription. + */ +export const AggregationMetricType = enumWithRetiredValues( + ['count', 'sum', 'avg', 'min', 'max', 'count_distinct'], + { + number: metricTypeExpressionRetired('number'), + string: metricTypeExpressionRetired('string'), + boolean: metricTypeExpressionRetired('boolean'), + }, +); export type AggregationMetricType = z.input; /** @@ -226,10 +268,10 @@ const CUBE_DIMENSION_NAME_REMOVED = cubeMemberNameRemoved('dimensions. { it('a measure refuses every non-column value at `sql`, naming the contract first and the dataset form after', () => { for (const sql of EXPRESSIONS) { - const issues = refusalOf(MetricSchema, { label: 'M', type: 'number', sql }); + const issues = refusalOf(MetricSchema, { label: 'M', type: 'sum', sql }); expect(issues, sql).toHaveLength(1); expect(issues[0]!.code).toBe('invalid_format'); expect(issues[0]!.path).toEqual(['sql']); @@ -180,7 +180,7 @@ describe('cube member sql — the rule reaches the published JSON Schema as a pa describe('cube member sql — every door that carries a cube refuses an expression member', () => { const withExpression = { ...CUBE, - measures: { ...CUBE.measures, done_rate: { label: 'Done Rate (%)', type: 'number', sql: DONE_RATE_EXPRESSION } }, + measures: { ...CUBE.measures, done_rate: { label: 'Done Rate (%)', type: 'sum', sql: DONE_RATE_EXPRESSION } }, }; it('the cube schema refuses it at measures..sql and dimensions..sql — one issue per member', () => { @@ -243,7 +243,7 @@ describe('cube member sql — every door that carries a cube refuses an expressi describe('cube member sql — the dataset form the prescription names is the structural equivalent', () => { it('the retired done-rate expression is refused, and its dataset form — a filtered count over a count — parses', () => { - expect(MetricSchema.safeParse({ label: 'Done Rate (%)', type: 'number', sql: DONE_RATE_EXPRESSION }).success).toBe(false); + expect(MetricSchema.safeParse({ label: 'Done Rate (%)', type: 'sum', sql: DONE_RATE_EXPRESSION }).success).toBe(false); const dataset = { name: 'task_metrics', label: 'Task Metrics', diff --git a/packages/spec/src/data/cube-metric-expression-types-retirement.test.ts b/packages/spec/src/data/cube-metric-expression-types-retirement.test.ts new file mode 100644 index 00000000000..9cbc42d74c7 --- /dev/null +++ b/packages/spec/src/data/cube-metric-expression-types-retirement.test.ts @@ -0,0 +1,217 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * A cube measure's custom-SQL-expression types — `number`, `string`, + * `boolean` — are RETIRED from `AggregationMetricType` (#21000, ADR-0049 + * enforce-or-remove). + * + * They declared "a custom SQL expression returning …": the measure's `sql` + * was the whole computation. Since `cube-member-sql-expression-retired` a + * member's `sql` is a column reference, so the three had nothing left to + * compute. What is pinned here, door by door: + * + * 1. Each of the three is refused at parse, by name, with the prescription — + * at the enum, at `MetricSchema.type`, and at a cube's + * `measures..type`. `sum` and `count` parse (the control), and a + * value the enum never declared keeps zod's own message. + * 2. Every door that carries a STORED or AUTHORED cube refuses it with the + * same prescription, never stands it down: the `analytics_cube` write + * door, the artifact boot door (`ObjectStackDefinitionSchema`, which + * `MetadataPlugin` parses a built artifact through), `defineCube()` and + * `defineStack()` (with its STACK_SCHEMA_INVALID / 422 envelope). + * The rehydration seam replays no conversion over it — a stored row + * reaches the parse exactly as stored, and the parse refuses it. + * 3. `tsc` refuses the three at a typed measure. + * 4. ADR-0087: the family's D3 entry is registered under step 18, with no + * D2 conversion naming the surface and no `RETIRED_KEYS_BY_MAJOR` row — + * no key left the shape. + * + * On the assertion set: a schema refusal raises a `ZodError` whose issues + * carry `code` and `path` but no ADR-0112 `status` — that envelope belongs to + * the authoring door, `defineStack`, pinned with its `code` and `status`. + */ + +import { describe, expect, it } from 'vitest'; + +import { CONVERSIONS_BY_MAJOR } from '../conversions/registry'; +import { applyConversionsToStoredItem } from '../conversions/stored'; +import { getMetadataTypeSchema } from '../kernel/metadata-type-schemas'; +import { MIGRATIONS_BY_MAJOR, RETIRED_KEYS_BY_MAJOR } from '../migrations/registry'; +import { ObjectStackDefinitionSchema, defineStack } from '../stack.zod'; +import { AggregationMetricType, CubeSchema, MetricSchema, defineCube, type Metric } from './analytics.zod'; + +const D3_ID = 'cube-metric-expression-types-retired'; +const RETIRED = ['number', 'string', 'boolean'] as const; +const AGGREGATES = ['count', 'sum', 'avg', 'min', 'max', 'count_distinct'] as const; + +/** The prescription's first sentence, per retired member. */ +const firstSentence = (member: string) => + `\`${member}\` was removed from \`AggregationMetricType\` (a cube measure's \`measures..type\`) ` + + 'in @objectstack/spec 17.7.0 (ADR-0049 enforce-or-remove)'; + +/** The fix every prescription carries: the six aggregates, and where a computed value goes instead. */ +const FIX = /Name the aggregate the measure means — `sum`, `avg`, `min` or `max` over the column, `count`[^]*`count_distinct`[^]*keep it as a field of the object[^]*ADR-0021 dataset/; + +const measureOf = (type: string) => ({ label: 'M', type, sql: 'amount' }); + +const CUBE = { + name: 'orders', + sql: 'shop_order', + measures: { + count: { label: 'Orders', type: 'count', sql: '*' }, + revenue: { label: 'Revenue', type: 'sum', sql: 'amount' }, + }, + dimensions: { status: { label: 'Status', type: 'string', sql: 'status' } }, +} as const; + +/** The control cube plus one measure of `type` — the shape a stored pre-retirement cube carries. */ +const cubeWith = (type: string) => ({ ...CUBE, measures: { ...CUBE.measures, m: measureOf(type) } }); + +function issuesOf(schema: { safeParse: (v: unknown) => { success: boolean; error?: { issues: Array<{ code: string; path: PropertyKey[]; message: string }> } } }, value: unknown) { + const r = schema.safeParse(value); + expect(r.success, 'expected a refusal').toBe(false); + return r.error!.issues; +} + +describe('the custom-SQL-expression metric types are refused at parse, by name', () => { + it.each(RETIRED)('`%s` is refused at the enum with its prescription', (member) => { + const issues = issuesOf(AggregationMetricType, member); + expect(issues).toHaveLength(1); + expect(issues[0]!.code).toBe('invalid_value'); + expect(issues[0]!.message.startsWith(firstSentence(member))).toBe(true); + expect(issues[0]!.message).toContain(`returning a ${member}`); + expect(issues[0]!.message).toMatch(FIX); + }); + + it.each(RETIRED)('`%s` is refused at a metric\'s `type`, and only there', (member) => { + const issues = issuesOf(MetricSchema, measureOf(member)); + expect(issues.map((i) => [i.code, i.path])).toEqual([['invalid_value', ['type']]]); + expect(issues[0]!.message.startsWith(firstSentence(member))).toBe(true); + }); + + it.each(RETIRED)('`%s` is refused at a cube\'s `measures..type`', (member) => { + const issues = issuesOf(CubeSchema, cubeWith(member)); + expect(issues.map((i) => [i.code, i.path])).toEqual([['invalid_value', ['measures', 'm', 'type']]]); + expect(issues[0]!.message.startsWith(firstSentence(member))).toBe(true); + }); + + it('CONTROL: `sum` and `count` parse, and the six aggregates are the whole vocabulary', () => { + expect(MetricSchema.safeParse(measureOf('sum')).success).toBe(true); + expect(MetricSchema.safeParse({ label: 'Rows', type: 'count', sql: '*' }).success).toBe(true); + expect(CubeSchema.safeParse(cubeWith('sum')).success).toBe(true); + expect([...AggregationMetricType.options]).toEqual([...AGGREGATES]); + }); + + it('a value the enum never declared keeps zod\'s own message — the prescription is the retired members\' alone', () => { + const issues = issuesOf(AggregationMetricType, 'median'); + expect(issues).toHaveLength(1); + expect(issues[0]!.message).not.toMatch(/was removed/); + for (const aggregate of AGGREGATES) expect(issues[0]!.message).toContain(aggregate); + }); + + it('`DimensionType` is a separate enum and keeps `string` / `number` / `boolean`', () => { + // The census reading this pins: the showcase cube's `type: 'string'` lines + // are DIMENSIONS, which this retirement does not touch. + expect(CubeSchema.safeParse({ + ...CUBE, + dimensions: { + status: { label: 'Status', type: 'string', sql: 'status' }, + amount: { label: 'Amount', type: 'number', sql: 'amount' }, + paid: { label: 'Paid', type: 'boolean', sql: 'paid' }, + }, + }).success).toBe(true); + }); +}); + +describe('every door that carries a stored or authored cube refuses it — never stands it down', () => { + it.each(RETIRED)('the `analytics_cube` write door refuses `%s`', (member) => { + // `getMetadataTypeSchema('analytics_cube')` is what a `PUT /api/v1/meta/analytics_cube` + // body is validated against; a rebinding to another shape would pass the + // pins above and still accept the retired type in production. + const door = getMetadataTypeSchema('analytics_cube'); + expect(door).toBe(CubeSchema); + const issues = issuesOf(door!, cubeWith(member)); + expect(issues.map((i) => i.path)).toEqual([['measures', 'm', 'type']]); + expect(issues[0]!.message.startsWith(firstSentence(member))).toBe(true); + }); + + it.each(RETIRED)('the artifact boot door refuses a built stack carrying `%s`', (member) => { + // `MetadataPlugin._parseAndRegisterArtifact` parses a built artifact + // through this schema; the refusal fails the boot with the prescription. + const issues = issuesOf(ObjectStackDefinitionSchema, { analyticsCubes: [cubeWith(member)] }); + expect(issues.map((i) => i.path)).toEqual([['analyticsCubes', 0, 'measures', 'm', 'type']]); + expect(issues[0]!.message.startsWith(firstSentence(member))).toBe(true); + }); + + it.each(RETIRED)('the rehydration seam replays nothing over a stored row carrying `%s` — it reaches the parse as stored', (member) => { + // CONTROL: the seam is live for this type — a retired KEY a D2 conversion + // strips (`refreshKey`, lossless: it never had an effect) is stripped here. + const withRetiredKey = applyConversionsToStoredItem('analytics_cube', { + ...cubeWith(member), + refreshKey: { every: '1 hour' }, + }) as Record; + expect(withRetiredKey).not.toHaveProperty('refreshKey'); + const stored = cubeWith(member); + const rehydrated = applyConversionsToStoredItem('analytics_cube', stored) as typeof stored; + // Nothing rewrote or dropped the retired measure on the way… + expect(rehydrated.measures.m).toEqual(measureOf(member)); + expect((withRetiredKey.measures as Record).m).toEqual(measureOf(member)); + // …so the parse that follows refuses it, with the prescription. + const issues = issuesOf(CubeSchema, rehydrated); + expect(issues[0]!.message.startsWith(firstSentence(member))).toBe(true); + }); + + it('`defineCube()` refuses it with the prescription', () => { + for (const member of RETIRED) { + expect(() => defineCube(cubeWith(member) as never)).toThrow(firstSentence(member)); + } + }); + + it('the authoring door, defineStack, refuses it with the STACK_SCHEMA_INVALID envelope', () => { + const stack = (cube: Record) => ({ + manifest: { id: 'com.example.cube-metric-type', name: 'cube_metric_type', version: '1.0.0', type: 'app' }, + analyticsCubes: [cube], + }); + for (const member of RETIRED) { + let thrown: unknown; + try { + defineStack(stack(cubeWith(member)) as never); + } catch (e) { + thrown = e; + } + const refusal = thrown as { code?: string; status?: number; issues?: Array<{ path: PropertyKey[]; message: string }> }; + expect(refusal?.code, member).toBe('STACK_SCHEMA_INVALID'); + expect(refusal?.status, member).toBe(422); + expect(refusal.issues?.map((i) => i.path), member).toEqual([['analyticsCubes', 0, 'measures', 'm', 'type']]); + expect(refusal.issues?.[0]?.message.startsWith(firstSentence(member)), member).toBe(true); + } + // CONTROL: the same stack with an aggregate measure is accepted by the same door. + expect(() => defineStack(stack(cubeWith('sum')) as never)).not.toThrow(); + }); +}); + +describe('tsc refuses the retired members at a typed measure', () => { + it('a typed `Metric` cannot name one', () => { + // @ts-expect-error — `number` left `AggregationMetricType` (#21000). + const retired: Metric = { label: 'M', type: 'number', sql: 'amount' }; + const live: Metric = { label: 'M', type: 'sum', sql: 'amount' }; + expect(MetricSchema.safeParse(retired).success).toBe(false); + expect(MetricSchema.safeParse(live).success).toBe(true); + }); +}); + +describe('ADR-0087 registration', () => { + it('carries the family D3 entry under step 18, with no D2 conversion and no retired-key row', () => { + const step = MIGRATIONS_BY_MAJOR[18]!; + const d3 = step.semantic.find((s) => s.id === D3_ID); + expect(d3, 'the family D3 entry').toBeDefined(); + expect(d3!.reason.length).toBeGreaterThan(0); + expect(d3!.acceptanceCriteria.length).toBeGreaterThan(0); + expect(step.rationale).toContain('`cube-metric-expression-types-retired`'); + // No conversion rewrites the type: a stored cube is refused, never stood down. + const surfaces = Object.values(CONVERSIONS_BY_MAJOR).flat().map((c) => c.surface); + expect(surfaces.filter((s) => /measures\.\.type/.test(s))).toEqual([]); + // No key left the shape, so no `${defKey}:${name}` entry is owed. + expect(RETIRED_KEYS_BY_MAJOR[18]).not.toContain('data/Metric:type'); + }); +}); diff --git a/packages/spec/src/migrations/entries/semantic/18.cube-metric-expression-types-retired.ts b/packages/spec/src/migrations/entries/semantic/18.cube-metric-expression-types-retired.ts new file mode 100644 index 00000000000..d3e0c85fdaf --- /dev/null +++ b/packages/spec/src/migrations/entries/semantic/18.cube-metric-expression-types-retired.ts @@ -0,0 +1,49 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import type { SemanticMigration } from '../../types.js'; + +// #21000 (ADR-0049 enforce-or-remove) — `AggregationMetricType`'s `number`, +// `string` and `boolean` declared a custom SQL expression returning that type, +// and a cube member's `sql` has been a column reference since +// `cube-member-sql-expression-retired`, so the three had nothing left to +// compute. A value-level retirement (`enumWithRetiredValues`), semantic only: +// no D2 conversion, because no rewrite can say which aggregate the author +// meant, and a stored cube carrying one is refused at every door rather than +// rewritten. +export const entry: SemanticMigration = { + id: 'cube-metric-expression-types-retired', + // No backticks and no pipes in `surface` — build-upgrade-guide.ts renders it + // inside a code span AND a table cell. + surface: + 'analyticsCubes[].measures..type (data.AggregationMetricType) authored as number, string ' + + 'or boolean — the custom-SQL-expression metric types', + replacement: + 'the aggregate the measure means: `sum`, `avg`, `min` or `max` over the column, `count` (over ' + + '`\'*\'` for a row count, or over a column for its non-null values), or `count_distinct`. A value ' + + 'computed per row becomes a field of the object (a stored or formula field) that the measure ' + + 'aggregates; a ratio or other value derived from measures is `derived: { op, of: [...] }` on an ' + + 'ADR-0021 dataset', + reason: + 'The three types existed to mark a measure whose `sql` was the whole computation — a ratio, a ' + + 'CASE, a window function — and named only what it returned. Since ' + + '`cube-member-sql-expression-retired` a member\'s `sql` is a column reference, so the types had ' + + 'nothing left to declare: measured before this retirement, the raw-SQL strategy emitted the ' + + 'referenced column unaggregated (a bare column in a grouped statement, which PostgreSQL refuses ' + + 'and SQLite answers with an arbitrary row\'s value) and the ObjectQL strategy refused the ' + + 'measure. There is no D2 conversion: the column alone does not say which aggregate the author ' + + 'wanted — a `number` over `amount` may have meant its sum, its average or its largest value — ' + + 'so only the author can choose, and a measure whose old expression computed something per row ' + + 'needs that value stored on the object before any aggregate can read it. Nothing is rewritten ' + + 'or dropped at rest: a stored or built cube that still carries one of the three is refused, ' + + 'with the prescription, at the boot and write doors, and a cube that reaches the analytics ' + + 'service without meeting the parse is refused at query time with the same text. ADR-0049 / ' + + 'ADR-0087', + acceptanceCriteria: + 'Every analytics cube parses: `CubeSchema`, the analytics_cube write door and defineStack refuse ' + + 'a measure typed number, string or boolean at its `type` with a prescription naming the six ' + + 'aggregates, so the sweep is mechanical — parse each cube, and each refusal is one measure to ' + + 'retype. For each retyped measure, a query over a fixture with more than one row per group ' + + 'returns the aggregate the author chose, and every dashboard, report or saved query that read ' + + 'the measure is checked against the number it now returns. A measure typed with one of the six ' + + 'aggregates parses byte-identically to before.', +}; diff --git a/packages/spec/src/migrations/registry.ts b/packages/spec/src/migrations/registry.ts index 933aadf1c97..922bf4f6869 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -5334,6 +5334,20 @@ const STEP18_RATIONALE: readonly RationaleFragment[] = [ + 'the semantic entry `cube-member-sql-expression-retired` carries the move, including ' + 'the scale change a ratio makes (a `derived` ratio is a 0–1 fraction).', }, + { + id: 'cube-metric-expression-types-retired', + order: 62, + text: + 'It also retires a cube measure\'s custom-SQL-expression types — `number`, `string` and ' + + '`boolean` from `AggregationMetricType`, so from `measures..type` (ADR-0049 ' + + 'enforce-or-remove). They marked a measure whose `sql` was the whole computation, and with ' + + 'that `sql` now a column reference they had nothing left to compute: the raw-SQL path ' + + 'returned the column unaggregated and the ObjectQL path refused the measure. Each is refused ' + + 'at parse with a prescription naming the six aggregates. No D2 conversion: the column alone ' + + 'does not say which aggregate the author meant, so the semantic entry ' + + '`cube-metric-expression-types-retired` carries the choice, and a stored cube that still ' + + 'carries one is refused rather than rewritten.', + }, { id: 'cube-metric-filters-retired', order: 10, @@ -8706,6 +8720,51 @@ const step18: MigrationStep = { + 'Every dashboard, report or saved query that named the cube member now names the dataset ' + 'measure. A cube member that aggregates a column parses byte-identically to before.', }, + // #21000 (ADR-0049 enforce-or-remove) — `AggregationMetricType`'s `number`, + // `string` and `boolean` declared a custom SQL expression returning that type, + // and a cube member's `sql` has been a column reference since + // `cube-member-sql-expression-retired`, so the three had nothing left to + // compute. A value-level retirement (`enumWithRetiredValues`), semantic only: + // no D2 conversion, because no rewrite can say which aggregate the author + // meant, and a stored cube carrying one is refused at every door rather than + // rewritten. + { + id: 'cube-metric-expression-types-retired', + // No backticks and no pipes in `surface` — build-upgrade-guide.ts renders it + // inside a code span AND a table cell. + surface: + 'analyticsCubes[].measures..type (data.AggregationMetricType) authored as number, string ' + + 'or boolean — the custom-SQL-expression metric types', + replacement: + 'the aggregate the measure means: `sum`, `avg`, `min` or `max` over the column, `count` (over ' + + '`\'*\'` for a row count, or over a column for its non-null values), or `count_distinct`. A value ' + + 'computed per row becomes a field of the object (a stored or formula field) that the measure ' + + 'aggregates; a ratio or other value derived from measures is `derived: { op, of: [...] }` on an ' + + 'ADR-0021 dataset', + reason: + 'The three types existed to mark a measure whose `sql` was the whole computation — a ratio, a ' + + 'CASE, a window function — and named only what it returned. Since ' + + '`cube-member-sql-expression-retired` a member\'s `sql` is a column reference, so the types had ' + + 'nothing left to declare: measured before this retirement, the raw-SQL strategy emitted the ' + + 'referenced column unaggregated (a bare column in a grouped statement, which PostgreSQL refuses ' + + 'and SQLite answers with an arbitrary row\'s value) and the ObjectQL strategy refused the ' + + 'measure. There is no D2 conversion: the column alone does not say which aggregate the author ' + + 'wanted — a `number` over `amount` may have meant its sum, its average or its largest value — ' + + 'so only the author can choose, and a measure whose old expression computed something per row ' + + 'needs that value stored on the object before any aggregate can read it. Nothing is rewritten ' + + 'or dropped at rest: a stored or built cube that still carries one of the three is refused, ' + + 'with the prescription, at the boot and write doors, and a cube that reaches the analytics ' + + 'service without meeting the parse is refused at query time with the same text. ADR-0049 / ' + + 'ADR-0087', + acceptanceCriteria: + 'Every analytics cube parses: `CubeSchema`, the analytics_cube write door and defineStack refuse ' + + 'a measure typed number, string or boolean at its `type` with a prescription naming the six ' + + 'aggregates, so the sweep is mechanical — parse each cube, and each refusal is one measure to ' + + 'retype. For each retyped measure, a query over a fixture with more than one row per group ' + + 'returns the aggregate the author chose, and every dashboard, report or saved query that read ' + + 'the measure is checked against the number it now returns. A measure typed with one of the six ' + + 'aggregates parses byte-identically to before.', + }, // #10414 (ADR-0049 enforce-or-remove) — the D3 entry of the // `metric-filters-removed` family (ruling B on #17152: one D3 entry per // retirement family, even when D2 is lossless). The unknown-keys entry From 6a5e98e5cb1ff5ccb7e98f9c07b71ee0845fc769 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 16:12:25 +0000 Subject: [PATCH 2/7] feat(service-analytics)!: one spec-worded verdict on a cube measure's type replaces the expression partition (WIP) Claude-Session: https://claude.ai/code/session_01UtnxvdiN376GF3sgXwAw4d Co-authored-by: Claude --- ...gregate-bridge-function-vocabulary.test.ts | 60 ++++- ...aller-member-column-reference-gate.test.ts | 13 +- .../cube-authored-format-granularity.test.ts | 52 ++-- .../field-read-admission-gate.test.ts | 2 +- ...measure-expression-both-strategies.test.ts | 242 +++++++++--------- .../__tests__/measure-expression-sql.test.ts | 84 +++--- .../unlisted-refusal-envelope.test.ts | 9 +- .../src/analytics-service.ts | 7 +- .../src/cube-measure-field-type-door.ts | 5 +- .../service-analytics/src/dataset-refusal.ts | 12 +- .../src/metric-type-coverage.test.ts | 76 ++++-- .../services/service-analytics/src/plugin.ts | 43 ++-- .../src/preview-evaluator.ts | 11 +- .../src/strategies/native-sql-strategy.ts | 153 ++++++----- .../src/strategies/objectql-strategy.ts | 72 ++---- packages/spec/liveness/analytics_cube.json | 6 +- 16 files changed, 469 insertions(+), 378 deletions(-) diff --git a/packages/services/service-analytics/src/__tests__/aggregate-bridge-function-vocabulary.test.ts b/packages/services/service-analytics/src/__tests__/aggregate-bridge-function-vocabulary.test.ts index 573727b6271..54ebeddca6c 100644 --- a/packages/services/service-analytics/src/__tests__/aggregate-bridge-function-vocabulary.test.ts +++ b/packages/services/service-analytics/src/__tests__/aggregate-bridge-function-vocabulary.test.ts @@ -19,15 +19,25 @@ * * ## Why this refusal is deliberately NOT in the ADR-0112 envelope * - * The reachable producer of a non-aggregate method — a custom-SQL measure - * (`AggregationMetricType` `number`/`string`/`boolean`) — is refused earlier - * and caller-facing by `ObjectQLStrategy.resolveMeasureAggregation` (commit 017130a09, - * `INVALID_FIELD` / 400). Anything still arriving at the bridge is host drift - * (an unparsed cube object, our own drift), which `dataset-refusal.ts`'s module - * header assigns to the bare-`Error`, undeclared-500 tier — the same tier it - * assigns to `native-sql-strategy.ts`'s "measure … has unrecognised type". The - * absence of a `code` is therefore asserted, not overlooked: enveloping this as - * a 400 would tell the author to fix something they did not write. + * A non-aggregate method is host drift (an unparsed cube object, our own + * drift), which `dataset-refusal.ts`'s module header assigns to the + * bare-`Error`, undeclared-500 tier. The absence of a `code` is therefore + * asserted, not overlooked: enveloping this as a 400 would tell the author to + * fix something they did not write. + * + * ## [#21000] Two seams, one tier + * + * The custom-SQL metric types (`AggregationMetricType` `number` / `string` / + * `boolean`) used to be refused `INVALID_FIELD` / 400 by + * `ObjectQLStrategy.resolveMeasureAggregation` (commit 017130a09), and every + * OTHER non-aggregate type — `median`, a cube that never met the parse — was + * forwarded on to this bridge. The three were retired from the spec, and the + * resolver now answers every type no aggregate lowers itself + * (`aggregateOfMeasure`), in the same undeclared-500 tier, in the spec's words. + * So the cube path no longer reaches the bridge with one, and this file pins + * both seams: the cube path refused at the resolver, nothing reaching the + * engine; and the bridge itself, driven directly, still parsing whatever + * method arrives before the engine sees it. */ import { describe, it, expect, vi } from 'vitest'; @@ -114,15 +124,41 @@ describe('[#11833] the aggregate auto-bridge speaks the engine contract vocabula expect(AggregationFunction.options).toContain(calls[0].aggregations?.[0].function); }); - it('refuses a method outside the engine vocabulary instead of forwarding it', async () => { + it('a cube measure whose type names no aggregate is refused at the resolver, before the bridge', async () => { // Host drift: a cube object registered without meeting `CubeSchema`, so its - // `type` never faced the enum's parse. This is the arrival path the tiering - // note above describes. + // `type` never faced the enum's parse. Since #21000 the strategy's resolver + // refuses it — the same tier the bridge answers in, in the spec's words. const calls: EngineAggregateCall[] = []; const service = await analyticsVia(fakeEngine(calls, schema), cubeWithMeasureType('median')); const err = await service.query(selection as never).then(() => null, (e: Error) => e); + expect(err).toBeInstanceOf(Error); + expect(err?.message).toContain('measure "revenue" on cube "sales" cannot be served: its type "median"'); + for (const fn of AggregationFunction.options) expect(err?.message).toContain(fn); + // Undeclared-500 tier, deliberately: no ADR-0112 envelope on this family. + expect((err as Error & { code?: string }).code).toBeUndefined(); + // The load-bearing half — the bad type never reached the engine. + expect(calls).toHaveLength(0); + }); + + it('the bridge itself still refuses a method outside the engine vocabulary instead of forwarding it', async () => { + // Driven DIRECTLY: no cube path reaches the bridge with a non-aggregate + // method any more (the case above), so the seam is exercised as the + // strategy calls it — the service's strategy context's `executeAggregate`, + // which is the plugin's auto-bridge — with a method the engine does not + // declare. + const calls: EngineAggregateCall[] = []; + const service = await analyticsVia(fakeEngine(calls, schema), cubeWithMeasureType('sum')); + const bridge = (service as unknown as { + baseCtx: { executeAggregate: (object: string, options: unknown) => Promise }; + }).baseCtx.executeAggregate; + + const err = await bridge('opportunity', { + groupBy: ['region'], + aggregations: [{ field: 'amount', method: 'median', alias: 'revenue' }], + }).then(() => null, (e: Error) => e); + expect(err).toBeInstanceOf(Error); // The wording IS the contract here: it must name the offending method, the // aggregation it belongs to, and the legal vocabulary. diff --git a/packages/services/service-analytics/src/__tests__/caller-member-column-reference-gate.test.ts b/packages/services/service-analytics/src/__tests__/caller-member-column-reference-gate.test.ts index 13bbae9a307..2a35e41b1d9 100644 --- a/packages/services/service-analytics/src/__tests__/caller-member-column-reference-gate.test.ts +++ b/packages/services/service-analytics/src/__tests__/caller-member-column-reference-gate.test.ts @@ -45,7 +45,7 @@ const REGISTERED: Cube = { public: true, measures: { count: { type: 'count', sql: '*', label: 'Count' }, - author_expr_measure: { type: 'number', sql: 'amount + 1', label: 'Author expression measure' }, + author_expr_measure: { type: 'sum', sql: 'amount + 1', label: 'Author expression measure' }, }, dimensions: { status: { type: 'string', sql: 'status', label: 'Status' }, @@ -195,11 +195,12 @@ describe('[#21156] analytics — a caller-named non-column member is refused at }); }); - // An author-declared expression MEASURE is a native-SQL-only feature (the - // ObjectQL aggregate AST cannot carry a raw SQL expression — a pre-existing - // strategy refusal, not this gate's). The non-regression claim is that THIS - // gate does not refuse it: on NativeSQL it still reaches the strategy with no - // security service. + // An author-declared expression MEASURE — an aggregate over an expression + // `sql`, written around the parse like the dimension above (the custom-SQL + // metric types it used to be typed as were retired from the spec, #21000, + // and both strategies refuse them by type, not this gate). The + // non-regression claim is that THIS gate does not refuse it: on NativeSQL it + // still reaches the strategy with no security service. it('an author-declared expression measure is not refused by this gate (served on NativeSQL, no security service)', async () => { const { service, executed } = makeService({ capabilities: nativeSqlOnly }); await service.query({ cube: 'cm_cube', measures: ['author_expr_measure'] } as never, CALLER); diff --git a/packages/services/service-analytics/src/__tests__/cube-authored-format-granularity.test.ts b/packages/services/service-analytics/src/__tests__/cube-authored-format-granularity.test.ts index 92355294d70..c459ad4fe01 100644 --- a/packages/services/service-analytics/src/__tests__/cube-authored-format-granularity.test.ts +++ b/packages/services/service-analytics/src/__tests__/cube-authored-format-granularity.test.ts @@ -29,12 +29,13 @@ * window-only `timeDimensions` entry stays a filter — the same five answers * the dataset path gives; * - the declared narrowing: a bucketed query is served by the engine path, - * which refuses every member it cannot evaluate — a custom-SQL measure, and, - * on a cube with `joins`, a cross-object member (`planCrossObject`) — so - * grouping such a query by a declared-default dimension is now refused, with - * the envelope, byte for byte, that stating the same granularity by hand - * already got. One pin per refusal source: the custom-SQL measure, and a - * cross-object measure on a joined cube. + * which refuses every member it cannot evaluate — on a cube with `joins`, a + * cross-object member (`planCrossObject`) — so grouping such a query by a + * declared-default dimension is now refused, with the envelope, byte for + * byte, that stating the same granularity by hand already got. Pinned on a + * cross-object measure on a joined cube. (A custom-SQL measure was the + * second refusal source until its metric types were retired, #21000: both + * paths now refuse it by type whatever the route, which its case pins.) */ import { describe, it, expect, vi } from 'vitest'; @@ -273,12 +274,14 @@ describe('analytics_cube.dimensions.granularities — the declared single granul expect(sqls[0]).toMatch(/created_at/); }); - it('DECLARED NARROWING: a custom-SQL measure grouped by a declared-default dimension gets the refusal a stated granularity gets', async () => { - // NOT parsed: since #20943 the cube contract admits a column reference - // only, so `CubeSchema` refuses this expression member at every authoring - // door. The engine path's refusal it pins is still owed to a cube that - // reaches the service without meeting that parse (a host registering one - // in-process), so the member is built directly on the parsed cube. + it('a retired custom-SQL metric type is refused whatever the route — by default bucket, by a stated granularity, and on the raw-SQL path', async () => { + // NOT parsed: `CubeSchema` refuses this member at every authoring door — + // its `sql` since #20943, its `type` since #21000. The refusal pinned here + // is still owed to a cube that reaches the service without meeting that + // parse (a host registering one in-process), so the member is built + // directly on the parsed cube. Before #21000 the engine path refused it and + // the raw-SQL path served it, so the route a declared default chose decided + // the answer; now both refuse it by type, before anything executes. const withExpression: Cube = { ...authored, measures: { @@ -302,7 +305,7 @@ describe('analytics_cube.dimensions.granularities — the declared single granul return []; }, }); - const envelope = (e: any) => ({ code: e?.code, status: e?.status }); + const refusal = (e: any) => ({ code: e?.code, status: e?.status, message: (e as Error)?.message }); const byDefault = await service .query({ cube: 'orders', measures: ['done_rate'], dimensions: ['placed_at'] }) @@ -315,20 +318,21 @@ describe('analytics_cube.dimensions.granularities — the declared single granul timeDimensions: [{ dimension: 'placed_at', granularity: 'month' }], }) .catch((e: unknown) => e); + // The control that used to be SERVED on the raw-SQL path: a dimension that + // declares no single default keeps the query there. + const rawSqlRoute = await service + .query({ cube: 'orders', measures: ['done_rate'], dimensions: ['shipped_at'] }) + .catch((e: unknown) => e); - expect(envelope(byDefault)).toEqual({ code: 'INVALID_FIELD', status: 400 }); - // Not a new refusal: the one stating the granularity by hand already got. - expect({ ...envelope(byDefault), message: (byDefault as Error).message }).toEqual({ - ...envelope(byHand), - message: (byHand as Error).message, - }); + expect(byDefault).toBeInstanceOf(Error); + expect(refusal(byDefault).message).toContain('measure "done_rate" on cube "orders" cannot be served: its type "number"'); + // One refusal, whichever route the query took. + expect(refusal(byDefault)).toEqual(refusal(byHand)); + expect(refusal(byDefault)).toEqual(refusal(rawSqlRoute)); + // The undeclared-500 tier, never the caller-blaming 400. + expect(refusal(byDefault).code).toBeUndefined(); expect(aggregated).toEqual([]); expect(sqls).toEqual([]); - - // Control: the same measure grouped by a dimension that declares no single - // default is still answered, on the raw-SQL path, as it was before. - await service.query({ cube: 'orders', measures: ['done_rate'], dimensions: ['shipped_at'] }); - expect(sqls).toHaveLength(1); }); it('DECLARED NARROWING, joined cube: a cross-object measure grouped by a declared-default dimension gets the refusal a stated granularity gets', async () => { diff --git a/packages/services/service-analytics/src/__tests__/field-read-admission-gate.test.ts b/packages/services/service-analytics/src/__tests__/field-read-admission-gate.test.ts index b2d9fc6373b..f519fd507f6 100644 --- a/packages/services/service-analytics/src/__tests__/field-read-admission-gate.test.ts +++ b/packages/services/service-analytics/src/__tests__/field-read-admission-gate.test.ts @@ -56,7 +56,7 @@ const AUTHORED: Cube = { measures: { count: { type: 'count', sql: '*', label: 'Count' }, alias_total: { type: 'sum', sql: 'hidden_number', label: 'Total' }, - expression_total: { type: 'number', sql: 'SUM(hidden_number) / 2', label: 'Expression total' }, + expression_total: { type: 'sum', sql: 'SUM(hidden_number) / 2', label: 'Expression total' }, }, dimensions: { title: { type: 'string', sql: 'title', label: 'Title' }, diff --git a/packages/services/service-analytics/src/__tests__/measure-expression-both-strategies.test.ts b/packages/services/service-analytics/src/__tests__/measure-expression-both-strategies.test.ts index 2e47015ace9..ad139e1ebf0 100644 --- a/packages/services/service-analytics/src/__tests__/measure-expression-both-strategies.test.ts +++ b/packages/services/service-analytics/src/__tests__/measure-expression-both-strategies.test.ts @@ -1,61 +1,48 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. /** - * Commit 017130a09 — a custom-SQL measure is refused loudly on the ObjectQL path, and - * BOTH strategies are pinned from one fixture so neither can hide the other. + * A cube measure whose `type` names no aggregate is refused on BOTH strategies, + * from one fixture, so neither can hide the other (commit 017130a09, #21000). * - * #4157 was fixed on one strategy of two: `NativeSQLStrategy` learned to emit - * a `number`/`string`/`boolean` measure's `sql` verbatim, and its regression - * pin (`measure-expression-sql.test.ts`) forces `objectqlAggregate: false` — - * so the pin covered exactly one strategy. `ObjectQLStrategy` never got the - * matching partition: `resolveMeasureAggregation` forwarded `Metric.type` - * verbatim as the engine method with the whole SQL expression in `field`, so - * `driver-sql` threw `INVALID_QUERY` blaming a `function` key the author never - * wrote, and the in-memory evaluator answered `null` for every bucket through - * its `switch` default — the #4157 class in its null variant, measured on - * #12053's probe: an admitted `sum` returned 300 per bucket where the - * custom-SQL measure returned `null`. + * The history this file carries: `AggregationMetricType` declared three + * custom-SQL-expression types — `number` / `string` / `boolean` — whose `sql` + * WAS the computation. `NativeSQLStrategy` emitted them verbatim (#4157) and + * `ObjectQLStrategy` refused them `INVALID_FIELD` / 400 (commit 017130a09), + * partitioned by a shared `EXPRESSION_METRIC_TYPES` set, and this file pinned + * the two postures side by side. A cube member's `sql` then became a column + * reference, so the three had nothing left to compute — measured before + * #21000 on this fixture's shape, the raw-SQL path emitted the referenced + * column UNAGGREGATED in a grouped statement — and they were retired from the + * spec with a prescription. The partition went with them. * - * This file is the pin the defect could not hide from: ONE cube whose measures - * cover all six aggregates, all three expression types and one enum-invalid - * drift type, driven through the real `AnalyticsService` routing under BOTH - * capability profiles. The native profile pins the expression measures still - * SERVED (emitted verbatim); the ObjectQL profile pins them REFUSED — same - * fixture, so a change that moves either posture turns a case here red. - * - * The load-bearing negatives, and why they are here: + * What is pinned now, ONE cube (all six aggregates, the three retired types, + * one never-declared drift type) driven through the real `AnalyticsService` + * routing under BOTH capability profiles: * + * - each retired type is REFUSED on both paths, never served, with the SPEC's + * prescription (`aggregateOfMeasure`) — in the undeclared-500 tier, since + * only a cube that never met `CubeSchema`'s parse can carry one — and + * nothing reaches the engine or the driver; * - every admitted AGGREGATE measure is still served on the ObjectQL path and - * still reaches the engine carrying its OWN method (`sum` stays `sum`). An - * implementation that refuses by method membership (e.g. reusing - * `RECOMBINABLE_METHODS`, which lacks `avg`/`count_distinct`) passes the - * refusal cases and goes red here. - * - an enum-INVALID metric type (`median` — host drift, not authorable) is NOT - * refused with the caller-shaped `INVALID_FIELD` envelope. The drift tier is - * the platform's own (`dataset-refusal.ts` header, #5716): an implementation - * that refuses "every method that is not one of the six aggregates" passes - * the refusal cases too, and goes red here — the arm must key on the - * DECLARED expression partition (`EXPRESSION_METRIC_TYPES`), not on a method - * allowlist. + * still reaches the engine carrying its OWN method (`sum` stays `sum`); + * - an enum-INVALID drift type (`median`) is NOT refused with the caller-shaped + * `INVALID_FIELD` envelope (#5716 / `dataset-refusal.ts`), on either path; * - the pre-existing cross-object non-recombinable refusal keeps its EXACT - * message — the new arm sits beside it, not over it. + * message, and a retired type beside a cross-object dimension is refused as + * the type refusal (the one resolver both doors call reaches it first). * * ## Dissolution verification, direction predicted BEFORE running * - * Restoring the accepting behaviour (deleting the arm commit 017130a09 added in - * `ObjectQLStrategy.resolveMeasureAggregation`) must turn the ObjectQL-profile - * REFUSAL cases red in the ordinary direction: each asserts the ADR-0112 - * envelope (`code`/`status`), the measure's own name in `member` and message, - * AND that nothing reached the engine (`calls`/`sqls` empty) — with the arm - * gone, the query "succeeds", the engine IS reached carrying the expression in - * `field`, so the cases cannot pass vacuously. Every other case — the six - * admitted aggregates, the drift tier, the native-profile SERVED block, the - * cross-object twin — is predicted to stay GREEN in both directions: none of - * them touches the arm. + * Restoring the native path's verbatim emit for the three must turn the + * native-profile REFUSAL cases red in the ordinary direction (the query + * "succeeds" and a statement is executed); removing the ObjectQL resolver's + * verdict must turn the ObjectQL-profile cases red the same way (the engine is + * reached carrying the retired type as its method). The aggregate, drift and + * cross-object cases are predicted to stay green in both directions. */ import { describe, it, expect, vi } from 'vitest'; -import type { Cube } from '@objectstack/spec/data'; +import { AggregationMetricType, type Cube } from '@objectstack/spec/data'; import { AnalyticsService } from '../analytics-service.js'; const silentLogger = { @@ -70,11 +57,12 @@ const silentLogger = { const ORDER_FIELDS = ['id', 'amount', 'cost', 'revenue', 'paid', 'buyer', 'status', 'account', 'created_at']; /** - * One cube, both strategies: all six aggregate types, all three custom-SQL - * expression types. The expression `sql`s are deliberately DOT-FREE — an - * expression containing a dot was already (mis)refused as a cross-object - * measure, so the dot-free ones are the exact shapes that used to reach - * `engine.aggregate` and answer `null`. + * One cube, both strategies: all six aggregate types, and the three retired + * custom-SQL types over a COLUMN — the shape a cube stored after #20943 made + * `sql` a column reference and before #21000 retired the types still carries. + * Built WITHOUT the parse (`as never`): `CubeSchema` refuses all three, so only + * a cube a host registers in-process from a literal can reach the service + * with one. */ const CUBE: Cube = { name: 'orders', @@ -87,15 +75,9 @@ const CUBE: Cube = { min_amount: { label: 'Min', type: 'min', sql: 'amount' }, max_amount: { label: 'Max', type: 'max', sql: 'amount' }, buyers: { label: 'Buyers', type: 'count_distinct', sql: 'buyer' }, - margin: { - label: 'Margin', type: 'number', - sql: 'SUM(revenue) / NULLIF(SUM(cost), 0)', - }, - top_status: { - label: 'Top status', type: 'string', - sql: "MAX(CASE WHEN paid THEN 'paid' ELSE 'open' END)", - }, - any_paid: { label: 'Any paid', type: 'boolean', sql: 'MAX(paid)' }, + margin: { label: 'Margin', type: 'number', sql: 'revenue' }, + top_status: { label: 'Top status', type: 'string', sql: 'status' }, + any_paid: { label: 'Any paid', type: 'boolean', sql: 'paid' }, }, dimensions: { status: { label: 'Status', type: 'string', sql: 'status' }, @@ -163,49 +145,74 @@ async function run(query: unknown, profile: 'objectql' | 'native') { return { rows, error, sqls, calls }; } -/** The one wire shape every custom-SQL refusal (commit 017130a09) must have (ADR-0112 / #5716). */ -function expectCustomSqlRefusal( +/** Run one dry-run `generateSql` (the `/analytics/sql` face) on a fresh service under `profile`. */ +async function runSql(query: unknown, profile: 'objectql' | 'native') { + const { service, sqls, calls } = makeService(profile); + let error: Refusal | undefined; + try { + await service.generateSql(query as never); + } catch (e) { + error = e as Refusal; + } + return { error, sqls, calls }; +} + +const RETIRED = [ + ['margin', 'number'], + ['top_status', 'string'], + ['any_paid', 'boolean'], +] as const; + +const PROFILES = ['native', 'objectql'] as const; + +/** + * The one refusal a retired metric type meets (`aggregateOfMeasure`, #21000): + * the measure and cube named, the SPEC's prescription for the type verbatim, + * the undeclared-500 tier (no ADR-0112 envelope — only a cube that never met + * the parse can carry one), and nothing executed. + */ +function expectRetiredTypeRefusal( r: { error?: Refusal; sqls: string[]; calls: unknown[] }, member: string, type: string, ) { expect(r.error).toBeInstanceOf(Error); - expect(r.error?.code).toBe('INVALID_FIELD'); - expect(r.error?.status).toBe(400); - // The measure AS THE AUTHOR WROTE IT — today's failure blames a `function` - // key the author never wrote, or answers null under this very name. - expect(r.error?.member).toBe(member); - expect(r.error?.param).toBe('measures'); - expect(r.error?.cube).toBe('orders'); - expect(r.error?.message).toContain(`("${member}")`); - expect(r.error?.message).toContain(`type "${type}"`); - // The in-file twin's posture: name the way out, both halves. - expect(r.error?.message).toContain('or run on a native-SQL driver'); + expect(r.error?.message).toContain(`measure "${member}" on cube "orders" cannot be served: its type "${type}"`); + // The prescription is the spec's own, read off the enum — so an operator + // reads what `os validate` would have printed for this cube. + const spec = AggregationMetricType.safeParse(type); + expect(spec.success).toBe(false); + expect(r.error?.message).toContain(spec.error!.issues[0]!.message); + expect(r.error?.message).toContain(`\`${type}\` was removed from \`AggregationMetricType\``); + // Undeclared-500 tier: never the caller-blaming INVALID_FIELD / 400. + expect(r.error?.code).toBeUndefined(); + expect(r.error?.status).toBeUndefined(); // The refusal is a refusal: the engine was never reached, nothing executed. expect(r.calls).toEqual([]); expect(r.sqls).toEqual([]); } -// ── 1. The ObjectQL path REFUSES what it cannot serve ──────────────────────── +// ── 1. Both paths REFUSE a retired type, on both doors ─────────────────────── + +describe.each(PROFILES)('%s path: a retired custom-SQL metric type is refused, never served', (profile) => { + it.each(RETIRED)('refuses "%s" (type %s) on /analytics/query, nothing executed', async (member, type) => { + const r = await run({ cube: 'orders', measures: [member], dimensions: ['status'] }, profile); + expectRetiredTypeRefusal(r, member, type); + }); -describe('ObjectQL path: custom-SQL measures are refused loudly', () => { - it.each([ - ['margin', 'number'], - ['top_status', 'string'], - ['any_paid', 'boolean'], - ] as const)('refuses "%s" (type %s) with INVALID_FIELD/400, engine never reached', async (member, type) => { - const r = await run({ cube: 'orders', measures: [member], dimensions: ['status'] }, 'objectql'); - expectCustomSqlRefusal(r, member, type); + it.each(RETIRED)('refuses "%s" (type %s) on the /analytics/sql dry run too', async (member, type) => { + const r = await runSql({ cube: 'orders', measures: [member], dimensions: ['status'] }, profile); + expectRetiredTypeRefusal(r, member, type); }); - it('an admitted measure beside it does not rescue the query — the custom-SQL member is named', async () => { - const r = await run({ cube: 'orders', measures: ['total', 'margin'], dimensions: ['status'] }, 'objectql'); - expectCustomSqlRefusal(r, 'margin', 'number'); + it('an admitted measure beside it does not rescue the query — the retired member is named', async () => { + const r = await run({ cube: 'orders', measures: ['total', 'margin'], dimensions: ['status'] }, profile); + expectRetiredTypeRefusal(r, 'margin', 'number'); }); it('refuses on the scalar (no-dimension) shape too', async () => { - const r = await run({ cube: 'orders', measures: ['margin'] }, 'objectql'); - expectCustomSqlRefusal(r, 'margin', 'number'); + const r = await run({ cube: 'orders', measures: ['margin'] }, profile); + expectRetiredTypeRefusal(r, 'margin', 'number'); }); }); @@ -237,41 +244,46 @@ describe('ObjectQL path: every admitted aggregate is still served, carrying its { field: 'buyer', method: 'count_distinct', alias: 'buyers' }, ]); }); - - it('an enum-invalid drift type is NOT refused as the caller\'s mistake', async () => { - // `median` is not authorable (`AggregationMetricType` is closed), so an - // arrival is OUR drift — the undeclared-500 tier, never the caller-shaped - // 400 (#5716). This is the case that reds a "refuse every method that is - // not one of the six aggregates" implementation: extensionally identical - // to the partition check on every enum-valid cube, it re-blames the - // caller exactly here. - const r = await run({ cube: 'orders_drift', measures: ['weird'], dimensions: ['status'] }, 'objectql'); - expect(r.error?.code).not.toBe('INVALID_FIELD'); - }); }); -// ── 3. The other strategy on the SAME fixture: still serves the expression ─── - -describe('native-SQL path: the same custom-SQL measures stay served', () => { - it('emits the number expression verbatim, no refusal', async () => { - const r = await run({ cube: 'orders', measures: ['margin'], dimensions: ['status'] }, 'native'); +describe('native-SQL path: every admitted aggregate is still wrapped', () => { + it('the six aggregates lower to their SQL functions in one statement', async () => { + const r = await run({ + cube: 'orders', + measures: ['orders_count', 'total', 'avg_amount', 'min_amount', 'max_amount', 'buyers'], + dimensions: ['status'], + }, 'native'); expect(r.error).toBeUndefined(); expect(r.sqls).toHaveLength(1); - expect(r.sqls[0]).toContain('SUM(revenue) / NULLIF(SUM(cost), 0) AS "margin"'); - expect(r.calls).toEqual([]); + for (const fragment of ['COUNT(*)', 'SUM(amount)', 'AVG(amount)', 'MIN(amount)', 'MAX(amount)', 'COUNT(DISTINCT buyer)']) { + expect(r.sqls[0]).toContain(fragment); + } }); +}); - it('emits string and boolean expressions verbatim, no refusal', async () => { - const r = await run({ cube: 'orders', measures: ['top_status', 'any_paid'] }, 'native'); - expect(r.error).toBeUndefined(); - expect(r.sqls[0]).toContain(`MAX(CASE WHEN paid THEN 'paid' ELSE 'open' END) AS "top_status"`); - expect(r.sqls[0]).toContain('MAX(paid) AS "any_paid"'); +// ── 3. The drift tier: never the caller's mistake, on either path ─────────── + +describe.each(PROFILES)('%s path: an enum-invalid drift type', (profile) => { + it('is NOT refused as the caller\'s mistake, and never reaches the engine', async () => { + // `median` was never authorable (`AggregationMetricType` is closed), so an + // arrival is OUR drift — the undeclared-500 tier, never the caller-shaped + // 400 (#5716). It is refused in the spec's words — its vocabulary, not a + // retirement — and on the ObjectQL path it is no longer forwarded to the + // engine as a method no driver declares. + const r = await run({ cube: 'orders_drift', measures: ['weird'], dimensions: ['status'] }, profile); + expect(r.error).toBeInstanceOf(Error); + expect(r.error?.code).not.toBe('INVALID_FIELD'); + expect(r.error?.code).toBeUndefined(); + expect(r.error?.message).toContain('cannot be served: its type "median"'); + expect(r.error?.message).not.toMatch(/was removed/); + expect(r.calls).toEqual([]); + expect(r.sqls).toEqual([]); }); }); // ── 4. The twin keeps its exact message ────────────────────────────────────── -describe('the cross-object non-recombinable refusal is untouched beside the new arm', () => { +describe('the cross-object non-recombinable refusal is untouched beside the type verdict', () => { it('still refuses avg + cross-object dimension with its exact shipped message', async () => { const r = await run( { cube: 'orders', dimensions: ['account.region'], measures: ['avg_amount'] }, @@ -289,15 +301,15 @@ describe('the cross-object non-recombinable refusal is untouched beside the new expect(r.calls).toEqual([]); }); - it('a custom-SQL measure beside a cross-object dimension is refused as custom-SQL', async () => { - // Deliberate precedence: the custom-SQL verdict names the real defect (the - // measure can never run on this engine, cross-object dimension or not), - // and both doors reach it through the one resolver — so the attribution - // cannot fork between /analytics/query and /analytics/sql. + it('a retired type beside a cross-object dimension is refused as the type refusal', async () => { + // Deliberate precedence: the type verdict names the real defect (the + // measure can never run, cross-object dimension or not), and both doors + // reach it through the one resolver — so the attribution cannot fork + // between /analytics/query and /analytics/sql. const r = await run( { cube: 'orders', dimensions: ['account.region'], measures: ['margin'] }, 'objectql', ); - expectCustomSqlRefusal(r, 'margin', 'number'); + expectRetiredTypeRefusal(r, 'margin', 'number'); }); }); diff --git a/packages/services/service-analytics/src/__tests__/measure-expression-sql.test.ts b/packages/services/service-analytics/src/__tests__/measure-expression-sql.test.ts index c5430cbe373..ff8c77b32fa 100644 --- a/packages/services/service-analytics/src/__tests__/measure-expression-sql.test.ts +++ b/packages/services/service-analytics/src/__tests__/measure-expression-sql.test.ts @@ -11,18 +11,30 @@ * `AggregationMetricType` — whose expression was discarded; * 3. an unrecognised `type`. * + * #4157 answered (2) by emitting the expression verbatim. A cube member's + * `sql` has since become a column reference and the three types were retired + * from the spec (#21000), so (2) and (3) are now ONE question with one answer: + * a type no aggregate lowers is refused, in the spec's words + * (`aggregateOfMeasure`) — never `COUNT(*)`, and never the column emitted + * unaggregated, which is what a retired type over a column got here. + * * And `qualifyAndRegisterJoin` treated any dot as a relationship hop, so an * expression like `SUM(account.amount)` was split into `"SUM(account"."amount)"` * plus a `LEFT JOIN "SUM(account"` — invalid SQL naming a table that does not * exist. That damage was invisible while the result was thrown away for - * `COUNT(*)`; emitting the expression makes it matter. + * `COUNT(*)`; emitting the expression made it matter, and an aggregate over an + * expression `sql` (a cube registered without the parse) still reaches it. */ import { describe, it, expect } from 'vitest'; -import type { Cube } from '@objectstack/spec/data'; +import { AggregationMetricType, type Cube } from '@objectstack/spec/data'; import type { AnalyticsQuery } from '@objectstack/spec/contracts'; import { NativeSQLStrategy } from '../strategies/native-sql-strategy.js'; -/** A cube whose measures cover both aggregate and custom-expression types. */ +/** + * A cube whose measures cover aggregate types and the three RETIRED + * custom-SQL types, the latter over a column — the shape a cube stored before + * the retirement carries. Unparsed (`as never`): `CubeSchema` refuses them. + */ const cube: Cube = { name: 'orders', title: 'Orders', @@ -30,16 +42,9 @@ const cube: Cube = { measures: { count: { label: 'Count', type: 'count', sql: '*' }, total: { label: 'Total', type: 'sum', sql: 'amount' }, - // The three custom-expression types. `sql` IS the computation. - margin: { - label: 'Margin', type: 'number', - sql: 'SUM(revenue) / NULLIF(SUM(cost), 0)', - }, - top_status: { - label: 'Top status', type: 'string', - sql: "MAX(CASE WHEN paid THEN 'paid' ELSE 'open' END)", - }, - any_paid: { label: 'Any paid', type: 'boolean', sql: 'MAX(paid)' }, + margin: { label: 'Margin', type: 'number', sql: 'revenue' }, + top_status: { label: 'Top status', type: 'string', sql: 'status' }, + any_paid: { label: 'Any paid', type: 'boolean', sql: 'paid' }, }, dimensions: { status: { label: 'Status', type: 'string', sql: 'status' }, @@ -55,26 +60,19 @@ const ctx = { const sqlFor = async (query: AnalyticsQuery) => (await new NativeSQLStrategy().generateSql(query, ctx)).sql; -describe('custom-expression measures emit their expression', () => { - it('emits a number expression verbatim, ungrouped', async () => { - const sql = await sqlFor({ cube: 'orders', measures: ['margin'] }); - expect(sql).toContain('SUM(revenue) / NULLIF(SUM(cost), 0) AS "margin"'); - expect(sql).not.toContain('COUNT(*)'); - }); - - it('emits a number expression verbatim in a grouped query', async () => { - const sql = await sqlFor({ cube: 'orders', measures: ['margin'], dimensions: ['status'] }); - expect(sql).toContain('SUM(revenue) / NULLIF(SUM(cost), 0) AS "margin"'); - expect(sql).toContain('GROUP BY'); - // Measures never join GROUP BY — only dimensions do. The expression must - // therefore be aggregate-shaped, which is the author's contract. - expect(sql.slice(sql.indexOf('GROUP BY'))).not.toContain('NULLIF'); - }); - - it('emits string and boolean expressions verbatim', async () => { - const sql = await sqlFor({ cube: 'orders', measures: ['top_status', 'any_paid'] }); - expect(sql).toContain(`MAX(CASE WHEN paid THEN 'paid' ELSE 'open' END) AS "top_status"`); - expect(sql).toContain('MAX(paid) AS "any_paid"'); +describe('a retired custom-SQL metric type is refused, never emitted', () => { + it.each([ + ['margin', 'number'], + ['top_status', 'string'], + ['any_paid', 'boolean'], + ] as const)('refuses "%s" (type %s), ungrouped and grouped, in the spec\'s words', async (member, type) => { + const prescription = AggregationMetricType.safeParse(type).error!.issues[0]!.message; + for (const query of [{ cube: 'orders', measures: [member] }, { cube: 'orders', measures: [member], dimensions: ['status'] }]) { + const err = await sqlFor(query as AnalyticsQuery).then(() => undefined, (e: Error) => e); + expect(err, JSON.stringify(query)).toBeInstanceOf(Error); + expect(err!.message).toContain(`measure "${member}" on cube "orders" cannot be served`); + expect(err!.message).toContain(prescription); + } }); it('still wraps the aggregate types', async () => { @@ -91,9 +89,11 @@ describe('an expression containing a dot is not mistaken for a join path', () => measures: { ...cube.measures, // A dot inside a function call — an expression, not `relation.column`. + // Aggregated by a live type: the retired custom-SQL types are refused + // before `sql` is lowered, so they can no longer reach this hazard. acct_total: { - label: 'Account total', type: 'number', - sql: 'SUM(account.amount) / 2', + label: 'Account total', type: 'sum', + sql: 'COALESCE(account.amount, 0) / 2', }, // A genuine relationship path, which MUST still be qualified and joined. acct_amount: { label: 'Account amount', type: 'sum', sql: 'account.amount' }, @@ -105,9 +105,9 @@ describe('an expression containing a dot is not mistaken for a join path', () => it('emits the expression intact and registers no phantom join', async () => { const sql = await dottedSql({ cube: 'orders', measures: ['acct_total'] }); - expect(sql).toContain('SUM(account.amount) / 2 AS "acct_total"'); - expect(sql).not.toContain('"SUM(account"'); - expect(sql).not.toContain('LEFT JOIN "SUM(account"'); + expect(sql).toContain('SUM(COALESCE(account.amount, 0) / 2) AS "acct_total"'); + expect(sql).not.toContain('"COALESCE(account"'); + expect(sql).not.toContain('LEFT JOIN "COALESCE(account"'); }); it('still lowers a real relationship path into a qualified column and a join', async () => { @@ -135,10 +135,10 @@ describe('the questions COUNT(*) used to answer now fail loudly', () => { } as never; const badCtx = { ...(ctx as object), getCube: () => bad } as never; await expect(new NativeSQLStrategy().generateSql({ cube: 'orders', measures: ['weird'] }, badCtx)) - .rejects.toThrow(/unrecognised type "median"/); + .rejects.toThrow(/cannot be served: its type "median"/); }); - it('the unrecognised-type error lists both vocabularies', async () => { + it('the unrecognised-type error lists the one vocabulary — the six aggregates, no custom-expression types', async () => { const bad = { ...cube, measures: { weird: { label: 'Weird', type: 'median', sql: 'amount' } }, @@ -147,7 +147,7 @@ describe('the questions COUNT(*) used to answer now fail loudly', () => { const err = await new NativeSQLStrategy() .generateSql({ cube: 'orders', measures: ['weird'] }, badCtx) .catch((e: Error) => e.message); - expect(err).toContain('count_distinct'); - expect(err).toContain('number'); + for (const aggregate of AggregationMetricType.options) expect(err).toContain(aggregate); + expect(err).not.toMatch(/custom-expression|"number"|"boolean"/); }); }); diff --git a/packages/services/service-analytics/src/__tests__/unlisted-refusal-envelope.test.ts b/packages/services/service-analytics/src/__tests__/unlisted-refusal-envelope.test.ts index d7042c5624d..0e2e0ac95db 100644 --- a/packages/services/service-analytics/src/__tests__/unlisted-refusal-envelope.test.ts +++ b/packages/services/service-analytics/src/__tests__/unlisted-refusal-envelope.test.ts @@ -522,9 +522,10 @@ describe('[#5716] the verdicts that deliberately stay an undeclared 500', () => // measurement is three-sided: // // - `Metric.type` is the CLOSED `AggregationMetricType` enum, and - // `metric-type-coverage.test.ts` pins that the strategy's aggregate and - // expression sets PARTITION it — its second case is literally "leaves no - // metric type to the unrecognised-type throw"; + // `metric-type-coverage.test.ts` pins that the strategy's aggregate set + // EQUALS it (the custom-SQL expression set that used to partition it + // with them was retired, #21000) — its second case is literally "leaves + // no metric type to the refusal"; // - `dataset-compiler` writes only a `SUPPORTED_AGGREGATES` member into a // cube (and refuses the other two aggregates with `DATASET_INVALID` // first), so no DATASET can produce one; @@ -548,7 +549,7 @@ describe('[#5716] the verdicts that deliberately stay an undeclared 500', () => ), ); - expect(String(err?.message)).toMatch(/has unrecognised type "median"/); + expect(String(err?.message)).toMatch(/cannot be served: its type "median"/); expect(err?.code).toBeUndefined(); expect(err?.status).toBeUndefined(); }); diff --git a/packages/services/service-analytics/src/analytics-service.ts b/packages/services/service-analytics/src/analytics-service.ts index 627d13ed97b..668e9a17df8 100644 --- a/packages/services/service-analytics/src/analytics-service.ts +++ b/packages/services/service-analytics/src/analytics-service.ts @@ -703,9 +703,10 @@ const AGGREGATION_FUNCTIONS: ReadonlySet = new Set(AggregationFunction.o * measure's aggregate and the declared type of the column it reads, and writes * only what it answers. It answers `undefined` for every pair it has * nothing to say about — the numeric and boolean classes, the count / sum / avg - * rows, an expression metric type — and for every pair the aggregate × - * field-type table refuses, which the cube door has refused before any strategy - * ran ({@link assertCubeMeasureFieldTypesAccepted}). [#21129] The column is + * rows, a type outside the six aggregates (which both strategies refuse) — and + * for every pair the aggregate × field-type table refuses, which the cube door + * has refused before any strategy ran + * ({@link assertCubeMeasureFieldTypesAccepted}). [#21129] The column is * {@link declaredMeasureColumn}'s, the door's own: a relationship-path column * is described by the declaration on the object its last hop reaches, as a * base-object column is by the base object's. diff --git a/packages/services/service-analytics/src/cube-measure-field-type-door.ts b/packages/services/service-analytics/src/cube-measure-field-type-door.ts index e71c540df58..7a9ad61aa5c 100644 --- a/packages/services/service-analytics/src/cube-measure-field-type-door.ts +++ b/packages/services/service-analytics/src/cube-measure-field-type-door.ts @@ -71,8 +71,9 @@ * - A host that wires no `sourceFieldMeta`, or a cube whose `sql` is not a * bare object name (the caller stands down for both). * - A member that resolves to no declared measure (the source-field gate's). - * - A measure type outside the table's vocabulary: the expression metric types - * (`number` / `string` / `boolean`). + * - A measure type outside the table's vocabulary — one the spec does not + * declare, such as the retired custom-SQL types (`number` / `string` / + * `boolean`): both strategies refuse it (`aggregateOfMeasure`). * - A `sql` that is not a column reference (`'*'`, an expression). * - A column the declaration hook cannot resolve — for a relationship path, * one whose hop reaches an object the host does not know (the resolver's diff --git a/packages/services/service-analytics/src/dataset-refusal.ts b/packages/services/service-analytics/src/dataset-refusal.ts index 803e82a6d13..004e04c60d9 100644 --- a/packages/services/service-analytics/src/dataset-refusal.ts +++ b/packages/services/service-analytics/src/dataset-refusal.ts @@ -93,15 +93,19 @@ * arrival there is our bug; an undeclared `500` is the honest answer, and * staying bare keeps it readable in the response (#5667's tiering) instead of * withheld like a declared server fault. [#5716] `native-sql-strategy.ts`'s - * "measure … has unrecognised type" joins this bullet after measurement, and + * "measure … has unrecognised type" joined this bullet after measurement, and * against #5716's own list, which had it down as author-shaped: `Metric.type` * is the CLOSED `AggregationMetricType` enum, `metric-type-coverage.test.ts` * pins that every member of it is handled (its second case is literally "leaves - * no metric type to the unrecognised-type throw"), the dataset compiler maps - * only `SUPPORTED_AGGREGATES` into a cube, and `inferMeasure` mints six known + * no metric type to the refusal"), the dataset compiler maps only + * `SUPPORTED_AGGREGATES` into a cube, and `inferMeasure` mints six known * types. So no spec-valid cube can reach it — an arrival is our own drift or a * host registering an unparsed cube object, which is the same 500 tier as the - * line above, not the author's 400. + * line above, not the author's 400. [#21000] That refusal is now + * `aggregateOfMeasure`'s "measure … cannot be served", the ONE both + * strategies give, worded by the spec's enum — the retired custom-SQL types + * (`number` / `string` / `boolean`) arrive here the same way, from a cube + * that never met the parse, and take the same tier. * - **Producer/consumer drift between two of OUR tables** — the posture * `objectql-strategy.ts`'s display-SQL renderer already states explicitly * ("Deliberately NOT `invalidFilterError`'s 400 envelope: this is drift diff --git a/packages/services/service-analytics/src/metric-type-coverage.test.ts b/packages/services/service-analytics/src/metric-type-coverage.test.ts index 533da175f4a..4ffc56d1302 100644 --- a/packages/services/service-analytics/src/metric-type-coverage.test.ts +++ b/packages/services/service-analytics/src/metric-type-coverage.test.ts @@ -2,49 +2,77 @@ /** * Every `AggregationMetricType` a measure can declare is handled, and handled - * as itself (#4157). + * as itself (#4157) — and a type outside it is refused in the spec's words + * (#21000). * * `resolveMeasureSql` used to answer `COUNT(*)` to three different questions: * an undeclared measure, a custom-SQL-expression metric type, and an * unrecognised type. Each returned a plausible number — aliased under the name * the caller asked for — for a query that asked for something else. * - * The two sets below must partition the spec's vocabulary. Deriving one as "the - * complement of the other" would defeat the point: a *new aggregate* the spec - * grows would be classified as an expression and emitted as a bare column, - * which is a different silent wrong answer. Naming both makes a new member fail - * here instead. + * The custom-SQL-expression types (`number` / `string` / `boolean`) were + * retired from the spec, so the metric vocabulary IS the six aggregates this + * runtime lowers, and the partition that used to split it in two is gone. The + * table must still EQUAL the spec's vocabulary rather than merely cover it: a + * new member the spec grows (`median`, …) fails here, before it can reach + * `aggregateOfMeasure`'s drift sentence. */ import { describe, it, expect } from 'vitest'; import { AggregationMetricType } from '@objectstack/spec/data'; import { SUPPORTED_AGGREGATE_SQL_KEYS, - EXPRESSION_METRIC_TYPES, + aggregateOfMeasure, } from './strategies/native-sql-strategy.js'; describe('AggregationMetricType coverage', () => { - it('is partitioned by the aggregate and expression sets', () => { - const handled = [...SUPPORTED_AGGREGATE_SQL_KEYS, ...EXPRESSION_METRIC_TYPES].sort(); - expect(handled).toEqual([...AggregationMetricType.options].sort()); + it('is exactly the aggregates this runtime lowers', () => { + expect([...SUPPORTED_AGGREGATE_SQL_KEYS].sort()).toEqual([...AggregationMetricType.options].sort()); }); - it('leaves no metric type to the unrecognised-type throw', () => { - const handled = new Set([...SUPPORTED_AGGREGATE_SQL_KEYS, ...EXPRESSION_METRIC_TYPES]); - const unhandled = AggregationMetricType.options.filter((t: string) => !handled.has(t)); - expect( - unhandled, - 'a declared metric type that reaches the throw is a spec member no query can use', - ).toEqual([]); + it('leaves no metric type to the refusal', () => { + for (const type of AggregationMetricType.options) { + expect(aggregateOfMeasure('orders', 'm', type), type).toBe(type); + } }); - it('classifies nothing as both an aggregate and an expression', () => { - const both = SUPPORTED_AGGREGATE_SQL_KEYS.filter((a) => EXPRESSION_METRIC_TYPES.has(a)); - expect(both).toEqual([]); - }); - - it('records the current split, so a vocabulary change shows up in review', () => { + it('records the current vocabulary, so a change shows up in review', () => { expect([...SUPPORTED_AGGREGATE_SQL_KEYS].sort()) .toEqual(['avg', 'count', 'count_distinct', 'max', 'min', 'sum']); - expect([...EXPRESSION_METRIC_TYPES].sort()).toEqual(['boolean', 'number', 'string']); + }); + + it.each(['number', 'string', 'boolean'])( + 'refuses the retired custom-SQL type "%s" with the spec\'s own prescription', + (type) => { + const err = (() => { + try { + aggregateOfMeasure('orders', 'm', type); + } catch (e) { + return e as Error & { code?: string; status?: number }; + } + return undefined; + })(); + const spec = AggregationMetricType.safeParse(type); + expect(spec.success).toBe(false); + expect(err).toBeInstanceOf(Error); + expect(err!.message).toContain(`measure "m" on cube "orders" cannot be served`); + // The words are the spec's, verbatim — no second copy to drift. + expect(err!.message).toContain(spec.error!.issues[0]!.message); + expect(err!.message).toContain(`\`${type}\` was removed from \`AggregationMetricType\``); + // Undeclared-500 tier: no ADR-0112 envelope (`dataset-refusal.ts` header). + expect(err!.code).toBeUndefined(); + expect(err!.status).toBeUndefined(); + }, + ); + + it('refuses a type the spec never declared with the spec\'s vocabulary, not a retirement', () => { + let message = ''; + try { + aggregateOfMeasure('orders', 'm', 'median'); + } catch (e) { + message = (e as Error).message; + } + expect(message).toContain('cannot be served: its type "median"'); + expect(message).not.toMatch(/was removed/); + for (const type of AggregationMetricType.options) expect(message).toContain(type); }); }); diff --git a/packages/services/service-analytics/src/plugin.ts b/packages/services/service-analytics/src/plugin.ts index 07cededfbf2..6ca3353bac7 100644 --- a/packages/services/service-analytics/src/plugin.ts +++ b/packages/services/service-analytics/src/plugin.ts @@ -109,11 +109,14 @@ type DialectNamingDriver = { readonly dialectName?: unknown }; * parse is DEFENCE IN DEPTH behind a compile-time check, not the only check * (#11833). * - * That is a reason to keep it, not to delete it. Types are erased: a - * JavaScript app supplying its own `executeAggregate`, or host drift arriving - * through a cube object that never met `CubeSchema`'s parse (the path - * `aggregate-bridge-function-vocabulary.test.ts` drives end to end), still - * reaches this seam carrying a method the engine does not declare. What the + * That is a reason to keep it, not to delete it. Types are erased, so the + * compile-time check proves nothing about the value a caller hands this seam + * at run time. A cube object that never met `CubeSchema`'s parse no longer + * reaches it with a non-aggregate type — since #21000 the strategy's resolver + * refuses that one step earlier (`aggregateOfMeasure`), in the same tier — so + * what this guards is the bridge itself: any method that arrives here, from + * whatever produced it, is still parsed before the engine sees it + * (`aggregate-bridge-function-vocabulary.test.ts` drives both). What the * refusal buys is in `plugin.ts`'s forward below and in commit 017130a09: the engine is * never handed a `function` no driver declares. * @@ -130,10 +133,10 @@ function parseEngineAggregateFunction( throw new Error( `[Analytics] The aggregate bridge cannot forward the aggregation ` + `"${alias}": "${method}" is not one of the engine's aggregate functions ` + - `(${AggregationFunction.options.join(', ')}). A custom-SQL measure is ` + - `refused earlier, with a caller-facing diagnostic, by ObjectQLStrategy; ` + - `reaching this point means the analytics layer produced a method the ` + - `engine contract does not declare.`, + `(${AggregationFunction.options.join(', ')}). A cube measure whose type ` + + `names no aggregate is refused earlier, by ObjectQLStrategy; reaching ` + + `this point means the analytics layer produced a method the engine ` + + `contract does not declare.`, ); } return parsed.data; @@ -386,15 +389,19 @@ export class AnalyticsServicePlugin implements Plugin { // so there is one vocabulary, and its own error map already // carries the `array_agg`/`string_agg` retirement prescriptions. // - // TIERING, deliberately: the reachable producer of a non-aggregate - // method — a custom-SQL measure (`AggregationMetricType` - // `number`/`string`/`boolean`) — is already refused upstream with a - // caller-blaming 400 by `ObjectQLStrategy.resolveMeasureAggregation` - // (commit 017130a09). Anything still arriving here is host drift, which that - // refusal's docblock assigns to the undeclared-500 tier — so this - // throws rather than re-blaming the caller, and it answers loudly - // instead of letting the engine answer `null` per bucket under the - // author's own measure name (the #4157 class). + // TIERING, deliberately: the producer of a non-aggregate method — + // a cube measure whose `type` names no aggregate: a custom-SQL + // type (`number`/`string`/`boolean`, retired from + // `AggregationMetricType`, #21000) or one the spec never declared + // — is already refused upstream by + // `ObjectQLStrategy.resolveMeasureAggregation`, in the + // undeclared-500 tier with the spec's own words + // (`aggregateOfMeasure`). So no cube measure reaches this point + // with one; what still could is a method the analytics layer + // itself produced, our own drift — so this throws in the same tier + // rather than blaming the caller, and it answers loudly instead of + // letting the engine answer `null` per bucket under the author's + // own measure name (the #4157 class). function: parseEngineAggregateFunction(a.method, a.alias), field: a.field, alias: a.alias, diff --git a/packages/services/service-analytics/src/preview-evaluator.ts b/packages/services/service-analytics/src/preview-evaluator.ts index 5eafd898fef..228976a23bd 100644 --- a/packages/services/service-analytics/src/preview-evaluator.ts +++ b/packages/services/service-analytics/src/preview-evaluator.ts @@ -467,7 +467,11 @@ function extremumOf(rows: Row[], field: string, kind: 'min' | 'max'): unknown { * | `avg` | the mean of the NON-NULL operands that read as numbers, | * | | and `null` when NO ROW CARRIED A VALUE (#16219) | * | `min` / `max` | the winning operand, IN ITS OWN TYPE ({@link extremumOf}) | - * | `number` / `string` / `boolean` | a custom-SQL metric the dataset path never mints — left on the historical numeric `default` | + * + * Those six are the whole vocabulary: its custom-SQL members (`number` / + * `string` / `boolean`) were retired from the spec (#21000), and the dataset + * path never minted them anyway. A type outside the six cannot come from a + * dataset measure; it is left on the historical numeric `default`. * * ⭐ `min`/`max` are why this function stopped returning `number`. Coercing * every operand with `Number()` and dropping the non-finite ones made a @@ -544,8 +548,9 @@ function aggregate(rows: Row[], metricType: string, field: string): unknown { // ⛔ Scoped to this arm rather than folded into `nums`: `sum` is immune to // the coercion (`0` is the additive identity, so both spellings answer the // same number) and the numeric `default` below is the historical answer for - // the custom-SQL metric types, which has no live standard to be moved - // towards. Widening either would be an unrequested value change. + // a type outside the six — the retired custom-SQL metric types once, a + // value no dataset measure can carry now — which has no live standard to + // be moved towards. Widening either would be an unrequested value change. // // ⛔ And the empty answer is not a hard-coded `null`: what an aggregate // answers over an empty operand set is the platform's ruling and lives in 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 d1ae690aef0..2f13576b9ba 100644 --- a/packages/services/service-analytics/src/strategies/native-sql-strategy.ts +++ b/packages/services/service-analytics/src/strategies/native-sql-strategy.ts @@ -2,7 +2,7 @@ import type { AnalyticsQuery, AnalyticsResult } from '@objectstack/spec/contracts'; import type { Cube } from '@objectstack/spec/data'; -import { NUMERIC_VALUE_TYPES, type AggregationFunction } from '@objectstack/spec/data'; +import { AggregationMetricType, NUMERIC_VALUE_TYPES, type AggregationFunction } from '@objectstack/spec/data'; import type { AnalyticsStrategy, StrategyContext, DatasetScopedStrategyContext } from './types.js'; import { declaredDatetimeLowering, @@ -49,9 +49,10 @@ import { explicitDateRangeWindow } from '../date-range-array-arm.js'; * `default: COUNT(*)`, so an aggregate the spec grew would have returned a row * count instead of the number the author asked for, silently. objectui#2945. * - * Non-aggregate metric types (`number`/`string`/`boolean`) are deliberately - * absent — they are handled by {@link EXPRESSION_METRIC_TYPES}, which emits the - * author's expression rather than wrapping it. + * [#21000] These six are also the whole cube metric vocabulary: the + * custom-SQL-expression metric types (`number`/`string`/`boolean`) were retired + * from `AggregationMetricType`, so a measure type this table does not key is + * one the spec does not declare, refused by {@link aggregateOfMeasure}. */ const AGGREGATE_SQL: Record string> = { // [#10298] `count` takes its COLUMN when the measure declares one. The @@ -109,27 +110,57 @@ export const SUPPORTED_AGGREGATE_SQL_KEYS = Object.keys(AGGREGATE_SQL); export const CONDITIONAL_AGGREGATE_SQL_KEYS = Object.keys(CONDITIONAL_AGGREGATE_SQL); /** - * Metric types that are a custom SQL *expression*, not an aggregate to wrap. + * [#21000] The ONE verdict both strategies give a cube measure's `type`: the + * aggregate it names, or a refusal. * - * `AggregationMetricType` (`data/analytics.zod.ts`) documents these three as - * "Custom SQL expression returning a number / string / boolean" — the measure's - * `sql` IS the whole computation (a ratio, a `CASE`, a window function), so the - * only correct emission is the expression itself. They used to fall through to - * `resolveMeasureSql`'s `COUNT(*)` fallback, which threw the expression away and - * returned a row count. #4157. + * The cube metric vocabulary IS the six aggregates {@link AGGREGATE_SQL} + * lowers. It used to carry three more — `number` / `string` / `boolean`, "a + * custom SQL expression returning …", which this strategy emitted verbatim and + * `ObjectQLStrategy` refused, partitioned by a shared `EXPRESSION_METRIC_TYPES` + * set. A cube member's `sql` became a column reference, so the three had + * nothing left to compute (this strategy emitted the column UNAGGREGATED in a + * grouped statement), and they were retired from `AggregationMetricType` with + * a prescription. The partition went with them. * - * Named rather than derived as "everything that is not an aggregate": deriving it - * would silently classify a *new* aggregate the spec grows (`median`, …) as an - * expression and emit a bare column. `metric-type-coverage.test.ts` asserts these - * two sets partition `AggregationMetricType`, so a new member fails a test - * instead of picking a default. + * So a type outside the table is one the spec does not declare, and only a + * cube that never met `CubeSchema`'s parse can carry one: every door that + * parses a cube — `defineStack`, the artifact boot, the `analytics_cube` write + * door — refuses it first. What still arrives here is a cube a host registered + * in-process from a literal (`CubeRegistry.register` never parses), one stored + * under the retired vocabulary included. It is REFUSED, never stood down: + * served, a retired type answered one row's value per group on this path, and + * the engine path would hand the engine a method no driver declares. * - * [commit 017130a09] `ObjectQLStrategy.resolveMeasureAggregation` keys its refusal arm on - * this same set — the engine aggregate AST cannot carry a raw SQL expression, - * so the ObjectQL path REFUSES exactly what this strategy emits verbatim. One - * set, two strategies, so the partition cannot fork per path. + * The words are the SPEC's, read off the enum itself — no local list of metric + * types, retired or otherwise, to drift. For a retired member the enum's error + * map answers the retirement prescription (the aggregate to write instead); + * for a value it never declared, zod's own message listing the six. An + * operator reads the sentence `os validate` would have printed for the cube. + * + * Bare `Error` — the undeclared-500 tier, unchanged: no spec-valid cube can + * reach it, and `dataset-refusal.ts`'s header assigns a cube registered + * without the parse to that tier, never to a 400 that would tell a dashboard + * user to fix metadata they cannot see. The message is self-authored, so the + * analytics doors still relay it readable. + * + * Keyed on what this runtime can LOWER, with the spec supplying the verdict: + * `metric-type-coverage.test.ts` pins the table's keys equal to the enum's + * options, so a member the spec grows fails a test before it reaches the + * drift sentence below. */ -export const EXPRESSION_METRIC_TYPES = new Set(['number', 'string', 'boolean']); +export function aggregateOfMeasure(cube: string, member: string, type: unknown): AggregationFunction { + if (typeof type === 'string' && Object.prototype.hasOwnProperty.call(AGGREGATE_SQL, type)) { + return type as AggregationFunction; + } + const verdict = AggregationMetricType.safeParse(type); + const why = verdict.success + ? `@objectstack/spec declares it, but no aggregate here lowers it (${SUPPORTED_AGGREGATE_SQL_KEYS.join(', ')}) — the two vocabularies have drifted.` + : (verdict.error.issues[0]?.message ?? 'It is not a declared metric type.'); + throw new Error( + `[Analytics] measure "${member}" on cube "${cube}" cannot be served: its type ` + + `${JSON.stringify(type)} is not one of the aggregates a cube measure declares. ${why}`, + ); +} /** * [#21365] The `LIMIT` an offset-only window carries, per dialect — `null` @@ -270,8 +301,9 @@ interface StatementClauses { * `resolveMeasureSql` used to answer `COUNT(*)` to three different questions it * could not otherwise answer — an undeclared measure, a custom-SQL-expression * metric type, and an unrecognised type. All three returned a plausible number - * for a query that asked for something else. They now emit the expression or - * throw; see that method. #4157. + * for a query that asked for something else. They now throw; see that method + * (#4157). The expression metric types, once emitted verbatim here, were + * retired from the spec (#21000) and are refused with the rest. */ export class NativeSQLStrategy implements AnalyticsStrategy { readonly name = 'NativeSQLStrategy'; @@ -777,8 +809,7 @@ export class NativeSQLStrategy implements AnalyticsStrategy { // `foldEmptyAggregateAnswers`, the rows path); this face answered `null` for // the same group. Read from the policy, never restated, for EVERY measure — // a measure-scoped one carries its aggregate in the same `type` — so - // `avg` / `min` / `max` (no identity) and the expression metric types - // (`undefined` too) keep their NULL. Only `null` folds, before the + // `avg` / `min` / `max` (no identity, `undefined`) keep their NULL. Only `null` folds, before the // presenter, in `driver-sql`'s order: an `undefined` would be a column // never projected, a different defect that must stay visible. The dataset // door's `DatasetExecutor` fill still runs after this and is idempotent on @@ -816,9 +847,8 @@ export class NativeSQLStrategy implements AnalyticsStrategy { // `declaredFieldType` on the object that declares the column // ({@link measureColumnOf}) — [#21129] for a relationship path, the object // its last hop reaches, as the statement joined it. A host that cannot - // answer leaves the value as the client gave it. Expression metric types - // (`number` / `string` / `boolean`) are the author's SQL and stay as they - // are. Rows are presented in place, as the driver presents its own. + // answer leaves the value as the client gave it. Rows are presented in + // place, as the driver presents its own. const declaredType = (ctx as DatasetScopedStrategyContext).declaredFieldType; const referenceOf = relationshipReferenceOf(ctx); const numberMeasures = (query.measures ?? []).filter((member) => { @@ -1371,6 +1401,14 @@ export class NativeSQLStrategy implements AnalyticsStrategy { ); } + // [#21000] The aggregate the measure names — or the one refusal both + // strategies give a type no aggregate lowers ({@link aggregateOfMeasure}): + // a retired custom-SQL-expression type (`number` / `string` / `boolean`), + // whose column this path used to emit unaggregated, or a type the spec + // never declared. Asked before anything is lowered, so nothing else the + // statement carries can route around it. + const aggregate = aggregateOfMeasure(cube.name, member, measure.type); + const column = measure.sql === '*' ? '*' : this.qualifyAndRegisterJoin(measure.sql, parentTable, joins, cube); @@ -1388,13 +1426,12 @@ export class NativeSQLStrategy implements AnalyticsStrategy { // relationship path's last hop reads ({@link measureColumnOf}, through the // one hop resolver). An expression, a column the host cannot describe, or a host // that names no dialect gets no class or no policy, and is aggregated as - // stored. The expression metric types are not aggregates and are never - // wrapped. + // stored. const target = measureColumnOf(cube, parentTable, measure.sql, joins.referenceOf); - const col = column === '*' || !Object.prototype.hasOwnProperty.call(AGGREGATE_ANSWER_KIND, measure.type) + const col = column === '*' || !Object.prototype.hasOwnProperty.call(AGGREGATE_ANSWER_KIND, aggregate) ? column : aggregandOperandSql( - measure.type as AggregationFunction, + aggregate, target ? aggregandColumnClass(declaredValueShapeResolver(ctx, target.object)?.(target.field)) : undefined, @@ -1403,51 +1440,27 @@ export class NativeSQLStrategy implements AnalyticsStrategy { ); if (predicate !== null) { - const wrapConditional = CONDITIONAL_AGGREGATE_SQL[measure.type]; + const wrapConditional = CONDITIONAL_AGGREGATE_SQL[aggregate]; if (wrapConditional) return wrapConditional(col, predicate); - // [#10298] Deliberately BARE — an undeclared 500, same tier and same - // reasoning as the "unrecognised type" throw below. A measure filter only - // ever arrives here from a COMPILED DATASET, and `DatasetMeasure.aggregate` - // is `AggregationFunction`, whose every member is a key of the table - // above — so an expression metric type (`number`/`string`/`boolean`, - // where `sql` IS the whole computation and there is no aggregate to make - // conditional) cannot carry one. What would reach here is our own drift. - // Emitting the unfiltered aggregate instead is precisely the defect this - // card closes: a 200 carrying different arithmetic than the author declared. + // [#10298] Deliberately BARE — an undeclared 500, the tier + // {@link aggregateOfMeasure} answers in. The conditional table is keyed + // identically to {@link AGGREGATE_SQL} (`aggregation-lockstep.test.ts`), + // and the aggregate above was admitted from that table, so what would + // reach here is our own drift between the two. Emitting the unfiltered + // aggregate instead is precisely the defect this card closes: a 200 + // carrying different arithmetic than the author declared. throw new Error( `[native-sql-strategy] measure "${member}" on cube "${cube.name}" carries a ` + - `scoped filter, but its type "${measure.type}" has no conditional form ` + + `scoped filter, but its type "${aggregate}" has no conditional form ` + `(conditional: ${CONDITIONAL_AGGREGATE_SQL_KEYS.join(', ')}).`, ); } - const wrap = AGGREGATE_SQL[measure.type]; - if (wrap) return wrap(col); - // A custom SQL expression: the measure's `sql` IS the computation, so emit - // it unwrapped. In a grouped query the expression must itself be - // aggregate-shaped — measures never join `GROUP BY` (only dimensions do), so - // a scalar expression there is invalid SQL. That is the author's contract to - // keep; silently substituting `COUNT(*)` did not keep it for them. - if (EXPRESSION_METRIC_TYPES.has(measure.type)) return col; - - // [#5716] Deliberately BARE — an undeclared 500, and the one site on that - // issue's list of nine that is NOT the author's mistake. `Metric.type` is the - // CLOSED `AggregationMetricType` enum; `metric-type-coverage.test.ts` pins - // that {@link AGGREGATE_SQL} ∪ {@link EXPRESSION_METRIC_TYPES} partitions it - // exactly, `dataset-compiler` only ever writes a `SUPPORTED_AGGREGATES` - // member into a cube, and `inferMeasure` mints six known types. So no - // spec-valid cube can arrive here: what does is our own drift or a host - // registering a cube object that never met `CubeSchema`. Answering the - // CALLER 400 for that would hide a platform bug from ops alerting and tell a - // dashboard user to fix metadata they cannot see. Same tier as - // `dataset-compiler`'s "non-derived measure has no aggregate"; the reasoning - // is written once in `dataset-refusal.ts`'s header. - throw new Error( - `[native-sql-strategy] measure "${member}" on cube "${cube.name}" has ` + - `unrecognised type "${measure.type}" — expected an aggregate ` + - `(${SUPPORTED_AGGREGATE_SQL_KEYS.join(', ')}) or a custom-expression type ` + - `(${[...EXPRESSION_METRIC_TYPES].join(', ')}).`, - ); + // [#5716 → #21000] Never `COUNT(*)` for a type this table does not key — + // {@link aggregateOfMeasure} admitted `aggregate` FROM this table, and + // refused (bare, undeclared 500, the spec's own words) every type it does + // not key, before anything was lowered. + return AGGREGATE_SQL[aggregate](col); } private resolveFieldSql( diff --git a/packages/services/service-analytics/src/strategies/objectql-strategy.ts b/packages/services/service-analytics/src/strategies/objectql-strategy.ts index 550f8c417f0..ef92b06c63f 100644 --- a/packages/services/service-analytics/src/strategies/objectql-strategy.ts +++ b/packages/services/service-analytics/src/strategies/objectql-strategy.ts @@ -45,10 +45,11 @@ import { type MeasureRecombine, type RecombinableMethod, } from './cross-object-rebucket.js'; -// [commit 017130a09] The custom-SQL half of the `AggregationMetricType` partition, ONE -// source shared with `NativeSQLStrategy` and pinned against the spec enum by -// `metric-type-coverage.test.ts` — a second literal set here would drift. -import { EXPRESSION_METRIC_TYPES } from './native-sql-strategy.js'; +// [#21000] The ONE verdict on a cube measure's `type`, shared with +// `NativeSQLStrategy` so the two paths accept and refuse the same set — it +// replaces the custom-SQL partition both used to key on, retired from the +// spec with the three types it named. +import { aggregateOfMeasure } from './native-sql-strategy.js'; /** * [#10861 / commit 399ecad58] Where a member in the cross-object envelope's inventory @@ -1540,55 +1541,32 @@ export class ObjectQLStrategy implements AnalyticsStrategy { | { sql: string; type: string } | undefined; if (direct) { - // [commit 017130a09] A custom-SQL measure (`AggregationMetricType` - // `number`/`string`/`boolean`) is REFUSED here rather than forwarded. Its - // `sql` IS the whole computation (a ratio, a `CASE`, a window function), - // and the engine aggregate AST has no place to carry a raw SQL - // expression: forwarding put the whole expression in `field` and the - // metric TYPE in `method`, so `driver-sql` threw `INVALID_QUERY`/400 - // blaming a `function` key the author never wrote, and the in-memory - // evaluator answered `null` for every bucket through its `switch` - // default — a silent wrong answer under the author's own metric name, - // the #4157 class in its null variant. #4157's fix landed on - // `NativeSQLStrategy` only (where the expression is legal and emitted - // verbatim, `EXPRESSION_METRIC_TYPES`); this arm is the matching - // partition on the strategy that cannot serve it. + // [#21000] The measure's aggregate, or the one refusal both strategies + // give a type no aggregate lowers ({@link aggregateOfMeasure}). + // + // This arm used to refuse exactly the custom-SQL partition — + // `AggregationMetricType`'s `number` / `string` / `boolean` (commit + // 017130a09), `INVALID_FIELD` / 400, because the engine aggregate AST + // cannot carry a raw SQL expression — and let every other type through + // unchecked ON PURPOSE: an enum-INVALID type (`median`, host drift) is + // OUR bug, the undeclared-500 tier, and a method allowlist answering 400 + // would have re-blamed the caller for it. The three were retired from + // the spec (a member's `sql` became a column reference, so they had + // nothing left to compute), and with them gone the partition had nothing + // to name. What replaces it keeps BOTH halves of that reasoning: the + // check is an allowlist of the aggregates, and its refusal is the + // undeclared-500 tier with the spec's own words — so a retired type is + // refused with its prescription and a never-declared one with zod's + // vocabulary, neither re-blamed on the caller, and neither handed to the + // engine as a method no driver declares (forwarded, `median` reached the + // host's `executeAggregate` and the SQL echo printed `MEDIAN(amount)`). // - // Same posture and same envelope as `planCrossObject`'s refusals below - // (`INVALID_FIELD` / 400, #5716; the non-recombinable-measure arm is the - // wording twin): the engine physically cannot evaluate this member, and - // a loud, correctly-attributed refusal beats a silent wrong number. // Sitting HERE — the one resolver both doors call — keeps // `/analytics/query` and `/analytics/sql` accepting/rejecting the same // set by construction (#10759's invariant). - // - // Keyed on the DECLARED metric-type partition, deliberately NOT on - // "method is not one of the six aggregates": the two read identically on - // every enum-valid cube, but an enum-INVALID type (host drift, e.g. a - // cube registered without meeting `CubeSchema`) is OUR bug — the - // undeclared-500 tier `dataset-refusal.ts`'s header assigns it — and a - // method allowlist would re-blame the caller for it with a 400. - if (EXPRESSION_METRIC_TYPES.has(direct.type)) { - throw invalidMemberError( - `[Analytics] ObjectQLStrategy cannot evaluate the custom-SQL measure ` + - `("${measureName}") — its type "${direct.type}" declares a raw SQL ` + - `expression, which the engine aggregate AST cannot carry; served ` + - `anyway it would answer null for every bucket under the measure's ` + - `own name. Use an aggregate measure ` + - `(count/sum/avg/min/max/count_distinct), or run on a native-SQL ` + - `driver.`, - { member: measureName, param: 'measures', cube: cube.name }, - ); - } return { field: direct.sql.replace(/^\$/, ''), - // The assertion, not a parse: for a CubeSchema-legal cube the type - // partition above leaves exactly the six `AggregationFunction` values. - // An enum-INVALID type (host drift, the comment above) still flows - // through unchecked ON PURPOSE — adding a method allowlist here would - // re-blame the caller with a 400 for OUR bug, so the cast keeps the - // compile-time contract (#12776) without changing that posture. - method: (direct.type === 'count_distinct' ? 'count_distinct' : direct.type) as AggregationFunction, + method: aggregateOfMeasure(cube.name, measureName, direct.type), }; } // Accept `${field}_${type}` aliases (e.g. 'amount_sum') for measures whose diff --git a/packages/spec/liveness/analytics_cube.json b/packages/spec/liveness/analytics_cube.json index 10a2c0e7fc5..9e3cf3f84c6 100644 --- a/packages/spec/liveness/analytics_cube.json +++ b/packages/spec/liveness/analytics_cube.json @@ -53,10 +53,10 @@ }, "type": { "status": "live", - "verifiedAt": "2026-09-17", - "evidence": "packages/services/service-analytics/src/strategies/native-sql-strategy.ts#resolveMeasureSql — `const wrap = AGGREGATE_SQL[measure.type]` picks the aggregate the column is wrapped in (SUM / COUNT / AVG / …), `CONDITIONAL_AGGREGATE_SQL[measure.type]` picks its conditional form for a scoped measure filter, and `EXPRESSION_METRIC_TYPES.has(measure.type)` is what emits a custom expression UNWRAPPED; an unrecognised type throws rather than substituting `COUNT(*)` (#4157). packages/services/service-analytics/src/analytics-service.ts#getMeta also publishes it as the measure's `type` on `GET /api/v1/analytics/meta`.", + "verifiedAt": "2026-10-02", + "evidence": "packages/services/service-analytics/src/strategies/native-sql-strategy.ts#resolveMeasureSql — `aggregateOfMeasure(cube.name, member, measure.type)` resolves the aggregate, `AGGREGATE_SQL[aggregate]` picks the function the column is wrapped in (SUM / COUNT / AVG / …) and `CONDITIONAL_AGGREGATE_SQL[aggregate]` its conditional form for a scoped measure filter; packages/services/service-analytics/src/strategies/objectql-strategy.ts#resolveMeasureAggregation hands the same verdict to the engine as the aggregation `method`. A type no aggregate lowers is refused in the spec's words rather than substituting `COUNT(*)` (#4157). packages/services/service-analytics/src/analytics-service.ts#getMeta also publishes it as the measure's `type` on `GET /api/v1/analytics/meta`.", "producer": "packages/cli/src/commands/serve.ts#CAPABILITY_PROVIDERS — the `analytics` entry declares `configKey: 'analyticsCubes'` and the capability resolver threads it into the plugin (`const cubes = (config as any).analyticsCubes ?? (config as any).cubes ?? []; arg = { cubes }`); packages/services/service-analytics/src/analytics-service.ts#registerAll (`if (config.cubes) this.cubeRegistry.registerAll(config.cubes)`) is where the authored array becomes the registry every consumer below resolves through. Without this thread an authored cube reaches no reader at all — the `seed.env` shape (#4837).", - "note": "The arithmetic itself: this key decides what number the query returns. `metric-type-coverage.test.ts` pins `AGGREGATE_SQL` ∪ `EXPRESSION_METRIC_TYPES` as an exact partition of the `AggregationMetricType` enum, so no spec-valid value can fall through." + "note": "The arithmetic itself: this key decides what number the query returns. NARROWED 2026-10-02 (#21000, ADR-0049 enforce-or-remove): the custom-SQL-expression values `number` / `string` / `boolean` were retired from `AggregationMetricType` — a member's `sql` is a column reference, so they had nothing left to compute — and are refused at parse with a prescription naming the six aggregates (`enumWithRetiredValues`); the strategies' `EXPRESSION_METRIC_TYPES` partition went with them, and both strategies refuse a cube that reached them unparsed with the same text (`aggregateOfMeasure`). Still LIVE: the key itself is unchanged and read at the sites above. `metric-type-coverage.test.ts` pins `AGGREGATE_SQL`'s keys EQUAL to the enum's options, so no spec-valid value can fall through. The D3 entry is `cube-metric-expression-types-retired`; there is no D2 conversion, because the column alone does not say which aggregate the author meant." }, "sql": { "status": "live", From 6d3659a29fe81f8a64091f7d83d82425d561de20 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 16:29:15 +0000 Subject: [PATCH 3/7] fix(spec): keep the analytics module header selected; cast the retired-type fixture (WIP) Claude-Session: https://claude.ai/code/session_01UtnxvdiN376GF3sgXwAw4d Co-authored-by: Claude --- .../src/__tests__/cube-authored-format-granularity.test.ts | 5 +++-- packages/spec/src/data/analytics.zod.ts | 4 +++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/services/service-analytics/src/__tests__/cube-authored-format-granularity.test.ts b/packages/services/service-analytics/src/__tests__/cube-authored-format-granularity.test.ts index c459ad4fe01..5b07696f1ff 100644 --- a/packages/services/service-analytics/src/__tests__/cube-authored-format-granularity.test.ts +++ b/packages/services/service-analytics/src/__tests__/cube-authored-format-granularity.test.ts @@ -282,13 +282,14 @@ describe('analytics_cube.dimensions.granularities — the declared single granul // directly on the parsed cube. Before #21000 the engine path refused it and // the raw-SQL path served it, so the route a declared default chose decided // the answer; now both refuse it by type, before anything executes. - const withExpression: Cube = { + // `as unknown as Cube`: `tsc` refuses the retired type at a typed cube too. + const withExpression = { ...authored, measures: { ...authored.measures, done_rate: { label: 'Done', type: 'number', sql: "SUM(CASE WHEN status = 'done' THEN 1 ELSE 0 END) * 1.0 / COUNT(*)" }, }, - }; + } as unknown as Cube; expect(CubeSchema.safeParse(withExpression).success).toBe(false); const aggregated: string[] = []; const sqls: string[] = []; diff --git a/packages/spec/src/data/analytics.zod.ts b/packages/spec/src/data/analytics.zod.ts index fb19a850767..ed736cd29b3 100644 --- a/packages/spec/src/data/analytics.zod.ts +++ b/packages/spec/src/data/analytics.zod.ts @@ -6,8 +6,10 @@ import { DATE_RANGE_PRESETS } from './date-range-presets'; import { DateGranularity } from './query.zod'; /** + * @module data/analytics + * * Analytics/Semantic Layer Protocol - * + * * Defines the "Business Logic" for data analysis. * Inspired by Cube.dev, LookML, and dbt MetricFlow. * From 9e40762aa4b021dcd85aa77ef2c3fafa172d670f Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 16:32:50 +0000 Subject: [PATCH 4/7] docs(spec): changesets, regenerated analytics reference, measured-vs-semantics wording (WIP) Claude-Session: https://claude.ai/code/session_01UtnxvdiN376GF3sgXwAw4d Co-authored-by: Claude --- .../21000-analytics-metric-type-verdict.md | 45 ++++++++++++ ...00-cube-metric-expression-types-retired.md | 69 +++++++++++++++++++ content/docs/references/data/analytics.mdx | 9 +-- packages/spec/src/data/analytics.zod.ts | 5 +- ...18.cube-metric-expression-types-retired.ts | 6 +- packages/spec/src/migrations/registry.ts | 6 +- 6 files changed, 126 insertions(+), 14 deletions(-) create mode 100644 .changeset/21000-analytics-metric-type-verdict.md create mode 100644 .changeset/21000-cube-metric-expression-types-retired.md diff --git a/.changeset/21000-analytics-metric-type-verdict.md b/.changeset/21000-analytics-metric-type-verdict.md new file mode 100644 index 00000000000..a1bf3e8de23 --- /dev/null +++ b/.changeset/21000-analytics-metric-type-verdict.md @@ -0,0 +1,45 @@ +--- +'@objectstack/service-analytics': minor +--- + +fix(service-analytics)!: both analytics strategies refuse a cube measure whose `type` names no aggregate, in the spec's words — the custom-SQL `EXPRESSION_METRIC_TYPES` partition is gone with the three types it named (#21000) + +**BREAKING** — `@objectstack/spec` retired the cube metric types `number`, `string` +and `boolean` from `AggregationMetricType` (a measure's `sql` is a column reference, +so they had nothing left to compute). Every door that parses a cube refuses them; +this release removes the runtime branches that still served them for a cube that +reached the analytics service WITHOUT meeting that parse — one a host registers +in-process from a literal, through `AnalyticsServicePlugin({ cubes })` or +`AnalyticsService({ cubes })` (the registry never parses). + +| | before | now | +| --- | --- | --- | +| `NativeSQLStrategy`, a measure typed `number` / `string` / `boolean` | served: the column emitted UNAGGREGATED in the statement (`amount AS "m"` beside `GROUP BY`) | refused, nothing executed | +| `ObjectQLStrategy`, the same measure | refused `INVALID_FIELD` / 400 | refused, nothing executed | +| either strategy, a type the spec never declared (`median`) | native: refused; ObjectQL: forwarded to `executeAggregate` as the method (the auto-bridge refused it; a host's own executor received it), and `/analytics/sql` echoed `MEDIAN(amount)` | refused, nothing executed | + +**The one refusal** is `aggregateOfMeasure`'s, shared by both strategies and both +doors (`POST /analytics/query` and `POST /analytics/sql`): it names the measure and +the cube, then quotes the spec's own verdict on the type — for a retired type the +retirement prescription (the six aggregates to choose from, and where a per-row or +derived value goes instead), for anything else zod's message listing the six. It is +a bare `Error`, the undeclared-500 tier this package assigns to a cube that never +met the parse, so the HTTP answer is `500` with the message readable in the body +(measured through the dispatcher's analytics route), never a caller-blaming `400`. +The ObjectQL envelope for the three retired types therefore moves from +`INVALID_FIELD` / 400 to that tier. + +**The fix:** give the measure one of the six aggregate types — `count`, `sum`, +`avg`, `min`, `max`, `count_distinct` — or parse the cube through `CubeSchema` +before registering it, which refuses the same types with the same prescription. + +**Removed export:** `EXPRESSION_METRIC_TYPES` from +`strategies/native-sql-strategy.ts` (internal to the package; not re-exported from +its entry point). **Unchanged:** every aggregate measure on both strategies, the +auto-bridge's own parse of an engine method (still pinned, driven directly), and +`GET /analytics/meta`, which keeps publishing each registered measure's `type` as +registered. + +Clause-②: no (narrowing) + + diff --git a/.changeset/21000-cube-metric-expression-types-retired.md b/.changeset/21000-cube-metric-expression-types-retired.md new file mode 100644 index 00000000000..ab71e17b317 --- /dev/null +++ b/.changeset/21000-cube-metric-expression-types-retired.md @@ -0,0 +1,69 @@ +--- +'@objectstack/spec': minor +--- + +feat(spec)!: retire the cube metric types `number`, `string` and `boolean` — a measure's `sql` is a column reference, so the custom-SQL-expression types had nothing left to compute (#21000) + +**BREAKING** — three members leave `AggregationMetricType`, so a cube measure's +`measures..type` no longer accepts `number`, `string` or `boolean`. ADR-0049 +enforce-or-remove. They declared "a custom SQL expression returning a number / +string / boolean": the measure's `sql` was the whole computation. A cube member's +`sql` is a column reference since `cube-member-sql-expression-retired` (#20943), so +the three were left naming nothing: measured before this change, the raw-SQL +analytics path returned the referenced column UNAGGREGATED (a bare column in a +grouped statement — by SQL's own rules an error on PostgreSQL and an arbitrary row's +value on SQLite), and the ObjectQL path refused the measure. The six aggregates — `count`, `sum`, +`avg`, `min`, `max`, `count_distinct` — are unchanged and are now the whole +vocabulary. + +### FROM → TO + +| removed | what to write instead | +| --- | --- | +| `measures..type: 'number'`, `'string'` or `'boolean'` | the aggregate the measure means: `sum`, `avg`, `min` or `max` over the column; `count` (over `'*'` for a row count, or over a column for its non-null values); or `count_distinct`. | +| a measure whose old expression computed a value per row | keep that value as a field of the object (a stored or formula field) and aggregate the field. | +| a measure whose old expression combined measures (a ratio, a difference) | `derived: { op, of: [...] }` on an ADR-0021 dataset over the same object. | + +**The one-line fix: give the measure an aggregate type.** There is no mechanical +rewrite — the column alone does not say whether `amount` meant its sum, its average +or its largest value — so `os migrate meta` lists nothing for this change. + +Each retired member is refused at parse with a prescription naming the six +aggregates, at the measure's `type`, and in `tsc` (the members are gone from the +`AggregationMetricType` type). A value the enum never declared keeps zod's own +message. + +### The retirement kit + +- **Value-level retirement.** `AggregationMetricType` is declared through + `enumWithRetiredValues` (`shared/retired-key.ts`), with the prescriptions + module-private. No authorable KEY and no def changed, so nothing lands in + `RETIRED_KEYS_BY_MAJOR`, and the four surface ratchets (`api-surface`, + `authorable-surface`, `json-schema.manifest`, `api-surface-signatures`) are + byte-identical. +- **No D2 conversion, by design.** A stored or built cube that still carries one of + the three is REFUSED, never rewritten or dropped: the boot door + (`ObjectStackDefinitionSchema`, which a built artifact is parsed through), the + `analytics_cube` write door and `defineStack` refuse it with the prescription, and + the rehydration seam replays no conversion over it. +- **D3 entry `cube-metric-expression-types-retired`**, with its step-18 rationale + fragment, carries the judgement the upgrader owes: which aggregate each measure + meant. +- **Liveness.** The `analytics_cube` row `measures.type` stays `live`, re-verified + 2026-10-02, with the narrowing recorded. +- **Docs.** The `data/analytics` reference page is regenerated. +- **No deprecation window**, per the project's startup-stage posture. + +### Reach, measured + +- This repository authors no cube measure of the three types outside tests: + `examples/**`, `packages/**` (the platform objects included) and the skills and + docs carry none. The showcase cube's `type: 'string'` entries are dimensions, + whose `DimensionType` is a separate enum and is unchanged. +- objectui at its pinned commit carries no `AggregationMetricType` mirror and no + cube measure of the three types. +- Out-of-repo authored cubes: NOT MEASURED. + +Clause-②: no (narrowing) + + diff --git a/content/docs/references/data/analytics.mdx b/content/docs/references/data/analytics.mdx index 422775d08b2..947a36364d1 100644 --- a/content/docs/references/data/analytics.mdx +++ b/content/docs/references/data/analytics.mdx @@ -40,9 +40,6 @@ const result = AggregationMetricType.parse(data); * `min` * `max` * `count_distinct` -* `number` -* `string` -* `boolean` --- @@ -126,7 +123,7 @@ Type: `[string, string]` | **title** | `string` | optional | | | **description** | `string` | optional | | | **sql** | `string` | ✅ | Base SQL statement or Table Name | -| **measures** | `Record; sql: string; … }>` | ✅ | Quantitative metrics, keyed by metric name: the record key IS the metric's name, published and queried as `.`. A metric declares no inner `name`. | +| **measures** | `Record; sql: string; … }>` | ✅ | Quantitative metrics, keyed by metric name: the record key IS the metric's name, published and queried as `.`. A metric declares no inner `name`. | | **dimensions** | `Record; sql: string; … }>` | ✅ | Qualitative attributes, keyed by dimension name: the record key IS the dimension's name, published and queried as `.`. A dimension declares no inner `name`. | | **joins** | `Record` | optional | | | **refreshKey** | `never` | optional | [REMOVED] `analytics_cube.refreshKey` was removed in @objectstack/spec 17 (ADR-0049 enforce-or-remove) — nothing read it: no analytics result is cached, so neither `every` nor `sql` ever refreshed anything. Delete the key; every analytics query is computed when it is asked. A refresh cadence is declared again when a result cache exists. Run `os migrate meta --from 17` to list the mechanical edits for existing sources; apply them by hand. | @@ -146,7 +143,7 @@ Type: `[string, string]` | **name** | `never` | optional | [REMOVED] `measures..name` was removed in @objectstack/spec 17.5.0 (ADR-0049 enforce-or-remove) — it never had an effect: the record key is the metric's name. Every consumer resolves a metric by its key in `measures` (`GET /analytics/meta` publishes it as `.`, and a query names it that way), so the inner `name` was a second copy of the identity that nothing read, and one that disagreed with its key was silently ignored. Delete the key. To rename a metric, rename its key in `measures` — and every query, dashboard and report that names `.`. Run `os migrate meta --from 17` to list the mechanical edits for existing sources; apply them by hand. | | **label** | `string` | ✅ | Human readable label | | **description** | `string` | optional | | -| **type** | `Enum<'count' \| 'sum' \| 'avg' \| 'min' \| 'max' \| 'count_distinct' \| 'number' \| 'string' \| 'boolean'>` | ✅ | | +| **type** | `Enum<'count' \| 'sum' \| 'avg' \| 'min' \| 'max' \| 'count_distinct'>` | ✅ | | | **sql** | `string` | ✅ | Column reference: a field of the cube's object ("amount"), a relationship path ending in one ("account.amount"), or "*" for a count. Never a SQL expression: a derived value is declared on an ADR-0021 dataset (a measure-scoped filter, or derived: `{ op, of }`). | | **format** | `string` | optional | Display format for this measure's result column: a numeral pattern such as "$0,0.00" or "0.0%". Relayed verbatim as fields[].format on POST /analytics/query results, and on the measure by GET /analytics/meta. | @@ -219,7 +216,7 @@ Type: `[string, string]` | **name** | `never` | optional | [REMOVED] `measures..name` was removed in @objectstack/spec 17.5.0 (ADR-0049 enforce-or-remove) — it never had an effect: the record key is the metric's name. Every consumer resolves a metric by its key in `measures` (`GET /analytics/meta` publishes it as `.`, and a query names it that way), so the inner `name` was a second copy of the identity that nothing read, and one that disagreed with its key was silently ignored. Delete the key. To rename a metric, rename its key in `measures` — and every query, dashboard and report that names `.`. Run `os migrate meta --from 17` to list the mechanical edits for existing sources; apply them by hand. | | **label** | `string` | ✅ | Human readable label | | **description** | `string` | optional | | -| **type** | `Enum<'count' \| 'sum' \| 'avg' \| 'min' \| 'max' \| 'count_distinct' \| 'number' \| 'string' \| 'boolean'>` | ✅ | | +| **type** | `Enum<'count' \| 'sum' \| 'avg' \| 'min' \| 'max' \| 'count_distinct'>` | ✅ | | | **sql** | `string` | ✅ | Column reference: a field of the cube's object ("amount"), a relationship path ending in one ("account.amount"), or "*" for a count. Never a SQL expression: a derived value is declared on an ADR-0021 dataset (a measure-scoped filter, or derived: `{ op, of }`). | | **format** | `string` | optional | Display format for this measure's result column: a numeral pattern such as "$0,0.00" or "0.0%". Relayed verbatim as fields[].format on POST /analytics/query results, and on the measure by GET /analytics/meta. | diff --git a/packages/spec/src/data/analytics.zod.ts b/packages/spec/src/data/analytics.zod.ts index ed736cd29b3..5c00b483303 100644 --- a/packages/spec/src/data/analytics.zod.ts +++ b/packages/spec/src/data/analytics.zod.ts @@ -33,8 +33,9 @@ import { MetadataProtectionFields } from '../kernel/metadata-protection.zod'; // `AnalyticsService` on both strategies before this retirement, with a column // `sql`: the raw-SQL path emitted the column UNAGGREGATED // (`SELECT status AS "status", amount AS "m" … GROUP BY status` — a bare -// column in a grouped statement, which PostgreSQL refuses and SQLite answers -// with an arbitrary row's value), and the ObjectQL path refused the measure. +// column in a grouped statement, by SQL's own rules an error on PostgreSQL and +// an arbitrary row's value on SQLite), and the ObjectQL path refused the +// measure. // // A VALUE-level retirement (`enumWithRetiredValues`, shared/retired-key.ts): // the members left the enum, so `tsc` refuses them, and the parse answers each diff --git a/packages/spec/src/migrations/entries/semantic/18.cube-metric-expression-types-retired.ts b/packages/spec/src/migrations/entries/semantic/18.cube-metric-expression-types-retired.ts index d3e0c85fdaf..90f459e620f 100644 --- a/packages/spec/src/migrations/entries/semantic/18.cube-metric-expression-types-retired.ts +++ b/packages/spec/src/migrations/entries/semantic/18.cube-metric-expression-types-retired.ts @@ -28,9 +28,9 @@ export const entry: SemanticMigration = { + 'CASE, a window function — and named only what it returned. Since ' + '`cube-member-sql-expression-retired` a member\'s `sql` is a column reference, so the types had ' + 'nothing left to declare: measured before this retirement, the raw-SQL strategy emitted the ' - + 'referenced column unaggregated (a bare column in a grouped statement, which PostgreSQL refuses ' - + 'and SQLite answers with an arbitrary row\'s value) and the ObjectQL strategy refused the ' - + 'measure. There is no D2 conversion: the column alone does not say which aggregate the author ' + + 'referenced column unaggregated (a bare column in a grouped statement, by SQL\'s own rules an ' + + 'error on PostgreSQL and an arbitrary row\'s value on SQLite) and the ObjectQL strategy refused ' + + 'the measure. There is no D2 conversion: the column alone does not say which aggregate the author ' + 'wanted — a `number` over `amount` may have meant its sum, its average or its largest value — ' + 'so only the author can choose, and a measure whose old expression computed something per row ' + 'needs that value stored on the object before any aggregate can read it. Nothing is rewritten ' diff --git a/packages/spec/src/migrations/registry.ts b/packages/spec/src/migrations/registry.ts index 922bf4f6869..1dd35ca452f 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -8746,9 +8746,9 @@ const step18: MigrationStep = { + 'CASE, a window function — and named only what it returned. Since ' + '`cube-member-sql-expression-retired` a member\'s `sql` is a column reference, so the types had ' + 'nothing left to declare: measured before this retirement, the raw-SQL strategy emitted the ' - + 'referenced column unaggregated (a bare column in a grouped statement, which PostgreSQL refuses ' - + 'and SQLite answers with an arbitrary row\'s value) and the ObjectQL strategy refused the ' - + 'measure. There is no D2 conversion: the column alone does not say which aggregate the author ' + + 'referenced column unaggregated (a bare column in a grouped statement, by SQL\'s own rules an ' + + 'error on PostgreSQL and an arbitrary row\'s value on SQLite) and the ObjectQL strategy refused ' + + 'the measure. There is no D2 conversion: the column alone does not say which aggregate the author ' + 'wanted — a `number` over `amount` may have meant its sum, its average or its largest value — ' + 'so only the author can choose, and a measure whose old expression computed something per row ' + 'needs that value stored on the object before any aggregate can read it. Nothing is rewritten ' From ab9da0a02a913d417a434b63094c15959e17102d Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 17:04:00 +0000 Subject: [PATCH 5/7] test(spec): prove the stored-row seam live with a retired interval, not a retired key another absence pin guards Claude-Session: https://claude.ai/code/session_01UtnxvdiN376GF3sgXwAw4d Co-authored-by: Claude --- ...metric-expression-types-retirement.test.ts | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/packages/spec/src/data/cube-metric-expression-types-retirement.test.ts b/packages/spec/src/data/cube-metric-expression-types-retirement.test.ts index 9cbc42d74c7..c65b0e1a771 100644 --- a/packages/spec/src/data/cube-metric-expression-types-retirement.test.ts +++ b/packages/spec/src/data/cube-metric-expression-types-retirement.test.ts @@ -144,18 +144,23 @@ describe('every door that carries a stored or authored cube refuses it — never }); it.each(RETIRED)('the rehydration seam replays nothing over a stored row carrying `%s` — it reaches the parse as stored', (member) => { - // CONTROL: the seam is live for this type — a retired KEY a D2 conversion - // strips (`refreshKey`, lossless: it never had an effect) is stripped here. - const withRetiredKey = applyConversionsToStoredItem('analytics_cube', { + // CONTROL: the seam is live for this type — a retired VALUE a D2 conversion + // does rewrite (`cube-sub-day-granularities-removed` drops a sub-day + // interval from a dimension's `granularities`) is rewritten here, on the + // same stored row, while the retired measure type beside it is not touched. + const withRetiredInterval = applyConversionsToStoredItem('analytics_cube', { ...cubeWith(member), - refreshKey: { every: '1 hour' }, - }) as Record; - expect(withRetiredKey).not.toHaveProperty('refreshKey'); + dimensions: { + ...CUBE.dimensions, + placed_at: { label: 'Placed', type: 'time', sql: 'placed_at', granularities: ['hour', 'day'] }, + }, + }) as { dimensions: Record; measures: Record }; + expect(withRetiredInterval.dimensions.placed_at!.granularities).toEqual(['day']); + expect(withRetiredInterval.measures.m).toEqual(measureOf(member)); const stored = cubeWith(member); const rehydrated = applyConversionsToStoredItem('analytics_cube', stored) as typeof stored; // Nothing rewrote or dropped the retired measure on the way… expect(rehydrated.measures.m).toEqual(measureOf(member)); - expect((withRetiredKey.measures as Record).m).toEqual(measureOf(member)); // …so the parse that follows refuses it, with the prescription. const issues = issuesOf(CubeSchema, rehydrated); expect(issues[0]!.message.startsWith(firstSentence(member))).toBe(true); From 228a253dc885c93dca4e3a865d31687d5069c400 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 17:29:51 +0000 Subject: [PATCH 6/7] =?UTF-8?q?test(lint):=20the=20cube-measure=20populati?= =?UTF-8?q?on=20is=20the=20aggregate=20table=20=E2=80=94=20the=20custom-SQ?= =?UTF-8?q?L=20metric=20types=20were=20retired?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Claude-Session: https://claude.ai/code/session_01UtnxvdiN376GF3sgXwAw4d Co-authored-by: Claude --- .../validate-dataset-measure-aggregates.test.ts | 14 +++++++++----- .../src/validate-dataset-measure-aggregates.ts | 6 +++--- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/packages/lint/src/validate-dataset-measure-aggregates.test.ts b/packages/lint/src/validate-dataset-measure-aggregates.test.ts index 1a8e0417dfb..f08031d104a 100644 --- a/packages/lint/src/validate-dataset-measure-aggregates.test.ts +++ b/packages/lint/src/validate-dataset-measure-aggregates.test.ts @@ -886,8 +886,8 @@ describe('the cube leg — an analyticsCubes member the analytics door refuses i * `shape`, as the two door modules state their rules — read off the spec's * table and predicates, never a retyped list: * - * - a `type` that is no row of the table (the expression metric types) is - * judged by neither door; + * - a `type` that is no row of the table (the retired custom-SQL metric + * types, which the schema refuses) is judged by neither door; * - `count_distinct` — `structured-json-dimension-door.ts`: refused when the * row refuses the type OR the declaration is multi-value; * - every other row — `cube-measure-field-type-door.ts`: refused when the row @@ -980,8 +980,11 @@ describe('the cube leg — every cube measure is judged by the aggregate × fiel expect(accepted, `${type} accepts`).toBeGreaterThan(5); } } - // The expression metric types are in the population and outside the table. - expect(types.filter((t) => !rows.includes(t)).sort()).toEqual(['boolean', 'number', 'string']); + // Every cube measure `type` IS a row: the custom-SQL metric types that + // used to sit outside the table (`number` / `string` / `boolean`) were + // retired from `AggregationMetricType` (#21000), so the population and the + // table are one vocabulary. + expect(types.filter((t) => !rows.includes(t)).sort()).toEqual([]); }); it('reads a relationship path on the object the last hop reaches — the lookup\'s reference, or the join the cube declares for it', () => { @@ -1002,7 +1005,8 @@ describe('the cube leg — every cube measure is judged by the aggregate × fiel it('never hands the predicate a guess: no type, a type outside the table, the row wildcard, an unresolved column', () => { const silent = (measures: Record, ledger: Record = {}) => expect(validateDatasetMeasureAggregates(cubeStack({ measures }, {}, ledger))).toEqual([]); - // The expression metric types are outside the table's vocabulary (skip 5). + // The retired custom-SQL metric types are outside the table's vocabulary + // (skip 5): the schema refuses them, so this leg leaves them to it. silent({ n: measure('number', 'name'), s: measure('string', 'name'), b: measure('boolean', 'name') }); // A prototype key is not a row. silent({ p: measure('toString', 'name') }); diff --git a/packages/lint/src/validate-dataset-measure-aggregates.ts b/packages/lint/src/validate-dataset-measure-aggregates.ts index 0d56d082134..0c450b5616f 100644 --- a/packages/lint/src/validate-dataset-measure-aggregates.ts +++ b/packages/lint/src/validate-dataset-measure-aggregates.ts @@ -159,9 +159,9 @@ * `cube-measure-field-type-door.ts` half, which reads the TYPE alone — the * `multiple` flag moves only `count_distinct`, see above), and never * `count`, which reads no value and accepts every type. A `type` outside - * the table's vocabulary — the expression metric types `number` / - * `string` / `boolean` — is skip 5, as on a dataset, and neither door - * judges it. + * the table's vocabulary — since the custom-SQL metric types `number` / + * `string` / `boolean` were retired (#21000), one the schema itself + * refuses — is skip 5, as on a dataset, and neither door judges it. * * How a cube names its column is read the way the door reads it * (`analytics-service.ts`, `hop-object.ts`): From 4278601b824d5c64fd7802cfc8cc4e7485b7d7ab Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 19:13:51 +0000 Subject: [PATCH 7/7] docs(spec): the measures.sql ledger note says both expression branches are gone and what still passes a non-column sql through Claude-Session: https://claude.ai/code/session_01UtnxvdiN376GF3sgXwAw4d Co-authored-by: Claude --- packages/spec/liveness/analytics_cube.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/spec/liveness/analytics_cube.json b/packages/spec/liveness/analytics_cube.json index e2e2b04bc85..d1a66db7a59 100644 --- a/packages/spec/liveness/analytics_cube.json +++ b/packages/spec/liveness/analytics_cube.json @@ -63,7 +63,7 @@ "verifiedAt": "2026-10-02", "evidence": "packages/services/service-analytics/src/strategies/native-sql-strategy.ts#resolveMeasureSql — `measure.sql` is the column the aggregate is applied to (`'*'` the COUNT(*) form), and `qualifyAndRegisterJoin(measure.sql, …)` is what lowers a dotted reference into a LEFT JOIN chain; packages/services/service-analytics/src/strategies/objectql-strategy.ts#resolveFieldName reads `measure.sql.replace(/^\\$/, '')` as the aggregate field for the `engine.aggregate` path; packages/services/service-analytics/src/analytics-service.ts#fieldsOfColumnSql resolves it to the field(s) the analytics door's field-level read gate judges.", "producer": "packages/cli/src/commands/serve.ts#CAPABILITY_PROVIDERS — the `analytics` entry declares `configKey: 'analyticsCubes'` and the capability resolver threads it into the plugin (`const cubes = (config as any).analyticsCubes ?? (config as any).cubes ?? []; arg = { cubes }`); packages/services/service-analytics/src/analytics-service.ts#registerAll (`if (config.cubes) this.cubeRegistry.registerAll(config.cubes)`) is where the authored array becomes the registry every consumer below resolves through. Without this thread an authored cube reaches no reader at all — the `seed.env` shape (#4837).", - "note": "REQUIRED. NARROWED 2026-09-30 (#20943, maintainer ruling D; ADR-0021 zero raw expressions, ADR-0049 enforce-or-remove): the value is a COLUMN REFERENCE — a bare identifier, a dotted identifier path (relationship hops, then the column), or `'*'` — and any SQL expression is refused at parse with a prescription naming the ADR-0021 dataset form (a measure-scoped `filter`, `derived: { op, of }`). The admitted pattern is the one `IDENTIFIER_PATH` (native-sql-strategy.ts) and the read gate already use to tell a column path from an expression (#4157), so every admitted value other than `'*'` (which reads no field value) resolves to a field the gate can judge. Still LIVE: the key itself is unchanged and read at the three sites above. The runtime's expression branches (verbatim emit on the raw-SQL path, the gate's stand-down) remain for a cube that reaches the service without meeting the parse; their deletion is the services-lane follow-up. The D3 entry is `cube-member-sql-expression-retired`; there is no D2 conversion, because an expression has no mechanical rewrite into a dataset. NARROWED again 2026-10-02 (#21409, the count-only boundary): `'*'` is admitted only under `type: 'count'` — the measure's refinement asks the one predicate it shares with the dataset measure (`data/analytics-column-reference.ts#rowWildcardOutsideCount`), and refuses `'*'` under any other type at `sql`, naming the slot and prescribing a `count` or a column; a dataset `sum` over `'*'`, which compiles to this very member, answered 500 DATABASE_ERROR at `POST /analytics/dataset/query` on both strategies before the rule. Cross-field, so a declared dropped-refinement site (`data/Metric` in `dropped-refinements.baseline.json`). D3 entry `analytics-row-wildcard-outside-count-refused`; no D2 conversion." + "note": "REQUIRED. NARROWED 2026-09-30 (#20943, maintainer ruling D; ADR-0021 zero raw expressions, ADR-0049 enforce-or-remove): the value is a COLUMN REFERENCE — a bare identifier, a dotted identifier path (relationship hops, then the column), or `'*'` — and any SQL expression is refused at parse with a prescription naming the ADR-0021 dataset form (a measure-scoped `filter`, `derived: { op, of }`). The admitted pattern is the one `IDENTIFIER_PATH` (native-sql-strategy.ts) and the read gate already use to tell a column path from an expression (#4157), so every admitted value other than `'*'` (which reads no field value) resolves to a field the gate can judge. Still LIVE: the key itself is unchanged and read at the three sites above. Both runtime expression branches are gone: the field-level read gate's stand-down with #20965, and the raw-SQL path's verbatim emit with #21000. What remains for a cube that reaches the service without meeting the parse is packages/services/service-analytics/src/strategies/native-sql-strategy.ts#qualifyAndRegisterJoin passing a non-column `sql` through as-is, inside the measure's aggregate (`SUM(amount + 1)`, `SUM(COALESCE(account.amount, 0) / 2)`, measured 2026-10-02 through `AnalyticsService#generateSql` with no field reader wired). The D3 entry is `cube-member-sql-expression-retired`; there is no D2 conversion, because an expression has no mechanical rewrite into a dataset. NARROWED again 2026-10-02 (#21409, the count-only boundary): `'*'` is admitted only under `type: 'count'` — the measure's refinement asks the one predicate it shares with the dataset measure (`data/analytics-column-reference.ts#rowWildcardOutsideCount`), and refuses `'*'` under any other type at `sql`, naming the slot and prescribing a `count` or a column; a dataset `sum` over `'*'`, which compiles to this very member, answered 500 DATABASE_ERROR at `POST /analytics/dataset/query` on both strategies before the rule. Cross-field, so a declared dropped-refinement site (`data/Metric` in `dropped-refinements.baseline.json`). D3 entry `analytics-row-wildcard-outside-count-refused`; no D2 conversion." }, "format": { "status": "live",