diff --git a/.changeset/21220-dataset-field-column-reference.md b/.changeset/21220-dataset-field-column-reference.md new file mode 100644 index 00000000000..986d89ebe7d --- /dev/null +++ b/.changeset/21220-dataset-field-column-reference.md @@ -0,0 +1,125 @@ +--- +'@objectstack/spec': minor +--- + +feat(spec)!: an analytics dataset dimension's and measure's `field` is a column reference — a SQL expression there is refused at parse, as it already is on the cube members a dataset compiles to (#21220) + +Clause-②: yes (narrowing) + +**BREAKING** — shipped as `minor` under the launch-window convention +(`check-changeset-no-major` refuses `major` until GA; breaking-ness is carried by +this banner, the `(narrowing)` arm above and the ADR-0087 disposition below, +never by the level). + +`DatasetDimensionSchema.field` and `DatasetMeasureSchema.field` — the `field` of +every entry in an ADR-0021 dataset's `dimensions` and `measures` — admit a column +reference only: a field of the dataset's object (`amount`), or a relationship path +of bare identifiers ending in one (`account.amount`, `account.owner.region`); a +measure also admits `'*'` for a count, and a count may still omit `field`. Any +other value — an arithmetic, an aggregate, a `CASE`, a subquery, a function call, +a quoted or `$`-prefixed spelling, a padded or empty string, a broken path — is +refused at `dimensions.N.field` / `measures.N.field` with a prescription, and so +is `'*'` on a dimension. + +Why: the dataset layer was declared to take no raw SQL (ADR-0021 "zero raw SQL / +zero raw expressions") and `field` was documented as a field or a relationship +path, but it was a bare string and parsed anything. The analytics dataset door +already refused an expression `field` on every query (`PERMISSION_DENIED` / 403, +inline or saved), so such a dataset could be saved and never answered — declared, +never enforced (ADR-0049). That door never judged an empty `field`: it skips one, +which is how a `count` measure with `field: ''` kept counting rows on SQLite's +native-SQL path (the D2 repair below). The accept set is the one the cube members a dataset +compiles to already hold: the dataset compiler copies `field` into the member's +`sql` verbatim, and both now read one shared declaration. `'*'` is refused on a +dimension because grouping by every column is no axis — both analytics strategies +answered such a dimension `500`. The rule is a `pattern` in the published JSON +Schema too, so a document validated against `json-schema/**` is judged as the +parse judges it. + +## FROM → TO + +``` +FROM defineDataset({ name: 'task_metrics', label: 'Task Metrics', object: 'task', + dimensions: [{ name: 'priority', field: 'priority' }], + measures: [ + { name: 'task_count', aggregate: 'count', field: '' }, + { name: 'done_points', aggregate: 'sum', + field: "CASE WHEN status = 'done' THEN points ELSE 0 END" }, + ] }) + -> parsed; the dataset door refused the expression on every query +TO -> ZodError at measures.0.field and measures.1.field (invalid_format): + `measures[].field` is a column reference: a field of the dataset's object … + + defineDataset({ name: 'task_metrics', label: 'Task Metrics', object: 'task', + dimensions: [{ name: 'priority', field: 'priority' }], + measures: [ + { name: 'task_count', aggregate: 'count' }, + { name: 'done_points', aggregate: 'sum', field: 'points', filter: { status: 'done' } }, + ] }) +``` + +A conditional count or sum is a measure with its own structured `filter`; a +ratio, sum, difference or product of measures is `derived: { op, of: [...] }` +over measures named in the same dataset. **Mind the scale:** a `derived` ratio is +a 0–1 fraction, so an expression that multiplied by 100 returned percentage +points — pair the ratio with a `%` numeral pattern. A dimension that bucketed a +column with an expression has no expression form: group by the column itself, or +keep the bucket as a field of the object and name that field. + +**The one-line fix:** parse each dataset; every refusal at `…field` is one member +to change — name the column, omit `field` on a plain count (never `field: ''`), +or move the computation to a measure `filter` or a `derived` measure. The one +mechanical case is done for you: `os migrate meta --from 17` lists, and every +stored-row rehydration replays, the D2 conversion +`dataset-count-measure-empty-field-removed`, which drops a `count` measure's empty +`field` (it still counts rows). Nothing else has a mechanical rewrite. + +**What an author who still writes it sees.** `DatasetSchema`, `defineStack({ +datasets })` (`STACK_SCHEMA_INVALID` / 422), the `dataset` write door and +`POST /api/v1/analytics/dataset/query` (which parses every dataset it is handed, +inline or saved, and now answers `400 VALIDATION_FAILED` at the path where it +answered `403 PERMISSION_DENIED` before) refuse the member at its `field` path +with the prescription. `tsc` does not: the key's type is still `string`. + +## The retirement kit + +- **Schema.** `ui/dataset.zod.ts` holds both keys to the pattern; the pattern is + declared once, in the non-public `data/analytics-column-reference.ts`, and the + cube layer's `CUBE_MEMBER_SQL` is that same `RegExp`. A dimension's pattern is + the same column path without the `'*'` arm. A column reference parses + byte-identically to before. +- **ADR-0087.** D2 carries the one lossless repair: the conversion + `dataset-count-measure-empty-field-removed` (`retiredFromLoadPath`, so an author + is refused at parse while stored rows and `os migrate meta` replay it) drops a + `count` measure's `field: ''`, which compiles to `COUNT(*)` without it. The D3 + entry `dataset-member-field-expression-refused`, linked to that conversion and + with its step-18 rationale fragment, carries the rest — a non-count measure or a + dimension with `''` and every expression have no mechanical rewrite into a + column. No `RETIRED_KEYS_BY_MAJOR` row: no key left the shape, so the + authorable-surface, api-surface and JSON-schema manifest ratchets are + unchanged. +- **Liveness.** The `dataset` ledger rows `dimensions.field` and + `measures.field` stay `live`, re-verified, with the narrowing recorded. +- **Docs.** The `ui/dataset` reference page is regenerated. +- **Runtime.** Unchanged: the analytics dataset door's refusal stays as defence + in depth for a dataset that reaches the service without meeting the parse — a + row stored before this change, which the build probe hands over as read. + +## Reach, measured + +- This repository: no authored dataset carries a non-column `field` — the + examples, `platform-objects`, the hand-written docs and the published skills + were read. Two test fixtures that sent an expression `field` on purpose were + re-pinned: the service door's test builds them unparsed, and the REST route's + test now expects the route's `400`. +- Studio's dataset inspector (objectui) seeds a new dimension or measure row with + `field: ''`. A plain count saved that way parsed before; its query answered + `500` on the ObjectQL path, while SQLite's native-SQL path accepted the + `COUNT()` it compiled to. A row already stored that way is repaired on load by + the D2 conversion above. A NEW save of that shape is refused at the save door + with the prescription to omit the key, because the write path parses with the + current schema and replays no conversion; the producer-side change is + objectui's. +- Out-of-repo authored datasets: NOT MEASURED. + + diff --git a/content/docs/data-modeling/analytics.mdx b/content/docs/data-modeling/analytics.mdx index f1412148186..4541d873fa0 100644 --- a/content/docs/data-modeling/analytics.mdx +++ b/content/docs/data-modeling/analytics.mdx @@ -81,6 +81,11 @@ export default defineStack({ - **No raw SQL, no hand-authored joins.** The author declares *which* relationships to include; the compiler derives the join from the object graph. +- **`field` is a column reference.** A dimension's `field` names a field of the + base object or a `relationship.field` path; a measure's may also be `'*'` (or + be omitted) for a count. A SQL expression there is refused at parse: a + conditional count or sum is a measure with its own `filter`, and a ratio is a + `derived` measure. - **Metric certification** — a `certified` flag that marks a measure as a human-blessed governance checkpoint — is a design goal of ADR-0021 but is **not yet implemented**; `DatasetMeasureSchema` has no `certified` field today. diff --git a/content/docs/references/ui/dataset.mdx b/content/docs/references/ui/dataset.mdx index 123cb60758a..be509f53cea 100644 --- a/content/docs/references/ui/dataset.mdx +++ b/content/docs/references/ui/dataset.mdx @@ -72,7 +72,7 @@ const result = DatasetSchema.parse(data); | :--- | :--- | :--- | :--- | | **name** | `string` | ✅ | Dimension name — referenced by presentations | | **label** | `string \| Record` | optional | Display label — the default-language string, or an inline locale map (`{ en, "zh-CN" }`) resolved at render time | -| **field** | `string` | ✅ | Base field, or `relationship[.relationship].field` path | +| **field** | `string` | ✅ | Base field, or `relationship[.relationship].field` path. A column reference, never a SQL expression. | | **type** | `Enum<'string' \| 'number' \| 'date' \| 'boolean' \| 'lookup'>` | optional | | | **dateGranularity** | `Enum<'day' \| 'week' \| 'month' \| 'quarter' \| 'year'>` | optional | | @@ -83,7 +83,7 @@ const result = DatasetSchema.parse(data); | **name** | `string` | ✅ | Measure name — e.g. "revenue"; defined once | | **label** | `string \| Record` | optional | Display label — the default-language string, or an inline locale map (`{ en, "zh-CN" }`) resolved at render time | | **aggregate** | `Enum<'count' \| 'sum' \| 'avg' \| 'min' \| 'max' \| 'count_distinct'>` | optional | Aggregation (sum/avg/count/...); omit when `derived` is set | -| **field** | `string` | optional | Aggregated field; optional for count(*) | +| **field** | `string` | optional | Aggregated field: a base field, a relationship path, or "*"; optional for count(*). Never a SQL expression. | | **filter** | `any` | optional | | | **format** | `string` | optional | Numeral pattern for a NUMERIC measure — grouping, decimals, percent; e.g. "0,0.00", "0.0%". An amount takes its symbol from `currency`, not from a "$" in the pattern. A DATE-valued measure never reads a date pattern: `"YYYY-MM-DD"` renders that arm's default face. A date or datetime value reads `format` as a display style — `short` or `relative`, honoured on both. | | **currency** | `string` | optional | Display currency code (ISO 4217) | @@ -108,7 +108,7 @@ const result = DatasetSchema.parse(data); | :--- | :--- | :--- | :--- | | **name** | `string` | ✅ | Dimension name — referenced by presentations | | **label** | `string \| Record` | optional | Display label — the default-language string, or an inline locale map (`{ en, "zh-CN" }`) resolved at render time | -| **field** | `string` | ✅ | Base field, or `relationship[.relationship].field` path | +| **field** | `string` | ✅ | Base field, or `relationship[.relationship].field` path. A column reference, never a SQL expression. | | **type** | `Enum<'string' \| 'number' \| 'date' \| 'boolean' \| 'lookup'>` | optional | | | **dateGranularity** | `Enum<'day' \| 'week' \| 'month' \| 'quarter' \| 'year'>` | optional | | @@ -124,7 +124,7 @@ const result = DatasetSchema.parse(data); | **name** | `string` | ✅ | Measure name — e.g. "revenue"; defined once | | **label** | `string \| Record` | optional | Display label — the default-language string, or an inline locale map (`{ en, "zh-CN" }`) resolved at render time | | **aggregate** | `Enum<'count' \| 'sum' \| 'avg' \| 'min' \| 'max' \| 'count_distinct'>` | optional | Aggregation (sum/avg/count/...); omit when `derived` is set | -| **field** | `string` | optional | Aggregated field; optional for count(*) | +| **field** | `string` | optional | Aggregated field: a base field, a relationship path, or "*"; optional for count(*). Never a SQL expression. | | **filter** | `any` | optional | | | **format** | `string` | optional | Numeral pattern for a NUMERIC measure — grouping, decimals, percent; e.g. "0,0.00", "0.0%". An amount takes its symbol from `currency`, not from a "$" in the pattern. A DATE-valued measure never reads a date pattern: `"YYYY-MM-DD"` renders that arm's default face. A date or datetime value reads `format` as a display style — `short` or `relative`, honoured on both. | | **currency** | `string` | optional | Display currency code (ISO 4217) | diff --git a/packages/rest/src/analytics-16019-driver-declared-fault.test.ts b/packages/rest/src/analytics-16019-driver-declared-fault.test.ts index b8725bbd9fb..3daf08501f8 100644 --- a/packages/rest/src/analytics-16019-driver-declared-fault.test.ts +++ b/packages/rest/src/analytics-16019-driver-declared-fault.test.ts @@ -31,6 +31,17 @@ * dataset whose `field` is not a column reference is refused too, and a saved * plain-column dataset is still served. * + * [#21220] The contract now refuses that `field` text one step earlier, at parse: + * `DatasetSchema` holds a dimension's and measure's `field` to a column reference, + * and this route parses every dataset it is handed — inline and saved alike — + * before calling `queryDataset`. So on THIS route both cases are answered by the + * route's own validation, `400 VALIDATION_FAILED` naming the path + * (`dimensions.N.field`), still before any strategy or driver runs and still + * without echoing the expression. The service door's `403 PERMISSION_DENIED` + * stays as defence in depth for a dataset that reaches `queryDataset` without + * that parse, and is pinned where it is reachable: in `service-analytics`'s + * `inline-dataset-field-admission-door.test.ts`. + * * The second block pins the ordering the ruling's execution notes name. A * DECLARED fault is withheld even when its text is one the heuristic does not * know (declared wins); an UNDECLARED knex-shaped fault still falls to the @@ -51,9 +62,11 @@ * first) and only that case goes RED (`ANALYTICS_QUERY_FAILED` in place of the * producer's code); the neighbouring "phrase the heuristic does not know" * case stays GREEN, which is precisely why it could not stand in for this one. - * The first block's leg: remove the caller-content gate and the expression - * reaches the real driver again — the refusal flips from `403 PERMISSION_DENIED` - * to the `500 DATABASE_ERROR` relay this file once asserted. + * The first block's leg (since #21220): admit anything in the contract's + * column-reference pattern and the route's parse passes the expression on — the + * refusal flips from `400 VALIDATION_FAILED` to the service door's + * `403 PERMISSION_DENIED`; remove that door as well and it reaches the real driver, + * the `500 DATABASE_ERROR` relay this file once asserted. */ import { describe, it, expect, vi, beforeEach, afterEach, beforeAll, afterAll } from 'vitest'; @@ -222,20 +235,28 @@ describe('[#16019] a driver fault on the raw-SQL path reaches the caller by decl await driver.disconnect(); }); - it('[#21177] a caller-supplied dimension-field expression is refused 403 PERMISSION_DENIED at the door — before any strategy or driver runs', async () => { + it('[#21177 / #21220] a caller-supplied dimension-field expression is refused 400 VALIDATION_FAILED at the route\'s parse — before any strategy or driver runs', async () => { + const execute = vi.spyOn(driver, 'execute'); const route = buildRoute(async () => realAnalytics(driver)); const res = await post(route, { dataset: expressionDataset, selection: { measures: ['account_count'], dimensions: ['folded_name'] } }); - expect(res.statusCode).toBe(403); - expect(res.body.code).toBe('PERMISSION_DENIED'); - // Caller text that names no attributable field is refused at the door, - // not evaluated — the driver never ran, so there is no driver fault line. + expect(res.statusCode).toBe(400); + expect(res.body.code).toBe('VALIDATION_FAILED'); + // The contract's refusal, at the expression's own path (`detail` is the + // parse's issue list, cut at 1000 characters — read, not re-parsed). + expect(res.body.detail).toMatch(/"code":\s*"invalid_format"/); + expect(res.body.detail).toMatch(/"path":\s*\[\s*"dimensions",\s*1,\s*"field"\s*\]/); + // Caller text that names no column is refused before it is evaluated — the + // driver never ran, so there is no driver fault line. + expect(execute).not.toHaveBeenCalled(); expect(warned.filter((m) => m.includes('[sql-driver] DATABASE_ERROR'))).toHaveLength(0); - // ⛔ The refusal names the member and its object (both the caller's own - // input), never the caller's `field` expression text or the compiled statement. + // ⛔ The refusal names the path, never the caller's `field` expression text + // or a compiled statement. The statement check reads the keywords as the + // strategies emit them (upper case): the prescription itself tells the + // author, in prose, to "Group by the column itself". const body = JSON.stringify(res.body); expect(body).not.toMatch(/translate/i); - expect(body).not.toMatch(/SELECT|GROUP BY/i); + expect(body).not.toMatch(/\bSELECT\b|\bGROUP BY\b/); }); it('POSITIVE CONTROL: a legitimate dataset on declared fields → 200 with rows', async () => { @@ -248,10 +269,11 @@ describe('[#16019] a driver fault on the raw-SQL path reaches the caller by decl }); // [#21177] The route's SAVED branch: `body.datasetName` loads the dataset from - // metadata and calls the same `queryDataset`, so the door judges a saved - // dataset's own `field` text exactly as it judges an inline one. The expression - // here is one SQLite can run, so without the door it would be served (200). - it('[#21177] a SAVED dataset (body.datasetName) whose dimension field is not a column reference is refused 403 PERMISSION_DENIED — nothing executed', async () => { + // metadata and calls the same `queryDataset`. [#21220] It parses the loaded + // row through `DatasetSchema` first, exactly as it parses an inline one, so a + // row stored before the contract narrowed is refused there. The expression + // here is one SQLite can run, so without a refusal it would be served (200). + it('[#21177 / #21220] a SAVED dataset (body.datasetName) whose dimension field is not a column reference is refused 400 VALIDATION_FAILED — nothing executed', async () => { const saved = { ...dataset, name: 'account_metrics_saved_expr', @@ -261,11 +283,12 @@ describe('[#16019] a driver fault on the raw-SQL path reaches the caller by decl const route = buildRoute(async () => realAnalytics(driver), [saved]); const res = await post(route, { datasetName: saved.name, selection: { measures: ['account_count'], dimensions: ['lowered_name'] } }); - expect(res.statusCode).toBe(403); - expect(res.body.code).toBe('PERMISSION_DENIED'); + expect(res.statusCode).toBe(400); + expect(res.body.code).toBe('VALIDATION_FAILED'); + expect(res.body.detail).toMatch(/"code":\s*"invalid_format"/); + expect(res.body.detail).toMatch(/"path":\s*\[\s*"dimensions",\s*0,\s*"field"\s*\]/); expect(execute).not.toHaveBeenCalled(); const body = JSON.stringify(res.body); - expect(body).toContain('lowered_name'); expect(body).not.toMatch(/lower\(name\)/i); }); diff --git a/packages/services/service-analytics/src/__tests__/inline-dataset-field-admission-door.test.ts b/packages/services/service-analytics/src/__tests__/inline-dataset-field-admission-door.test.ts index e0ded543985..55410c7b2aa 100644 --- a/packages/services/service-analytics/src/__tests__/inline-dataset-field-admission-door.test.ts +++ b/packages/services/service-analytics/src/__tests__/inline-dataset-field-admission-door.test.ts @@ -26,6 +26,15 @@ * through `query()`, where #21156 leaves its members to the existing gates). The * `/analytics/query` door's own caller members are #21156's and are pinned in * `caller-member-column-reference-gate.test.ts` / `field-read-admission-gate.test.ts`. + * + * [#21220] Since then the CONTRACT refuses the same `field` text at parse: + * `DatasetSchema` holds a dimension's and measure's `field` to a column reference, + * so the route's own `DatasetSchema.parse` answers such a dataset `400` before it + * reaches this door. The door stays as defence in depth for a dataset that reaches + * `queryDataset` WITHOUT meeting that parse — a row stored before the narrowing, + * handed over as read (the build probe's dashboard-widget path does exactly that). + * The expression fixtures here are therefore built UNPARSED, the way such a row + * arrives, and one case asserts the parse refuses them; the controls still parse. */ import { describe, it, expect } from 'vitest'; @@ -90,21 +99,39 @@ const PROVIDERS: ReadonlyArray<{ label: string; provider?: ReadableFields; ctx: /** An expression that reads another object's column — not attributable to any field. */ const EXPRESSION = `(SELECT secret FROM ${OTHER})`; +const DATASET_BASE = { + name: 'id_ds', label: 'DS', object: BASE, + dimensions: [{ name: 'status', field: 'status', type: 'string' }], + measures: [{ name: 'total', aggregate: 'sum', field: 'amount' }], +}; + +/** A dataset that met the contract's parse — the controls. */ function datasetWith(overrides: Record) { - return DatasetSchema.parse({ - name: 'id_ds', label: 'DS', object: BASE, - dimensions: [{ name: 'status', field: 'status', type: 'string' }], - measures: [{ name: 'total', aggregate: 'sum', field: 'amount' }], - ...overrides, - }); + return DatasetSchema.parse({ ...DATASET_BASE, ...overrides }); +} + +/** + * [#21220] A dataset whose `field` the contract now refuses, built UNPARSED — the + * shape a row stored before the narrowing still has when it is read back and + * handed to `queryDataset`. + */ +function storedDatasetWith(overrides: Record) { + return { ...DATASET_BASE, ...overrides }; } describe('[#21177] inline-dataset `field` admission — the dataset door', () => { + it('[#21220] the contract refuses both expression fixtures at parse — the door below is defence in depth', () => { + const dimension = DatasetSchema.safeParse(storedDatasetWith({ dimensions: [{ name: 'leaked', field: EXPRESSION, type: 'string' }] })); + const measure = DatasetSchema.safeParse(storedDatasetWith({ measures: [{ name: 'leaked', aggregate: 'sum', field: `amount + ${EXPRESSION}` }] })); + expect(dimension.success ? [] : dimension.error.issues.map((i) => [i.code, i.path])).toEqual([['invalid_format', ['dimensions', 0, 'field']]]); + expect(measure.success ? [] : measure.error.issues.map((i) => [i.code, i.path])).toEqual([['invalid_format', ['measures', 0, 'field']]]); + }); + describe.each(STRATEGY_PATHS)('$label', ({ capabilities }) => { describe.each(PROVIDERS)('$label', ({ provider, ctx }) => { it('refuses a dimension-field expression — PERMISSION_DENIED / 403, strategy never called', async () => { const { service, executed } = makeService({ capabilities, getReadableFields: provider }); - const dataset = datasetWith({ dimensions: [{ name: 'leaked', field: EXPRESSION, type: 'string' }] }); + const dataset = storedDatasetWith({ dimensions: [{ name: 'leaked', field: EXPRESSION, type: 'string' }] }); const err = await service.queryDataset(dataset as never, { dimensions: ['leaked'], measures: ['total'] } as never, ctx) .then(() => null, (e) => e as Record); expect(err).toMatchObject({ code: 'PERMISSION_DENIED', status: 403, member: 'leaked' }); @@ -116,7 +143,7 @@ describe('[#21177] inline-dataset `field` admission — the dataset door', () => it('refuses a measure-field expression', async () => { const { service, executed } = makeService({ capabilities, getReadableFields: provider }); - const dataset = datasetWith({ measures: [{ name: 'leaked', aggregate: 'sum', field: `amount + ${EXPRESSION}` }] }); + const dataset = storedDatasetWith({ measures: [{ name: 'leaked', aggregate: 'sum', field: `amount + ${EXPRESSION}` }] }); const err = await service.queryDataset(dataset as never, { dimensions: ['status'], measures: ['leaked'] } as never, ctx) .then(() => null, (e) => e as Record); expect(err).toMatchObject({ code: 'PERMISSION_DENIED', status: 403, member: 'leaked' }); diff --git a/packages/spec/liveness/dataset.json b/packages/spec/liveness/dataset.json index bc2d93827ac..cc02fd29415 100644 --- a/packages/spec/liveness/dataset.json +++ b/packages/spec/liveness/dataset.json @@ -53,8 +53,8 @@ "field": { "status": "live", "evidence": "packages/services/service-analytics/src/dataset-compiler.ts#compileDataset (`sql: d.field`, guarded by `assertDeclared(d.field, 'dimension', d.name)` — the relationship-path validation)", - "note": "emitted as the dimension's SQL column reference (relationship-validated). 2026-08-28: RE-ANCHORED (commit 9ee2dcfbd) and REPOINTED — `:144` had rotted onto a docblock terminator `*/`. Re-closed by hand against c459da6bc; DATED for the first time (this file carried no `verifiedAt` anywhere, which the review accepting that re-anchoring sorts as OLDEST).", - "verifiedAt": "2026-08-28" + "note": "emitted as the dimension's SQL column reference (relationship-validated). NARROWED 2026-10-01 (#21220; ADR-0021 zero raw expressions, ADR-0049 enforce-or-remove): the value is a COLUMN REFERENCE — a bare identifier or a dotted identifier path (relationship hops, then the column), the cube member's accept set from the one shared declaration in `data/analytics-column-reference.ts`, minus the row wildcard `'*'`, which both analytics strategies answered 500 as a dimension (`GROUP BY *`). A SQL expression, a padded or empty string and `'*'` are refused at parse with a prescription naming the ADR-0021 form. Still LIVE: the key is unchanged and read at the site above; the analytics dataset door's 403 for an expression `field` (#21190) stays as defence in depth for a dataset that reaches the service without meeting the parse. The D3 entry is `dataset-member-field-expression-refused`; a dimension has no D2 conversion (no lossless rewrite). 2026-08-28: RE-ANCHORED (commit 9ee2dcfbd) and REPOINTED — `:144` had rotted onto a docblock terminator `*/`. Re-closed by hand against c459da6bc; DATED for the first time (this file carried no `verifiedAt` anywhere, which the review accepting that re-anchoring sorts as OLDEST).", + "verifiedAt": "2026-10-01" }, "type": { "status": "live", @@ -94,8 +94,8 @@ "field": { "status": "live", "evidence": "packages/services/service-analytics/src/dataset-compiler.ts#compileDataset (`sql: m.field ?? '*'` — `count` with no field aggregates over rows — guarded by `assertDeclared(m.field, 'measure', m.name)`)", - "note": "SQL aggregate operand (count omits field → '*'); relationship-validated. 2026-08-28: RE-ANCHORED (commit 9ee2dcfbd) and REPOINTED — `:170` had rotted into a comment about members the spec now refuses at parse. Re-closed by hand against c459da6bc; DATED for the first time (this file carried no `verifiedAt` anywhere, which the review accepting that re-anchoring sorts as OLDEST).", - "verifiedAt": "2026-08-28" + "note": "SQL aggregate operand (count omits field → '*'); relationship-validated. NARROWED 2026-10-01 (#21220, with `dimensions.field`): the value is a COLUMN REFERENCE — a bare identifier, a dotted identifier path, or `'*'` — exactly the cube member `sql` accept set, from the one shared declaration in `data/analytics-column-reference.ts`. A SQL expression and a padded or empty string are refused at parse (a count spells \"no field\" by omitting the key), with a prescription naming the ADR-0021 form (a measure-scoped `filter`, `derived: { op, of }`). The one lossless repair is the D2 conversion `dataset-count-measure-empty-field-removed`: a `count` measure's empty `field` is dropped from stored rows and sources and still counts rows; the D3 entry `dataset-member-field-expression-refused` carries the rest. Still LIVE, at the site above; the dataset door's 403 for an expression (#21190) stays as defence in depth. 2026-08-28: RE-ANCHORED (commit 9ee2dcfbd) and REPOINTED — `:170` had rotted into a comment about members the spec now refuses at parse. Re-closed by hand against c459da6bc; DATED for the first time (this file carried no `verifiedAt` anywhere, which the review accepting that re-anchoring sorts as OLDEST).", + "verifiedAt": "2026-10-01" }, "filter": { "status": "live", diff --git a/packages/spec/src/conversions/dataset-count-measure-empty-field-removed.test.ts b/packages/spec/src/conversions/dataset-count-measure-empty-field-removed.test.ts new file mode 100644 index 00000000000..27ffb48abe3 --- /dev/null +++ b/packages/spec/src/conversions/dataset-count-measure-empty-field-removed.test.ts @@ -0,0 +1,113 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, expect, it } from 'vitest'; + +import { MIGRATIONS_BY_MAJOR } from '../migrations/registry.js'; +import { DatasetSchema } from '../ui/dataset.zod.js'; +import { applyConversions } from './apply.js'; +import { ALL_CONVERSIONS, CONVERSIONS_BY_MAJOR } from './registry.js'; +import { applyConversionsToStoredItem } from './stored.js'; +import type { ConversionNotice, ConversionTodoNotice } from './types.js'; + +/** + * [#21220] `dataset-count-measure-empty-field-removed` — the D2 half of the + * dataset `field` narrowing: the one sub-shape with a working row and a + * lossless repair. + * + * A dataset measure's `field` is a column reference, so `''` is refused at + * parse. A `count` measure with `field: ''` (the shape Studio's dataset + * inspector stores when the Field box is left blank) parsed before, the + * dataset door skipped it, and it compiled to the row count on SQLite's native + * path. Dropping the key compiles it to `COUNT(*)`, the same row count. + * + * The fixture pair in `conversions.test.ts` proves before → after over the + * whole table. This file pins it on a STORED row, the seam that replays it: + * the key is dropped and the row then parses; everything outside the + * sub-shape is the same reference and stays refused where it was. + */ + +const ID = 'dataset-count-measure-empty-field-removed'; +const D3_ID = 'dataset-member-field-expression-refused'; + +function storedDataset(measures: unknown[], dimensions: unknown[] = [{ name: 'stage', field: 'stage' }]) { + return { name: 'deal_metrics', label: 'Deal Metrics', object: 'deal', dimensions, measures }; +} + +function convertStored(row: Record) { + const notices: ConversionNotice[] = []; + const todos: ConversionTodoNotice[] = []; + const item = applyConversionsToStoredItem('dataset', row, { + onNotice: (n) => notices.push(n), + onTodo: (t) => todos.push(t), + }); + return { item, notices: notices.filter((n) => n.conversionId === ID), allNotices: notices, todos }; +} + +function fieldIssuePaths(row: unknown): string[] { + const r = DatasetSchema.safeParse(row); + return r.success ? [] : r.error.issues.map((i) => i.path.join('.')).filter((p) => p.endsWith('field')); +} + +describe('[#21220] dataset-count-measure-empty-field-removed (ADR-0087 D2)', () => { + it('is registered for protocol 18, retired from the authoring load path, and linked from its D3 entry', () => { + const entry = ALL_CONVERSIONS.find((c) => c.id === ID); + expect(entry, 'the conversion is registered').toBeDefined(); + expect(entry!.toMajor).toBe(18); + expect(entry!.retiredFromLoadPath).toBe(true); + expect(CONVERSIONS_BY_MAJOR[18]!.map((c) => c.id)).toContain(ID); + expect(MIGRATIONS_BY_MAJOR[18]!.conversionIds).toContain(ID); + const d3 = MIGRATIONS_BY_MAJOR[18]!.semantic.find((s) => s.id === D3_ID); + expect(d3?.conversionIds).toEqual([ID]); + }); + + it('a stored count measure with `field: \'\'` loses the key, and the row then parses', () => { + const row = storedDataset([ + { name: 'deal_count', aggregate: 'count', field: '' }, + { name: 'won_count', aggregate: 'count', field: '', filter: { stage: 'won' } }, + ]); + expect(fieldIssuePaths(row), 'refused before the replay').toEqual(['measures.0.field', 'measures.1.field']); + const { item, notices, todos } = convertStored(row); + const measures = (item as typeof row).measures as Array>; + expect(measures).toEqual([ + { name: 'deal_count', aggregate: 'count' }, + { name: 'won_count', aggregate: 'count', filter: { stage: 'won' } }, + ]); + expect(notices.map((n) => [n.path, n.from, n.to])).toEqual([ + ['datasets[0].measures[0].field', 'field', '(removed)'], + ['datasets[0].measures[1].field', 'field', '(removed)'], + ]); + expect(todos).toEqual([]); + expect(DatasetSchema.safeParse(item).success, 'parses after the replay').toBe(true); + }); + + it('control: outside the sub-shape a row is the same reference — a sum or a dimension over `\'\'` stays refused', () => { + const rows = [ + storedDataset([{ name: 'blank_sum', aggregate: 'sum', field: '' }]), + storedDataset([{ name: 'row_count', aggregate: 'count' }], [{ name: 'blank_axis', field: '' }]), + storedDataset([{ name: 'star_count', aggregate: 'count', field: '*' }]), + storedDataset([{ name: 'row_count', aggregate: 'count' }]), + storedDataset([{ name: 'owner_count', aggregate: 'count', field: 'owner' }]), + storedDataset([{ name: 'padded_count', aggregate: 'count', field: ' ' }]), + storedDataset([{ name: 'expr_count', aggregate: 'count', field: 'COUNT(*)' }]), + ]; + for (const row of rows) { + const { item, allNotices, todos } = convertStored(row); + expect(item, JSON.stringify(row.measures)).toBe(row); + expect(allNotices).toEqual([]); + expect(todos).toEqual([]); + } + expect(fieldIssuePaths(rows[0])).toEqual(['measures.0.field']); + expect(fieldIssuePaths(rows[1])).toEqual(['dimensions.0.field']); + }); + + it('is idempotent — the converted result replays to itself with no second notice', () => { + const once = applyConversions( + { datasets: [storedDataset([{ name: 'deal_count', aggregate: 'count', field: '' }])] }, + { includeRetired: true }, + ); + const notices: ConversionNotice[] = []; + const twice = applyConversions(once, { includeRetired: true, onNotice: (n) => notices.push(n) }); + expect(twice).toBe(once); + expect(notices).toEqual([]); + }); +}); diff --git a/packages/spec/src/conversions/registry.ts b/packages/spec/src/conversions/registry.ts index 5bbf5efb032..75f4166c468 100644 --- a/packages/spec/src/conversions/registry.ts +++ b/packages/spec/src/conversions/registry.ts @@ -6894,6 +6894,96 @@ const elementInputTargetVariableRemoved: MetadataConversion = { }, }; +/** + * A dataset `count` measure drops an empty `field` (protocol 18, #21220). + * + * A dataset measure's `field` is a column reference (`ui/dataset.zod.ts`, the + * one accept set in `data/analytics-column-reference.ts`), so `''` is refused + * at parse. On ONE sub-shape that refusal has a lossless repair: a measure + * with `aggregate: 'count'` and `field: ''`. The dataset's own refinement + * already read `''` as "no field" on a count (only `count` may omit it), the + * analytics dataset door never judged it (it skips an empty `field`), and the + * measure compiled to `sql: m.field ?? '*'` with the `''` intact — answered on + * SQLite's native-SQL path as `COUNT()` (its row count) and refused on the + * ObjectQL path. Without the key it compiles to `COUNT(*)`: the row count both + * meant. Its producer is named: Studio's dataset inspector seeds every new + * measure row with `field: ''`, so a count whose Field box was left blank was + * stored that way. + * + * Scope — this sub-shape and nothing else. A non-count measure with `''` (the + * refinement already refused it: only `count` may omit `field`), a dimension + * with `''` (it groups by nothing), a padded value and every expression have no + * working row or no mechanical rewrite; they stay as stored and the D3 entry + * `dataset-member-field-expression-refused` carries them. + * + * Retired from the load path: an author is refused at parse with the + * prescription to omit the key; data at rest and `os migrate meta` replay it. + */ +const datasetCountMeasureEmptyFieldRemoved: MetadataConversion = { + id: 'dataset-count-measure-empty-field-removed', + toMajor: 18, + retiredFromLoadPath: true, + retiredAfter: '17.5.0', + surface: 'dataset.measures[].field (aggregate count, empty string)', + summary: + "a `count` dataset measure's empty `field` is removed: a count with no `field` counts rows, which " + + "is what the empty string compiled to, and a measure's `field` is now a column reference that " + + 'refuses an empty string', + apply(stack, emit) { + return mapCollection(stack, 'datasets', (dataset, path) => { + const measures = dataset.measures; + if (!Array.isArray(measures)) return dataset; + let changed = false; + const next = measures.map((m, i) => { + if (!isDict(m) || m.aggregate !== 'count' || m.field !== '') return m; + changed = true; + return stripKeys(m, ['field'], emit, `${path}.measures[${i}]`); + }); + return changed ? { ...dataset, measures: next } : dataset; + }); + }, + fixture: { + before: { + datasets: [{ + name: 'deal_metrics', + label: 'Deal Metrics', + object: 'deal', + // A dimension's empty `field` has no lossless rewrite: left as stored. + dimensions: [{ name: 'stage', field: 'stage' }, { name: 'blank_axis', field: '' }], + measures: [ + // The Studio-seeded shape: a count whose Field box was left blank. + { name: 'deal_count', aggregate: 'count', field: '' }, + { name: 'won_count', aggregate: 'count', field: '', filter: { stage: 'won' } }, + // Untouched: a count over `*`, a count with no `field`, a count over a + // column, and a `sum` over `''` (refused already, no lossless rewrite). + { name: 'star_count', aggregate: 'count', field: '*' }, + { name: 'row_count', aggregate: 'count' }, + { name: 'owner_count', aggregate: 'count', field: 'owner' }, + { name: 'blank_sum', aggregate: 'sum', field: '' }, + ], + }], + }, + after: { + datasets: [{ + name: 'deal_metrics', + label: 'Deal Metrics', + object: 'deal', + dimensions: [{ name: 'stage', field: 'stage' }, { name: 'blank_axis', field: '' }], + measures: [ + { name: 'deal_count', aggregate: 'count' }, + { name: 'won_count', aggregate: 'count', filter: { stage: 'won' } }, + { name: 'star_count', aggregate: 'count', field: '*' }, + { name: 'row_count', aggregate: 'count' }, + { name: 'owner_count', aggregate: 'count', field: 'owner' }, + { name: 'blank_sum', aggregate: 'sum', field: '' }, + ], + }], + }, + // One per removed `field`: the two blank counts. + expectedNotices: 2, + }, +}; + /** * `element:filter` — the whole element retired (protocol 18, #9220, ADR-0049 * enforce-or-remove at ELEMENT grain). @@ -13595,6 +13685,7 @@ const MAJOR_18_CONVERSIONS: readonly OrderedConversion[] = [ { conversion: currencyConfigPrecisionRemoved, order: 41 }, { conversion: dashboardRefreshIntervalToRefreshIntervalSeconds, order: 24 }, { conversion: dashboardWidgetChartConfigStructureRemoved, order: 33 }, + { conversion: datasetCountMeasureEmptyFieldRemoved, order: 54 }, { conversion: elementFilterRemoved, order: 4 }, { conversion: elementFormRemoved, order: 5 }, { conversion: elementInputTargetVariableRemoved, order: 3 }, diff --git a/packages/spec/src/data/analytics-column-reference.ts b/packages/spec/src/data/analytics-column-reference.ts new file mode 100644 index 00000000000..436b0518c4b --- /dev/null +++ b/packages/spec/src/data/analytics-column-reference.ts @@ -0,0 +1,69 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The COLUMN REFERENCE of the analytics author surface, declared once (ADR-0021 + * "zero raw SQL / zero raw expressions"; ADR-0049 enforce-or-remove). + * + * Two layers name a column, and they name it in ONE value at two depths: the + * dataset compiler copies a dataset dimension's or measure's `field` into the + * `sql` of the cube member it compiles to, verbatim. So the two slots take one + * accept set, from this module: + * + * - the cube layer — `MetricSchema.sql` / `DimensionSchema.sql` + * (`./analytics.zod.ts`, as its `CUBE_MEMBER_SQL`; #20943); + * - the dataset layer — `DatasetMeasureSchema.field` / + * `DatasetDimensionSchema.field` (`../ui/dataset.zod.ts`; #21220). + * + * Admitted, and parsed byte-identically to before: a column of the object + * (`amount`), and a relationship path of bare identifiers ending in one + * (`account.amount`, `account.owner.region` — the chain + * `NativeSQLStrategy#qualifyAndRegisterJoin` lowers into its LEFT JOINs and + * the dataset compiler checks against `Dataset.include`). The path half is the + * pattern the readers already use to tell a column path from an expression: + * `IDENTIFIER_PATH` in `native-sql-strategy.ts`, and the field-level read + * gate's bare-identifier / identifier-path pair in `analytics-service.ts`. + * + * Everything else is refused at parse: a `CASE WHEN …`, an aggregate or a + * ratio of aggregates, a quoted or `$`-prefixed spelling, a padded or empty + * string. Such a value names no single field, so no platform check could judge + * which fields it reads. + * + * ## The row wildcard, and the one restriction stated here + * + * `'*'` is the row wildcard — what a `count` aggregates (`COUNT(*)`), reading + * no field value. {@link ANALYTICS_COLUMN_REFERENCE} admits it, for the slots + * whose ruling admits it: both cube members (maintainer ruling D on #20943 named + * one accept set "on a measure and a dimension alike") and a dataset MEASURE. + * {@link ANALYTICS_COLUMN_PATH} is the same path WITHOUT that arm, for the one + * slot where the wildcard has no meaning: a dataset DIMENSION. Grouping by + * every column at once is not an axis, and the runtime never answered one — + * measured on #21220 at `POST /analytics/dataset/query`, a dimension whose + * `field` is `'*'` compiled to `SELECT * AS … GROUP BY *` on the native-SQL + * strategy and to `groupBy: ['*']` on the ObjectQL one, and was answered + * `500 DATABASE_ERROR` on both. Both patterns are built from the one + * {@link COLUMN_PATH} source below: one pattern, one stated restriction, never a + * second copy that can drift. + * + * They are `RegExp`s for `.regex()`, never refinements, so the published JSON + * Schema carries each as a `pattern`: a document validated against + * `json-schema/**` is judged as the parse judges it. + * + * A module of its own, and outside the `data` barrel, so the two layers share + * one declaration without it becoming published API (the + * `../ui/analytics-carrier-filter.ts` precedent). + */ + +/** A bare identifier, then zero or more `.identifier` hops — the column path. */ +const COLUMN_PATH = '[A-Za-z_][A-Za-z0-9_]*(?:\\.[A-Za-z_][A-Za-z0-9_]*)*'; + +/** + * A column of the object, a relationship path ending in one, or the row + * wildcard `'*'` — a cube member's `sql` and a dataset measure's `field`. + */ +export const ANALYTICS_COLUMN_REFERENCE = new RegExp(`^(?:\\*|${COLUMN_PATH})$`); + +/** + * {@link ANALYTICS_COLUMN_REFERENCE} without the row wildcard — a column of the + * object or a relationship path ending in one: a dataset dimension's `field`. + */ +export const ANALYTICS_COLUMN_PATH = new RegExp(`^${COLUMN_PATH}$`); diff --git a/packages/spec/src/data/analytics.zod.ts b/packages/spec/src/data/analytics.zod.ts index 6f03ec88387..efde10ad72e 100644 --- a/packages/spec/src/data/analytics.zod.ts +++ b/packages/spec/src/data/analytics.zod.ts @@ -22,6 +22,7 @@ import { DateGranularity } from './query.zod'; import { lazySchema } from '../shared/lazy-schema'; import { strictObject } from '../shared/strict-object'; import { 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', @@ -215,6 +216,12 @@ const CUBE_DIMENSION_NAME_REMOVED = cubeMemberNameRemoved('dimensions. { success: boolean; error?: { issues: Issue[] } } }, value: unknown) => { + const r = schema.safeParse(value); + expect(r.success, JSON.stringify(value)).toBe(false); + return r.error!.issues; +}; + +describe('dataset field — every column reference parses byte-identically to before', () => { + it('a dimension admits a bare column and relationship paths', () => { + for (const field of ['stage', 'account.region', 'account.owner.region', '_private', 'Region_2']) { + const dataset = { ...BASE, dimensions: [{ name: 'axis', field, type: 'string' }], measures: [COUNT] }; + const r = DatasetSchema.safeParse(dataset); + expect(r.success, `${field}: ${JSON.stringify(r.error?.issues)}`).toBe(true); + if (r.success) expect(r.data).toEqual(dataset); + } + }); + + it('a measure admits a bare column, relationship paths and `*` — and a count may still omit `field`', () => { + const measures = [ + COUNT, + { name: 'star_count', aggregate: 'count', field: '*' }, + { name: 'revenue', aggregate: 'sum', field: 'amount' }, + { name: 'account_revenue', aggregate: 'sum', field: 'account.annual_revenue' }, + { name: 'regions', aggregate: 'count_distinct', field: 'account.owner.region' }, + ]; + const dataset = { ...BASE, dimensions: [STAGE], measures }; + const r = DatasetSchema.safeParse(dataset); + expect(r.success, JSON.stringify(r.error?.issues)).toBe(true); + if (r.success) expect(r.data).toEqual(dataset); + }); +}); + +describe('dataset field — a non-column value is refused at parse, at its path, with the prescription', () => { + it('a dimension refuses every non-column value — and `*` — at `dimensions.N.field`', () => { + for (const field of [...NON_COLUMNS, '*']) { + const issues = refusalOf(DatasetSchema, { + ...BASE, + dimensions: [STAGE, { name: 'bucket', field, type: 'string' }], + measures: [COUNT], + }); + expect(issues.map((i) => [i.code, i.path]), field).toEqual([['invalid_format', ['dimensions', 1, 'field']]]); + expect(issues[0]!.message.startsWith(DIMENSION_FIRST_SENTENCE), field).toBe(true); + expect(issues[0]!.message).toMatch(/ADR-0021/); + } + }); + + it('a measure refuses every non-column value at `measures.N.field`, naming the ADR-0021 form', () => { + for (const field of NON_COLUMNS) { + const issues = refusalOf(DatasetSchema, { + ...BASE, + dimensions: [STAGE], + measures: [COUNT, { name: 'computed', aggregate: 'sum', field }], + }); + // An empty `field` on a `sum` also fails the "requires `field`" rule — + // the column-reference issue is the one at the path. + const atField = issues.filter((i) => i.path.join('.') === 'measures.1.field'); + expect(atField.map((i) => i.code), field).toEqual(['invalid_format']); + expect(atField[0]!.message.startsWith(MEASURE_FIRST_SENTENCE), field).toBe(true); + expect(atField[0]!.message).toMatch(DATASET_FORM); + } + }); + + it('the refusal does not depend on the aggregate — a count with an empty or expression `field` is refused too', () => { + for (const field of ['', 'COUNT(*)', 'DISTINCT owner']) { + const issues = refusalOf(DatasetSchema, { + ...BASE, + dimensions: [STAGE], + measures: [{ name: 'row_count', aggregate: 'count', field }], + }); + expect(issues.map((i) => [i.code, i.path]), field).toEqual([['invalid_format', ['measures', 0, 'field']]]); + } + }); + + it('the member schemas refuse it on their own, at `field`', () => { + expect(refusalOf(DatasetDimensionSchema, { name: 'bucket', field: 'amount * 2' }).map((i) => [i.code, i.path])) + .toEqual([['invalid_format', ['field']]]); + expect(refusalOf(DatasetMeasureSchema, { name: 'computed', aggregate: 'sum', field: 'amount * 2' }).map((i) => [i.code, i.path])) + .toEqual([['invalid_format', ['field']]]); + }); +}); + +describe('dataset field — ONE pattern, published as the JSON Schema `pattern`', () => { + const patternOf = (schema: z.ZodType) => + (z.toJSONSchema(schema, { io: 'input', unrepresentable: 'any' }) as { + properties: Record; + }).properties; + + it("a measure's `field` carries the cube member's own `sql` pattern; a dimension's is that path without the `*` arm", () => { + const cubeSql = patternOf(MetricSchema).sql!.pattern!; + const measureField = patternOf(DatasetMeasureSchema).field!.pattern!; + const dimensionField = patternOf(DatasetDimensionSchema).field!.pattern!; + expect(measureField).toBe(cubeSql); + // The one stated restriction: the row-wildcard arm, and nothing else. + expect(cubeSql.startsWith('^(?:\\*|') && cubeSql.endsWith(')$')).toBe(true); + expect(dimensionField).toBe(`^${cubeSql.slice('^(?:\\*|'.length, -')$'.length)}$`); + }); + + it('each pattern judges values as the parse does', () => { + const measure = new RegExp(patternOf(DatasetMeasureSchema).field!.pattern!); + const dimension = new RegExp(patternOf(DatasetDimensionSchema).field!.pattern!); + for (const v of ['amount', 'account.amount', 'account.owner.region']) { + expect(measure.test(v), v).toBe(true); + expect(dimension.test(v), v).toBe(true); + } + expect(measure.test('*')).toBe(true); + expect(dimension.test('*')).toBe(false); + for (const v of NON_COLUMNS) { + expect(measure.test(v), v).toBe(false); + expect(dimension.test(v), v).toBe(false); + } + }); +}); + +describe('dataset field — every door that carries a dataset refuses a non-column field', () => { + const withExpression = { + ...BASE, + dimensions: [STAGE], + measures: [COUNT, { name: 'double_amount', aggregate: 'sum', field: 'amount * 2' }], + }; + + it('the `dataset` write door (the registry binding) refuses it', () => { + // What a `PUT /api/v1/meta/dataset` body is validated against; a rebinding + // to another shape would pass the pins above and still accept the + // expression in production. + const door = getMetadataTypeSchema('dataset'); + expect(door).toBe(DatasetSchema); + const issues = refusalOf(door!, withExpression); + expect(issues.map((i) => [i.code, i.path])).toEqual([['invalid_format', ['measures', 1, 'field']]]); + }); + + it('the authoring door, defineStack, refuses it with the STACK_SCHEMA_INVALID envelope', () => { + const stack = (dataset: Record) => ({ + manifest: { id: 'com.example.dataset-field', name: 'dataset_field', version: '1.0.0', type: 'app' }, + datasets: [dataset], + }); + let thrown: unknown; + try { + defineStack(stack(withExpression) as never); + } catch (e) { + thrown = e; + } + const refusal = thrown as { code?: string; status?: number; issues?: Issue[] }; + expect(refusal?.code).toBe('STACK_SCHEMA_INVALID'); + expect(refusal?.status).toBe(422); + expect(refusal.issues?.map((i) => i.path)).toEqual([['datasets', 0, 'measures', 1, 'field']]); + expect(refusal.issues?.[0]?.message.startsWith(MEASURE_FIRST_SENTENCE)).toBe(true); + // CONTROL: the same stack with a column `field` is accepted by the same door. + expect(() => defineStack(stack({ ...BASE, dimensions: [STAGE], measures: [COUNT] }) as never)).not.toThrow(); + }); +}); + +describe('dataset field — ADR-0087 registration', () => { + it('carries the family D3 entry under step 18, linked to its one D2 repair, with no retired-key row', () => { + const d3 = MIGRATIONS_BY_MAJOR[18]!.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); + // The D2 half — a `count` measure's empty `field` — is pinned on a stored + // row in `conversions/dataset-count-measure-empty-field-removed.test.ts`. + expect(d3!.conversionIds).toEqual(['dataset-count-measure-empty-field-removed']); + expect(MIGRATIONS_BY_MAJOR[18]!.conversionIds).toContain('dataset-count-measure-empty-field-removed'); + // No key left the shape, so no `${defKey}:${name}` entry is owed. + expect(RETIRED_KEYS_BY_MAJOR[18]!.filter((k) => /Dataset(Dimension|Measure):field$/.test(k))).toEqual([]); + }); +}); diff --git a/packages/spec/src/ui/dataset.zod.ts b/packages/spec/src/ui/dataset.zod.ts index b9c62c41d69..f1b8a2b1a42 100644 --- a/packages/spec/src/ui/dataset.zod.ts +++ b/packages/spec/src/ui/dataset.zod.ts @@ -9,6 +9,7 @@ import { analyticsCarrierFilter } from './analytics-carrier-filter'; import { SnakeCaseIdentifierSchema } from '../shared/identifiers.zod'; import { I18nLabelSchema } from './i18n.zod'; import { AggregationFunction, DateGranularity } from '../data/query.zod'; +import { ANALYTICS_COLUMN_PATH, ANALYTICS_COLUMN_REFERENCE } from '../data/analytics-column-reference'; /** * Analytics Dataset — the one semantic layer (ADR-0021). @@ -79,6 +80,53 @@ const DATASET_NO_SQL = + '`derived: { op, of: [...] }`, which combines OTHER measures in this dataset by name. ' + 'Joins are compiled from `Dataset.include` — you never write an ON clause.'; +/** + * A dataset dimension's and measure's `field` is a COLUMN REFERENCE, never a + * SQL expression (#21220; ADR-0021 "zero raw SQL / zero raw expressions", + * ADR-0049 enforce-or-remove) — the accept set the cube members it compiles to + * already hold (#20943), from the one shared declaration in + * `../data/analytics-column-reference.ts`: the dataset compiler copies `field` + * into the cube member's `sql` verbatim, so the two slots are one value. + * + * The module header said so from the start ("no raw SQL"), and the field's own + * description named a field or a relationship path, but the slot was a bare + * `z.string()` and parsed anything. The runtime has refused an expression + * `field` at the analytics dataset door since #21190 (`PERMISSION_DENIED` / + * 403, inline or saved), so an expression could be saved and never answered — + * declared, never enforced. That door stays, as defence in depth for a dataset + * that reaches the service without meeting this parse. It never judged an + * empty `field` (it skips one); a stored `count` measure with `field: ''` is + * repaired on load by the D2 conversion `dataset-count-measure-empty-field-removed`. + * + * A measure admits the row wildcard `'*'` (a count's `COUNT(*)`); a dimension + * does not — the restriction, and its measurement, are stated on + * {@link ANALYTICS_COLUMN_PATH}. An empty string is refused on both: on a + * measure the wildcard's spelling is `'*'` or no `field` at all, and on a + * dimension it names nothing to group by. + */ +const DATASET_FIELD_EXPRESSION_REFUSED = + 'A SQL expression there names no single field, so no platform check can judge which fields it reads, ' + + 'and the analytics dataset door refuses it on every query (ADR-0021: the dataset layer takes no raw SQL ' + + 'and no raw expressions; ADR-0049 enforce-or-remove).'; + +const DATASET_DIMENSION_FIELD_NOT_COLUMN = + '`dimensions[].field` is a column reference: a field of the dataset\'s object (`stage`), or a relationship ' + + 'path ending in one (`account.region`) whose relationships are declared in `include`. ' + + `${DATASET_FIELD_EXPRESSION_REFUSED} Group by the column itself. \`'*'\` is no dimension: it names every ` + + 'column at once, which is not an axis. A bucket computed over a column\'s values (a CASE over them) has no ' + + 'expression form in the dataset layer: keep the bucket as a field of the object and name that field here.'; + +const DATASET_MEASURE_FIELD_NOT_COLUMN = + '`measures[].field` is a column reference: a field of the dataset\'s object (`amount`), a relationship path ' + + 'ending in one (`account.amount`) whose relationships are declared in `include`, or `\'*\'` for a count; ' + + `a count may also omit \`field\`. ${DATASET_FIELD_EXPRESSION_REFUSED} Name the column the measure ` + + 'aggregates. A derived value is declared in the ADR-0021 form, where the platform judges every field it ' + + 'reads: a conditional count or sum is a measure with its own structured `filter` ' + + '(`{ name: \'done_count\', aggregate: \'count\', filter: { status: \'done\' } }`), and a ratio, sum, ' + + 'difference or product of measures is `derived: { op, of: [...] }` over measures named in this dataset ' + + '(`{ name: \'done_rate\', derived: { op: \'ratio\', of: [\'done_count\', \'task_count\'] }, format: ' + + '\'0.0%\' }` — a 0–1 fraction, which the `%` pattern displays as a percentage).'; + /** * Dimension — a groupable axis (e.g. "region", "close_date by quarter"). */ @@ -121,8 +169,13 @@ export const DatasetDimensionSchema = lazySchema(() => strictObject({ * ending in a field — e.g. `account.region` or `account.owner.region` * (ADR-0071 multi-hop). The join chain is DERIVED from the relationship(s) * declared in `Dataset.include`; the author never writes a predicate. + * A column reference only (#21220, see `DATASET_FIELD_EXPRESSION_REFUSED`): + * a SQL expression, `'*'` or an empty string is refused at parse. */ - field: z.string().describe('Base field, or `relationship[.relationship].field` path').meta({ title: 'Field' }), + field: z.string() + .regex(ANALYTICS_COLUMN_PATH, { error: () => DATASET_DIMENSION_FIELD_NOT_COLUMN }) + .describe('Base field, or `relationship[.relationship].field` path. A column reference, never a SQL expression.') + .meta({ title: 'Field' }), type: z.enum(['string', 'number', 'date', 'boolean', 'lookup']).optional().meta({ title: 'Type' }), /** Default bucketing for date dimensions (day/week/month/quarter/year). */ dateGranularity: DateGranularity.optional().meta({ title: 'Date Granularity' }), @@ -185,8 +238,17 @@ export const DatasetMeasureSchema = lazySchema(() => strictObject({ /** Aggregation function — reuses the canonical query.zod enum. */ aggregate: AggregationFunction.optional().describe('Aggregation (sum/avg/count/...); omit when `derived` is set') .meta({ title: 'Aggregate' }), - /** Base field, or `relationship[.relationship].field` path. Optional for `count` (count(*)). */ - field: z.string().optional().describe('Aggregated field; optional for count(*)').meta({ title: 'Field' }), + /** + * Base field, or `relationship[.relationship].field` path, or `'*'`. Optional + * for `count` (count(*)). A column reference only (#21220, see + * `DATASET_FIELD_EXPRESSION_REFUSED`): a SQL expression or an empty string is + * refused at parse. + */ + field: z.string() + .regex(ANALYTICS_COLUMN_REFERENCE, { error: () => DATASET_MEASURE_FIELD_NOT_COLUMN }) + .optional() + .describe('Aggregated field: a base field, a relationship path, or "*"; optional for count(*). Never a SQL expression.') + .meta({ title: 'Field' }), /** * Measure-scoped filter (e.g. only won deals for "won_amount"). [#20080] A * list in the equality slot inside a nested relation is refused on save, as