Skip to content

Commit 2e57fa2

Browse files
committed
feat(spec): a dataset count measure's empty field is dropped by a D2 conversion
dataset-count-measure-empty-field-removed (retiredFromLoadPath, retiredAfter 17.5.0) drops field: '' from a count measure, which then compiles to COUNT(*); the D3 entry links it and states that the dataset door never judged an empty field. Changeset, ledger notes and rationale fragment corrected to match. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UtnxvdiN376GF3sgXwAw4d
1 parent 494acb0 commit 2e57fa2

8 files changed

Lines changed: 299 additions & 48 deletions

File tree

‎.changeset/21220-dataset-field-column-reference.md‎

Lines changed: 27 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,11 @@ is `'*'` on a dimension.
2424
Why: the dataset layer was declared to take no raw SQL (ADR-0021 "zero raw SQL /
2525
zero raw expressions") and `field` was documented as a field or a relationship
2626
path, but it was a bare string and parsed anything. The analytics dataset door
27-
already refused a non-column `field` on every query (`PERMISSION_DENIED` / 403,
27+
already refused an expression `field` on every query (`PERMISSION_DENIED` / 403,
2828
inline or saved), so such a dataset could be saved and never answered — declared,
29-
never enforced (ADR-0049). The accept set is the one the cube members a dataset
29+
never enforced (ADR-0049). That door never judged an empty `field`: it skips one,
30+
which is how a `count` measure with `field: ''` kept counting rows on SQLite's
31+
native-SQL path (the D2 repair below). The accept set is the one the cube members a dataset
3032
compiles to already hold: the dataset compiler copies `field` into the member's
3133
`sql` verbatim, and both now read one shared declaration. `'*'` is refused on a
3234
dimension because grouping by every column is no axis — both analytics strategies
@@ -66,7 +68,11 @@ keep the bucket as a field of the object and name that field.
6668

6769
**The one-line fix:** parse each dataset; every refusal at `…field` is one member
6870
to change — name the column, omit `field` on a plain count (never `field: ''`),
69-
or move the computation to a measure `filter` or a `derived` measure.
71+
or move the computation to a measure `filter` or a `derived` measure. The one
72+
mechanical case is done for you: `os migrate meta --from 17` lists, and every
73+
stored-row rehydration replays, the D2 conversion
74+
`dataset-count-measure-empty-field-removed`, which drops a `count` measure's empty
75+
`field` (it still counts rows). Nothing else has a mechanical rewrite.
7076

7177
**What an author who still writes it sees.** `DatasetSchema`, `defineStack({
7278
datasets })` (`STACK_SCHEMA_INVALID` / 422), the `dataset` write door and
@@ -82,11 +88,16 @@ with the prescription. `tsc` does not: the key's type is still `string`.
8288
cube layer's `CUBE_MEMBER_SQL` is that same `RegExp`. A dimension's pattern is
8389
the same column path without the `'*'` arm. A column reference parses
8490
byte-identically to before.
85-
- **ADR-0087.** The D3 entry `dataset-member-field-expression-refused`, with its
86-
step-18 rationale fragment. No D2 conversion — an expression has no mechanical
87-
rewrite into a column — and no `RETIRED_KEYS_BY_MAJOR` row: no key left the
88-
shape, so the authorable-surface, api-surface and JSON-schema manifest ratchets
89-
are unchanged.
91+
- **ADR-0087.** D2 carries the one lossless repair: the conversion
92+
`dataset-count-measure-empty-field-removed` (`retiredFromLoadPath`, so an author
93+
is refused at parse while stored rows and `os migrate meta` replay it) drops a
94+
`count` measure's `field: ''`, which compiles to `COUNT(*)` without it. The D3
95+
entry `dataset-member-field-expression-refused`, linked to that conversion and
96+
with its step-18 rationale fragment, carries the rest — a non-count measure or a
97+
dimension with `''` and every expression have no mechanical rewrite into a
98+
column. No `RETIRED_KEYS_BY_MAJOR` row: no key left the shape, so the
99+
authorable-surface, api-surface and JSON-schema manifest ratchets are
100+
unchanged.
90101
- **Liveness.** The `dataset` ledger rows `dimensions.field` and
91102
`measures.field` stay `live`, re-verified, with the narrowing recorded.
92103
- **Docs.** The `ui/dataset` reference page is regenerated.
@@ -102,10 +113,13 @@ with the prescription. `tsc` does not: the key's type is still `string`.
102113
re-pinned: the service door's test builds them unparsed, and the REST route's
103114
test now expects the route's `400`.
104115
- Studio's dataset inspector (objectui) seeds a new dimension or measure row with
105-
`field: ''`; a measure left that way as a plain count is now refused at save
106-
with the prescription to omit the key. It parsed before; its query answered
107-
`500` on the ObjectQL path (SQLite's native-SQL path happened to accept the
108-
`COUNT()` it compiled to).
116+
`field: ''`. A plain count saved that way parsed before; its query answered
117+
`500` on the ObjectQL path, while SQLite's native-SQL path accepted the
118+
`COUNT()` it compiled to. A row already stored that way is repaired on load by
119+
the D2 conversion above. A NEW save of that shape is refused at the save door
120+
with the prescription to omit the key, because the write path parses with the
121+
current schema and replays no conversion; the producer-side change is
122+
objectui's.
109123
- Out-of-repo authored datasets: NOT MEASURED.
110124

111-
<!-- adr-0087: registered dataset-member-field-expression-refused -->
125+
<!-- adr-0087: registered dataset-member-field-expression-refused, dataset-count-measure-empty-field-removed -->

‎packages/spec/liveness/dataset.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@
5353
"field": {
5454
"status": "live",
5555
"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)",
56-
"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 a non-column `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`; no D2 conversion. 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).",
56+
"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).",
5757
"verifiedAt": "2026-10-01"
5858
},
5959
"type": {
@@ -94,7 +94,7 @@
9494
"field": {
9595
"status": "live",
9696
"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)`)",
97-
"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 }`). Still LIVE, at the site above; the dataset door's 403 (#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).",
97+
"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).",
9898
"verifiedAt": "2026-10-01"
9999
},
100100
"filter": {
Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
import { describe, expect, it } from 'vitest';
4+
5+
import { MIGRATIONS_BY_MAJOR } from '../migrations/registry.js';
6+
import { DatasetSchema } from '../ui/dataset.zod.js';
7+
import { applyConversions } from './apply.js';
8+
import { ALL_CONVERSIONS, CONVERSIONS_BY_MAJOR } from './registry.js';
9+
import { applyConversionsToStoredItem } from './stored.js';
10+
import type { ConversionNotice, ConversionTodoNotice } from './types.js';
11+
12+
/**
13+
* [#21220] `dataset-count-measure-empty-field-removed` — the D2 half of the
14+
* dataset `field` narrowing: the one sub-shape with a working row and a
15+
* lossless repair.
16+
*
17+
* A dataset measure's `field` is a column reference, so `''` is refused at
18+
* parse. A `count` measure with `field: ''` (the shape Studio's dataset
19+
* inspector stores when the Field box is left blank) parsed before, the
20+
* dataset door skipped it, and it compiled to the row count on SQLite's native
21+
* path. Dropping the key compiles it to `COUNT(*)`, the same row count.
22+
*
23+
* The fixture pair in `conversions.test.ts` proves before → after over the
24+
* whole table. This file pins it on a STORED row, the seam that replays it:
25+
* the key is dropped and the row then parses; everything outside the
26+
* sub-shape is the same reference and stays refused where it was.
27+
*/
28+
29+
const ID = 'dataset-count-measure-empty-field-removed';
30+
const D3_ID = 'dataset-member-field-expression-refused';
31+
32+
function storedDataset(measures: unknown[], dimensions: unknown[] = [{ name: 'stage', field: 'stage' }]) {
33+
return { name: 'deal_metrics', label: 'Deal Metrics', object: 'deal', dimensions, measures };
34+
}
35+
36+
function convertStored(row: Record<string, unknown>) {
37+
const notices: ConversionNotice[] = [];
38+
const todos: ConversionTodoNotice[] = [];
39+
const item = applyConversionsToStoredItem('dataset', row, {
40+
onNotice: (n) => notices.push(n),
41+
onTodo: (t) => todos.push(t),
42+
});
43+
return { item, notices: notices.filter((n) => n.conversionId === ID), allNotices: notices, todos };
44+
}
45+
46+
function fieldIssuePaths(row: unknown): string[] {
47+
const r = DatasetSchema.safeParse(row);
48+
return r.success ? [] : r.error.issues.map((i) => i.path.join('.')).filter((p) => p.endsWith('field'));
49+
}
50+
51+
describe('[#21220] dataset-count-measure-empty-field-removed (ADR-0087 D2)', () => {
52+
it('is registered for protocol 18, retired from the authoring load path, and linked from its D3 entry', () => {
53+
const entry = ALL_CONVERSIONS.find((c) => c.id === ID);
54+
expect(entry, 'the conversion is registered').toBeDefined();
55+
expect(entry!.toMajor).toBe(18);
56+
expect(entry!.retiredFromLoadPath).toBe(true);
57+
expect(CONVERSIONS_BY_MAJOR[18]!.map((c) => c.id)).toContain(ID);
58+
expect(MIGRATIONS_BY_MAJOR[18]!.conversionIds).toContain(ID);
59+
const d3 = MIGRATIONS_BY_MAJOR[18]!.semantic.find((s) => s.id === D3_ID);
60+
expect(d3?.conversionIds).toEqual([ID]);
61+
});
62+
63+
it('a stored count measure with `field: \'\'` loses the key, and the row then parses', () => {
64+
const row = storedDataset([
65+
{ name: 'deal_count', aggregate: 'count', field: '' },
66+
{ name: 'won_count', aggregate: 'count', field: '', filter: { stage: 'won' } },
67+
]);
68+
expect(fieldIssuePaths(row), 'refused before the replay').toEqual(['measures.0.field', 'measures.1.field']);
69+
const { item, notices, todos } = convertStored(row);
70+
const measures = (item as typeof row).measures as Array<Record<string, unknown>>;
71+
expect(measures).toEqual([
72+
{ name: 'deal_count', aggregate: 'count' },
73+
{ name: 'won_count', aggregate: 'count', filter: { stage: 'won' } },
74+
]);
75+
expect(notices.map((n) => [n.path, n.from, n.to])).toEqual([
76+
['datasets[0].measures[0].field', 'field', '(removed)'],
77+
['datasets[0].measures[1].field', 'field', '(removed)'],
78+
]);
79+
expect(todos).toEqual([]);
80+
expect(DatasetSchema.safeParse(item).success, 'parses after the replay').toBe(true);
81+
});
82+
83+
it('control: outside the sub-shape a row is the same reference — a sum or a dimension over `\'\'` stays refused', () => {
84+
const rows = [
85+
storedDataset([{ name: 'blank_sum', aggregate: 'sum', field: '' }]),
86+
storedDataset([{ name: 'row_count', aggregate: 'count' }], [{ name: 'blank_axis', field: '' }]),
87+
storedDataset([{ name: 'star_count', aggregate: 'count', field: '*' }]),
88+
storedDataset([{ name: 'row_count', aggregate: 'count' }]),
89+
storedDataset([{ name: 'owner_count', aggregate: 'count', field: 'owner' }]),
90+
storedDataset([{ name: 'padded_count', aggregate: 'count', field: ' ' }]),
91+
storedDataset([{ name: 'expr_count', aggregate: 'count', field: 'COUNT(*)' }]),
92+
];
93+
for (const row of rows) {
94+
const { item, allNotices, todos } = convertStored(row);
95+
expect(item, JSON.stringify(row.measures)).toBe(row);
96+
expect(allNotices).toEqual([]);
97+
expect(todos).toEqual([]);
98+
}
99+
expect(fieldIssuePaths(rows[0])).toEqual(['measures.0.field']);
100+
expect(fieldIssuePaths(rows[1])).toEqual(['dimensions.0.field']);
101+
});
102+
103+
it('is idempotent — the converted result replays to itself with no second notice', () => {
104+
const once = applyConversions(
105+
{ datasets: [storedDataset([{ name: 'deal_count', aggregate: 'count', field: '' }])] },
106+
{ includeRetired: true },
107+
);
108+
const notices: ConversionNotice[] = [];
109+
const twice = applyConversions(once, { includeRetired: true, onNotice: (n) => notices.push(n) });
110+
expect(twice).toBe(once);
111+
expect(notices).toEqual([]);
112+
});
113+
});

0 commit comments

Comments
 (0)