diff --git a/.changeset/analytics-timedimension-projection.md b/.changeset/analytics-timedimension-projection.md new file mode 100644 index 0000000000..a203f87d38 --- /dev/null +++ b/.changeset/analytics-timedimension-projection.md @@ -0,0 +1,19 @@ +--- +"@objectstack/service-analytics": patch +--- + +fix(analytics): project a `timeDimensions` bucket into the result rows and fields (#4033) + +An analytics query that buckets 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 never reached the caller: rows carried just the measures and `fields` +never mentioned the dimension. A trend chart got N values and no x-axis. The +same query written with `dimensions: ['due_date']` was unaffected, which is why +it went unnoticed. + +Grouping, row mapping and field metadata now derive the projected set from one +`projectedDimensions()` helper — `dimensions` plus every *granular* +`timeDimensions` entry not already among them. A `timeDimensions` entry without +a granularity contributes only its `dateRange` predicate and stays out of the +projection, so no phantom column is declared. diff --git a/packages/services/service-analytics/src/__tests__/objectql-timedimension-projection.test.ts b/packages/services/service-analytics/src/__tests__/objectql-timedimension-projection.test.ts new file mode 100644 index 0000000000..477c0bea0c --- /dev/null +++ b/packages/services/service-analytics/src/__tests__/objectql-timedimension-projection.test.ts @@ -0,0 +1,183 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #4033 — a `timeDimensions`-only bucket must reach the caller. + * + * `timeDimensions` is not merely a filter carrier: an entry with a + * `granularity` is GROUPED BY, so its bucket is a column of the result. The + * strategy grouped by it correctly — the echoed SQL even read + * `date_trunc('month', due_date) AS "due_date"` — but the row mapper and + * `buildFieldMeta` both enumerated `query.dimensions` alone, 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. Measured on a live + * showcase app, which is what makes this worth pinning: the numbers were right, + * so nothing downstream errored — the labels were simply gone. The identical + * query written with `dimensions: ['due_date']` was always fine, which is why + * it survived so long. + * + * A `timeDimensions` entry WITHOUT a granularity contributes only a `dateRange` + * predicate and is not grouped — projecting it would invent a column, so the + * negative case is pinned too. + */ + +import { describe, it, expect } from 'vitest'; +import { DatasetSchema } from '@objectstack/spec/ui'; +import type { ExecutionContext } from '@objectstack/spec/kernel'; +import { AnalyticsService } from '../analytics-service.js'; +import { compileDataset } from '../dataset-compiler.js'; + +const dataset = DatasetSchema.parse({ + name: 'delivery', + label: 'Delivery', + object: 'task', + dimensions: [ + { name: 'due_date', field: 'due_date', type: 'date' }, + { name: 'priority', field: 'priority', type: 'string' }, + ], + measures: [{ name: 'count', aggregate: 'count', field: 'id' }], +}); + +const TABLE = [ + { id: 1, due_date: '2026-01-10', priority: 'high' }, + { id: 2, due_date: '2026-01-20', priority: 'low' }, + { id: 3, due_date: '2026-02-05', priority: 'high' }, +]; + +const ctx = { tenantId: 'org_A', userId: 'u_a' } as ExecutionContext; +const objectqlOnly = () => ({ nativeSql: false, objectqlAggregate: true, inMemory: false }); + +type Row = Record; +type GroupByItem = string | { field: string; dateGranularity: string }; +type AggOpts = { + groupBy?: GroupByItem[]; + aggregations?: Array<{ field: string; method: string; alias: string }>; + filter?: Record; +}; + +/** `engine.aggregate()` stand-in: buckets and counts, honestly. */ +const aggregate = async (_object: string, opts: AggOpts): Promise => { + const buckets = new Map(); + for (const r of TABLE) { + const key: Row = {}; + for (const g of opts.groupBy ?? []) { + if (typeof g === 'string') key[g] = r[g]; + else key[g.field] = String(r[g.field]).slice(0, 7); // month + } + const id = JSON.stringify(key); + const b = buckets.get(id) ?? { ...key }; + for (const a of opts.aggregations ?? []) { + if (a.method === 'count') b[a.alias] = Number(b[a.alias] ?? 0) + 1; + } + buckets.set(id, b); + } + return [...buckets.values()]; +}; + +const service = () => + new AnalyticsService({ + cubes: [compileDataset(dataset).cube], + queryCapabilities: objectqlOnly, + executeAggregate: aggregate, + }); + +describe('ObjectQLStrategy — timeDimensions bucket projection (#4033)', () => { + it('projects a timeDimensions-only bucket into the rows', async () => { + const result = await service().query( + { + cube: 'delivery', + measures: ['count'], + timeDimensions: [{ dimension: 'due_date', granularity: 'month' }], + }, + ctx, + ); + + // Before the fix these rows were `[{count:2},{count:1}]` — right numbers, + // no labels. + expect(result.rows).toEqual([ + { due_date: '2026-01', count: 2 }, + { due_date: '2026-02', count: 1 }, + ]); + }); + + it('declares the bucket in fields, so a chart can find its x-axis', async () => { + const result = await service().query( + { + cube: 'delivery', + measures: ['count'], + timeDimensions: [{ dimension: 'due_date', granularity: 'month' }], + }, + ctx, + ); + + // `type` here is the CUBE dimension vocabulary, where every temporal + // dimension is `time` — not the `Field.date` / `Field.datetime` / + // `Field.time` vocabulary the driver stores by. The dataset declares + // `type: 'date'`; the compiled cube dimension is `time`. + expect(result.fields).toEqual([ + { name: 'due_date', type: 'time' }, + { name: 'count', type: 'number' }, + ]); + }); + + it('presents identically whether the bucket is named in dimensions or only in timeDimensions', async () => { + const viaTimeDimensions = await service().query( + { + cube: 'delivery', + measures: ['count'], + timeDimensions: [{ dimension: 'due_date', granularity: 'month' }], + }, + ctx, + ); + const viaDimensions = await service().query( + { + cube: 'delivery', + dimensions: ['due_date'], + measures: ['count'], + timeDimensions: [{ dimension: 'due_date', granularity: 'month' }], + }, + ctx, + ); + + // The two spellings mean the same query; they used to disagree about + // whether the answer carried its labels. + expect(viaTimeDimensions.rows).toEqual(viaDimensions.rows); + expect(viaTimeDimensions.fields).toEqual(viaDimensions.fields); + }); + + it('does not duplicate a bucket that is also listed in dimensions', async () => { + const result = await service().query( + { + cube: 'delivery', + dimensions: ['due_date'], + measures: ['count'], + timeDimensions: [{ dimension: 'due_date', granularity: 'month' }], + }, + ctx, + ); + + expect(result.fields.filter((f) => f.name === 'due_date')).toHaveLength(1); + }); + + it('keeps a non-granular timeDimension OUT of the projection — it only filters', async () => { + const result = await service().query( + { + cube: 'delivery', + dimensions: ['priority'], + measures: ['count'], + // No granularity: a window, not a bucket. Projecting it would declare a + // column the engine never grouped by. + timeDimensions: [{ dimension: 'due_date', dateRange: ['2026-01-01', '2026-12-31'] }], + }, + ctx, + ); + + expect(result.fields.map((f) => f.name)).toEqual(['priority', 'count']); + for (const row of result.rows) { + expect(row).not.toHaveProperty('due_date'); + } + }); +}); diff --git a/packages/services/service-analytics/src/strategies/objectql-strategy.ts b/packages/services/service-analytics/src/strategies/objectql-strategy.ts index 945d80b5ec..9b2c8955eb 100644 --- a/packages/services/service-analytics/src/strategies/objectql-strategy.ts +++ b/packages/services/service-analytics/src/strategies/objectql-strategy.ts @@ -148,14 +148,15 @@ export class ObjectQLStrategy implements AnalyticsStrategy { context: ctx.context, }); - // Remap short field names back to cube-qualified names + // Remap short field names back to cube-qualified names. Driven by + // `projectedDimensions`, so a `timeDimensions`-only bucket — grouped by + // just above, and therefore present in `row` — reaches the caller instead + // of being silently dropped (#4033). const mappedRows = rows.map(row => { const mapped: Record = {}; - if (query.dimensions) { - for (const dim of query.dimensions) { - const shortName = this.resolveFieldName(cube, dim, 'dimension'); - if (shortName in row) mapped[dim] = row[shortName]; - } + for (const dim of this.projectedDimensions(query)) { + const shortName = this.resolveFieldName(cube, dim, 'dimension'); + if (shortName in row) mapped[dim] = row[shortName]; } if (query.measures) { for (const m of query.measures) { @@ -543,7 +544,10 @@ export class ObjectQLStrategy implements AnalyticsStrategy { // Map resolved group keys back to the caller's dimension names. const mappedRows = merged.map((row) => { const out: Record = {}; - for (const dim of query.dimensions ?? []) { + // Same projection set as the direct path and `buildFieldMeta` + // ({@link projectedDimensions}) — a cross-object dimension carries the + // caller's name already, everything else is remapped from its short name. + for (const dim of this.projectedDimensions(query)) { if (crossByDim.has(dim)) { if (dim in row) out[dim] = row[dim]; } else { @@ -551,11 +555,6 @@ export class ObjectQLStrategy implements AnalyticsStrategy { if (field in row) out[dim] = row[field]; } } - for (const td of query.timeDimensions ?? []) { - if (query.dimensions?.includes(td.dimension)) continue; - const field = this.resolveFieldName(cube, td.dimension, 'dimension'); - if (field in row) out[td.dimension] = row[field]; - } for (const m of query.measures ?? []) { if (m in row) out[m] = row[m]; } @@ -831,13 +830,37 @@ export class ObjectQLStrategy implements AnalyticsStrategy { return cube.sql.trim(); } + /** + * The dimensions this query PROJECTS, in the order the result carries them: + * every `dimensions` entry, then every granular `timeDimensions` entry that + * is not already one of them. + * + * `timeDimensions` is not merely a filter carrier. An entry with a + * `granularity` is GROUPED BY — see the `td.granularity` sites that build + * groupBy here, in `generateSql` and in the cross-object path — so its + * bucket is a COLUMN of the result; an entry without one only contributes a + * `dateRange` predicate and must NOT be projected. + * + * Grouping, row mapping and {@link buildFieldMeta} have to agree on exactly + * that set. When they did not, a bucketed query returned rows carrying only + * the measures and a `fields` list that never mentioned the bucket — a trend + * chart got N values and no x-axis (#4033) — even though the SQL had + * selected `date_trunc(…) AS ""` all along. One definition, every + * consumer. + */ + private projectedDimensions(query: AnalyticsQuery): string[] { + const out = [...(query.dimensions ?? [])]; + for (const td of query.timeDimensions ?? []) { + if (td.granularity && !out.includes(td.dimension)) out.push(td.dimension); + } + return out; + } + private buildFieldMeta(query: AnalyticsQuery, cube: Cube): Array<{ name: string; type: string }> { const fields: Array<{ name: string; type: string }> = []; - if (query.dimensions) { - for (const dim of query.dimensions) { - const d = this.lookupMember(cube, dim, 'dimension'); - fields.push({ name: dim, type: d?.type || 'string' }); - } + for (const dim of this.projectedDimensions(query)) { + const d = this.lookupMember(cube, dim, 'dimension'); + fields.push({ name: dim, type: d?.type || 'string' }); } if (query.measures) { for (const m of query.measures) {