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
20 changes: 20 additions & 0 deletions .changeset/dataset-authoring-form.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
---
"@objectstack/spec": minor
---

feat(spec): dataset authoring form + derived measures without a dummy aggregate

`dataset` was the only UI-authorable metadata type without a `defineForm`
layout, so Studio's create surface fell back to the auto-generated flat layout
(free-text `object`, no grouping). Adds `dataset.form.ts` (registered in
`METADATA_FORM_REGISTRY`): sectioned Basics / Source / Dimensions / Measures
with an `object` picker (`ref:object`) and guidance — matching the sibling
`report` editor.

Also makes `DatasetMeasureSchema.aggregate` optional. A derived measure
(`derived: { op, of }`) combines other measures by name and `aggregate` is
ignored for it at compile time, but the schema still required it — so a derived
measure failed validation unless you added a meaningless aggregate. `aggregate`
is now required only for non-derived measures (enforced in the existing
`superRefine`). Backward compatible: existing measures that carry an aggregate
stay valid.
13 changes: 13 additions & 0 deletions .changeset/dataset-join-ambiguous-column.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
---
"@objectstack/service-analytics": patch
---

fix(analytics): qualify base-object columns in joined dataset queries

A dataset that joins a related object (`include` + a `relationship.field`
dimension/measure) emitted BARE base-table columns in SELECT/GROUP BY while the
joined columns were alias-qualified. When the base and joined tables share a
column name (e.g. both have `status`), the query failed at runtime with
"ambiguous column name". `NativeSQLStrategy` now qualifies plain base-column
identifiers with the base table when the cube has joins; single-object cubes
are unchanged (byte-for-byte identical SQL).
Original file line number Diff line number Diff line change
Expand Up @@ -108,3 +108,51 @@ describe('NativeSQLStrategy — D-C RLS hardening', () => {
expect(sql).toContain('$2');
});
});

