diff --git a/.changeset/28744-objectql-two-dimensions-one-column.md b/.changeset/28744-objectql-two-dimensions-one-column.md new file mode 100644 index 00000000000..46df5acd418 --- /dev/null +++ b/.changeset/28744-objectql-two-dimensions-one-column.md @@ -0,0 +1,12 @@ +--- +'@objectstack/service-analytics': patch +--- + +fix(service-analytics): the ObjectQL strategy serves two dataset dimensions over one column + +Clause-②: yes (widening) +Metadata-protocol: none + +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. + +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. diff --git a/packages/services/service-analytics/src/__tests__/preview-time-filter-fk-parity.test.ts b/packages/services/service-analytics/src/__tests__/preview-time-filter-fk-parity.test.ts index 2ac57e3e13b..640fc6bc32a 100644 --- a/packages/services/service-analytics/src/__tests__/preview-time-filter-fk-parity.test.ts +++ b/packages/services/service-analytics/src/__tests__/preview-time-filter-fk-parity.test.ts @@ -21,6 +21,11 @@ * two dimensions through one relationship, with the native strategy's * groups. Before, it handed the engine a groupBy naming one column twice and * answered `INVALID_QUERY`. + * - #28744. The ObjectQL strategy answers two dataset dimensions over one + * column (`region`, and `zone` declared over `region`) with one group per + * value, keyed by both names, as native and the preview do, on its direct + * path and beside a cross-object dimension. Before, it answered + * `INVALID_QUERY` for the same duplicate groupBy column. */ import { describe, it, expect, beforeAll, afterAll } from 'vitest'; @@ -248,6 +253,15 @@ describe('[#28690–#28692] time-dimension buckets, column-spelled filter member ); }); + it('reads a filter on a measure key from the measure\'s column, as the live faces resolve it', async () => { + // `total` is a measure over `amount`; the strategies' filter-member + // lookup falls through to the measures, so the row filter is `amount > 60`. + await answeredAlike(FACES, UNBACKED, { dimensions: ['region'], measures: ['total'], runtimeFilter: { total: { $gt: 60 } } }, { + rows: [{ region: 'amer', total: 100 }], + fields: ['region:string', 'total:number'], + }); + }); + it('filters on zone beside a time-dimension-only bucket', async () => { await answeredAlike( FACES, @@ -297,4 +311,61 @@ describe('[#28690–#28692] time-dimension buckets, column-spelled filter member }); }); }); + describe('#28744 — the ObjectQL strategy groups a column once when two dimensions read it', () => { + it.each([ + ['region, zone', ['region', 'zone']], + ['zone, region', ['zone', 'region']], + ])('serves dimensions %s over one column with one group per value, where it answered INVALID_QUERY', async (_label, dimensions) => { + await answeredAlike(FACES, UNBACKED, { dimensions, measures: ['total'] }, { + rows: [ + { region: 'amer', zone: 'amer', total: 150 }, + { region: 'emea', zone: 'emea', total: 30 }, + ], + fields: [...dimensions.map((d) => `${d}:string`), 'total:number'], + }); + }); + + it('serves them beside a month bucket', async () => { + await answeredAlike( + FACES, + UNBACKED, + { dimensions: ['region', 'zone'], measures: ['total'], timeDimensions: [{ dimension: 'closed_at', granularity: 'month' }] }, + { + rows: [ + { region: 'amer', zone: 'amer', closed_at: '2026-03', total: 100 }, + { region: 'amer', zone: 'amer', closed_at: '2026-04', total: 50 }, + { region: 'emea', zone: 'emea', closed_at: '2026-04', total: 30 }, + ], + fields: ['region:string', 'zone:string', 'closed_at:time', 'total:number'], + }, + ); + }); + + it("still hands the engine a column raw beside its bucket, which it refuses (two values under one name)", async () => { + // `closed_at` raw and `OBJECT.closed_at` bucketed by month are different + // keys over one column: nothing is merged, and the engine's + // duplicate-column refusal stands on both live faces. + for (const face of LIVE) { + await expect( + ask(face, UNBACKED, { + dimensions: ['closed_at'], + measures: ['total'], + timeDimensions: [{ dimension: `${OBJECT}.closed_at`, granularity: 'month' }], + }), + face, + ).rejects.toMatchObject({ code: 'INVALID_QUERY', status: 400 }); + } + }); + + it("serves them beside a cross-object 'account.name' dimension", async () => { + await answeredAlike(LIVE, BACKED, { dimensions: ['region', 'zone', 'account.name'], measures: ['total'] }, { + rows: [ + { region: 'amer', zone: 'amer', 'account.name': 'Acme', total: 100 }, + { region: 'amer', zone: 'amer', 'account.name': 'Globex', total: 50 }, + { region: 'emea', zone: 'emea', 'account.name': 'Acme', total: 30 }, + ], + fields: ['region:string', 'zone:string', 'account.name:string', 'total:number'], + }); + }); + }); }); diff --git a/packages/services/service-analytics/src/analytics-service.ts b/packages/services/service-analytics/src/analytics-service.ts index 057ef2cfbb9..d4dbdde397a 100644 --- a/packages/services/service-analytics/src/analytics-service.ts +++ b/packages/services/service-analytics/src/analytics-service.ts @@ -452,6 +452,25 @@ const RELATIONSHIP_MEMBER_NOUN: Readonly leaf.member); + } catch { + return []; + } +} + /** * [#28555] Every entry of `query` that reads a column through a relationship * path, in request order: `dimensions`, `timeDimensions`, then (with @@ -483,16 +502,7 @@ function relationshipMembers(query: AnalyticsQuery, cube: Cube, withMeasures: bo for (const member of query.dimensions ?? []) read(member, 'dimensions', 'dimension'); for (const td of query.timeDimensions ?? []) read(td?.dimension, 'timeDimensions', 'any'); if (withMeasures) for (const member of query.measures ?? []) read(member, 'measures', 'measure'); - const where = (query as { where?: unknown }).where; - if (where && typeof where === 'object') { - let leaves: string[] = []; - try { - leaves = collectFilterLeaves(normalizeAnalyticsFilterTree({ where }, NO_DATETIME_COLUMNS)).map((leaf) => leaf.member); - } catch { - leaves = []; - } - for (const member of leaves) read(member, 'where', 'any'); - } + for (const member of whereLeafMembers((query as { where?: unknown }).where)) read(member, 'where', 'any'); return out; } @@ -630,23 +640,14 @@ function namedQueryFields( } return true; }; - const filterMembers = (where: unknown): string[] => { - if (!where || typeof where !== 'object') return []; - try { - return collectFilterLeaves(normalizeAnalyticsFilterTree({ where }, NO_DATETIME_COLUMNS)).map((leaf) => leaf.member); - } catch { - return []; - } - }; - for (const member of query.dimensions ?? []) name(member, 'dimension', 'aggregate'); for (const td of query.timeDimensions ?? []) name(td.dimension, 'any', td.granularity ? 'aggregate' : 'predicate'); for (const member of query.measures ?? []) if (!durationColumns(member, 'aggregate')) name(member, 'measure', 'aggregate'); - for (const member of filterMembers((query as { where?: unknown }).where)) name(member, 'any', 'predicate'); + for (const member of whereLeafMembers((query as { where?: unknown }).where)) name(member, 'any', 'predicate'); for (const measure of query.measures ?? []) { - for (const member of filterMembers(scopedMeasureFilter(datasetScope, measure, cube.name))) name(member, 'any', 'predicate'); + for (const member of whereLeafMembers(scopedMeasureFilter(datasetScope, measure, cube.name))) name(member, 'any', 'predicate'); } - for (const member of filterMembers(datasetScope?.filter)) name(member, 'any', 'predicate'); + for (const member of whereLeafMembers(datasetScope?.filter)) name(member, 'any', 'predicate'); const order = (query as { order?: unknown }).order; if (order && typeof order === 'object' && !Array.isArray(order)) { // An order key naming both a duration measure and a declared member is @@ -4798,16 +4799,8 @@ function previewEvaluationCube(query: AnalyticsQuery, cube: Cube): Cube { * before the evaluator runs. */ function previewWhereColumns(query: AnalyticsQuery, cube: Cube): Record { - const where = (query as { where?: unknown }).where; - if (!where || typeof where !== 'object') return {}; - let leaves: string[]; - try { - leaves = collectFilterLeaves(normalizeAnalyticsFilterTree({ where }, NO_DATETIME_COLUMNS)).map((leaf) => leaf.member); - } catch { - return {}; - } const out: Record = {}; - for (const member of leaves) { + for (const member of whereLeafMembers((query as { where?: unknown }).where)) { if (out[member] !== undefined) continue; const entry = declaredMemberEntry(cube, member, 'any'); if (!entry) continue; diff --git a/packages/services/service-analytics/src/strategies/objectql-strategy.ts b/packages/services/service-analytics/src/strategies/objectql-strategy.ts index c5327455533..b891268af80 100644 --- a/packages/services/service-analytics/src/strategies/objectql-strategy.ts +++ b/packages/services/service-analytics/src/strategies/objectql-strategy.ts @@ -220,6 +220,28 @@ function bucketEchoRefused(dimension: string, granularity: string, cause: Bucket return err; } +/** + * [#28692, #28744] Append `item` to an `engine.aggregate` `groupBy` unless an + * identical key is already there: the same column, or the same column bucketed + * at the same granularity. Several dimensions can read one column (two dataset + * dimensions declared over it, a relationship's FK column under each dimension + * through it), and they share its one group, which every dimension name is + * mapped from afterwards. The engine refuses a `groupBy` naming a column twice + * (`INVALID_QUERY`), and that refusal stands for a caller that really asks for + * two values under one name: the column raw beside its bucket, or two buckets + * of it, are different keys, so both are handed on and the engine refuses them. + */ +function pushGroupKey( + groupBy: Array, + item: string | { field: string; dateGranularity: string }, +): void { + const same = (a: (typeof groupBy)[number]) => + typeof a === 'string' || typeof item === 'string' + ? a === item + : a.field === item.field && a.dateGranularity === item.dateGranularity; + if (!groupBy.some(same)) groupBy.push(item); +} + /** * ObjectQLStrategy — Priority 2 * @@ -253,18 +275,22 @@ export class ObjectQLStrategy implements AnalyticsStrategy { for (const td of query.timeDimensions ?? []) { if (td.granularity) granByDim.set(td.dimension, td.granularity); } + // [#28744] Each group key is named once (see {@link pushGroupKey}): two + // dimensions over one column (`region`, and `zone` declared over + // `region`) share its group, and the row mapping below keys the answer by + // every dimension name over it. const groupBy: GroupByItem[] = []; if (query.dimensions && query.dimensions.length > 0) { for (const dim of query.dimensions) { const field = this.resolveFieldName(cube, dim, 'dimension'); const gran = granByDim.get(dim); - groupBy.push(gran ? { field, dateGranularity: gran } : field); + pushGroupKey(groupBy, gran ? { field, dateGranularity: gran } : field); granByDim.delete(dim); } } // Time dimensions not also listed in `dimensions` still bucket + group. for (const [dim, gran] of granByDim) { - groupBy.push({ field: this.resolveFieldName(cube, dim, 'dimension'), dateGranularity: gran }); + pushGroupKey(groupBy, { field: this.resolveFieldName(cube, dim, 'dimension'), dateGranularity: gran }); } // The compiled dataset's own scope (#10298 / PR #10758), read ONCE and @@ -1345,21 +1371,21 @@ export class ObjectQLStrategy implements AnalyticsStrategy { // share it, and the engine refuses a groupBy naming a column twice // (`INVALID_QUERY`), where the native strategy served the groups. The // re-bucket reads the one column for each dimension. - if (!groupBy.includes(cd.fkField)) groupBy.push(cd.fkField); + pushGroupKey(groupBy, cd.fkField); continue; } const field = this.resolveFieldName(cube, dim, 'dimension'); const gran = granByDim.get(dim); - // …and a base dimension over that FK column (`account` after - // `account.name`) reads the one already grouped. - const fkGrouped = !gran && groupBy.includes(field) && plan.crossDims.some((c) => c.fkField === field); - if (!fkGrouped) groupBy.push(gran ? { field, dateGranularity: gran } : field); + // …and so is a base column two dimensions read: the FK column itself + // (`account` beside `account.name`), or one column under two dimension + // names (`region`, `zone` over `region`, #28744). + pushGroupKey(groupBy, gran ? { field, dateGranularity: gran } : field); baseDimFields.push(field); granByDim.delete(dim); } for (const [dim, gran] of granByDim) { const field = this.resolveFieldName(cube, dim, 'dimension'); - groupBy.push({ field, dateGranularity: gran }); + pushGroupKey(groupBy, { field, dateGranularity: gran }); baseDimFields.push(field); }