Skip to content

Commit b2ddd35

Browse files
committed
fix(analytics): review round 1 on the duration measure
- The field read gate resolves a duration measure first, as every other seam does: a dimension sharing its name no longer hides its two columns. - MySQL divides by a DOUBLE literal, so the arm is double precision like the other two; the docs and changeset claim live parity for SQLite and PostgreSQL only. - getMeta lists a registered dataset's duration measures with their aggregate, label and format, and every seam accepts the `<cube>.<name>` spelling it lists them under, its scoped filter included. - Docs: path and expression refusals happen at parse, not in the lint rule. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AKPixnkg91qCkQCVFDJuqW
1 parent eda01f1 commit b2ddd35

12 files changed

Lines changed: 205 additions & 46 deletions

File tree

‎.changeset/25778-dataset-duration-measure.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ Clause-②: yes (widening)
1010
Metadata-protocol: added
1111

1212
- **`@objectstack/spec`**: `DatasetMeasureSchema` gains an optional `duration: { from, to, unit }`, in place of `field`. `from` and `to` name two `datetime` fields of the dataset's own object by their bare names, `unit` is `seconds`, `minutes`, `hours` or `days`, and the measure's `aggregate` is `avg`, `min`, `max` or `sum` of `to` minus `from`. A relationship path, an expression or the row wildcard in either column, the same column twice, another unit, a counting aggregate, or a `field`, `derived` or `currency` beside it is refused at parse. New exports: `DatasetMeasureDurationSchema`, `DatasetDurationUnit`, `DATASET_DURATION_UNIT_SECONDS`, `DATASET_DURATION_AGGREGATES`, `DATASET_DURATION_FIELD_TYPE` and the `DatasetMeasureDuration` / `DatasetDurationUnit` types.
13-
- **`@objectstack/service-analytics`**: the dataset compile refuses a duration column declared anything but `datetime` (`400 DATASET_INVALID`, naming the column and its type). The native-SQL strategy computes the duration as one statement on SQLite, PostgreSQL and MySQL, with the same number on each: a record missing either instant is left out, a `to` before its `from` counts as negative, and over a group with no such record `sum` answers `0` and the others `null`. Measure filters, derived measures, ordering and the draft preview serve it. A query the native strategy does not serve (a time dimension bucketed by `granularity`, a field-to-field filter, an object served through engine middleware or an external datasource, a non-SQL datasource) is refused with `400 INVALID_FIELD` naming the measure. Its two columns pass the field-level read gate like any measure's field. The temporal half of the aggregate × field-type refusal now names the duration measure instead of a stored "days open" number.
13+
- **`@objectstack/service-analytics`**: the dataset compile refuses a duration column declared anything but `datetime` (`400 DATASET_INVALID`, naming the column and its type). The native-SQL strategy computes the duration as one statement on SQLite, PostgreSQL and MySQL, in double precision on each; SQLite and PostgreSQL are tested live to answer the same number: a record missing either instant is left out, a `to` before its `from` counts as negative, and over a group with no such record `sum` answers `0` and the others `null`. Measure filters, derived measures, ordering and the draft preview serve it. A query the native strategy does not serve (a time dimension bucketed by `granularity`, a field-to-field filter, an object served through engine middleware or an external datasource, a non-SQL datasource) is refused with `400 INVALID_FIELD` naming the measure. Its two columns pass the field-level read gate like any measure's field. The temporal half of the aggregate × field-type refusal now names the duration measure instead of a stored "days open" number.
1414
- **`@objectstack/lint`**: `os validate` refuses a duration column not declared `datetime` with the new gating rule `measure-duration-field-type-refused` (at `datasets[N].measures[M].duration.from` / `.to`), with an `os explain` entry. `dataset-field-unknown` now also reports a duration column the object does not declare.
1515

1616
What to do: nothing. A dataset without `duration` parses, compiles and answers exactly as before.

