From 722fbe7c32a853bd5bb883bf8957778a7d108754 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 1 Nov 2026 19:00:39 +0000 Subject: [PATCH 1/2] fix(service-analytics)!: the draft preview refuses one column asked raw beside its own time bucket, as the live faces do - objectql-strategy.ts: the direct path's groupBy build moves into a public groupByOf(query, cube), which execute() calls; exported type AggregateGroupKey. No behaviour change on the live faces. - preview-group-column-door.ts (new): assertPreviewGroupColumnsDistinct(), the draft preview's copy of objectql's duplicate-group-column refusal (objectql is a dev dependency only), INVALID_QUERY / 400 with the engine's message, path, param, object and fields. - analytics-service.ts: the preview gate list asks it over the ObjectQL strategy's own keys, after assertPreviewJoinsNothing. - Tests on real SQLite: three refusal cases compared field by field with both live faces, and a dimension that is its own time dimension still served. Co-Authored-By: Claude --- ...review-refuses-column-raw-beside-bucket.md | 19 +++++ .../preview-time-filter-fk-parity.test.ts | 61 ++++++++++++++++ .../src/analytics-service.ts | 13 +++- .../src/preview-group-column-door.ts | 72 +++++++++++++++++++ .../src/strategies/objectql-strategy.ts | 39 ++++++---- 5 files changed, 191 insertions(+), 13 deletions(-) create mode 100644 .changeset/28765-preview-refuses-column-raw-beside-bucket.md create mode 100644 packages/services/service-analytics/src/preview-group-column-door.ts diff --git a/.changeset/28765-preview-refuses-column-raw-beside-bucket.md b/.changeset/28765-preview-refuses-column-raw-beside-bucket.md new file mode 100644 index 00000000000..88f3dcf6eeb --- /dev/null +++ b/.changeset/28765-preview-refuses-column-raw-beside-bucket.md @@ -0,0 +1,19 @@ +--- +'@objectstack/service-analytics': patch +--- + +fix(service-analytics)!: the draft preview refuses one column asked raw beside its own time bucket, as the live faces do + +Clause-②: no (narrowing) — the draft preview (`queryDataset` with `previewDrafts: true`) now refuses `INVALID_QUERY` / 400 a selection that asks one column for two values under one name (a dimension `X` beside a `timeDimensions` entry `OBJECT.X` with a granularity, or two buckets of one column), with the message, `path`, `param`, `object` and `fields` the native and ObjectQL strategies already answer; it served both columns. Nothing is widened: no export, answer member, error code or status is added, and the code was already on the wire. +Metadata-protocol: none + + + +**BREAKING** accept-set narrowing on the `queryDataset` draft preview (`@objectstack/service-analytics`). + +- **Before.** Measured on real SQLite: `dimensions: ['closed_at']` with `timeDimensions: [{ dimension: 'OBJECT.closed_at', granularity: 'month' }]` was refused by both live strategies with the engine's duplicate-column refusal (`groupBy.1.field names the column 'closed_at', which groupBy.0 already names`), while the draft preview answered one row per raw timestamp carrying both columns. The same held for a month and a year bucket of one column. +- **Now.** The preview judges the keys the ObjectQL strategy would hand the engine for the selection, at the same positions, and refuses with the engine's refusal for them, so all three faces answer one `INVALID_QUERY` / 400 with one message. +- **Unchanged.** A raw dimension beside a bucket of a different column, a dimension that is also its own time dimension (one bucket), and two dimensions over one column with no bucket (#28744) are still served on every face. The live refusal does not move. +- **Who sees it.** An author previewing a draft dataset with such a selection: the preview now says what publish would say, instead of a table publish refuses. + +**The remedy.** Ask for the column once: as the bucketed time dimension, or as the raw dimension. 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 640fc6bc32a..5c7a0bb280b 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 @@ -26,6 +26,10 @@ * 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. + * - #28765. A selection that asks one column raw beside its own bucket (or for + * two buckets of it) under one name is refused by the draft preview with the + * live faces' refusal: `INVALID_QUERY` / 400, the same message, path, param, + * object and fields. Before, the preview served both columns. */ import { describe, it, expect, beforeAll, afterAll } from 'vitest'; @@ -368,4 +372,61 @@ describe('[#28690–#28692] time-dimension buckets, column-spelled filter member }); }); }); + describe('#28765 — the draft preview refuses one column asked twice under one name, as the live faces do', () => { + /** Every face refuses `selection`, and the preview's refusal is the ObjectQL face's, field for field. */ + const refusedAsLive = async (selection: Selection, path: string) => { + const refusals: Array> = []; + for (const face of FACES) { + const err = (await ask(face, UNBACKED, selection).then( + () => { throw new Error(`${face}: expected a refusal, but the query resolved`); }, + (e: unknown) => e, + )) as Error & Record; + expect(err.code, `${face}: ${err.message}`).toBe('INVALID_QUERY'); + expect(err.status, face).toBe(400); + expect(err.path, face).toBe(path); + refusals.push({ + message: err.message, code: err.code, status: err.status, httpStatus: err.httpStatus, + object: err.object, param: err.param, path: err.path, fields: err.fields, + }); + } + expect(refusals[2], 'preview vs ObjectQL').toEqual(refusals[1]); + expect(refusals[2], 'preview vs native').toEqual(refusals[0]); + }; + + it('refuses a column raw beside its own month bucket, where the preview served both', async () => { + await refusedAsLive( + { dimensions: ['closed_at'], measures: ['total'], timeDimensions: [{ dimension: `${OBJECT}.closed_at`, granularity: 'month' }] }, + 'groupBy.1.field', + ); + }); + + it('refuses it after another dimension, naming the later position', async () => { + await refusedAsLive( + { dimensions: ['region', 'closed_at'], measures: ['total'], timeDimensions: [{ dimension: `${OBJECT}.closed_at`, granularity: 'month' }] }, + 'groupBy.2.field', + ); + }); + + it('refuses two buckets of one column', async () => { + await refusedAsLive( + { + measures: ['total'], + timeDimensions: [ + { dimension: 'closed_at', granularity: 'month' }, + { dimension: `${OBJECT}.closed_at`, granularity: 'year' }, + ], + }, + 'groupBy.1.field', + ); + }); + + it('still serves a dimension that is also its own time dimension, as one bucket', async () => { + await answeredAlike( + FACES, + UNBACKED, + { dimensions: ['closed_at'], measures: ['total'], timeDimensions: [{ dimension: 'closed_at', granularity: 'month' }] }, + { rows: [{ closed_at: '2026-03', total: 100 }, { closed_at: '2026-04', total: 80 }], fields: ['closed_at:time', 'total:number'] }, + ); + }); + }); }); diff --git a/packages/services/service-analytics/src/analytics-service.ts b/packages/services/service-analytics/src/analytics-service.ts index d4dbdde397a..36a520faa98 100644 --- a/packages/services/service-analytics/src/analytics-service.ts +++ b/packages/services/service-analytics/src/analytics-service.ts @@ -116,6 +116,7 @@ import { type DimensionLabelDeps, } from './dimension-labels.js'; import { evaluateAnalyticsQueryOverRows } from './preview-evaluator.js'; +import { assertPreviewGroupColumnsDistinct } from './preview-group-column-door.js'; // [#25778] The one lookup of a dataset duration measure by name. import { durationMeasureIn } from './duration-measure-sql.js'; import { scopedMeasureFilter } from './scoped-measure-key.js'; @@ -1464,6 +1465,11 @@ interface CubeReads { */ export class AnalyticsService implements IAnalyticsService { private readonly strategies: AnalyticsStrategy[]; + /** + * The built-in ObjectQL strategy, held by name as well so the draft preview + * judges the `groupBy` it would hand the engine (#28765). + */ + private readonly objectqlStrategy = new ObjectQLStrategy(); /** Context-independent part of the StrategyContext (no per-request scope). */ private readonly baseCtx: DatasetScopedStrategyContext; /** Context-aware read-scope provider (bound to the request's context per call). */ @@ -1708,7 +1714,7 @@ export class AnalyticsService implements IAnalyticsService { // When fallbackService is configured, an internal delegate is added at P3. const builtIn: AnalyticsStrategy[] = [ new NativeSQLStrategy(), - new ObjectQLStrategy(), + this.objectqlStrategy, ]; // Auto-add fallback delegate when fallbackService is provided @@ -2890,6 +2896,11 @@ export class AnalyticsService implements IAnalyticsService { // under one `null` key (and a filter on one matched no row) where // the live strategies join it. See {@link assertPreviewJoinsNothing}. this.assertPreviewJoinsNothing(q, cube); + // [#28765] …and a selection asking one column for two values + // under one name (raw beside its own bucket, or two buckets of + // it), with the refusal the engine gives both live faces for the + // same keys, where the evaluator served both columns. + assertPreviewGroupColumnsDistinct(dataset.object, this.objectqlStrategy.groupByOf(q, cube)); return evaluateAnalyticsQueryOverRows( q, previewEvaluationCube(q, cube), diff --git a/packages/services/service-analytics/src/preview-group-column-door.ts b/packages/services/service-analytics/src/preview-group-column-door.ts new file mode 100644 index 00000000000..a7bbd7876b4 --- /dev/null +++ b/packages/services/service-analytics/src/preview-group-column-door.ts @@ -0,0 +1,72 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#28765] The draft preview's copy of the engine's duplicate-group-column + * refusal. + * + * Both live faces hand `engine.aggregate` the `groupBy` the ObjectQL strategy + * builds (the native strategy delegates a bucketed time dimension to it), and + * the engine refuses one naming a column twice: `@objectstack/objectql`'s + * `assertAggregatedColumnsAreDistinct` (`aggregate-query-shape-door.ts`), + * `INVALID_QUERY` / 400. The strategy already names a key once when several + * dimensions read it (#28744), so what still reaches that refusal is a + * selection asking one column for two values under one name: the column raw + * beside its own bucket, or two buckets of it. + * + * The preview evaluates drafted rows in memory and hands no engine anything, + * so it served both columns. It now judges the strategy's own keys + * (`ObjectQLStrategy.groupByOf`, so the positions are the engine's) with this + * function and refuses the same way. + * + * Why a copy and not the engine's function: `@objectstack/objectql` is a dev + * dependency of this package, never a runtime one, and its root does not + * export the door. The message, code, status and keys here are the engine's, + * byte for byte, and `preview-time-filter-fk-parity.test.ts` compares the + * preview's refusal with both live faces' field by field, so a change to the + * engine's wording reds that test. Only the `groupBy` half is copied: the + * strategy's keys carry no `alias`, and a measure alias colliding with a group + * column is a different question. + */ + +import { StandardErrorCode } from '@objectstack/spec/api'; +import type { AggregateGroupKey } from './strategies/objectql-strategy.js'; + +type GroupColumnRefusal = Error & { + code?: string; status?: number; httpStatus?: number; + fields?: string[]; object?: string; param?: string; path?: string; +}; + +/** The column a key projects, and how the engine names its position. */ +function columnOf(key: AggregateGroupKey, i: number): { column: string; path: string } { + return typeof key === 'string' + ? { column: key, path: `groupBy.${i}` } + : { column: key.field, path: `groupBy.${i}.field` }; +} + +/** + * Refuse `groupBy` (the keys the ObjectQL strategy would hand the engine for + * a query over `object`) when two of them project one column, with the + * engine's refusal for it. + */ +export function assertPreviewGroupColumnsDistinct(object: string, groupBy: readonly AggregateGroupKey[]): void { + const seen = new Map(); + for (const [i, key] of groupBy.entries()) { + const { column, path } = columnOf(key, i); + const first = seen.get(column); + if (first === undefined) { + seen.set(column, path); + continue; + } + const err = new Error( + `aggregate('${object}'): ${path} names the column '${column}', which ${first} already names — ` + + `one row cannot carry two values under one column. The query was NOT run. Give one of the group ` + + "entries its own 'alias'. A repeated column name kept only the last value written under it: a " + + 'count overwritten by a sum, or the group keys replaced by the numbers, while the answer looked served.', + ) as GroupColumnRefusal; + err.code = StandardErrorCode.enum.INVALID_QUERY; + err.status = 400; + err.httpStatus = 400; + Object.assign(err, { object, param: 'groupBy', path, fields: [column] }); + throw err; + } +} diff --git a/packages/services/service-analytics/src/strategies/objectql-strategy.ts b/packages/services/service-analytics/src/strategies/objectql-strategy.ts index b891268af80..4e3f69e7aa3 100644 --- a/packages/services/service-analytics/src/strategies/objectql-strategy.ts +++ b/packages/services/service-analytics/src/strategies/objectql-strategy.ts @@ -220,6 +220,9 @@ function bucketEchoRefused(dimension: string, granularity: string, cause: Bucket return err; } +/** One `engine.aggregate` group key: a column, or a column bucketed by a granularity. */ +export type AggregateGroupKey = string | { field: string; dateGranularity: string }; + /** * [#28692, #28744] Append `item` to an `engine.aggregate` `groupBy` unless an * identical key is already there: the same column, or the same column bucketed @@ -231,10 +234,7 @@ function bucketEchoRefused(dimension: string, granularity: string, cause: Bucket * 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 { +function pushGroupKey(groupBy: AggregateGroupKey[], item: AggregateGroupKey): void { const same = (a: (typeof groupBy)[number]) => typeof a === 'string' || typeof item === 'string' ? a === item @@ -259,10 +259,18 @@ export class ObjectQLStrategy implements AnalyticsStrategy { return caps.objectqlAggregate && typeof ctx.executeAggregate === 'function'; } - async execute(query: AnalyticsQuery, ctx: StrategyContext): Promise { - const cube = ctx.getCube(query.cube!)!; - const objectName = this.extractObjectName(cube); - + /** + * The `engine.aggregate` `groupBy` {@link execute} hands the engine for + * `query`, in order: one key per `dimensions` entry, then one per bucketed + * `timeDimensions` entry not also listed there, each named once + * ({@link pushGroupKey}). + * + * [#28765] Public so the draft preview, which hands no engine anything, + * judges the same keys at the same positions and refuses a selection the + * engine would refuse with the engine's own words + * ({@link assertPreviewGroupColumnsDistinct}). + */ + groupByOf(query: AnalyticsQuery, cube: Cube): AggregateGroupKey[] { // Build groupBy from dimensions, honouring `timeDimensions` granularity. // A date dimension with a granularity becomes a STRUCTURED groupBy item // `{ field, dateGranularity }` — which `engine.aggregate()` buckets (the @@ -270,16 +278,15 @@ export class ObjectQLStrategy implements AnalyticsStrategy { // path grouped raw timestamps (one bucket per row) and date-bucketed // dataset widgets never matched their legacy `categoryGranularity` // counterpart. - type GroupByItem = string | { field: string; dateGranularity: string }; const granByDim = new Map(); 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[] = []; + // `region`) share its group, and the row mapping in {@link execute} keys + // the answer by every dimension name over it. + const groupBy: AggregateGroupKey[] = []; if (query.dimensions && query.dimensions.length > 0) { for (const dim of query.dimensions) { const field = this.resolveFieldName(cube, dim, 'dimension'); @@ -292,6 +299,14 @@ export class ObjectQLStrategy implements AnalyticsStrategy { for (const [dim, gran] of granByDim) { pushGroupKey(groupBy, { field: this.resolveFieldName(cube, dim, 'dimension'), dateGranularity: gran }); } + return groupBy; + } + + async execute(query: AnalyticsQuery, ctx: StrategyContext): Promise { + const cube = ctx.getCube(query.cube!)!; + const objectName = this.extractObjectName(cube); + + const groupBy = this.groupByOf(query, cube); // The compiled dataset's own scope (#10298 / PR #10758), read ONCE and // reused below for both the per-measure aggregation filter (#10413 phase From d26d29b40aaf118afde7ab286928849d70991e0d Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 1 Nov 2026 20:22:44 +0000 Subject: [PATCH 2/2] fix(service-analytics): build the preview's group keys with a module-level function, not a public strategy method Contract review of the previous commit: ObjectQLStrategy is re-exported from the package root, so its new public groupByOf() widened the published surface under a narrowing declaration. The key builder is now the module-level aggregateGroupKeys(query, cube) in objectql-strategy.ts (not re-exported from the root), with the member lookups it reads (lookupCubeMember, resolveCubeFieldName) and aggregateObjectOf(cube) beside it; the strategy's private methods delegate to them. execute() and the preview door both call it, and the service no longer holds a named ObjectQLStrategy. assertPreviewGroupColumnsDistinct(query, cube) now reads the object off the cube the way the strategy does (review should-fix 1). Co-Authored-By: Claude --- .../src/analytics-service.ts | 9 +- .../src/preview-group-column-door.ts | 19 +- .../src/strategies/objectql-strategy.ts | 163 ++++++++++-------- 3 files changed, 109 insertions(+), 82 deletions(-) diff --git a/packages/services/service-analytics/src/analytics-service.ts b/packages/services/service-analytics/src/analytics-service.ts index 36a520faa98..67997ef6763 100644 --- a/packages/services/service-analytics/src/analytics-service.ts +++ b/packages/services/service-analytics/src/analytics-service.ts @@ -1465,11 +1465,6 @@ interface CubeReads { */ export class AnalyticsService implements IAnalyticsService { private readonly strategies: AnalyticsStrategy[]; - /** - * The built-in ObjectQL strategy, held by name as well so the draft preview - * judges the `groupBy` it would hand the engine (#28765). - */ - private readonly objectqlStrategy = new ObjectQLStrategy(); /** Context-independent part of the StrategyContext (no per-request scope). */ private readonly baseCtx: DatasetScopedStrategyContext; /** Context-aware read-scope provider (bound to the request's context per call). */ @@ -1714,7 +1709,7 @@ export class AnalyticsService implements IAnalyticsService { // When fallbackService is configured, an internal delegate is added at P3. const builtIn: AnalyticsStrategy[] = [ new NativeSQLStrategy(), - this.objectqlStrategy, + new ObjectQLStrategy(), ]; // Auto-add fallback delegate when fallbackService is provided @@ -2900,7 +2895,7 @@ export class AnalyticsService implements IAnalyticsService { // under one name (raw beside its own bucket, or two buckets of // it), with the refusal the engine gives both live faces for the // same keys, where the evaluator served both columns. - assertPreviewGroupColumnsDistinct(dataset.object, this.objectqlStrategy.groupByOf(q, cube)); + assertPreviewGroupColumnsDistinct(q, cube); return evaluateAnalyticsQueryOverRows( q, previewEvaluationCube(q, cube), diff --git a/packages/services/service-analytics/src/preview-group-column-door.ts b/packages/services/service-analytics/src/preview-group-column-door.ts index a7bbd7876b4..052e88f077c 100644 --- a/packages/services/service-analytics/src/preview-group-column-door.ts +++ b/packages/services/service-analytics/src/preview-group-column-door.ts @@ -15,8 +15,9 @@ * * The preview evaluates drafted rows in memory and hands no engine anything, * so it served both columns. It now judges the strategy's own keys - * (`ObjectQLStrategy.groupByOf`, so the positions are the engine's) with this - * function and refuses the same way. + * (`aggregateGroupKeys`, the function the strategy's direct path builds them + * with, so the positions are the engine's) on the object the strategy names + * (`aggregateObjectOf`), and refuses the same way. * * Why a copy and not the engine's function: `@objectstack/objectql` is a dev * dependency of this package, never a runtime one, and its root does not @@ -29,7 +30,9 @@ */ import { StandardErrorCode } from '@objectstack/spec/api'; -import type { AggregateGroupKey } from './strategies/objectql-strategy.js'; +import type { AnalyticsQuery } from '@objectstack/spec/contracts'; +import type { Cube } from '@objectstack/spec/data'; +import { aggregateGroupKeys, aggregateObjectOf, type AggregateGroupKey } from './strategies/objectql-strategy.js'; type GroupColumnRefusal = Error & { code?: string; status?: number; httpStatus?: number; @@ -44,11 +47,13 @@ function columnOf(key: AggregateGroupKey, i: number): { column: string; path: st } /** - * Refuse `groupBy` (the keys the ObjectQL strategy would hand the engine for - * a query over `object`) when two of them project one column, with the - * engine's refusal for it. + * Refuse `query` when two of the keys the ObjectQL strategy would hand the + * engine for it over `cube` project one column, with the engine's refusal for + * it. */ -export function assertPreviewGroupColumnsDistinct(object: string, groupBy: readonly AggregateGroupKey[]): void { +export function assertPreviewGroupColumnsDistinct(query: AnalyticsQuery, cube: Cube): void { + const object = aggregateObjectOf(cube); + const groupBy = aggregateGroupKeys(query, cube); const seen = new Map(); for (const [i, key] of groupBy.entries()) { const { column, path } = columnOf(key, i); diff --git a/packages/services/service-analytics/src/strategies/objectql-strategy.ts b/packages/services/service-analytics/src/strategies/objectql-strategy.ts index 4e3f69e7aa3..3b60e719b13 100644 --- a/packages/services/service-analytics/src/strategies/objectql-strategy.ts +++ b/packages/services/service-analytics/src/strategies/objectql-strategy.ts @@ -242,6 +242,95 @@ function pushGroupKey(groupBy: AggregateGroupKey[], item: AggregateGroupKey): vo if (!groupBy.some(same)) groupBy.push(item); } +/** + * The member `member` resolves to in `cube`'s `kind` bag: its own key, a + * `.`-qualified or other dotted spelling of a key, the key with its dots + * flattened, or (for a dotted dimension) a synthetic relation traversal. + * Module-level because it reads only the cube; `ObjectQLStrategy.lookupMember` + * is this function. + */ +function lookupCubeMember( + cube: Cube, + member: string, + kind: 'dimension' | 'measure', +): { sql: string; type?: string } | undefined { + const bag = kind === 'dimension' ? cube.dimensions : cube.measures; + if (bag[member]) return bag[member]; + if (member.includes('.')) { + const [first, ...rest] = member.split('.'); + const tail = rest.join('.'); + if (first === cube.name && bag[tail]) return bag[tail]; + if (bag[tail]) return bag[tail]; + const flat = member.replace(/\./g, '_'); + if (bag[flat]) return bag[flat]; + if (kind === 'dimension') return { sql: member, type: 'string' }; + } else if (bag[member]) { + return bag[member]; + } + return undefined; +} + +/** The column `member` reads on the base object; `ObjectQLStrategy.resolveFieldName` is this function. */ +function resolveCubeFieldName(cube: Cube, member: string, kind: 'dimension' | 'measure' | 'any'): string { + if (kind === 'dimension' || kind === 'any') { + const dim = lookupCubeMember(cube, member, 'dimension'); + if (dim) return dim.sql.replace(/^\$/, ''); + } + if (kind === 'measure' || kind === 'any') { + const measure = lookupCubeMember(cube, member, 'measure'); + if (measure) return measure.sql.replace(/^\$/, ''); + } + return member.includes('.') ? member.split('.')[1] : member; +} + +/** The object an aggregate over `cube` runs on; `ObjectQLStrategy.extractObjectName` is this function. */ +export function aggregateObjectOf(cube: Cube): string { + return cube.sql.trim(); +} + +/** + * The `engine.aggregate` `groupBy` the strategy's direct path hands the engine + * for `query`, in order: one key per `dimensions` entry, then one per bucketed + * `timeDimensions` entry not also listed there, each named once + * ({@link pushGroupKey}). + * + * [#28765] Module-level, and not exported from the package root, so the draft + * preview, which hands no engine anything, judges the same keys at the same + * positions and refuses a selection the engine would refuse with the engine's + * own words (`assertPreviewGroupColumnsDistinct`). It reads only the cube. + */ +export function aggregateGroupKeys(query: AnalyticsQuery, cube: Cube): AggregateGroupKey[] { + // Build groupBy from dimensions, honouring `timeDimensions` granularity. + // A date dimension with a granularity becomes a STRUCTURED groupBy item + // `{ field, dateGranularity }` — which `engine.aggregate()` buckets (the + // driver's own SQL expression, or in memory). Without this the ObjectQL + // path grouped raw timestamps (one bucket per row) and date-bucketed + // dataset widgets never matched their legacy `categoryGranularity` + // counterpart. + const granByDim = new Map(); + 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 in `ObjectQLStrategy.execute` keys + // the answer by every dimension name over it. + const groupBy: AggregateGroupKey[] = []; + if (query.dimensions && query.dimensions.length > 0) { + for (const dim of query.dimensions) { + const field = resolveCubeFieldName(cube, dim, 'dimension'); + const gran = granByDim.get(dim); + 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) { + pushGroupKey(groupBy, { field: resolveCubeFieldName(cube, dim, 'dimension'), dateGranularity: gran }); + } + return groupBy; +} + /** * ObjectQLStrategy — Priority 2 * @@ -259,54 +348,11 @@ export class ObjectQLStrategy implements AnalyticsStrategy { return caps.objectqlAggregate && typeof ctx.executeAggregate === 'function'; } - /** - * The `engine.aggregate` `groupBy` {@link execute} hands the engine for - * `query`, in order: one key per `dimensions` entry, then one per bucketed - * `timeDimensions` entry not also listed there, each named once - * ({@link pushGroupKey}). - * - * [#28765] Public so the draft preview, which hands no engine anything, - * judges the same keys at the same positions and refuses a selection the - * engine would refuse with the engine's own words - * ({@link assertPreviewGroupColumnsDistinct}). - */ - groupByOf(query: AnalyticsQuery, cube: Cube): AggregateGroupKey[] { - // Build groupBy from dimensions, honouring `timeDimensions` granularity. - // A date dimension with a granularity becomes a STRUCTURED groupBy item - // `{ field, dateGranularity }` — which `engine.aggregate()` buckets (the - // driver's own SQL expression, or in memory). Without this the ObjectQL - // path grouped raw timestamps (one bucket per row) and date-bucketed - // dataset widgets never matched their legacy `categoryGranularity` - // counterpart. - const granByDim = new Map(); - 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 in {@link execute} keys - // the answer by every dimension name over it. - const groupBy: AggregateGroupKey[] = []; - 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); - 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) { - pushGroupKey(groupBy, { field: this.resolveFieldName(cube, dim, 'dimension'), dateGranularity: gran }); - } - return groupBy; - } - async execute(query: AnalyticsQuery, ctx: StrategyContext): Promise { const cube = ctx.getCube(query.cube!)!; const objectName = this.extractObjectName(cube); - const groupBy = this.groupByOf(query, cube); + const groupBy = aggregateGroupKeys(query, cube); // The compiled dataset's own scope (#10298 / PR #10758), read ONCE and // reused below for both the per-measure aggregation filter (#10413 phase @@ -1716,22 +1762,10 @@ export class ObjectQLStrategy implements AnalyticsStrategy { member: string, kind: 'dimension' | 'measure', ): { sql: string; type?: string } | undefined { - const bag = kind === 'dimension' ? cube.dimensions : cube.measures; - if (bag[member]) return bag[member]; - if (member.includes('.')) { - const [first, ...rest] = member.split('.'); - const tail = rest.join('.'); - if (first === cube.name && bag[tail]) return bag[tail]; - if (bag[tail]) return bag[tail]; - const flat = member.replace(/\./g, '_'); - if (bag[flat]) return bag[flat]; - if (kind === 'dimension') return { sql: member, type: 'string' }; - } else if (bag[member]) { - return bag[member]; - } - return undefined; + return lookupCubeMember(cube, member, kind); } + /** * [#14079] The (object, field) a filter member binds against — the echo's * copy of `NativeSQLStrategy.resolveStorageTarget`, kept beside the @@ -1762,17 +1796,10 @@ export class ObjectQLStrategy implements AnalyticsStrategy { } private resolveFieldName(cube: Cube, member: string, kind: 'dimension' | 'measure' | 'any'): string { - if (kind === 'dimension' || kind === 'any') { - const dim = this.lookupMember(cube, member, 'dimension'); - if (dim) return dim.sql.replace(/^\$/, ''); - } - if (kind === 'measure' || kind === 'any') { - const measure = this.lookupMember(cube, member, 'measure'); - if (measure) return measure.sql.replace(/^\$/, ''); - } - return member.includes('.') ? member.split('.')[1] : member; + return resolveCubeFieldName(cube, member, kind); } + private resolveMeasureAggregation(cube: Cube, measureName: string): { field: string; method: AggregationFunction } { const direct = this.lookupMember(cube, measureName, 'measure') as | { sql: string; type: string } @@ -2322,7 +2349,7 @@ export class ObjectQLStrategy implements AnalyticsStrategy { } private extractObjectName(cube: Cube): string { - return cube.sql.trim(); + return aggregateObjectOf(cube); } /**