Skip to content

Commit cb122c7

Browse files
committed
fix(service-analytics): the ObjectQL strategy groups a column once when two dimensions read it
- objectql-strategy.ts: a module-level pushGroupKey() appends a groupBy key unless an identical one (same column, or same column at the same granularity) is already there. The direct path and the cross-object path both use it, replacing #28692's two FK-only checks. Two dataset dimensions over one column now share its group, and the existing row mapping keys the answer by every dimension name. A column raw beside its bucket stays two keys, so the engine's duplicate-column refusal still answers it. - analytics-service.ts (review S1): one whereLeafMembers(where) reader for namedQueryFields, relationshipMembers and previewWhereColumns, in place of three copies of the guarded collectFilterLeaves read. - Tests (review S2 and #28744): a filter on a measure key over a plain column, two dimensions over one column on the direct path, beside a month bucket and beside a cross-object dimension, and the raw-beside-bucket refusal, all on real SQLite. Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 7a48bf7 commit cb122c7

4 files changed

Lines changed: 141 additions & 39 deletions

File tree

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
'@objectstack/service-analytics': patch
3+
---
4+
5+
fix(service-analytics): the ObjectQL strategy serves two dataset dimensions over one column
6+
7+
Clause-②: yes (widening)
8+
Metadata-protocol: none
9+
10+
A dataset that exposes one column under two dimension names (`region`, and `zone` declared over `region`) and is queried with both is now served by the ObjectQL strategy with one group per value, keyed by both names, as the native strategy and the draft preview answer it. This holds on the strategy's direct path, beside a bucketed time dimension, and beside a cross-object dimension such as `account.name`. Before, the strategy handed the engine a `groupBy` naming the column twice and answered `INVALID_QUERY` / 400, measured on real SQLite.
11+
12+
The strategy now names each group key once: the same column, or the same column bucketed at the same granularity. The engine's duplicate-column refusal is unchanged, and it still answers a selection that asks for two values under one name, such as a column raw beside its own month bucket. Nothing is refused that was served, and no export, key, error code or status is added.

‎packages/services/service-analytics/src/__tests__/preview-time-filter-fk-parity.test.ts‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,11 @@
2121
* two dimensions through one relationship, with the native strategy's
2222
* groups. Before, it handed the engine a groupBy naming one column twice and
2323
* answered `INVALID_QUERY`.
24+
* - #28744. The ObjectQL strategy answers two dataset dimensions over one
25+
* column (`region`, and `zone` declared over `region`) with one group per
26+
* value, keyed by both names, as native and the preview do, on its direct
27+
* path and beside a cross-object dimension. Before, it answered
28+
* `INVALID_QUERY` for the same duplicate groupBy column.
2429
*/
2530

2631
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
@@ -248,6 +253,15 @@ describe('[#28690–#28692] time-dimension buckets, column-spelled filter member
248253
);
249254
});
250255