‎content/docs/data-modeling/analytics.mdx‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -107,10 +107,10 @@ in place of `field`:
107107
```
108108

109109
- `from` and `to` name two `datetime` fields of the dataset's own object, by
110-
their bare names. A relationship path, an expression, or a `date`, `time`,
111-
number or text field is refused: `os validate` reports
112-
`measure-duration-field-type-refused`, and the dataset compile answers
113-
`400 DATASET_INVALID`.
110+
their bare names. A relationship path or an expression is refused when the
111+
dataset is parsed. A `date`, `time`, number or text field is refused too:
112+
`os validate` reports `measure-duration-field-type-refused`, and the dataset
113+
compile answers `400 DATASET_INVALID`.
114114
- `unit` is `seconds`, `minutes`, `hours` or `days`, and the measure answers a
115115
number of them (`1.5` hours). `aggregate` is `avg`, `min`, `max` or `sum`.
116116
- A record missing either instant is left out, as a null is left out of an
@@ -119,8 +119,8 @@ in place of `field`:
119119
nothing.
120120
- A measure `filter`, and a `derived` measure naming the duration measure in
121121
`of`, work as they do for any measure.
122-
- It runs as one SQL statement on SQLite, PostgreSQL and MySQL, and answers the
123-
same number on each. A query the service cannot compile to one such
122+
- It runs as one SQL statement on SQLite, PostgreSQL and MySQL, computed in
123+
double precision on each. A query the service cannot compile to one such
124124
statement is refused with `400 INVALID_FIELD` naming the measure, rather than
125125
answered another way: a time dimension bucketed by `granularity` (a monthly
126126
trend), a field-to-field comparison in a filter, an object served through

‎packages/services/service-analytics/src/__tests__/dataset-duration-measure.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,7 @@ for (const cell of CELLS) {
277277
);
278278
expect(run.error, run.error?.message).toBeUndefined();
279279
expect(run.rawSql).toBe(1);
280-
expect(reads.statements.at(-1)).toContain('ORDER BY "total_seconds" ASC');
280+
expect(reads.statements[reads.statements.length - 1]).toContain('ORDER BY "total_seconds" ASC');
281281
expect(run.value!.rows).toEqual([{ category: 'b', total_seconds: -3599.25 }]);
282282
});
283283

‎packages/services/service-analytics/src/__tests__/native-sql-duration-measure.test.ts‎

Lines changed: 84 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,8 +77,8 @@ describe('[#25778] NativeSQLStrategy — the duration measure as a statement', (
7777
],
7878
[
7979
'mysql',
80-
'AVG(((TIMESTAMPDIFF(MICROSECOND, opened_at, resolved_at) / 1000000.0) / 3600.0)) AS "avg_hours"',
81-
'SUM(CASE WHEN priority = $1 THEN (TIMESTAMPDIFF(MICROSECOND, opened_at, resolved_at) / 1000000.0) END) AS "total_seconds"',
80+
'AVG(((TIMESTAMPDIFF(MICROSECOND, opened_at, resolved_at) / 1.0E6) / 3600.0)) AS "avg_hours"',
81+
'SUM(CASE WHEN priority = $1 THEN (TIMESTAMPDIFF(MICROSECOND, opened_at, resolved_at) / 1.0E6) END) AS "total_seconds"',
8282
],
8383
])('%s: the per-dialect difference, divided by the unit, under the aggregate and its measure filter', async (dialect, avg, filteredSum) => {
8484
const { sql, params } = await native.generateSql(q(), nativeCtx({ dialect }));
@@ -258,6 +258,88 @@ describe('[#25778] AnalyticsService — the gates a duration measure passes thro
258258
expect(executed).toEqual([]);
259259
});
260260

261+
it('a dimension sharing its name does not hide its columns from the read gate', async () => {
262+
const shadowed = DatasetSchema.parse({
263+
...DATASET_BASE,
264+
dimensions: [...DATASET_BASE.dimensions, { name: 'resolution_seconds_sum', field: 'category', type: 'string' }],
265+
});
266+
const { service, executed } = makeService({ readable: FIELDS.filter((f) => f !== 'resolved_at') });
267+
const err = await refusalOf(service.queryDataset(shadowed, { measures: ['resolution_seconds_sum'], dimensions: ['category'] } as never, MEMBER));
268+
expect(err?.code, err?.message).toBe('PERMISSION_DENIED');
269+
expect(err?.status).toBe(403);
270+
expect(err?.message).toContain('resolved_at');
271+
expect(executed).toEqual([]);
272+
});
273+
274+
describe('on the cube door, under the `<cube>.<name>` spelling the discovery surface lists', () => {
275+
const listed = DatasetSchema.parse({
276+
...DATASET_BASE,
277+
measures: [
278+
...DATASET_BASE.measures,
279+
{
280+
name: 'high_hours',
281+
label: 'High-priority hours',
282+
aggregate: 'avg',
283+
format: '0.0',
284+
filter: { priority: 'high' },
285+
duration: { from: 'opened_at', to: 'resolved_at', unit: 'hours' },
286+
},
287+
],
288+
});
289+
290+
it('getMeta lists each duration measure with its aggregate, label and format', async () => {
291+
const { service } = makeService();
292+
service.registerDataset(listed);
293+
const [meta] = await service.getMeta(listed.name);
294+
expect(meta.measures.filter((m) => m.name !== `${listed.name}.tickets`)).toEqual([
295+
{ name: 'resolution.resolution_seconds_sum', type: 'sum', title: 'resolution_seconds_sum' },
296+
{ name: 'resolution.high_hours', type: 'avg', title: 'High-priority hours', format: '0.0' },
297+
]);
298+
});
299+
300+
it('the listed spelling is served as the bare one, its scoped filter included, under the name asked', async () => {
301+
const { service, executed } = makeService();
302+
service.registerDataset(listed);
303+
await service.query({ cube: listed.name, measures: ['resolution.high_hours'], dimensions: ['category'] } as never, MEMBER);
304+
expect(executed).toHaveLength(1);
305+
expect(executed[0]).toContain('AVG(CASE WHEN priority = $1 THEN ((ROUND((julianday(resolved_at) - julianday(opened_at))');
306+
expect(executed[0]).toContain('AS "resolution.high_hours"');
307+
});
308+
309+
it('the read gate and the strategy refusal know the listed spelling too', async () => {
310+
const gated = makeService({ readable: FIELDS.filter((f) => f !== 'opened_at') });
311+
gated.service.registerDataset(listed);
312+
const denied = await refusalOf(gated.service.query({ cube: listed.name, measures: ['resolution.high_hours'] } as never, MEMBER));
313+
expect(denied?.code, denied?.message).toBe('PERMISSION_DENIED');
314+
expect(denied?.status).toBe(403);
315+
expect(gated.executed).toEqual([]);
316+
317+
const offNative = makeService({ capabilities: () => ({ nativeSql: false, objectqlAggregate: true, inMemory: false }) });
318+
offNative.service.registerDataset(listed);
319+
const refused = await refusalOf(offNative.service.query({ cube: listed.name, measures: ['resolution.high_hours'] } as never, MEMBER));
320+
expect(refused?.code, refused?.message).toBe('INVALID_FIELD');
321+
expect(refused?.status).toBe(400);
322+
expect(refused?.member).toBe('resolution.high_hours');
323+
expect(offNative.executed).toEqual([]);
324+
});
325+
});
326+
327+
it('a field-to-field filter routes the query off the native strategy, and the measure is refused by name', async () => {
328+
const { service, executed } = makeService();
329+
service.registerDataset(dataset);
330+
const err = await refusalOf(
331+
service.query({
332+
cube: dataset.name,
333+
measures: ['resolution_seconds_sum'],
334+
where: { resolved_at: { $gt: { $field: 'opened_at' } } },
335+
} as never, MEMBER),
336+
);
337+
expect(err?.code, err?.message).toBe('INVALID_FIELD');
338+
expect(err?.status).toBe(400);
339+
expect(err?.member).toBe('resolution_seconds_sum');
340+
expect(executed).toEqual([]);
341+
});
342+
261343
describe('a dataset that reaches the service without meeting the spec\'s parse', () => {
262344
const stored = (duration: Record<string, unknown>) => ({
263345
...DATASET_BASE,

‎packages/services/service-analytics/src/analytics-service.ts‎

Lines changed: 41 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ import {
112112
} from './dimension-labels.js';
113113
import { evaluateAnalyticsQueryOverRows } from './preview-evaluator.js';
114114
// [#25778] The one lookup of a dataset duration measure by name.
115-
import { durationMeasureIn } from './duration-measure-sql.js';
115+
import { durationMeasureIn, durationMeasureKey } from './duration-measure-sql.js';
116116
// [#5918] The measure mint refuses a dotted member through the SAME constructor
117117
// the strategies' member-level refusals use — `INVALID_FIELD` / 400, naming the
118118
// member as the request spelled it (see `dataset-refusal.ts`'s header for why
@@ -547,10 +547,13 @@ function namedQueryFields(
547547
};
548548
// [#25778] A dataset duration measure is absent from the cube (fail-closed,
549549
// `DatasetScope.measureDurations`), so `name` would read nothing for it. It
550-
// reads its two columns, and is judged on both.
550+
// reads its two columns, and is judged on both. Resolved FIRST, as the mint,
551+
// `assertCallerMembersResolvable` and `NativeSQLStrategy` resolve it: a
552+
// dimension may share the measure's name, and that dimension's column is not
553+
// what the measure aggregates.
551554
const durationColumns = (member: string, role: FieldReadRole): boolean => {
552-
const duration = durationMeasureIn(datasetScope?.measureDurations, member);
553-
if (!duration || declaredMemberEntry(cube, member, 'any')) return false;
555+
const duration = durationMeasureIn(datasetScope?.measureDurations, member, cube.name);
556+
if (!duration) return false;
554557
for (const column of [duration.from, duration.to]) {
555558
const fields = fieldsOfColumnSql(cube, baseObject, column, role, referenceOf);
556559
if (fields) out.push(...fields);
@@ -572,12 +575,17 @@ function namedQueryFields(
572575
for (const member of query.measures ?? []) if (!durationColumns(member, 'aggregate')) name(member, 'measure', 'aggregate');
573576
for (const member of filterMembers((query as { where?: unknown }).where)) name(member, 'any', 'predicate');
574577
for (const measure of query.measures ?? []) {
575-
for (const member of filterMembers(datasetScope?.measureFilters?.[measure])) name(member, 'any', 'predicate');
578+
const filterKey = durationMeasureKey(datasetScope?.measureDurations, measure, cube.name) ?? measure;
579+
for (const member of filterMembers(datasetScope?.measureFilters?.[filterKey])) name(member, 'any', 'predicate');
576580
}
577581
for (const member of filterMembers(datasetScope?.filter)) name(member, 'any', 'predicate');
578582
const order = (query as { order?: unknown }).order;
579583
if (order && typeof order === 'object' && !Array.isArray(order)) {
580-
for (const key of Object.keys(order)) if (!durationColumns(key, 'predicate')) name(key, 'any', 'predicate');
584+
// An order key naming both a duration measure and a declared member is
585+
// judged as both.
586+
for (const key of Object.keys(order)) {
587+
if (!durationColumns(key, 'predicate') || declaredMemberEntry(cube, key, 'any')) name(key, 'any', 'predicate');
588+
}
581589
}
582590
return [...out.filter((f) => f.object === baseObject), ...out.filter((f) => f.object !== baseObject)];
583591
}
@@ -2171,7 +2179,7 @@ export class AnalyticsService implements IAnalyticsService {
21712179
// Author-declared measures are not this gate's business. [#25778] A
21722180
// dataset's duration measure is one, declared beside the cube.
21732181
if (authorCube && declaredMemberEntry(authorCube, measure, 'measure')) continue;
2174-
if (durationMeasureIn(scope.getCompiledDataset(name)?.durations, measure)) continue;
2182+
if (durationMeasureIn(scope.getCompiledDataset(name)?.durations, measure, name)) continue;
21752183
const inferredSql = inferredCallerMeasureSql(measure, name);
21762184
// `null` — a dotted non-qualifier measure #5918 already refused — and a
21772185
// column reference (`'*'`, a bare identifier, a relationship path) both
@@ -3238,13 +3246,24 @@ export class AnalyticsService implements IAnalyticsService {
32383246
name: cube.name,
32393247
title: cube.title,
32403248
...(cube.description === undefined ? {} : { description: cube.description }),
3241-
measures: Object.entries(cube.measures).map(([key, measure]) => ({
3242-
name: `${cube.name}.${key}`,
3243-
type: measure.type,
3244-
title: measure.label,
3245-
...(measure.description === undefined ? {} : { description: measure.description }),
3246-
...(measure.format === undefined ? {} : { format: measure.format }),
3247-
})),
3249+
measures: [
3250+
...Object.entries(cube.measures).map(([key, measure]) => ({
3251+
name: `${cube.name}.${key}`,
3252+
type: measure.type,
3253+
title: measure.label,
3254+
...(measure.description === undefined ? {} : { description: measure.description }),
3255+
...(measure.format === undefined ? {} : { format: measure.format }),
3256+
})),
3257+
// [#25778] A registered dataset's duration measures are absent from
3258+
// `cube.measures` (fail-closed) yet answer on the cube door, so the
3259+
// discovery surface lists them from the compiled dataset of THIS cube.
3260+
...Object.entries(this.compiledDatasetOf(cube)?.durations ?? {}).map(([key, duration]) => ({
3261+
name: `${cube.name}.${key}`,
3262+
type: duration.aggregate,
3263+
title: duration.label ?? key,
3264+
...(duration.format === undefined ? {} : { format: duration.format }),
3265+
})),
3266+
],
32483267
dimensions: Object.entries(cube.dimensions).map(([key, dimension]) => ({
32493268
name: `${cube.name}.${key}`,
32503269
type: dimension.type,
@@ -3300,6 +3319,12 @@ export class AnalyticsService implements IAnalyticsService {
33003319

33013320
// ── Internal ─────────────────────────────────────────────────────
33023321

3322+
/** The compiled dataset a registered cube came from, or nothing when another cube took the name. */
3323+
private compiledDatasetOf(cube: Cube): CompiledDataset | undefined {
3324+
const compiled = this.datasetRegistry.get(cube.name);
3325+
return compiled?.cube === cube ? compiled : undefined;
3326+
}
3327+
33033328
/**
33043329
* [#25778] Refuse a dataset duration measure the chosen strategy does not
33053330
* serve — `INVALID_FIELD` / 400, naming the measure, before anything runs.
@@ -3318,7 +3343,7 @@ export class AnalyticsService implements IAnalyticsService {
33183343
private assertDurationMeasuresServed(query: AnalyticsQuery, scope: CubeScope, strategy: AnalyticsStrategy): void {
33193344
if (strategy instanceof NativeSQLStrategy) return;
33203345
const durations = scope.getCompiledDataset(query.cube!)?.durations;
3321-
const measure = (query.measures ?? []).find((m) => durationMeasureIn(durations, m));
3346+
const measure = (query.measures ?? []).find((m) => durationMeasureIn(durations, m, query.cube));
33223347
if (measure === undefined) return;
33233348
throw invalidMemberError(
33243349
`Measure "${measure}" of dataset "${query.cube}" is a duration, the time between two datetime fields ` +
@@ -3468,7 +3493,7 @@ export class AnalyticsService implements IAnalyticsService {
34683493
const durations = scope.getCompiledDataset(name)?.durations;
34693494
for (const m of query.measures || []) {
34703495
if (cube.measures[m] || extraMeasures[m]) continue;
3471-
if (durationMeasureIn(durations, m)) continue;
3496+
if (durationMeasureIn(durations, m, name)) continue;
34723497
const key = mintableMeasureKey(m, name);
34733498
if (cube.measures[key] || extraMeasures[key]) continue;
34743499
// [#21437] A spelling whose source names no field (`_sum`, `*`) is

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

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -456,7 +456,12 @@ function assertAggregateFieldTypeCompatible(
456456
);
457457
}
458458

459-
/** One column of the base object by its bare name — the spec's `duration.from` / `to` shape. */
459+
/**
460+
* One column of the base object by its bare name — the spec's `duration.from` /
461+
* `to` shape (`ANALYTICS_BASE_COLUMN`, which `@objectstack/spec` does not
462+
* export). Judged again here because a dataset can reach the compile without
463+
* passing the schema.
464+
*/
460465
const DURATION_BASE_COLUMN = /^[A-Za-z_][A-Za-z0-9_]*$/;
461466

462467
/**
@@ -519,7 +524,15 @@ function compileDurationMeasure(
519524
);
520525
}
521526
}
522-
return { aggregate: aggregate as DurationMeasureSpec['aggregate'], from, to, unit };
527+
return {
528+
aggregate: aggregate as DurationMeasureSpec['aggregate'],
529+
from,
530+
to,
531+
unit,
532+
// [#6761] The same registry-locale label a cube metric gets.
533+
label: resolveI18nLabel(measure.label, REGISTRY_LOCALE) ?? measure.name,
534+
...(typeof measure.format === 'string' ? { format: measure.format } : {}),
535+
};
523536
}
524537

525538
/** Map a dataset dimension type to the Cube dimension `type`. */

‎packages/services/service-analytics/src/dataset-executor.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1196,7 +1196,7 @@ export class DatasetExecutor {
11961196
// aggregate beside it.
11971197
const fillColumns: Record<string, string | undefined> = {};
11981198
for (const m of baseMeasures) {
1199-
const aggregate = compiled.cube.measures?.[m]?.type ?? durationMeasureIn(compiled.durations, m)?.aggregate;
1199+
const aggregate = compiled.cube.measures?.[m]?.type ?? durationMeasureIn(compiled.durations, m, compiled.cube.name)?.aggregate;
12001200
fillColumns[m] = aggregate;
12011201
if (selection.compareTo) fillColumns[`${m}__compare`] = aggregate;
12021202
}

0 commit comments

Comments
 (0)