Skip to content

Commit 715d667

Browse files
os-zhuangclaude
andauthored
feat(studio): online dataset authoring — create form, derived measures, joined-query fix (#2248)
* feat(studio): online dataset authoring — create form, derived measures, joined-query fix Dogfooded online dataset (ADR-0021 semantic layer) authoring in Studio as a business user and fixed three issues found: - spec: add `dataset.form.ts` (sectioned create form + `object` picker) and register it in `METADATA_FORM_REGISTRY`. `dataset` was the only UI-authorable metadata type without a `defineForm` layout, so its create surface fell back to the flat auto-layout. - spec: make `DatasetMeasure.aggregate` optional so derived measures are authorable without a dummy aggregate (required for non-derived only, enforced in `superRefine`). Backward compatible. - service-analytics: qualify base-object columns in joined dataset queries to fix "ambiguous column name" 500s when base/joined tables share a column (e.g. `status`). Single-object cubes unchanged. Tests: spec 6624, service-analytics 151 (+4 regression), full build 76/76. Browser-verified: improved create form; joined+derived dataset query returns correct values (utilization = spent/budget = 70.0% / 97.8% / 40.0% / 96.0%). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore(spec): regenerate api-surface for new datasetForm export The public API-surface snapshot check flagged `+ datasetForm (const)`; regenerate api-surface.json to include the new export. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent b6a4972 commit 715d667

11 files changed

Lines changed: 200 additions & 4 deletions

File tree

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
---
2+
"@objectstack/spec": minor
3+
---
4+
5+
feat(spec): dataset authoring form + derived measures without a dummy aggregate
6+
7+
`dataset` was the only UI-authorable metadata type without a `defineForm`
8+
layout, so Studio's create surface fell back to the auto-generated flat layout
9+
(free-text `object`, no grouping). Adds `dataset.form.ts` (registered in
10+
`METADATA_FORM_REGISTRY`): sectioned Basics / Source / Dimensions / Measures
11+
with an `object` picker (`ref:object`) and guidance — matching the sibling
12+
`report` editor.
13+
14+
Also makes `DatasetMeasureSchema.aggregate` optional. A derived measure
15+
(`derived: { op, of }`) combines other measures by name and `aggregate` is
16+
ignored for it at compile time, but the schema still required it — so a derived
17+
measure failed validation unless you added a meaningless aggregate. `aggregate`
18+
is now required only for non-derived measures (enforced in the existing
19+
`superRefine`). Backward compatible: existing measures that carry an aggregate
20+
stay valid.
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
"@objectstack/service-analytics": patch
3+
---
4+
5+
fix(analytics): qualify base-object columns in joined dataset queries
6+
7+
A dataset that joins a related object (`include` + a `relationship.field`
8+
dimension/measure) emitted BARE base-table columns in SELECT/GROUP BY while the
9+
joined columns were alias-qualified. When the base and joined tables share a
10+
column name (e.g. both have `status`), the query failed at runtime with
11+
"ambiguous column name". `NativeSQLStrategy` now qualifies plain base-column
12+
identifiers with the base table when the cube has joins; single-object cubes
13+
are unchanged (byte-for-byte identical SQL).

packages/services/service-analytics/src/__tests__/native-sql-rls.test.ts

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,3 +108,51 @@ describe('NativeSQLStrategy — D-C RLS hardening', () => {
108108
expect(sql).toContain('$2');
109109
});
110110
});
111+
112+
describe('NativeSQLStrategy — base-column qualification under joins', () => {
113+
// Regression: a joined dataset whose base table and joined table share a
114+
// column name (e.g. `status`) produced `GROUP BY status` → "ambiguous column
115+
// name" at runtime. Base columns must be qualified with the base table when
116+
// the cube can join. (Found by dogfooding a joined dataset in Studio.)
117+
const joinCube: Cube = {
118+
name: 'sales',
119+
title: 'Sales',
120+
sql: 'opportunity',
121+
measures: { revenue: { name: 'revenue', label: 'Revenue', type: 'sum', sql: 'amount' } },
122+
dimensions: {
123+
status: { name: 'status', label: 'Status', type: 'string', sql: 'status' },
124+
region: { name: 'region', label: 'Region', type: 'string', sql: 'account.region' },
125+
},
126+
joins: { account: { name: 'account', relationship: 'many_to_one', sql: '' } },
127+
public: false,
128+
};
129+
130+
it('qualifies a base-table dimension with the base table when the cube has joins', async () => {
131+
const strategy = new NativeSQLStrategy();
132+
const ctx = ctxWith({
133+
getCube: (n) => (n === 'sales' ? joinCube : undefined),
134+
getAllowedRelationships: () => new Set(['account']),
135+
});
136+
const q: AnalyticsQuery = { cube: 'sales', measures: ['revenue'], dimensions: ['status', 'region'], timezone: 'UTC' };
137+
const { sql } = await strategy.generateSql(q, ctx);
138+
expect(sql).toContain('"opportunity"."status"'); // base column qualified (was bare `status`)
139+
expect(sql).toContain('"account"."region"'); // relationship column still qualified
140+
expect(sql).toContain('LEFT JOIN "account"');
141+
expect(sql).not.toMatch(/GROUP BY\s+status\b/); // no bare, ambiguous base column
142+
});
143+
144+
it('leaves base columns BARE for a single-object cube (no joins) — generated SQL unchanged', async () => {
145+
const soloCube: Cube = {
146+
name: 'tasks', title: 'Tasks', sql: 'task',
147+
measures: { c: { name: 'c', label: 'Count', type: 'count', sql: '*' } },
148+
dimensions: { status: { name: 'status', label: 'Status', type: 'string', sql: 'status' } },
149+
public: false,
150+
};
151+
const strategy = new NativeSQLStrategy();
152+
const ctx = ctxWith({ getCube: (n) => (n === 'tasks' ? soloCube : undefined) });
153+
const q: AnalyticsQuery = { cube: 'tasks', measures: ['c'], dimensions: ['status'], timezone: 'UTC' };
154+
const { sql } = await strategy.generateSql(q, ctx);
155+
expect(sql).toContain('GROUP BY status'); // bare — unchanged for single-object cubes
156+
expect(sql).not.toContain('"task"."status"');
157+
});
158+
});