256+
it('reads a filter on a measure key from the measure\'s column, as the live faces resolve it', async () => {
257+
// `total` is a measure over `amount`; the strategies' filter-member
258+
// lookup falls through to the measures, so the row filter is `amount > 60`.
259+
await answeredAlike(FACES, UNBACKED, { dimensions: ['region'], measures: ['total'], runtimeFilter: { total: { $gt: 60 } } }, {
260+
rows: [{ region: 'amer', total: 100 }],
261+
fields: ['region:string', 'total:number'],
262+
});
263+
});
264+
251265
it('filters on zone beside a time-dimension-only bucket', async () => {
252266
await answeredAlike(
253267
FACES,
@@ -297,4 +311,61 @@ describe('[#28690–#28692] time-dimension buckets, column-spelled filter member
297311
});
298312
});
299313
});
314+
describe('#28744 — the ObjectQL strategy groups a column once when two dimensions read it', () => {
315+
it.each([
316+
['region, zone', ['region', 'zone']],
317+
['zone, region', ['zone', 'region']],
318+
])('serves dimensions %s over one column with one group per value, where it answered INVALID_QUERY', async (_label, dimensions) => {
319+
await answeredAlike(FACES, UNBACKED, { dimensions, measures: ['total'] }, {
320+
rows: [
321+
{ region: 'amer', zone: 'amer', total: 150 },
322+
{ region: 'emea', zone: 'emea', total: 30 },
323+
],
324+
fields: [...dimensions.map((d) => `${d}:string`), 'total:number'],
325+
});
326+
});
327+
328+
it('serves them beside a month bucket', async () => {
329+
await answeredAlike(
330+
FACES,
331+
UNBACKED,
332+
{ dimensions: ['region', 'zone'], measures: ['total'], timeDimensions: [{ dimension: 'closed_at', granularity: 'month' }] },
333+
{
334+
rows: [
335+
{ region: 'amer', zone: 'amer', closed_at: '2026-03', total: 100 },
336+
{ region: 'amer', zone: 'amer', closed_at: '2026-04', total: 50 },
337+
{ region: 'emea', zone: 'emea', closed_at: '2026-04', total: 30 },
338+
],
339+
fields: ['region:string', 'zone:string', 'closed_at:time', 'total:number'],
340+
},
341+
);
342+
});
343+
344+
it("still hands the engine a column raw beside its bucket, which it refuses (two values under one name)", async () => {
345+
// `closed_at` raw and `OBJECT.closed_at` bucketed by month are different
346+
// keys over one column: nothing is merged, and the engine's
347+
// duplicate-column refusal stands on both live faces.
348+
for (const face of LIVE) {
349+
await expect(
350+
ask(face, UNBACKED, {
351+
dimensions: ['closed_at'],
352+
measures: ['total'],
353+
timeDimensions: [{ dimension: `${OBJECT}.closed_at`, granularity: 'month' }],
354+
}),
355+
face,
356+
).rejects.toMatchObject({ code: 'INVALID_QUERY', status: 400 });
357+
}
358+
});
359+
360+
it("serves them beside a cross-object 'account.name' dimension", async () => {
361+
await answeredAlike(LIVE, BACKED, { dimensions: ['region', 'zone', 'account.name'], measures: ['total'] }, {
362+
rows: [
363+
{ region: 'amer', zone: 'amer', 'account.name': 'Acme', total: 100 },
364+
{ region: 'amer', zone: 'amer', 'account.name': 'Globex', total: 50 },
365+
{ region: 'emea', zone: 'emea', 'account.name': 'Acme', total: 30 },
366+
],
367+
fields: ['region:string', 'zone:string', 'account.name:string', 'total:number'],
368+
});
369+
});
370+
});
300371
});

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

Lines changed: 24 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -452,6 +452,25 @@ const RELATIONSHIP_MEMBER_NOUN: Readonly<Record<RelationshipMember['param'], str
452452
where: 'Filter member',
453453
};
454454

