From a46b93443dc24cbbc38bf40bb72d397f3935a1fa Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 27 Jul 2026 14:39:10 +0000 Subject: [PATCH] fix(analytics): sort dataset selections by the display label for select/lookup dimensions (#3680) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `DatasetSelection.order` sorted a select/lookup dimension by its stored value (option value / FK id) while the response renders the resolved display label, so "sort by Account" presented as arbitrary. Order keys naming a label-bearing dimension now sort by the label the user reads: - `queryDataset` builds an `OrderLabelResolver` over the same label capabilities (and #3602 referenced-object read scope, fail-closed) as the display pass and injects it into the executor, which stays engine-free. - The executor substitutes the label as the SORT KEY only — rows keep raw values for the drill sidecars — and resolves it before `applyWindow`, so a "top 10 by account name" truncates the right ten. Such keys are excluded from SQL window pushdown. - A per-request label-fetch cache makes the display pass reuse the ids the sort already fetched (one id→name read total), and the plugin's `fetchRecordLabels` now chunks its `$in` (500/batch) since the sort pass hands over the PRE-window id set. - Measure / plain-dimension / date-bucket ordering is unchanged, pushdown included. Closes #3680. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_016ZsNHvYoujMGq66RLrCice --- .../analytics-order-by-display-label.md | 26 ++ content/docs/ui/dashboards.mdx | 5 + .../__tests__/dataset-order-labels.test.ts | 274 ++++++++++++++++++ .../src/analytics-service.ts | 57 +++- .../service-analytics/src/dataset-executor.ts | 71 ++++- .../service-analytics/src/dimension-labels.ts | 125 ++++++++ .../services/service-analytics/src/index.ts | 9 +- .../services/service-analytics/src/plugin.ts | 50 ++-- 8 files changed, 574 insertions(+), 43 deletions(-) create mode 100644 .changeset/analytics-order-by-display-label.md create mode 100644 packages/services/service-analytics/src/__tests__/dataset-order-labels.test.ts diff --git a/.changeset/analytics-order-by-display-label.md b/.changeset/analytics-order-by-display-label.md new file mode 100644 index 0000000000..d5b3dab032 --- /dev/null +++ b/.changeset/analytics-order-by-display-label.md @@ -0,0 +1,26 @@ +--- +"@objectstack/service-analytics": patch +--- + +fix(analytics): sort dataset selections by the display label for select/lookup dimensions (#3680) + +`DatasetSelection.order` (what a widget's `options.sortBy` lowers to) sorted a +`select` or `lookup`/`master_detail` dimension by its STORED value — the option +value or the foreign-key id — while the response rows carry the resolved display +label. A "sort by Account" therefore ordered by opaque ids and read as arbitrary; +a localized select sorted by its ASCII value while showing a non-ASCII label. + +Order keys naming a label-bearing dimension now sort by the display label the +user reads. The executor receives an injected sort-key hook (`OrderLabelResolver`, +built by `queryDataset` over the same label-resolution capabilities and #3602 +read scoping as the display pass); only the COMPARISON substitutes the label — +rows keep their raw values until the display pass, so drill metadata still +snapshots stored values, and ordering + windowing stay one adjacent step (a +"top 10 by account name" truncates the right ten). + +Cost model: sorting by a measure or a plain/date dimension is unchanged (SQL +pushdown included). A label-ordered `select` resolves from field metadata (no +query). A label-ordered `lookup` costs one batched id→name read over the +pre-window grouped ids (chunked, and reused by the display pass via a +per-request cache), and its window can no longer be pushed into SQL — the +inherent price of ordering by a value the database doesn't store. diff --git a/content/docs/ui/dashboards.mdx b/content/docs/ui/dashboards.mdx index 3337bf52e6..929d8ff14a 100644 --- a/content/docs/ui/dashboards.mdx +++ b/content/docs/ui/dashboards.mdx @@ -145,6 +145,11 @@ Notes on behaviour: truncates a reproducible window instead of an arbitrary subset. - Ordering is applied to the finished grid, so a **derived measure** is a valid `sortBy` even though no single SQL statement computes it. +- A `sortBy` naming a **select** or **lookup** dimension orders by the + **display label** the rows render (the option label / the related record's + name), not the stored value or foreign-key id — and the label is resolved + before `limit` applies, so a top-N by name truncates the right N. Sorting by + a measure (the common case) involves no label lookup and is unaffected. - A `funnel` with no declared stage order falls back to sorting by value descending. diff --git a/packages/services/service-analytics/src/__tests__/dataset-order-labels.test.ts b/packages/services/service-analytics/src/__tests__/dataset-order-labels.test.ts new file mode 100644 index 0000000000..8f3694370e --- /dev/null +++ b/packages/services/service-analytics/src/__tests__/dataset-order-labels.test.ts @@ -0,0 +1,274 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #3680 — `DatasetSelection.order` on a select/lookup dimension sorts by the + * DISPLAY label the user reads, not the stored value. + * + * The executor sorts the assembled grid before `queryDataset` rewrites stored + * values into display labels, so a "sort by Account" used to order by the FK + * id — deterministic, but arbitrary to the reader once the names render. These + * tests pin the fixed contract: the sort key is the label, the window cuts + * AFTER the label sort (top-N by name is the right N), the rows still carry + * raw values until the display pass (drill metadata depends on that), and the + * label fetch is paid at most once per request. + */ + +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 { applyOrdering } from '../dataset-executor.js'; +import type { DimensionLabelDeps, FieldMetaLite } from '../dimension-labels.js'; + +const CTX = { tenantId: 'org_A' } as ExecutionContext; + +/** Opportunities grouped by a lookup (account) and a select (status). */ +const byAccount = DatasetSchema.parse({ + name: 'sales_by_account', label: 'Sales', object: 'opportunity', include: [], + dimensions: [ + { name: 'account', field: 'account', type: 'lookup', label: 'Account' }, + { name: 'status', field: 'status', type: 'string', label: 'Status' }, + ], + measures: [{ name: 'revenue', aggregate: 'sum', field: 'amount' }], +}); + +type FetchCall = { target: string; ids: unknown[]; scope: unknown }; + +/** + * Label capabilities for the fixture: `account` references `crm_account` + * (ids deliberately ordered OPPOSITE to their names), `status` is a select + * whose option labels invert the stored-value order. + */ +const FIELDS: Record> = { + opportunity: { + account: { type: 'lookup', reference: 'crm_account' }, + status: { + type: 'select', + options: [ + { value: 'a_active', label: 'Working' }, + { value: 'b_churned', label: 'Ended' }, + ], + }, + }, + crm_account: { name: { type: 'text' } }, +}; + +function labelResolver(names: Record, calls: FetchCall[] = []): DimensionLabelDeps { + return { + getObjectFields: (obj) => FIELDS[obj], + fetchRecordLabels: async (target, ids, scope) => { + calls.push({ target, ids: [...ids], scope }); + const m = new Map(); + if (target === 'crm_account') { + for (const id of ids) if (names[String(id)]) m.set(id, names[String(id)]); + } + return m; + }, + }; +} + +/** ObjectQL-aggregate service (the path with no ordering grammar of its own). */ +function aggSvc(rows: Record[], names: Record, calls: FetchCall[] = []) { + return new AnalyticsService({ + queryCapabilities: () => ({ nativeSql: false, objectqlAggregate: true, inMemory: false }), + executeAggregate: async () => rows, + labelResolver: labelResolver(names, calls), + }); +} + +/** Native-SQL service; captures every emitted statement. */ +function sqlSvc(rows: Record[], names: Record, captured: string[] = [], calls: FetchCall[] = []) { + return new AnalyticsService({ + queryCapabilities: () => ({ nativeSql: true, objectqlAggregate: false, inMemory: false }), + executeRawSql: async (_o, sql) => { captured.push(sql); return rows; }, + labelResolver: labelResolver(names, calls), + }); +} + +describe('#3680 — a lookup order key sorts by the related record name', () => { + it('orders by display name, not the FK id, and keeps the drill sidecar aligned', async () => { + // Raw-id ascending would put a1 (Zebra) before b2 (Apple). + const svc = aggSvc( + [ + { account: 'a1', revenue: 10 }, + { account: 'b2', revenue: 20 }, + ], + { a1: 'Zebra', b2: 'Apple' }, + ); + const result = await svc.queryDataset( + byAccount, + { dimensions: ['account'], measures: ['revenue'], order: { account: 'asc' } }, + CTX, + ) as never as { rows: Record[]; drillRawRows: Record[] }; + expect(result.rows.map((r) => r.account)).toEqual(['Apple', 'Zebra']); + // The drill sidecar snapshots the STORED ids, aligned to the sorted rows. + expect(result.drillRawRows).toEqual([{ account: 'b2' }, { account: 'a1' }]); + }); + + it('a value the resolver cannot map sorts (and renders) by its raw form', async () => { + const svc = aggSvc( + [ + { account: 'zzz_9', revenue: 1 }, // orphaned — no label + { account: 'a1', revenue: 2 }, + { account: 'b2', revenue: 3 }, + ], + { a1: 'Zebra', b2: 'Alpha' }, + ); + const result = await svc.queryDataset( + byAccount, + { dimensions: ['account'], measures: ['revenue'], order: { account: 'asc' } }, + CTX, + ); + expect(result.rows.map((r) => r.account)).toEqual(['Alpha', 'Zebra', 'zzz_9']); + }); +}); + +describe('#3680 — a select order key sorts by the option label', () => { + it('orders by label even when it inverts the stored-value order', async () => { + // Stored ascending: a_active < b_churned. Labels invert it: Ended < Working. + const svc = aggSvc( + [ + { status: 'a_active', revenue: 10 }, + { status: 'b_churned', revenue: 20 }, + ], + {}, + ); + const result = await svc.queryDataset( + byAccount, + { dimensions: ['status'], measures: ['revenue'], order: { status: 'asc' } }, + CTX, + ); + expect(result.rows.map((r) => r.status)).toEqual(['Ended', 'Working']); + }); +}); + +describe('#3680 — windowing and query pushdown around a label sort', () => { + it('cuts the window AFTER the label sort, so a top-N by name is the right N', async () => { + const calls: FetchCall[] = []; + const svc = aggSvc( + [ + { account: 'r1', revenue: 1 }, + { account: 'r2', revenue: 2 }, + { account: 'r3', revenue: 3 }, + ], + { r1: 'Charlie', r2: 'Alpha', r3: 'Bravo' }, + calls, + ); + const result = await svc.queryDataset( + byAccount, + { dimensions: ['account'], measures: ['revenue'], order: { account: 'asc' }, limit: 2 }, + CTX, + ); + // By stored id the first two would be r1/r2 (Charlie, Alpha) — by name they + // are Alpha and Bravo. + expect(result.rows.map((r) => r.account)).toEqual(['Alpha', 'Bravo']); + // The sort-key fetch covered the FULL pre-window id set… + expect(new Set(calls[0]?.ids)).toEqual(new Set(['r1', 'r2', 'r3'])); + // …and the display pass reused it via the per-request cache: ONE fetch total. + expect(calls).toHaveLength(1); + }); + + it('keeps the window OUT of the SQL when ordering by a label-bearing dimension', async () => { + const captured: string[] = []; + const svc = sqlSvc( + [ + { account: 'a1', revenue: 10 }, + { account: 'b2', revenue: 20 }, + ], + { a1: 'Zebra', b2: 'Apple' }, + captured, + ); + const result = await svc.queryDataset( + byAccount, + { dimensions: ['account'], measures: ['revenue'], order: { account: 'asc' }, limit: 1 }, + CTX, + ); + // A SQL ORDER BY would sort the stored id and LIMIT would truncate the + // wrong window — both must stay in memory for a label-bearing order key. + expect(captured[0]).not.toContain('ORDER BY'); + expect(captured[0]).not.toContain('LIMIT'); + expect(result.rows.map((r) => r.account)).toEqual(['Apple']); + }); + + it('still pushes a MEASURE ordering into the SQL (the common case is unchanged)', async () => { + const captured: string[] = []; + const svc = sqlSvc([{ account: 'a1', revenue: 10 }], { a1: 'Zebra' }, captured); + await svc.queryDataset( + byAccount, + { dimensions: ['account'], measures: ['revenue'], order: { revenue: 'desc' }, limit: 5 }, + CTX, + ); + expect(captured[0]).toContain('ORDER BY "revenue" DESC'); + expect(captured[0]).toContain('LIMIT 5'); + }); +}); + +describe('#3680 × #3602 — the sort-key label fetch stays scoped', () => { + it('carries the REFERENCED object read scope into the sort-time fetch', async () => { + const calls: FetchCall[] = []; + const svc = new AnalyticsService({ + queryCapabilities: () => ({ nativeSql: false, objectqlAggregate: true, inMemory: false }), + executeAggregate: async () => [ + { account: 'a1', revenue: 10 }, + { account: 'b2', revenue: 20 }, + ], + getReadScope: (object, ctx?: ExecutionContext) => { + if (!ctx?.tenantId) return undefined; + return object === 'crm_account' + ? { organization_id: ctx.tenantId, is_public: true } + : { organization_id: ctx.tenantId }; + }, + labelResolver: labelResolver({ a1: 'Zebra', b2: 'Apple' }, calls), + }); + const result = await svc.queryDataset( + byAccount, + { dimensions: ['account'], measures: ['revenue'], order: { account: 'asc' } }, + CTX, + ); + expect(result.rows.map((r) => r.account)).toEqual(['Apple', 'Zebra']); + expect(calls).toHaveLength(1); + expect(calls[0].scope).toEqual({ organization_id: 'org_A', is_public: true }); + }); + + it('fails CLOSED when the referenced object scope cannot be resolved: sorts by the stored value, fetches nothing', async () => { + const calls: FetchCall[] = []; + const svc = new AnalyticsService({ + queryCapabilities: () => ({ nativeSql: false, objectqlAggregate: true, inMemory: false }), + executeAggregate: async () => [ + { account: 'b2', revenue: 20 }, + { account: 'a1', revenue: 10 }, + ], + getReadScope: (object) => { + if (object === 'crm_account') throw new Error('scope resolution failed'); + return undefined; + }, + labelResolver: labelResolver({ a1: 'Zebra', b2: 'Apple' }, calls), + }); + const result = await svc.queryDataset( + byAccount, + { dimensions: ['account'], measures: ['revenue'], order: { account: 'asc' } }, + CTX, + ); + // No unscoped fetch happened, for the sort key OR the display pass… + expect(calls).toHaveLength(0); + // …and the rows fall back to stored-value order, rendering the raw ids — + // exactly what the display pass does for an unresolvable label. + expect(result.rows.map((r) => r.account)).toEqual(['a1', 'b2']); + }); +}); + +describe('#3680 — applyOrdering sort-key substitution (unit)', () => { + it('compares by the mapped value and falls back to the raw cell where unmapped', () => { + const rows = [{ v: 'id_z' }, { v: 'id_a' }, { v: 'raw' }]; + const sorted = applyOrdering(rows, { v: 'asc' }, { + v: new Map([['id_z', 'Apple'], ['id_a', 'Zebra']]), + }); + expect(sorted.map((r) => r.v)).toEqual(['id_z', 'raw', 'id_a']); + }); + + it('keeps nulls last regardless of the substitution map', () => { + const rows = [{ v: null }, { v: 'id_a' }]; + const sorted = applyOrdering(rows, { v: 'desc' }, { v: new Map([['id_a', 'Alpha']]) }); + expect(sorted.map((r) => r.v)).toEqual(['id_a', null]); + }); +}); diff --git a/packages/services/service-analytics/src/analytics-service.ts b/packages/services/service-analytics/src/analytics-service.ts index c784077e4f..30b3bf36c5 100644 --- a/packages/services/service-analytics/src/analytics-service.ts +++ b/packages/services/service-analytics/src/analytics-service.ts @@ -18,7 +18,12 @@ import { NativeSQLStrategy } from './strategies/native-sql-strategy.js'; import { ObjectQLStrategy } from './strategies/objectql-strategy.js'; import { compileDataset, type CompiledDataset, type RelationshipResolver } from './dataset-compiler.js'; import { DatasetExecutor, resolveDimensionGranularity, type DateGranularityValue } from './dataset-executor.js'; -import { resolveDimensionLabels, type DimensionLabelDeps } from './dimension-labels.js'; +import { + resolveDimensionLabels, + createOrderLabelResolver, + withLabelFetchCache, + type DimensionLabelDeps, +} from './dimension-labels.js'; import { evaluateAnalyticsQueryOverRows } from './preview-evaluator.js'; /** @@ -500,6 +505,33 @@ export class AnalyticsService implements IAnalyticsService { } } + // #3602 — every label lookup in this request (sort keys below, display + // labels further down) reads the REFERENCED object, so bind that object's + // own read scope to this request once, up front. + const provider = this.readScopeProvider; + const resolveScope = provider + ? (targetObject: string) => provider(targetObject, context) + : undefined; + // #3680 — per-request label-fetch cache. A selection that sorts by a + // lookup dimension resolves labels twice (pre-window sort keys, then + // post-window display); the cache makes the display pass reuse the ids the + // sort already fetched, so label-ordering costs ONE id→name read total. + const labelDeps = this.labelResolver ? withLabelFetchCache(this.labelResolver) : undefined; + // #3680 — hand the executor the sort-key label hook so an `order` on a + // select/lookup dimension sorts by the label the user reads. Built over + // the SAME capabilities (and read scope) as the display resolution below. + const orderLabels = labelDeps && dataset.dimensions?.length + ? createOrderLabelResolver( + dataset.object, + dataset.dimensions + .filter((d) => !!d.field) + .map((d) => ({ name: d.name, field: d.field as string })), + labelDeps, + resolveScope, + context, + ) + : undefined; + // Graceful degradation: a dashboard/report widget whose backing object or // table is not present in this kernel (e.g. a platform dashboard like // System Overview that charts `sys_audit_log`, opened in an environment @@ -508,7 +540,7 @@ export class AnalyticsService implements IAnalyticsService { // hard-failed on a missing source. let result: AnalyticsResult; try { - result = await new DatasetExecutor(this).execute(compiled, selection, context); + result = await new DatasetExecutor(this, orderLabels).execute(compiled, selection, context); } catch (err) { if (isMissingSourceError(err)) { this.logger.warn( @@ -622,7 +654,7 @@ export class AnalyticsService implements IAnalyticsService { // (select option label, lookup related-record name). Charts render the // dimension key verbatim, so this is the single place that turns a stored // value / FK id into the text a user expects to read. - if (this.labelResolver && selectedDims.length) { + if (labelDeps && selectedDims.length) { // Same single-source rule as the drill ranges above: a date bucket must be // LABELLED with the granularity it was actually grouped by (#3588). // Formatting a `year` bucket with the dataset's `month` default rendered @@ -636,26 +668,23 @@ export class AnalyticsService implements IAnalyticsService { dateGranularity: resolveDimensionGranularity(selection, d.name, d.dateGranularity), })); if (dims.length) { - // #3602 — bind the referenced object's read scope to THIS request so the - // label lookup (a per-record read of the related object) cannot surface a - // record the referenced object's RLS would hide. Same provider the - // aggregate path uses; `undefined` when no provider is configured, in - // which case labels fetch unscoped exactly as before. - const provider = this.readScopeProvider; - const resolveScope = provider - ? (targetObject: string) => provider(targetObject, context) - : undefined; + // `resolveScope` (hoisted above) binds the referenced object's read + // scope to THIS request so the label lookup (a per-record read of the + // related object) cannot surface a record the referenced object's RLS + // would hide (#3602). `labelDeps` is the per-request cache over the + // configured resolver, so ids the sort-key pass (#3680) already fetched + // are not fetched again here. try { // `context` rides alongside `resolveScope` — the label lookup's SECOND // belt. `resolveScope` is this layer's own predicate; the context lets // the engine's middleware scope the same per-record read itself. - await resolveDimensionLabels(dataset.object, dims, result.rows, this.labelResolver, resolveScope, context); + await resolveDimensionLabels(dataset.object, dims, result.rows, labelDeps, resolveScope, context); // Totals rows (#1753) carry dimension values too (a row subtotal is // keyed by its row bucket) — resolve each grouping's own subset. for (const total of result.totals ?? []) { const subset = dims.filter((d) => total.dimensions.includes(d.name)); if (subset.length) { - await resolveDimensionLabels(dataset.object, subset, total.rows, this.labelResolver, resolveScope, context); + await resolveDimensionLabels(dataset.object, subset, total.rows, labelDeps, resolveScope, context); } } } catch (e) { diff --git a/packages/services/service-analytics/src/dataset-executor.ts b/packages/services/service-analytics/src/dataset-executor.ts index cc0bf2a8f1..b4d11a7cd8 100644 --- a/packages/services/service-analytics/src/dataset-executor.ts +++ b/packages/services/service-analytics/src/dataset-executor.ts @@ -10,6 +10,7 @@ import type { import type { FilterCondition } from '@objectstack/spec/data'; import type { ExecutionContext } from '@objectstack/spec/kernel'; import type { CompiledDataset, DerivedMeasureSpec } from './dataset-compiler.js'; +import type { OrderLabelResolver } from './dimension-labels.js'; // Re-export the shared protocol shapes so existing importers keep working. export type { DatasetSelection } from '@objectstack/spec/contracts'; @@ -52,9 +53,22 @@ export type CompareTo = DatasetCompareTo; * (see `canPushDownWindow`) so the database does the work and the echoed `sql` * shows it; the post-pass is then a no-op re-sort of already-sorted rows. * + * **What the sort key IS for a label-bearing dimension (#3680).** An order key + * naming a `select` or `lookup`/`master_detail` dimension sorts by the DISPLAY + * label the response will carry (option label / related record name), not the + * stored value — a "sort by Account" ordered by opaque FK ids presents as + * arbitrary once the labels render. The mapping comes through an injected + * {@link OrderLabelResolver} (built by `queryDataset` over the same + * label-resolution capabilities the display pass uses); rows keep their raw + * values — only the COMPARISON substitutes the label — so drill metadata still + * snapshots stored values downstream. Such keys are never pushed into SQL (the + * label is not a column there), and the label fetch happens BEFORE `applyWindow` + * so a "top 10 by account name" truncates the right ten. + * * RLS/tenant scoping is NOT handled here — it is enforced inside the strategy * via the StrategyContext read-scope hook (D-C). This layer is pure query - * shaping + arithmetic. + * shaping + arithmetic; the order-label hook is an injected interface, not an + * engine dependency. */ /** AND two optional FilterConditions into one (MongoDB-style). */ @@ -191,10 +205,17 @@ function compareValues(a: unknown, b: unknown): number { * Order rows by each key in `order`, in the object's own key order (first key is * the primary sort). Returns a NEW array; the input is not mutated. Null/empty * cells stay last in both directions (see {@link compareValues}). + * + * `sortKeys` substitutes the COMPARED value per key (#3680): when it holds a map + * for an order key, each cell compares by its mapped value — the display label a + * label-bearing dimension will render as — falling back to the raw cell where + * unmapped (an orphaned id or RLS-hidden record renders raw too, so sort and + * display stay consistent). The rows themselves are never rewritten here. */ export function applyOrdering( rows: Record[], order: Record | undefined, + sortKeys?: Record>, ): Record[] { const keys = Object.entries(order ?? {}); if (keys.length === 0 || rows.length < 2) return rows; @@ -202,8 +223,9 @@ export function applyOrdering( // grouping produced — an important property for reproducible LIMITs. return [...rows].sort((ra, rb) => { for (const [key, dir] of keys) { - const av = ra[key]; - const bv = rb[key]; + const map = sortKeys?.[key]; + const av = map?.get(ra[key]) ?? ra[key]; + const bv = map?.get(rb[key]) ?? rb[key]; const aNull = av == null || av === ''; const bNull = bv == null || bv === ''; // Nulls last in BOTH directions — decided before `desc` negation. @@ -307,7 +329,18 @@ export function shiftRange(range: [string, string], kind: CompareTo['kind']): [s } export class DatasetExecutor { - constructor(private readonly service: IAnalyticsService) {} + /** + * @param service - The analytics service the executor issues its queries to. + * @param orderLabels - Optional sort-key label hook (#3680). When provided, + * an order key naming a label-bearing (`select`/`lookup`) dimension sorts + * by its display label instead of the stored value. Omit to sort by stored + * values everywhere (e.g. the draft-preview path, whose seed rows already + * carry display names). + */ + constructor( + private readonly service: IAnalyticsService, + private readonly orderLabels?: OrderLabelResolver, + ) {} /** * Execute a dataset selection and return the shaped rows (+ field metadata). @@ -390,6 +423,17 @@ export class DatasetExecutor { // a deterministic dimension order synthesized for a bare `limit` (#3588). const order = resolveOrdering(selection, dimensions); + // #3680 — order keys naming a select/lookup dimension sort by the DISPLAY + // label the response will carry, not the stored value / FK id. Resolved + // over the assembled grid below; identified up front because such a key + // also disqualifies SQL pushdown (the label is not a column the database + // could ORDER BY, and a SQL LIMIT would truncate the wrong window). + const labelOrderKeys = this.orderLabels + ? Object.keys(order ?? {}).filter( + (k) => dimensions.includes(k) && this.orderLabels!.isLabelBearing(k), + ) + : []; + // Push `order`/`limit`/`offset` down into the SQL only when this selection // is ONE query whose columns can satisfy them. With supplementary // measure-scoped queries, a compareTo pass, or derived measures in play, the @@ -399,7 +443,8 @@ export class DatasetExecutor { const singleQuery = filtered.length === 0 && !selection.compareTo && selectedDerived.length === 0; const pushDownKeys = new Set([...dimensions, ...unfiltered]); const canPushDownWindow = - singleQuery && Object.keys(order ?? {}).every((k) => pushDownKeys.has(k)); + singleQuery && labelOrderKeys.length === 0 && + Object.keys(order ?? {}).every((k) => pushDownKeys.has(k)); const windowQuery = canPushDownWindow ? { order, limit: selection.limit, offset: selection.offset } : undefined; @@ -454,7 +499,21 @@ export class DatasetExecutor { // re-slices an already-sliced one; when it could not be (the ObjectQL // aggregate path has no ordering grammar, and date-bucketed queries are // forced down it), this is what makes `sortBy` work at all. - result.rows = applyOrdering(result.rows, order); + // + // #3680 — for label-bearing order keys, substitute the display label as + // the SORT KEY, resolved over the grid's distinct values BEFORE the window + // (a "top 10 by account name" must pick the ten by name). Rows keep their + // raw values — display rewriting stays in `queryDataset`, after the drill + // metadata snapshots the stored values. A select dimension resolves from + // field metadata (no query); a lookup costs one batched id→name read. + let sortKeys: Record> | undefined; + for (const key of labelOrderKeys) { + const values = [...new Set(result.rows.map((r) => r[key]).filter((v) => v != null))]; + if (values.length === 0) continue; + const labels = await this.orderLabels!.resolveLabels(key, values); + if (labels && labels.size > 0) (sortKeys ??= {})[key] = labels; + } + result.rows = applyOrdering(result.rows, order, sortKeys); result.rows = applyWindow(result.rows, selection.limit, selection.offset); return result; diff --git a/packages/services/service-analytics/src/dimension-labels.ts b/packages/services/service-analytics/src/dimension-labels.ts index 131d664b8f..6377e05e5a 100644 --- a/packages/services/service-analytics/src/dimension-labels.ts +++ b/packages/services/service-analytics/src/dimension-labels.ts @@ -79,6 +79,131 @@ export type LabelScopeResolver = ( const LOOKUP_TYPES = new Set(['lookup', 'master_detail']); +/** + * Sort-key label resolution for `DatasetSelection.order` (#3680). + * + * The executor sorts the assembled grid BEFORE `queryDataset` rewrites stored + * dimension values into display labels, so an order key naming a `select` or + * `lookup`/`master_detail` dimension used to sort by the stored value / FK id — + * an order that presents as arbitrary once the labels render. This hook hands + * the executor JUST the value→label mapping for such a dimension so it can sort + * by what the user will actually read, while the rows keep their raw values + * (drill metadata depends on them) and ordering + windowing stay one adjacent + * step. The executor stays engine-free: it sees this interface, never the + * engine behind it. + */ +export interface OrderLabelResolver { + /** + * Whether the dimension's stored value differs from the label it renders as + * (`select` options, `lookup`/`master_detail` FK ids). Synchronous — the + * executor consults it when deciding whether the window may be pushed into + * SQL, before any query runs. + */ + isLabelBearing(dimension: string): boolean; + /** + * Map the given raw stored values of one dimension to display labels. + * Values missing from the map sort by their raw form — the same thing the + * user will see rendered for them. + */ + resolveLabels(dimension: string, values: unknown[]): Promise | undefined>; +} + +/** + * Build the executor's {@link OrderLabelResolver} from the dataset's dimension + * list and the injected label capabilities. Mirrors the classification in + * {@link resolveDimensionLabels}: a dimension is label-bearing when its field + * carries select `options` or is a lookup/master_detail with a `reference`. + * + * - `select` resolves from field metadata — no query at all. + * - `lookup`/`master_detail` costs ONE batched id→name read over the distinct + * grouped values, scoped to the REFERENCED object's own RLS (#3602). Fail + * closed: an unresolvable scope degrades to sorting by the stored id rather + * than fetching unscoped — consistent with the display pass, which renders + * the raw id in that case too. + */ +export function createOrderLabelResolver( + baseObject: string, + dims: Array<{ name: string; field: string }>, + deps: DimensionLabelDeps, + resolveScope?: LabelScopeResolver, + context?: ExecutionContext, +): OrderLabelResolver { + const dimByName = new Map(dims.map((d) => [d.name, d])); + const metaFor = (dimension: string): FieldMetaLite | undefined => { + const dim = dimByName.get(dimension); + return dim ? deps.getObjectFields(baseObject)?.[dim.field] : undefined; + }; + return { + isLabelBearing(dimension) { + const meta = metaFor(dimension); + if (!meta) return false; + if (Array.isArray(meta.options) && meta.options.length > 0) return true; + return !!(meta.type && LOOKUP_TYPES.has(meta.type) && meta.reference); + }, + async resolveLabels(dimension, values) { + const meta = metaFor(dimension); + if (!meta) return undefined; + if (Array.isArray(meta.options) && meta.options.length > 0) { + const labelByValue = new Map(); + for (const opt of meta.options) { + if (opt && opt.label != null) labelByValue.set(opt.value, String(opt.label)); + } + return labelByValue; + } + if (meta.type && LOOKUP_TYPES.has(meta.type) && meta.reference) { + let scope: Record | null | undefined; + if (resolveScope) { + try { + scope = await resolveScope(meta.reference); + } catch { + return undefined; + } + } + return deps.fetchRecordLabels(meta.reference, values, scope ?? undefined, context); + } + return undefined; + }, + }; +} + +/** + * Wrap a {@link DimensionLabelDeps} so repeated `fetchRecordLabels` calls + * within ONE request fetch each id at most once. A selection that sorts by a + * lookup dimension resolves labels twice — once PRE-window for the sort keys + * (#3680, over the full grid's ids), once post-window for display (a subset of + * the same ids) — so with this cache the display pass costs no extra query. + * + * Per-request only: entries are keyed by target object alone, which is safe + * because an object's read scope is constant within one request. Never share + * an instance across requests. + */ +export function withLabelFetchCache(deps: DimensionLabelDeps): DimensionLabelDeps { + // Per target object: id → label, with `null` marking "fetched, no label" + // (RLS-hidden or orphaned) so unresolvable ids are not re-fetched every call. + const cache = new Map>(); + return { + getObjectFields: (objectName) => deps.getObjectFields(objectName), + async fetchRecordLabels(targetObject, ids, scope, context) { + let known = cache.get(targetObject); + if (!known) { + known = new Map(); + cache.set(targetObject, known); + } + const missing = ids.filter((id) => !known.has(id)); + if (missing.length > 0) { + const fetched = await deps.fetchRecordLabels(targetObject, missing, scope, context); + for (const id of missing) known.set(id, fetched.get(id) ?? null); + } + const out = new Map(); + for (const id of ids) { + const label = known.get(id); + if (label != null) out.set(id, label); + } + return out; + }, + }; +} + /** Date-dimension granularity (mirrors the dataset `dateGranularity` enum). */ export type DateGranularity = 'day' | 'week' | 'month' | 'quarter' | 'year'; diff --git a/packages/services/service-analytics/src/index.ts b/packages/services/service-analytics/src/index.ts index aa4617d1dc..19573997df 100644 --- a/packages/services/service-analytics/src/index.ts +++ b/packages/services/service-analytics/src/index.ts @@ -15,8 +15,13 @@ export { CubeRegistry } from './cube-registry.js'; export { compileDataset } from './dataset-compiler.js'; export type { CompiledDataset, DerivedMeasureSpec, RelationshipResolver, RelationshipTarget } from './dataset-compiler.js'; -export { resolveDimensionLabels, pickDisplayField } from './dimension-labels.js'; -export type { DimensionLabelDeps, FieldMetaLite } from './dimension-labels.js'; +export { + resolveDimensionLabels, + pickDisplayField, + createOrderLabelResolver, + withLabelFetchCache, +} from './dimension-labels.js'; +export type { DimensionLabelDeps, FieldMetaLite, OrderLabelResolver } from './dimension-labels.js'; export { DatasetExecutor, evaluateDerivedMeasures, diff --git a/packages/services/service-analytics/src/plugin.ts b/packages/services/service-analytics/src/plugin.ts index caca6d39f2..170c25d02d 100644 --- a/packages/services/service-analytics/src/plugin.ts +++ b/packages/services/service-analytics/src/plugin.ts @@ -375,27 +375,35 @@ export class AnalyticsServicePlugin implements Plugin { const map = new Map(); const displayField = pickDisplayField(dataEngine()?.getObject?.(targetObject)?.fields); if (!displayField || !executeAggregate || ids.length === 0) return map; - // #3602 — AND the referenced object's own read scope into the id filter, - // with `$and` (never key-merge) so it cannot be displaced by the id - // predicate — the same composition the strategy uses for the aggregate. - // Without it this per-record read leaks display names the target's RLS - // would hide (fires when the referenced object is stricter than the base). - const idFilter: Record = { id: { $in: ids } }; - const filter = scope ? { $and: [idFilter, scope] } : idFilter; - // Group by (id, displayField) — one row per record — reusing the aggregate - // bridge rather than adding a record-fetch capability. A count keeps engines - // that require ≥1 aggregation happy; the count itself is unused. - const rows = await executeAggregate(targetObject, { - groupBy: ['id', displayField], - aggregations: [{ field: 'id', method: 'count', alias: '_c' }], - filter, - // #3602 second belt — `scope` above is the analytics layer's own - // predicate on this per-record read; the context makes the engine's - // middleware scope it as well. - context, - }); - for (const r of rows) { - if (r.id != null && r[displayField] != null) map.set(r.id, String(r[displayField])); + // #3680 — the sort-key pass hands over the PRE-window id set (every + // grouped value, not just the displayed page), so a high-cardinality + // lookup dimension can push thousands of ids through here. Chunk the + // `$in` so the bound-parameter count stays under every driver's limit + // (SQLite's historic floor is 999 variables). + const CHUNK = 500; + for (let i = 0; i < ids.length; i += CHUNK) { + // #3602 — AND the referenced object's own read scope into the id filter, + // with `$and` (never key-merge) so it cannot be displaced by the id + // predicate — the same composition the strategy uses for the aggregate. + // Without it this per-record read leaks display names the target's RLS + // would hide (fires when the referenced object is stricter than the base). + const idFilter: Record = { id: { $in: ids.slice(i, i + CHUNK) } }; + const filter = scope ? { $and: [idFilter, scope] } : idFilter; + // Group by (id, displayField) — one row per record — reusing the aggregate + // bridge rather than adding a record-fetch capability. A count keeps engines + // that require ≥1 aggregation happy; the count itself is unused. + const rows = await executeAggregate(targetObject, { + groupBy: ['id', displayField], + aggregations: [{ field: 'id', method: 'count', alias: '_c' }], + filter, + // #3602 second belt — `scope` above is the analytics layer's own + // predicate on this per-record read; the context makes the engine's + // middleware scope it as well. + context, + }); + for (const r of rows) { + if (r.id != null && r[displayField] != null) map.set(r.id, String(r[displayField])); + } } return map; },