packages/services/service-analytics/src/dataset-compiler.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,11 @@ export type RelationshipResolver = (
5858

5959
/** Map a dataset measure's aggregate to the Cube metric `type`. */
6060
function aggregateToMetricType(m: DatasetMeasure): Metric['type'] {
61+
// Only reached for non-derived measures, where the spec refinement guarantees
62+
// an aggregate; guard defensively so the type narrows from `optional`.
63+
if (!m.aggregate) {
64+
throw new Error(`[dataset-compiler] non-derived measure "${m.name}" has no aggregate`);
65+
}
6166
if (UNSUPPORTED_AGGREGATES.has(m.aggregate)) {
6267
throw new Error(
6368
`[dataset-compiler] measure "${m.name}" uses aggregate "${m.aggregate}" which is ` +

packages/services/service-analytics/src/strategies/native-sql-strategy.ts

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,19 @@ export class NativeSQLStrategy implements AnalyticsStrategy {
234234
joins: Map<string, string>,
235235
cube?: Cube,
236236
): string {
237-
if (!rawSql.includes('.')) return rawSql;
237+
if (!rawSql.includes('.')) {
238+
// Base-table column. When the cube can join other tables, a bare column
239+
// that also exists on a joined table (e.g. base `status` vs joined
240+
// `account.status`) makes the SQL engine raise "ambiguous column name".
241+
// Qualify plain identifiers with the base table; leave SQL expressions
242+
// and `*` untouched. Single-object cubes (no joins) keep bare columns so
243+
// their generated SQL is byte-for-byte unchanged.
244+
const canJoin = !!cube?.joins && Object.keys(cube.joins).length > 0;
245+
if (canJoin && /^[A-Za-z_][A-Za-z0-9_]*$/.test(rawSql)) {
246+
return `"${parentTable}"."${rawSql}"`;
247+
}
248+
return rawSql;
249+
}
238250
// Only the first dotted hop is supported (single-level relation).
239251
const [alias, ...rest] = rawSql.split('.');
240252
if (!alias || rest.length === 0) return rawSql;

packages/spec/api-surface.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3342,6 +3342,7 @@
33423342
"actionForm (const)",
33433343
"appForm (const)",
33443344
"dashboardForm (const)",
3345+
"datasetForm (const)",
33453346
"defineAction (function)",
33463347
"defineApp (function)",
33473348
"defineDataset (function)",

packages/spec/src/system/metadata-form-registry.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ import {
3737
viewForm,
3838
appForm,
3939
dashboardForm,
40+
datasetForm,
4041
actionForm,
4142
pageForm,
4243
reportForm,
@@ -67,6 +68,7 @@ export const METADATA_FORM_REGISTRY: Readonly<Record<string, FormView>> = Object
6768
field: fieldForm,
6869
hook: hookForm,
6970
report: reportForm,
71+
dataset: datasetForm,
7072
view: viewForm,
7173
app: appForm,
7274
dashboard: dashboardForm,
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
import { defineForm } from './view.zod';
4+
5+
/**
6+
* Form layout for the Dataset metadata editor (ADR-0021 analytics semantic layer).
7+
*
8+
* Bound to {@link DatasetSchema}. Until this entry existed, `dataset` was the
9+
* only UI-authorable metadata type WITHOUT a registered {@link FormView}, so the
10+
* metadata-admin create surface fell back to the auto-generated single-section
11+
* layout. That fallback (a) silently DROPPED the optional `include` (joins) and
12+
* `filter` (intrinsic scope) fields — making joined / scoped datasets
13+
* un-authorable online — and (b) rendered the base `object` and dimension/
14+
* measure `field`s as bare free-text inputs with no object context.
15+
*
16+
* This layout restores `include` + `filter`, groups the surface into sections
17+
* with guidance, and uses pickers (`ref:object`, `filter-builder` scoped to the
18+
* base object) so a business user can author a dataset without memorising
19+
* machine names. Mirrors {@link reportForm} — the sibling analytics editor.
20+
*/
21+
export const datasetForm = defineForm({
22+
schemaId: 'dataset',
23+
type: 'simple',
24+
sections: [
25+
{
26+
name: 'basics',
27+
label: 'Basics',
28+
description: 'Dataset identity.',
29+
columns: 2,
30+
fields: [
31+
{ field: 'name', type: 'text', colSpan: 1, required: true, immutable: true, helpText: 'snake_case unique identifier' },
32+
{ field: 'label', type: 'text', colSpan: 1, required: true, helpText: 'Display name' },
33+
{ field: 'description', type: 'textarea', colSpan: 2, helpText: 'What this dataset measures' },
34+
],
35+
},
36+
{
37+
name: 'source',
38+
label: 'Source',
39+
description: 'The base object, the relationships to join, and the dataset’s intrinsic scope. Joins are derived from the object graph — pick relationship (lookup / master_detail) names, never write an ON clause.',
40+
fields: [
41+
{ field: 'object', widget: 'ref:object', required: true, helpText: 'Base object — the FROM' },
42+
{ field: 'include', widget: 'string-tags', helpText: 'Relationship (lookup / master_detail) field names to join — enables `relationship.field` dimensions/measures (e.g. include "account" → group by account.region)' },
43+
{ field: 'filter', widget: 'filter-builder', dependsOn: 'object', helpText: 'Intrinsic scope filter (e.g. exclude soft-deleted records), ANDed into every query' },
44+
],
45+
},
46+
{
47+
name: 'dimensions',
48+
label: 'Dimensions',
49+
description: 'Groupable axes. Use a base field, or `relationship.field` (e.g. account.region) for a relationship included above.',
50+
fields: [
51+
{ field: 'dimensions', type: 'repeater', required: true, helpText: 'Each: name (referenced by presentations), field, type, and — for dates — a default bucketing granularity' },
52+
],
53+
},
54+
{
55+
name: 'measures',
56+
label: 'Measures',
57+
description: 'Aggregatable values defined once and referenced by name. A measure is sum/avg/count/… of a field; a derived measure combines other measures (ratio/sum/difference/product). Measure-scoped filters and derived ops are edited per-row in the dataset designer.',
58+
fields: [
59+
{ field: 'measures', type: 'repeater', required: true, helpText: 'Each: name, aggregate, field (optional for count), display format/currency, and a “certified” governance flag' },
60+
],
61+
},
62+
],
63+
});

packages/spec/src/ui/dataset.test.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,3 +88,26 @@ describe('DatasetSchema', () => {
8888
expect(d).toBe(base);
8989
});
9090
});
91+
92+
describe('DatasetSchema — derived measure aggregate optionality', () => {
93+
// Regression: a derived measure had to carry a meaningless `aggregate` or
94+
// validation failed, even though `aggregate` is ignored for derived measures.
95+
it('accepts a derived measure with NO aggregate', () => {
96+
expect(() =>
97+
DatasetSchema.parse({
98+
...base,
99+
measures: [
100+
{ name: 'won_amount', aggregate: 'sum', field: 'amount' },
101+
{ name: 'win_rate', derived: { op: 'ratio', of: ['won_amount', 'revenue'] } },
102+
{ name: 'revenue', aggregate: 'sum', field: 'amount' },
103+
],
104+
}),
105+
).not.toThrow();
106+
});
107+
108+
it('still rejects a NON-derived measure with no aggregate', () => {
109+
expect(() =>
110+
DatasetSchema.parse({ ...base, measures: [{ name: 'revenue', field: 'amount' }] }),
111+
).toThrowError(/requires `aggregate`/);
112+
});
113+
});

packages/spec/src/ui/dataset.zod.ts

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ export const DatasetMeasureSchema = lazySchema(() => z.object({
6565
name: SnakeCaseIdentifierSchema.describe('Measure name — e.g. "revenue"; defined once'),
6666
label: I18nLabelSchema.optional(),
6767
/** Aggregation function — reuses the canonical query.zod enum. */
68-
aggregate: AggregationFunction.describe('Aggregation (sum/avg/count/...)'),
68+
aggregate: AggregationFunction.optional().describe('Aggregation (sum/avg/count/...); omit when `derived` is set'),
6969
/** Base or `relationship.field`. Optional for `count` (count(*)). */
7070
field: z.string().optional().describe('Aggregated field; optional for count(*)'),
7171
/** Measure-scoped filter (e.g. only won deals for "won_amount"). */
@@ -152,8 +152,16 @@ export const DatasetSchema = lazySchema(() => z.object({
152152
// Derived measures may only reference OTHER measures declared in this dataset.
153153
for (const m of ds.measures) {
154154
if (!m.derived) {
155-
// A non-derived measure needs a field unless it is a plain count.
156-
if (!m.field && m.aggregate !== 'count') {
155+
// A non-derived measure must declare an aggregate (a derived measure
156+
// omits it — it combines other measures by name instead).
157+
if (!m.aggregate) {
158+
ctx.addIssue({
159+
code: 'custom',
160+
message: `measure "${m.name}" requires \`aggregate\` (or a \`derived\` spec)`,
161+
path: ['measures'],
162+
});
163+
} else if (!m.field && m.aggregate !== 'count') {
164+
// A non-derived measure needs a field unless it is a plain count.
157165
ctx.addIssue({
158166
code: 'custom',
159167
message: `measure "${m.name}" requires \`field\` (only \`count\` may omit it)`,

0 commit comments

Comments
 (0)