describe('NativeSQLStrategy — base-column qualification under joins', () => {
// Regression: a joined dataset whose base table and joined table share a
// column name (e.g. `status`) produced `GROUP BY status` → "ambiguous column
// name" at runtime. Base columns must be qualified with the base table when
// the cube can join. (Found by dogfooding a joined dataset in Studio.)
const joinCube: Cube = {
name: 'sales',
title: 'Sales',
sql: 'opportunity',
measures: { revenue: { name: 'revenue', label: 'Revenue', type: 'sum', sql: 'amount' } },
dimensions: {
status: { name: 'status', label: 'Status', type: 'string', sql: 'status' },
region: { name: 'region', label: 'Region', type: 'string', sql: 'account.region' },
},
joins: { account: { name: 'account', relationship: 'many_to_one', sql: '' } },
public: false,
};

it('qualifies a base-table dimension with the base table when the cube has joins', async () => {
const strategy = new NativeSQLStrategy();
const ctx = ctxWith({
getCube: (n) => (n === 'sales' ? joinCube : undefined),
getAllowedRelationships: () => new Set(['account']),
});
const q: AnalyticsQuery = { cube: 'sales', measures: ['revenue'], dimensions: ['status', 'region'], timezone: 'UTC' };
const { sql } = await strategy.generateSql(q, ctx);
expect(sql).toContain('"opportunity"."status"'); // base column qualified (was bare `status`)
expect(sql).toContain('"account"."region"'); // relationship column still qualified
expect(sql).toContain('LEFT JOIN "account"');
expect(sql).not.toMatch(/GROUP BY\s+status\b/); // no bare, ambiguous base column
});

it('leaves base columns BARE for a single-object cube (no joins) — generated SQL unchanged', async () => {
const soloCube: Cube = {
name: 'tasks', title: 'Tasks', sql: 'task',
measures: { c: { name: 'c', label: 'Count', type: 'count', sql: '*' } },
dimensions: { status: { name: 'status', label: 'Status', type: 'string', sql: 'status' } },
public: false,
};
const strategy = new NativeSQLStrategy();
const ctx = ctxWith({ getCube: (n) => (n === 'tasks' ? soloCube : undefined) });
const q: AnalyticsQuery = { cube: 'tasks', measures: ['c'], dimensions: ['status'], timezone: 'UTC' };
const { sql } = await strategy.generateSql(q, ctx);
expect(sql).toContain('GROUP BY status'); // bare — unchanged for single-object cubes
expect(sql).not.toContain('"task"."status"');
});
});
5 changes: 5 additions & 0 deletions packages/services/service-analytics/src/dataset-compiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,11 @@ export type RelationshipResolver = (

/** Map a dataset measure's aggregate to the Cube metric `type`. */
function aggregateToMetricType(m: DatasetMeasure): Metric['type'] {
// Only reached for non-derived measures, where the spec refinement guarantees
// an aggregate; guard defensively so the type narrows from `optional`.
if (!m.aggregate) {
throw new Error(`[dataset-compiler] non-derived measure "${m.name}" has no aggregate`);
}
if (UNSUPPORTED_AGGREGATES.has(m.aggregate)) {
throw new Error(
`[dataset-compiler] measure "${m.name}" uses aggregate "${m.aggregate}" which is ` +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,19 @@ export class NativeSQLStrategy implements AnalyticsStrategy {
joins: Map<string, string>,
cube?: Cube,
): string {
if (!rawSql.includes('.')) return rawSql;
if (!rawSql.includes('.')) {
// Base-table column. When the cube can join other tables, a bare column
// that also exists on a joined table (e.g. base `status` vs joined
// `account.status`) makes the SQL engine raise "ambiguous column name".
// Qualify plain identifiers with the base table; leave SQL expressions
// and `*` untouched. Single-object cubes (no joins) keep bare columns so
// their generated SQL is byte-for-byte unchanged.
const canJoin = !!cube?.joins && Object.keys(cube.joins).length > 0;
if (canJoin && /^[A-Za-z_][A-Za-z0-9_]*$/.test(rawSql)) {
return `"${parentTable}"."${rawSql}"`;
}
return rawSql;
}
// Only the first dotted hop is supported (single-level relation).
const [alias, ...rest] = rawSql.split('.');
if (!alias || rest.length === 0) return rawSql;
Expand Down
1 change: 1 addition & 0 deletions packages/spec/api-surface.json
Original file line number Diff line number Diff line change
Expand Up @@ -3342,6 +3342,7 @@
"actionForm (const)",
"appForm (const)",
"dashboardForm (const)",
"datasetForm (const)",
"defineAction (function)",
"defineApp (function)",
"defineDataset (function)",
Expand Down
2 changes: 2 additions & 0 deletions packages/spec/src/system/metadata-form-registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import {
viewForm,
appForm,
dashboardForm,
datasetForm,
actionForm,
pageForm,
reportForm,
Expand Down Expand Up @@ -67,6 +68,7 @@ export const METADATA_FORM_REGISTRY: Readonly<Record<string, FormView>> = Object
field: fieldForm,
hook: hookForm,
report: reportForm,
dataset: datasetForm,
view: viewForm,
app: appForm,
dashboard: dashboardForm,
Expand Down
63 changes: 63 additions & 0 deletions packages/spec/src/ui/dataset.form.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

import { defineForm } from './view.zod';

/**
* Form layout for the Dataset metadata editor (ADR-0021 analytics semantic layer).
*
* Bound to {@link DatasetSchema}. Until this entry existed, `dataset` was the
* only UI-authorable metadata type WITHOUT a registered {@link FormView}, so the
* metadata-admin create surface fell back to the auto-generated single-section
* layout. That fallback (a) silently DROPPED the optional `include` (joins) and
* `filter` (intrinsic scope) fields — making joined / scoped datasets
* un-authorable online — and (b) rendered the base `object` and dimension/
* measure `field`s as bare free-text inputs with no object context.
*
* This layout restores `include` + `filter`, groups the surface into sections
* with guidance, and uses pickers (`ref:object`, `filter-builder` scoped to the
* base object) so a business user can author a dataset without memorising
* machine names. Mirrors {@link reportForm} — the sibling analytics editor.
*/
export const datasetForm = defineForm({
schemaId: 'dataset',
type: 'simple',
sections: [
{
name: 'basics',
label: 'Basics',
description: 'Dataset identity.',
columns: 2,
fields: [
{ field: 'name', type: 'text', colSpan: 1, required: true, immutable: true, helpText: 'snake_case unique identifier' },
{ field: 'label', type: 'text', colSpan: 1, required: true, helpText: 'Display name' },
{ field: 'description', type: 'textarea', colSpan: 2, helpText: 'What this dataset measures' },
],
},
{
name: 'source',
label: 'Source',
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.',
fields: [
{ field: 'object', widget: 'ref:object', required: true, helpText: 'Base object — the FROM' },
{ 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)' },
{ field: 'filter', widget: 'filter-builder', dependsOn: 'object', helpText: 'Intrinsic scope filter (e.g. exclude soft-deleted records), ANDed into every query' },
],
},
{
name: 'dimensions',
label: 'Dimensions',
description: 'Groupable axes. Use a base field, or `relationship.field` (e.g. account.region) for a relationship included above.',
fields: [
{ field: 'dimensions', type: 'repeater', required: true, helpText: 'Each: name (referenced by presentations), field, type, and — for dates — a default bucketing granularity' },
],
},
{
name: 'measures',
label: 'Measures',
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.',
fields: [
{ field: 'measures', type: 'repeater', required: true, helpText: 'Each: name, aggregate, field (optional for count), display format/currency, and a “certified” governance flag' },
],
},
],
});
23 changes: 23 additions & 0 deletions packages/spec/src/ui/dataset.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,3 +88,26 @@ describe('DatasetSchema', () => {
expect(d).toBe(base);
});
});

describe('DatasetSchema — derived measure aggregate optionality', () => {
// Regression: a derived measure had to carry a meaningless `aggregate` or
// validation failed, even though `aggregate` is ignored for derived measures.
it('accepts a derived measure with NO aggregate', () => {
expect(() =>
DatasetSchema.parse({
...base,
measures: [
{ name: 'won_amount', aggregate: 'sum', field: 'amount' },
{ name: 'win_rate', derived: { op: 'ratio', of: ['won_amount', 'revenue'] } },
{ name: 'revenue', aggregate: 'sum', field: 'amount' },
],
}),
).not.toThrow();
});

it('still rejects a NON-derived measure with no aggregate', () => {
expect(() =>
DatasetSchema.parse({ ...base, measures: [{ name: 'revenue', field: 'amount' }] }),
).toThrowError(/requires `aggregate`/);
});
});
14 changes: 11 additions & 3 deletions packages/spec/src/ui/dataset.zod.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ export const DatasetMeasureSchema = lazySchema(() => z.object({
name: SnakeCaseIdentifierSchema.describe('Measure name — e.g. "revenue"; defined once'),
label: I18nLabelSchema.optional(),
/** Aggregation function — reuses the canonical query.zod enum. */
aggregate: AggregationFunction.describe('Aggregation (sum/avg/count/...)'),
aggregate: AggregationFunction.optional().describe('Aggregation (sum/avg/count/...); omit when `derived` is set'),
/** Base or `relationship.field`. Optional for `count` (count(*)). */
field: z.string().optional().describe('Aggregated field; optional for count(*)'),
/** Measure-scoped filter (e.g. only won deals for "won_amount"). */
Expand Down Expand Up @@ -152,8 +152,16 @@ export const DatasetSchema = lazySchema(() => z.object({
// Derived measures may only reference OTHER measures declared in this dataset.
for (const m of ds.measures) {
if (!m.derived) {
// A non-derived measure needs a field unless it is a plain count.
if (!m.field && m.aggregate !== 'count') {
// A non-derived measure must declare an aggregate (a derived measure
// omits it — it combines other measures by name instead).
if (!m.aggregate) {
ctx.addIssue({
code: 'custom',
message: `measure "${m.name}" requires \`aggregate\` (or a \`derived\` spec)`,
path: ['measures'],
});
} else if (!m.field && m.aggregate !== 'count') {
// A non-derived measure needs a field unless it is a plain count.
ctx.addIssue({
code: 'custom',
message: `measure "${m.name}" requires \`field\` (only \`count\` may omit it)`,
Expand Down
1 change: 1 addition & 0 deletions packages/spec/src/ui/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export { reportForm } from './report.form';
export { viewForm } from './view.form';
export { appForm } from './app.form';
export { dashboardForm } from './dashboard.form';
export { datasetForm } from './dataset.form';
export { actionForm } from './action.form';
export { pageForm } from './page.form';
export * from './action.zod';
Expand Down