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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
125 changes: 125 additions & 0 deletions .changeset/21220-dataset-field-column-reference.md
Original file line number Diff line number Diff line change
@@ -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.

<!-- adr-0087: registered dataset-member-field-expression-refused, dataset-count-measure-empty-field-removed -->
5 changes: 5 additions & 0 deletions content/docs/data-modeling/analytics.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
8 changes: 4 additions & 4 deletions content/docs/references/ui/dataset.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ const result = DatasetSchema.parse(data);
| :--- | :--- | :--- | :--- |
| **name** | `string` | ✅ | Dimension name — referenced by presentations |
| **label** | `string \| Record<string, string>` | 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 | |

Expand All @@ -83,7 +83,7 @@ const result = DatasetSchema.parse(data);
| **name** | `string` | ✅ | Measure name — e.g. "revenue"; defined once |
| **label** | `string \| Record<string, string>` | 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) |
Expand All @@ -108,7 +108,7 @@ const result = DatasetSchema.parse(data);
| :--- | :--- | :--- | :--- |
| **name** | `string` | ✅ | Dimension name — referenced by presentations |
| **label** | `string \| Record<string, string>` | 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 | |

Expand All @@ -124,7 +124,7 @@ const result = DatasetSchema.parse(data);
| **name** | `string` | ✅ | Measure name — e.g. "revenue"; defined once |
| **label** | `string \| Record<string, string>` | 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) |
Expand Down
59 changes: 41 additions & 18 deletions packages/rest/src/analytics-16019-driver-declared-fault.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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';
Expand Down Expand Up @@ -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 () => {
Expand All @@ -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',
Expand All @@ -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);
});

Expand Down
Loading
Loading