Skip to content

Commit 3abd233

Browse files
os-zhuangclaude
andauthored
fix(analytics): project a timeDimensions bucket into the result rows and fields (#4033) (#4069)
A query bucketing by `timeDimensions` alone grouped correctly — the echoed SQL read `date_trunc('month', due_date) AS "due_date"` — but the row mapper and buildFieldMeta both enumerated `query.dimensions` only, so the bucket was dropped on the way out: rows: [{ count: 2 }, { count: 8 }] fields: [{ name: 'count', type: 'number' }] A trend chart therefore got N values and no x-axis. Found by driving a live showcase app, not by static review: the numbers were right, so nothing downstream errored — only the labels were gone. Writing the same query as `dimensions: ['due_date']` was always fine, which is why it survived. Grouping, row mapping (both the direct and cross-object paths) and field metadata now derive their projected set from one `projectedDimensions()` helper: `dimensions` plus every GRANULAR `timeDimensions` entry not already among them. Granularity is the right gate because it is exactly what the three groupBy sites key on — an entry without one contributes only a `dateRange` predicate, and projecting it would declare a column the engine never grouped by. The cross-object path already had its own correct-but-parallel loop; it now shares the single definition rather than being a second place to keep in sync. Closes #4033 Claude-Session: https://claude.ai/code/session_01TPxNwPnjcn599ujXpU3ibJ Co-authored-by: Claude <noreply@anthropic.com>
1 parent 839982e commit 3abd233

3 files changed

Lines changed: 242 additions & 17 deletions

File tree

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
---
2+
"@objectstack/service-analytics": patch
3+
---
4+
5+
fix(analytics): project a `timeDimensions` bucket into the result rows and fields (#4033)
6+
7+
An analytics query that buckets by `timeDimensions` alone grouped correctly —
8+
the echoed SQL read `date_trunc('month', due_date) AS "due_date"` — but the row
9+
mapper and `buildFieldMeta` both enumerated `query.dimensions` only, so the
10+
bucket never reached the caller: rows carried just the measures and `fields`
11+
never mentioned the dimension. A trend chart got N values and no x-axis. The
12+
same query written with `dimensions: ['due_date']` was unaffected, which is why
13+
it went unnoticed.
14+
15+
Grouping, row mapping and field metadata now derive the projected set from one
16+
`projectedDimensions()` helper — `dimensions` plus every *granular*
17+
`timeDimensions` entry not already among them. A `timeDimensions` entry without
18+
a granularity contributes only its `dateRange` predicate and stays out of the
19+
projection, so no phantom column is declared.
Lines changed: 183 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,183 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* #4033 — a `timeDimensions`-only bucket must reach the caller.
5+
*
6+
* `timeDimensions` is not merely a filter carrier: an entry with a
7+
* `granularity` is GROUPED BY, so its bucket is a column of the result. The
8+
* strategy grouped by it correctly — the echoed SQL even read
9+
* `date_trunc('month', due_date) AS "due_date"` — but the row mapper and
10+
* `buildFieldMeta` both enumerated `query.dimensions` alone, so the bucket was
11+
* dropped on the way out:
12+
*
13+
* rows: [{ count: 2 }, { count: 8 }]
14+
* fields: [{ name: 'count', type: 'number' }]
15+
*
16+
* A trend chart therefore got N values and no x-axis. Measured on a live
17+
* showcase app, which is what makes this worth pinning: the numbers were right,
18+
* so nothing downstream errored — the labels were simply gone. The identical
19+
* query written with `dimensions: ['due_date']` was always fine, which is why
20+
* it survived so long.
21+
*
22+
* A `timeDimensions` entry WITHOUT a granularity contributes only a `dateRange`
23+
* predicate and is not grouped — projecting it would invent a column, so the
24+
* negative case is pinned too.
25+
*/
26+
27+
import { describe, it, expect } from 'vitest';
28+
import { DatasetSchema } from '@objectstack/spec/ui';
29+
import type { ExecutionContext } from '@objectstack/spec/kernel';
30+
import { AnalyticsService } from '../analytics-service.js';
31+
import { compileDataset } from '../dataset-compiler.js';
32+
33+
const dataset = DatasetSchema.parse({
34+
name: 'delivery',
35+
label: 'Delivery',
36+
object: 'task',
37+
dimensions: [
38+
{ name: 'due_date', field: 'due_date', type: 'date' },
39+
{ name: 'priority', field: 'priority', type: 'string' },
40+
],
41+
measures: [{ name: 'count', aggregate: 'count', field: 'id' }],
42+
});
43+
44+
const TABLE = [
45+
{ id: 1, due_date: '2026-01-10', priority: 'high' },
46+
{ id: 2, due_date: '2026-01-20', priority: 'low' },
47+
{ id: 3, due_date: '2026-02-05', priority: 'high' },
48+
];
49+
50+
const ctx = { tenantId: 'org_A', userId: 'u_a' } as ExecutionContext;
51+
const objectqlOnly = () => ({ nativeSql: false, objectqlAggregate: true, inMemory: false });
52+
53+
type Row = Record<string, unknown>;
54+
type GroupByItem = string | { field: string; dateGranularity: string };
55+
type AggOpts = {
56+
groupBy?: GroupByItem[];
57+
aggregations?: Array<{ field: string; method: string; alias: string }>;
58+
filter?: Record<string, unknown>;
59+
};
60+
61+
/** `engine.aggregate()` stand-in: buckets and counts, honestly. */
62+
const aggregate = async (_object: string, opts: AggOpts): Promise<Row[]> => {
63+
const buckets = new Map<string, Row>();
64+
for (const r of TABLE) {
65+
const key: Row = {};
66+
for (const g of opts.groupBy ?? []) {
67+
if (typeof g === 'string') key[g] = r[g];
68+
else key[g.field] = String(r[g.field]).slice(0, 7); // month
69+
}
70+
const id = JSON.stringify(key);
71+
const b = buckets.get(id) ?? { ...key };
72+
for (const a of opts.aggregations ?? []) {
73+
if (a.method === 'count') b[a.alias] = Number(b[a.alias] ?? 0) + 1;
74+
}
75+
buckets.set(id, b);
76+
}
77+
return [...buckets.values()];
78+
};
79+
80+
const service = () =>
81+
new AnalyticsService({
82+
cubes: [compileDataset(dataset).cube],
83+
queryCapabilities: objectqlOnly,
84+
executeAggregate: aggregate,
85+
});
86+
87+
describe('ObjectQLStrategy — timeDimensions bucket projection (#4033)', () => {
88+
it('projects a timeDimensions-only bucket into the rows', async () => {
89+
const result = await service().query(
90+
{
91+
cube: 'delivery',
92+
measures: ['count'],
93+
timeDimensions: [{ dimension: 'due_date', granularity: 'month' }],
94+
},
95+
ctx,
96+
);
97+
98+
// Before the fix these rows were `[{count:2},{count:1}]` — right numbers,
99+
// no labels.
100+
expect(result.rows).toEqual([
101+
{ due_date: '2026-01', count: 2 },
102+
{ due_date: '2026-02', count: 1 },
103+
]);
104+
});
105+
106+
it('declares the bucket in fields, so a chart can find its x-axis', async () => {
107+
const result = await service().query(
108+
{
109+
cube: 'delivery',
110+
measures: ['count'],
111+
timeDimensions: [{ dimension: 'due_date', granularity: 'month' }],
112+
},
113+
ctx,
114+
);
115+
116+
// `type` here is the CUBE dimension vocabulary, where every temporal
117+
// dimension is `time` — not the `Field.date` / `Field.datetime` /
118+
// `Field.time` vocabulary the driver stores by. The dataset declares
119+
// `type: 'date'`; the compiled cube dimension is `time`.
120+
expect(result.fields).toEqual([
121+
{ name: 'due_date', type: 'time' },
122+
{ name: 'count', type: 'number' },
123+
]);
124+
});
125+
126+
it('presents identically whether the bucket is named in dimensions or only in timeDimensions', async () => {
127+
const viaTimeDimensions = await service().query(
128+
{
129+
cube: 'delivery',
130+
measures: ['count'],
131+
timeDimensions: [{ dimension: 'due_date', granularity: 'month' }],
132+
},
133+
ctx,
134+
);
135+
const viaDimensions = await service().query(
136+
{
137+
cube: 'delivery',
138+
dimensions: ['due_date'],
139+
measures: ['count'],
140+
timeDimensions: [{ dimension: 'due_date', granularity: 'month' }],
141+
},
142+
ctx,
143+
);
144+
145+
// The two spellings mean the same query; they used to disagree about
146+
// whether the answer carried its labels.
147+
expect(viaTimeDimensions.rows).toEqual(viaDimensions.rows);
148+
expect(viaTimeDimensions.fields).toEqual(viaDimensions.fields);
149+
});
150+
151+
it('does not duplicate a bucket that is also listed in dimensions', async () => {
152+
const result = await service().query(
153+
{
154+
cube: 'delivery',
155+
dimensions: ['due_date'],
156+
measures: ['count'],
157+
timeDimensions: [{ dimension: 'due_date', granularity: 'month' }],
158+
},
159+
ctx,
160+
);
161+
162+
expect(result.fields.filter((f) => f.name === 'due_date')).toHaveLength(1);
163+
});
164+
165+
it('keeps a non-granular timeDimension OUT of the projection — it only filters', async () => {
166+
const result = await service().query(
167+
{
168+
cube: 'delivery',
169+
dimensions: ['priority'],
170+
measures: ['count'],
171+
// No granularity: a window, not a bucket. Projecting it would declare a
172+
// column the engine never grouped by.
173+
timeDimensions: [{ dimension: 'due_date', dateRange: ['2026-01-01', '2026-12-31'] }],
174+
},
175+
ctx,
176+
);
177+
178+
expect(result.fields.map((f) => f.name)).toEqual(['priority', 'count']);
179+
for (const row of result.rows) {
180+
expect(row).not.toHaveProperty('due_date');
181+
}
182+
});
183+
});

packages/services/service-analytics/src/strategies/objectql-strategy.ts

Lines changed: 40 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -148,14 +148,15 @@ export class ObjectQLStrategy implements AnalyticsStrategy {
148148
context: ctx.context,
149149
});
150150

151-
// Remap short field names back to cube-qualified names
151+
// Remap short field names back to cube-qualified names. Driven by
152+
// `projectedDimensions`, so a `timeDimensions`-only bucket — grouped by
153+
// just above, and therefore present in `row` — reaches the caller instead
154+
// of being silently dropped (#4033).
152155
const mappedRows = rows.map(row => {
153156
const mapped: Record<string, unknown> = {};
154-
if (query.dimensions) {
155-
for (const dim of query.dimensions) {
156-
const shortName = this.resolveFieldName(cube, dim, 'dimension');
157-
if (shortName in row) mapped[dim] = row[shortName];
158-
}
157+
for (const dim of this.projectedDimensions(query)) {
158+
const shortName = this.resolveFieldName(cube, dim, 'dimension');
159+
if (shortName in row) mapped[dim] = row[shortName];
159160
}
160161
if (query.measures) {
161162
for (const m of query.measures) {
@@ -543,19 +544,17 @@ export class ObjectQLStrategy implements AnalyticsStrategy {
543544
// Map resolved group keys back to the caller's dimension names.
544545
const mappedRows = merged.map((row) => {
545546
const out: Record<string, unknown> = {};
546-
for (const dim of query.dimensions ?? []) {
547+
// Same projection set as the direct path and `buildFieldMeta`
548+
// ({@link projectedDimensions}) — a cross-object dimension carries the
549+
// caller's name already, everything else is remapped from its short name.
550+
for (const dim of this.projectedDimensions(query)) {
547551
if (crossByDim.has(dim)) {
548552
if (dim in row) out[dim] = row[dim];
549553
} else {
550554
const field = this.resolveFieldName(cube, dim, 'dimension');
551555
if (field in row) out[dim] = row[field];
552556
}
553557
}
554-
for (const td of query.timeDimensions ?? []) {
555-
if (query.dimensions?.includes(td.dimension)) continue;
556-
const field = this.resolveFieldName(cube, td.dimension, 'dimension');
557-
if (field in row) out[td.dimension] = row[field];
558-
}
559558
for (const m of query.measures ?? []) {
560559
if (m in row) out[m] = row[m];
561560
}
@@ -831,13 +830,37 @@ export class ObjectQLStrategy implements AnalyticsStrategy {
831830
return cube.sql.trim();
832831
}
833832

833+
/**
834+
* The dimensions this query PROJECTS, in the order the result carries them:
835+
* every `dimensions` entry, then every granular `timeDimensions` entry that
836+
* is not already one of them.
837+
*
838+
* `timeDimensions` is not merely a filter carrier. An entry with a
839+
* `granularity` is GROUPED BY — see the `td.granularity` sites that build
840+
* groupBy here, in `generateSql` and in the cross-object path — so its
841+
* bucket is a COLUMN of the result; an entry without one only contributes a
842+
* `dateRange` predicate and must NOT be projected.
843+
*
844+
* Grouping, row mapping and {@link buildFieldMeta} have to agree on exactly
845+
* that set. When they did not, a bucketed query returned rows carrying only
846+
* the measures and a `fields` list that never mentioned the bucket — a trend
847+
* chart got N values and no x-axis (#4033) — even though the SQL had
848+
* selected `date_trunc(…) AS "<dim>"` all along. One definition, every
849+
* consumer.
850+
*/
851+
private projectedDimensions(query: AnalyticsQuery): string[] {
852+
const out = [...(query.dimensions ?? [])];
853+
for (const td of query.timeDimensions ?? []) {
854+
if (td.granularity && !out.includes(td.dimension)) out.push(td.dimension);
855+
}
856+
return out;
857+
}
858+
834859
private buildFieldMeta(query: AnalyticsQuery, cube: Cube): Array<{ name: string; type: string }> {
835860
const fields: Array<{ name: string; type: string }> = [];
836-
if (query.dimensions) {
837-
for (const dim of query.dimensions) {
838-
const d = this.lookupMember(cube, dim, 'dimension');
839-
fields.push({ name: dim, type: d?.type || 'string' });
840-
}
861+
for (const dim of this.projectedDimensions(query)) {
862+
const d = this.lookupMember(cube, dim, 'dimension');
863+
fields.push({ name: dim, type: d?.type || 'string' });
841864
}
842865
if (query.measures) {
843866
for (const m of query.measures) {

0 commit comments

Comments
 (0)