455+
/**
456+
* The member each leaf of a `where` tree binds against, in tree order, read
457+
* through the lowering both strategies compile with
458+
* (`normalizeAnalyticsFilterTree` + `collectFilterLeaves`), structure
459+
* discarded. A `where` that is not an object, or that the lowering refuses,
460+
* names nothing here: it is refused where it already is. One reader for
461+
* {@link namedQueryFields}, {@link relationshipMembers} and
462+
* {@link previewWhereColumns}, so what each of them judges is the same leaf
463+
* list.
464+
*/
465+
function whereLeafMembers(where: unknown): string[] {
466+
if (!where || typeof where !== 'object') return [];
467+
try {
468+
return collectFilterLeaves(normalizeAnalyticsFilterTree({ where }, NO_DATETIME_COLUMNS)).map((leaf) => leaf.member);
469+
} catch {
470+
return [];
471+
}
472+
}
473+
455474
/**
456475
* [#28555] Every entry of `query` that reads a column through a relationship
457476
* path, in request order: `dimensions`, `timeDimensions`, then (with
@@ -483,16 +502,7 @@ function relationshipMembers(query: AnalyticsQuery, cube: Cube, withMeasures: bo
483502
for (const member of query.dimensions ?? []) read(member, 'dimensions', 'dimension');
484503
for (const td of query.timeDimensions ?? []) read(td?.dimension, 'timeDimensions', 'any');
485504
if (withMeasures) for (const member of query.measures ?? []) read(member, 'measures', 'measure');
486-
const where = (query as { where?: unknown }).where;
487-
if (where && typeof where === 'object') {
488-
let leaves: string[] = [];
489-
try {
490-
leaves = collectFilterLeaves(normalizeAnalyticsFilterTree({ where }, NO_DATETIME_COLUMNS)).map((leaf) => leaf.member);
491-
} catch {
492-
leaves = [];
493-
}
494-
for (const member of leaves) read(member, 'where', 'any');
495-
}
505+
for (const member of whereLeafMembers((query as { where?: unknown }).where)) read(member, 'where', 'any');
496506
return out;
497507
}
498508

@@ -630,23 +640,14 @@ function namedQueryFields(
630640
}
631641
return true;
632642
};
633-
const filterMembers = (where: unknown): string[] => {
634-
if (!where || typeof where !== 'object') return [];
635-
try {
636-
return collectFilterLeaves(normalizeAnalyticsFilterTree({ where }, NO_DATETIME_COLUMNS)).map((leaf) => leaf.member);
637-
} catch {
638-
return [];
639-
}
640-
};
641-
642643
for (const member of query.dimensions ?? []) name(member, 'dimension', 'aggregate');
643644
for (const td of query.timeDimensions ?? []) name(td.dimension, 'any', td.granularity ? 'aggregate' : 'predicate');
644645
for (const member of query.measures ?? []) if (!durationColumns(member, 'aggregate')) name(member, 'measure', 'aggregate');
645-
for (const member of filterMembers((query as { where?: unknown }).where)) name(member, 'any', 'predicate');
646+
for (const member of whereLeafMembers((query as { where?: unknown }).where)) name(member, 'any', 'predicate');
646647
for (const measure of query.measures ?? []) {
647-
for (const member of filterMembers(scopedMeasureFilter(datasetScope, measure, cube.name))) name(member, 'any', 'predicate');
648+
for (const member of whereLeafMembers(scopedMeasureFilter(datasetScope, measure, cube.name))) name(member, 'any', 'predicate');
648649
}
649-
for (const member of filterMembers(datasetScope?.filter)) name(member, 'any', 'predicate');
650+
for (const member of whereLeafMembers(datasetScope?.filter)) name(member, 'any', 'predicate');
650651
const order = (query as { order?: unknown }).order;
651652
if (order && typeof order === 'object' && !Array.isArray(order)) {
652653
// An order key naming both a duration measure and a declared member is
@@ -4798,16 +4799,8 @@ function previewEvaluationCube(query: AnalyticsQuery, cube: Cube): Cube {
47984799
* before the evaluator runs.
47994800
*/
48004801
function previewWhereColumns(query: AnalyticsQuery, cube: Cube): Record<string, string> {
4801-
const where = (query as { where?: unknown }).where;
4802-
if (!where || typeof where !== 'object') return {};
4803-
let leaves: string[];
4804-
try {
4805-
leaves = collectFilterLeaves(normalizeAnalyticsFilterTree({ where }, NO_DATETIME_COLUMNS)).map((leaf) => leaf.member);
4806-
} catch {
4807-
return {};
4808-
}
48094802
const out: Record<string, string> = {};
4810-
for (const member of leaves) {
4803+
for (const member of whereLeafMembers((query as { where?: unknown }).where)) {
48114804
if (out[member] !== undefined) continue;
48124805
const entry = declaredMemberEntry(cube, member, 'any');
48134806
if (!entry) continue;

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

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,28 @@ function bucketEchoRefused(dimension: string, granularity: string, cause: Bucket
220220
return err;
221221
}
222222

223+
/**
224+
* [#28692, #28744] Append `item` to an `engine.aggregate` `groupBy` unless an
225+
* identical key is already there: the same column, or the same column bucketed
226+
* at the same granularity. Several dimensions can read one column (two dataset
227+
* dimensions declared over it, a relationship's FK column under each dimension
228+
* through it), and they share its one group, which every dimension name is
229+
* mapped from afterwards. The engine refuses a `groupBy` naming a column twice
230+
* (`INVALID_QUERY`), and that refusal stands for a caller that really asks for
231+
* two values under one name: the column raw beside its bucket, or two buckets
232+
* of it, are different keys, so both are handed on and the engine refuses them.
233+
*/
234+
function pushGroupKey(
235+
groupBy: Array<string | { field: string; dateGranularity: string }>,
236+
item: string | { field: string; dateGranularity: string },
237+
): void {
238+
const same = (a: (typeof groupBy)[number]) =>
239+
typeof a === 'string' || typeof item === 'string'
240+
? a === item
241+
: a.field === item.field && a.dateGranularity === item.dateGranularity;
242+
if (!groupBy.some(same)) groupBy.push(item);
243+
}
244+
223245
/**
224246
* ObjectQLStrategy — Priority 2
225247
*
@@ -253,18 +275,22 @@ export class ObjectQLStrategy implements AnalyticsStrategy {
253275
for (const td of query.timeDimensions ?? []) {
254276
if (td.granularity) granByDim.set(td.dimension, td.granularity);
255277
}
278+
// [#28744] Each group key is named once (see {@link pushGroupKey}): two
279+
// dimensions over one column (`region`, and `zone` declared over
280+
// `region`) share its group, and the row mapping below keys the answer by
281+
// every dimension name over it.
256282
const groupBy: GroupByItem[] = [];
257283
if (query.dimensions && query.dimensions.length > 0) {
258284
for (const dim of query.dimensions) {
259285
const field = this.resolveFieldName(cube, dim, 'dimension');
260286
const gran = granByDim.get(dim);
261-
groupBy.push(gran ? { field, dateGranularity: gran } : field);
287+
pushGroupKey(groupBy, gran ? { field, dateGranularity: gran } : field);
262288
granByDim.delete(dim);
263289
}
264290
}
265291
// Time dimensions not also listed in `dimensions` still bucket + group.
266292
for (const [dim, gran] of granByDim) {
267-
groupBy.push({ field: this.resolveFieldName(cube, dim, 'dimension'), dateGranularity: gran });
293+
pushGroupKey(groupBy, { field: this.resolveFieldName(cube, dim, 'dimension'), dateGranularity: gran });
268294
}
269295

270296
// The compiled dataset's own scope (#10298 / PR #10758), read ONCE and
@@ -1345,21 +1371,21 @@ export class ObjectQLStrategy implements AnalyticsStrategy {
13451371
// share it, and the engine refuses a groupBy naming a column twice
13461372
// (`INVALID_QUERY`), where the native strategy served the groups. The
13471373
// re-bucket reads the one column for each dimension.
1348-
if (!groupBy.includes(cd.fkField)) groupBy.push(cd.fkField);
1374+
pushGroupKey(groupBy, cd.fkField);
13491375
continue;
13501376
}
13511377
const field = this.resolveFieldName(cube, dim, 'dimension');
13521378
const gran = granByDim.get(dim);
1353-
// …and a base dimension over that FK column (`account` after
1354-
// `account.name`) reads the one already grouped.
1355-
const fkGrouped = !gran && groupBy.includes(field) && plan.crossDims.some((c) => c.fkField === field);
1356-
if (!fkGrouped) groupBy.push(gran ? { field, dateGranularity: gran } : field);
1379+
// …and so is a base column two dimensions read: the FK column itself
1380+
// (`account` beside `account.name`), or one column under two dimension
1381+
// names (`region`, `zone` over `region`, #28744).
1382+
pushGroupKey(groupBy, gran ? { field, dateGranularity: gran } : field);
13571383
baseDimFields.push(field);
13581384
granByDim.delete(dim);
13591385
}
13601386
for (const [dim, gran] of granByDim) {
13611387
const field = this.resolveFieldName(cube, dim, 'dimension');
1362-
groupBy.push({ field, dateGranularity: gran });
1388+
pushGroupKey(groupBy, { field, dateGranularity: gran });
13631389
baseDimFields.push(field);
13641390
}
13651391

0 commit comments

Comments
 (0)