From 1fa5dee3af87669c6039ff0f08c14017207d92c7 Mon Sep 17 00:00:00 2001 From: Claude Fable 5 Date: Thu, 6 Aug 2026 11:38:45 +0000 Subject: [PATCH] =?UTF-8?q?fix(service-analytics):=20=E5=8D=B3=E5=B8=AD?= =?UTF-8?q?=E6=8E=A8=E6=96=AD=E7=9A=84=20Cube=20=E6=8A=8A=20owner.region?= =?UTF-8?q?=20=E5=BD=93=E6=88=90=E5=85=B3=E7=B3=BB=E7=A9=BF=E8=B6=8A,?= =?UTF-8?q?=E4=B8=8D=E5=86=8D=E9=93=B8=E6=88=90=E5=9F=BA=E8=A1=A8=E5=88=97?= =?UTF-8?q?=20region=20(#5739)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `inferCubeFromQuery` 的每个铸造点都先把成员过一遍 `stripPrefix` —— 一个把任何 点号名首段剥掉的判定。对 `.` 限定符这是对的;对关系穿越则不是: `owner.region` 被铸成 `dimensions.region = { sql: 'region' }`,一个基表列, `lookupMember` 的 plain second-segment 档随即命中它,赶在 synthetic relation traversal 档把点号路径交给 JOIN 机制之前就返回 —— 穿越被基表列遮蔽。基表恰好 有同名列时,两个策略 × 两个请求键四个组合全部静默筛/分组错列。 维护者 2026-08-06 裁 B(原样铸造):只剥真正的 `.` 限定前缀(首段 == cube 名),其余点号 member 原样铸造,与数组 where 写法一直在走的 synthetic 档收敛到 同一条 LEFT JOIN 谓词 —— 两种写法逐字生成同一条语句。#5353 留在 `where` 上的 点号残留循环随之折叠进 lowered 循环,两种写法现在也铸出同一个 cube。 measures 循环刻意保持原样:`lookupMember` 的 synthetic 穿越档是 dimension-only, dotted measure 没有可收敛的穿越答案,原样铸造只会把 #4437 的 400 INVALID_FIELD 换成 ObjectQL 不带 code/status 的 cross-object measure 抛错。其残留另立 #5918。 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_015a5qkLzpGXhLL2F5gvJ7dD --- .changeset/infer-cube-relation-traversal.md | 57 +++ .../infer-cube-relation-traversal.test.ts | 461 ++++++++++++++++++ .../infer-cube-where-spelling-parity.test.ts | 123 +++-- .../__tests__/where-source-field-gate.test.ts | 66 ++- .../src/analytics-service.ts | 111 +++-- 5 files changed, 697 insertions(+), 121 deletions(-) create mode 100644 .changeset/infer-cube-relation-traversal.md create mode 100644 packages/services/service-analytics/src/__tests__/infer-cube-relation-traversal.test.ts diff --git a/.changeset/infer-cube-relation-traversal.md b/.changeset/infer-cube-relation-traversal.md new file mode 100644 index 0000000000..f5619f0cae --- /dev/null +++ b/.changeset/infer-cube-relation-traversal.md @@ -0,0 +1,57 @@ +--- +"@objectstack/service-analytics": patch +--- + +fix(service-analytics): 即席推断的 Cube 把 `owner.region` 当成关系穿越,不再铸成基表列 `region` (#5739) + +`inferCubeFromQuery` 为「没有注册 Cube 的自由查询」即席合成一个 Cube,并从查询提 +到的字段里播种 `dimensions`。每个铸造点都先把成员过一遍 `stripPrefix` —— 一个把 +**任何**点号名的首段剥掉的判定。对 `.` 限定符(`crm_account.industry` → +`industry`)这是对的;对**关系穿越**则不是:`owner.region` 被铸成 +`dimensions.region = { sql: 'region' }`,一个**基表列**。下游 `lookupMember` 的 +「plain second-segment」那一档随即命中它,**赶在**「synthetic relation traversal」 +那一档把点号路径交给 JOIN 机制之前就返回了 —— 关系穿越被基表列遮蔽。 + +危害分两档,而更糟的是安静的那一档。当基表**恰好有同名列**时(`crm_account` 自己 +就有 `region`),四个组合全部静默通过、无任何拒收: + +``` +① ObjectQL, where: {'owner.region':'NA'} → executeAggregate 收到 {"region":"NA"} +② NativeSQL, where: {'owner.region':'NA'} → … FROM "crm_account" WHERE region = $1 +③ ObjectQL, dimensions: ['owner.region'] → groupBy: ["region"] +④ NativeSQL, dimensions: ['owner.region'] → SELECT region AS "owner.region" … GROUP BY region +``` + +行数与图表都是错的,而没有任何错误可读 —— ④ 尤甚:响应列名标着 `owner.region`,值 +却来自基表,读者无法从结果里看出来。基表**没有**同名列时则落到 `400 INVALID_FIELD` +且点名 `region`,而调用方写的是 `owner.region`。 + +维护者 2026-08-06 裁定(issue #5739):即席路径**支持**关系穿越。铸造改为**原样** +(`dimensions['owner.region'] = { sql: 'owner.region' }`),真正的 `.` 限定 +前缀(首段 == cube 名)仍然剥。这同时收敛了一处早有的分叉:同一个过滤器写成数组 +(`[['owner.region','=','NA']]`)时铸不出 dimension,于是一直走 synthetic 档、一直 +编出正确的 JOIN —— 两种写法现在逐字生成同一条语句。 + +**Observable behaviour change —— 若你按状态码告警/重试,或消费即席 cube 的元数据, +请读这一段。** + +- **对象写法的点号 member 从「静默错列」/「`INVALID_FIELD` 指错名」变为 JOIN 穿越。** + NativeSQL 上 `where: {'owner.region': 'NA'}` 与 + `dimensions: ['owner.region']` 现在编出 + `LEFT JOIN "owner" ON "crm_account"."owner" = "owner"."id"` 并按 `"owner"."region"` + 筛选/分组;此前它们筛/分组的是基表 `region`(有同名列时),或以 + `400 INVALID_FIELD "constrains field 'region'"` 被拒(无同名列时)。**同一个请求 + 现在返回的行可能与此前不同 —— 此前那些行是错的。** +- **ObjectQL 上同一个 member 改为响亮拒收或正确穿越,不再有第三种更安静的答案。** + `where` 得到 `cannot evaluate a cross-object filter ("owner.region")` —— 与**已 + 注册 cube** 上的既有答案逐字一致;`dimensions` 走 FK-expand 正确穿越,返回关联对象 + 的值。带 `granularity` 的跨对象 `timeDimensions` 得到 + `cannot bucket a cross-object time dimension`。 +- **即席 cube 的 `dimensions` 词汇表里现在出现点号键**(`getMeta` 上是 + `crm_account.owner.region`)。此前该穿越要么以剥掉的尾段出现(`crm_account.region`), + 要么(数组写法)完全不出现。 +- **不变的部分**:真正的 `.` 限定符照旧剥除;裸列名照旧是基表列(基表自己的 + `region` 仍可作为 `region` 分组);#4437 / #5520 / #5669 三道源字段闸门的代码一行未 + 动,它们对裸名拼错的 `400 INVALID_FIELD` 拒收原样保留;点号 **measure**(如 + `total.sum`)仍按 #4437 的 `400 INVALID_FIELD` 拒收 —— `lookupMember` 的 synthetic + 穿越档是 dimension-only,dotted measure 没有可收敛的穿越答案。 diff --git a/packages/services/service-analytics/src/__tests__/infer-cube-relation-traversal.test.ts b/packages/services/service-analytics/src/__tests__/infer-cube-relation-traversal.test.ts new file mode 100644 index 0000000000..582e5c5a60 --- /dev/null +++ b/packages/services/service-analytics/src/__tests__/infer-cube-relation-traversal.test.ts @@ -0,0 +1,461 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #5739 — a relation traversal on the AD-HOC cube path is a traversal, not a + * base-table column. + * + * `inferCubeFromQuery` mints a Cube for a query naming no registered cube. Every + * mint site ran the member through a blanket `stripPrefix`, which dropped the + * first segment of ANY dotted name. That is right for the `.` QUALIFIER + * (`crm_account.industry` → `industry`) and wrong for a relation TRAVERSAL: + * `owner.region` was minted as `dimensions.region = {sql: 'region'}`, a BASE + * column. `lookupMember`'s "plain second-segment" tier then found that key + * BEFORE its synthetic-traversal tier could hand the dotted path to the JOIN + * machinery, so the traversal was shadowed by a base column. + * + * Measured on `origin/main` (`dc6abfd`), with `crm_account` carrying a base + * column ALSO called `region` — the shape that makes the defect silent: + * + * ``` + * ① ObjectQL, where: {'owner.region':'NA'} → executeAggregate ← {"region":"NA"} + * ② NativeSQL, where: {'owner.region':'NA'} → … FROM "crm_account" WHERE region = $1 + * ③ ObjectQL, dimensions: ['owner.region'] → groupBy: ["region"] + * ④ NativeSQL, dimensions: ['owner.region'] → SELECT region AS "owner.region" … GROUP BY region + * ``` + * + * Four silent wrong-column answers: right-looking rows, wrong rows. ④ is the + * worst of them — the response column is LABELLED `owner.region` while the value + * comes from the base table, so the result cannot be read as wrong. + * + * Maintainer ruling, 2026-08-06 (issue #5739, the 08:28Z and 10:37Z comments): + * mint the traversal VERBATIM and let the ad-hoc path serve it, converging with + * the array `where` spelling, which compiled the correct JOIN all along. The + * public floor is that the silent wrong column must disappear; this file's + * block 1 is that floor, one case per combination. + * + * ## Reverse verification, direction predicted BEFORE running + * + * Restoring the blanket `stripPrefix` (and the dotted residue loop it fed) turns + * RED every case that names a traversal — the four in block 1, the three in + * block 2, both in block 4 — because each asserts on a JOIN, an FK-expand or a + * dotted cube key that only exists once the mint stops stripping. Ordinary + * direction, no inversion: the assertions are on statements and rows that GAIN + * a join, and every one of them also asserts the base-column answer is ABSENT, + * so a case cannot pass by producing nothing. + * + * Blocks 3 and 5 stay GREEN in both directions, and that is their point: they + * fence what the ruling does NOT move — the real `.` qualifier still + * strips, and every rejection the source-field gates (#4437 / #5520 / #5669) + * already produced is still produced, with the same envelope. + * + * **Predicted: blocks 1, 2 and 4 red (10 of this file's 17), blocks 3 and 5 + * green; measured exactly that set** — plus, in the same run, the three flipped + * cases in `infer-cube-where-spelling-parity.test.ts` and the one in + * `where-source-field-gate.test.ts`, for 14 red across the package. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { AnalyticsService } from '../analytics-service.js'; + +const silentLogger = { + info: vi.fn(), + debug: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + child: vi.fn().mockReturnThis(), +} as any; + +/** + * `crm_account`'s real columns. `region` is deliberately one of them: it is the + * base column `owner.region` used to be mis-cast onto, and the reason the defect + * was silent rather than a `no such column`. + */ +const ACCOUNT_FIELDS = ['id', 'name', 'industry', 'region', 'owner', 'created_at']; + +/** What the BASE table's own `region` column answers — never the right answer here. */ +const BASE_REGION = 'BASE-REGION'; + +/** + * A service with NO registered cube for `crm_account`, so every query takes the + * auto-inference path. One service per query: `ensureCube` registers what it + * infers, so a second query on the same service would find the cube. + * + * `executeAggregate` is a two-object double. It serves the base aggregate AND + * the FK→attribute read the ObjectQL cross-object plan issues against `owner`, + * and it answers the related `region` with values that differ from the base + * column's — so a mis-cast is caught by the VALUES in the result, not only by + * the shape of the call. + */ +function makeService(opts: { native?: boolean; fields?: string[] } = {}) { + const sqls: string[] = []; + const calls: Array<{ object: string; groupBy?: unknown; filter?: unknown }> = []; + const service = new AnalyticsService({ + logger: silentLogger, + queryCapabilities: () => ({ + nativeSql: !!opts.native, + objectqlAggregate: !opts.native, + inMemory: false, + }), + executeAggregate: async (object: string, options: any) => { + calls.push({ object, groupBy: options?.groupBy, filter: options?.filter }); + // The related object, read by the FK-expand: two owners in two regions. + if (object === 'owner') { + return [ + { id: 'u1', region: 'NA' }, + { id: 'u2', region: 'EMEA' }, + ]; + } + const groupBy = (options?.groupBy ?? []) as unknown[]; + if (groupBy.includes('owner')) { + return [ + { owner: 'u1', count: 2 }, + { owner: 'u2', count: 3 }, + ]; + } + // The mis-cast's own answer: grouping on the BASE `region` column. + if (groupBy.includes('region')) return [{ region: BASE_REGION, count: 5 }]; + return [{ count: 5 }]; + }, + executeRawSql: async (_object: string, sql: string) => { + sqls.push(sql); + return [{ count: 1 }]; + }, + isRegisteredObject: (n: string) => n === 'crm_account', + getObjectFieldNames: (n: string) => + n === 'crm_account' ? (opts.fields ?? ACCOUNT_FIELDS) : undefined, + } as any); + return { service, sqls, calls }; +} + +/** Run one query and report everything it produced, however it settled. */ +async function run(query: unknown, opts: { native?: boolean; fields?: string[] } = {}) { + const { service, sqls, calls } = makeService(opts); + let rows: unknown[] | undefined; + let error: (Error & { code?: string; status?: number; field?: string }) | undefined; + try { + rows = (await service.query(query as never)).rows as unknown[]; + } catch (e) { + error = e as Error & { code?: string }; + } + const [meta] = await service.getMeta((query as { cube: string }).cube); + const cubePrefix = `${(query as { cube: string }).cube}.`; + return { + rows, + error, + sqls, + calls, + // `getMeta` hands members out cube-prefixed; the KEY is what the mint produced. + dimensions: (meta?.dimensions ?? []).map((d) => d.name.replace(cubePrefix, '')).sort(), + measures: (meta?.measures ?? []).map((m) => m.name.replace(cubePrefix, '')).sort(), + }; +} + +// ── 1. The four combinations the issue measured ────────────────────────────── + +describe('[#5739] a dotted member traverses on the ad-hoc path — the silent wrong column is gone', () => { + it('① ObjectQL / `where` — declines LOUDLY instead of filtering the base column', async () => { + // The ad-hoc cube is single-table and `engine.aggregate()` cannot join, so a + // cross-object FILTER is out of the envelope. That is the answer a REGISTERED + // cube already gives for this member (issue #5739's own control measurement), + // and the ruling requires this path to give the same loud one — never a third, + // quieter answer. The engine must not run at all. + const { error, calls } = await run({ + cube: 'crm_account', + measures: ['count'], + where: { 'owner.region': 'NA' }, + }); + + expect(error?.message).toMatch(/cannot evaluate a cross-object filter \("owner\.region"\)/); + // The floor: no aggregate ran, so nothing was filtered on the base `region`. + expect(calls).toEqual([]); + }); + + it('② NativeSQL / `where` — joins the relation instead of filtering the base column', async () => { + const { sqls } = await run( + { cube: 'crm_account', measures: ['count'], where: { 'owner.region': 'NA' } }, + { native: true }, + ); + + expect(sqls[0]).toBe( + 'SELECT COUNT(*) AS "count" FROM "crm_account" ' + + 'LEFT JOIN "owner" ON "crm_account"."owner" = "owner"."id" ' + + 'WHERE "owner"."region" = $1', + ); + // The mis-cast's statement, named so the case cannot pass by producing nothing. + expect(sqls[0]).not.toContain('WHERE region = '); + }); + + it('③ ObjectQL / `dimensions` — FK-expands to the RELATED value, not the base column', async () => { + // In envelope: a single-hop cross-object DIMENSION with a recombinable + // measure is served by grouping on the FK, resolving the FK to the related + // attribute with a SCOPED read, and re-bucketing. This is the strongest + // assertion in the file — it is about VALUES, and the double answers the base + // `region` column with a value the related object never carries. + const { rows, calls } = await run({ + cube: 'crm_account', + measures: ['count'], + dimensions: ['owner.region'], + }); + + expect(rows).toEqual([ + { 'owner.region': 'NA', count: 2 }, + { 'owner.region': 'EMEA', count: 3 }, + ]); + // Grouped by the FK on the base object, then the related object read by id. + expect(calls.map((c) => c.object)).toEqual(['crm_account', 'owner']); + expect(calls[0].groupBy).toEqual(['owner']); + expect(calls[1].groupBy).toEqual(['id', 'region']); + expect(calls[1].filter).toEqual({ id: { $in: ['u1', 'u2'] } }); + // The mis-cast's answer, absent: the BASE object was never grouped by its own + // `region` column, and no row carries that column's value. (`region` DOES + // appear in the second call's groupBy — that is the related object's column, + // read on `owner`, which is the whole point.) + expect( + calls + .filter((c) => c.object === 'crm_account') + .some((c) => (c.groupBy as unknown[])?.includes('region')), + ).toBe(false); + expect(JSON.stringify(rows)).not.toContain(BASE_REGION); + }); + + it('④ NativeSQL / `dimensions` — the labelled column and the value finally agree', async () => { + // The issue's worst case: the response column was already LABELLED + // `owner.region` while `GROUP BY region` fed it from the base table, so the + // result could not be read as wrong. + const { sqls } = await run( + { cube: 'crm_account', measures: ['count'], dimensions: ['owner.region'] }, + { native: true }, + ); + + expect(sqls[0]).toBe( + 'SELECT "owner"."region" AS "owner.region", COUNT(*) AS "count" FROM "crm_account" ' + + 'LEFT JOIN "owner" ON "crm_account"."owner" = "owner"."id" ' + + 'GROUP BY "owner"."region"', + ); + expect(sqls[0]).not.toContain('GROUP BY region'); + expect(sqls[0]).not.toContain('SELECT region AS'); + }); +}); + +// ── 2. One traversal, two `where` spellings, one answer ────────────────────── + +/** + * The ruling's mechanism: mint verbatim so the OBJECT spelling resolves through + * `lookupMember`'s direct hit, landing on the same `qualifyAndRegisterJoin` + * input the ARRAY spelling has always reached through the synthetic tier. #5353 + * (PR #5764) could unify only BARE keys and left this per-spelling; these are the + * cases that then flip. + */ +describe('[#5739] the object and array `where` spellings converge on one traversal', () => { + it('compiles the byte-identical statement for both spellings', async () => { + const object = await run( + { cube: 'crm_account', measures: ['count'], where: { 'owner.region': 'NA' } }, + { native: true }, + ); + const array = await run( + { cube: 'crm_account', measures: ['count'], where: [['owner.region', '=', 'NA']] }, + { native: true }, + ); + + expect(array.sqls).toEqual(object.sqls); + // Stated separately so a failure reads as "they diverged" and not merely as a + // wrong expected string — and pinned to the JOIN so both cannot be empty. + expect(object.sqls[0]).toContain('LEFT JOIN "owner" ON "crm_account"."owner" = "owner"."id"'); + expect(object.sqls[0]).toContain('WHERE "owner"."region" = '); + }); + + it('mints the identical cube for both spellings — the traversal, spelled as written', async () => { + const object = await run({ + cube: 'crm_account', + measures: ['count'], + where: { 'owner.region': 'NA' }, + }); + const array = await run({ + cube: 'crm_account', + measures: ['count'], + where: [['owner.region', '=', 'NA']], + }); + + expect(object.dimensions).toEqual(['owner.region']); + expect(array.dimensions).toEqual(object.dimensions); + // The stripped tail is what the mis-cast produced; it must not be in the bag. + expect(object.dimensions).not.toContain('region'); + }); + + it('declines identically on ObjectQL for both spellings', async () => { + const object = await run({ + cube: 'crm_account', + measures: ['count'], + where: { 'owner.region': 'NA' }, + }); + const array = await run({ + cube: 'crm_account', + measures: ['count'], + where: [['owner.region', '=', 'NA']], + }); + + expect(array.error?.message).toBe(object.error?.message); + expect(object.error?.message).toMatch(/cannot evaluate a cross-object filter/); + }); + + it('unifies a dotted key riding alongside a bare one, on both spellings', async () => { + const { sqls, dimensions } = await run( + { + cube: 'crm_account', + measures: ['count'], + where: [ + ['industry', '=', 'tech'], + ['owner.region', '=', 'NA'], + ], + }, + { native: true }, + ); + + expect(dimensions).toEqual(['industry', 'owner.region']); + // The bare column stays BARE: `qualifyAndRegisterJoin` qualifies plain + // identifiers only when the cube declares `joins`, and an inferred cube never + // does — minting a dotted dimension does not change that (#5353's block 2). + expect(sqls[0]).toContain('WHERE (industry = $1 AND "owner"."region" = $2)'); + }); +}); + +// ── 3. What the ruling does NOT move: the real `.` qualifier ─────────── + +describe('[#5739] a real `.` qualifier is still stripped', () => { + it('strips it from `dimensions` and `measures` — base columns, no join', async () => { + const { sqls, dimensions, measures } = await run( + { + cube: 'crm_account', + measures: ['crm_account.count'], + dimensions: ['crm_account.industry'], + }, + { native: true }, + ); + + expect(dimensions).toEqual(['industry']); + expect(measures).toEqual(['count']); + expect(sqls[0]).toBe( + 'SELECT industry AS "crm_account.industry", COUNT(*) AS "crm_account.count" ' + + 'FROM "crm_account" GROUP BY industry', + ); + expect(sqls[0]).not.toContain('LEFT JOIN'); + }); + + it('strips it from a `where` key — no phantom join to a table named after the cube', async () => { + const { sqls, dimensions } = await run( + { cube: 'crm_account', measures: ['count'], where: { 'crm_account.industry': 'tech' } }, + { native: true }, + ); + + expect(dimensions).toEqual(['industry']); + expect(sqls[0]).toBe('SELECT COUNT(*) AS "count" FROM "crm_account" WHERE industry = $1'); + expect(sqls[0]).not.toContain('LEFT JOIN'); + }); + + it('leaves the base table\'s OWN same-named column reachable as itself', async () => { + // The control for block 1: `region` is a real column of `crm_account`, and + // naming it bare must still group by it. The ruling narrows what a DOTTED + // member means, not what a bare one does. + const { sqls } = await run( + { cube: 'crm_account', measures: ['count'], dimensions: ['region'] }, + { native: true }, + ); + + expect(sqls[0]).toBe( + 'SELECT region AS "region", COUNT(*) AS "count" FROM "crm_account" GROUP BY region', + ); + expect(sqls[0]).not.toContain('LEFT JOIN'); + }); +}); + +// ── 4. The third mint site: `timeDimensions` ───────────────────────────────── + +describe('[#5739] `timeDimensions` mint the traversal too', () => { + it('windows on the RELATED timestamp instead of the base one (NativeSQL)', async () => { + // The quietest face of the defect: every object has `created_at`, so the + // stripped tail ALWAYS resolved to a real base column and no gate ever fired — + // the window silently moved to the wrong table's timestamps. + const { sqls, dimensions } = await run( + { + cube: 'crm_account', + measures: ['count'], + timeDimensions: [{ dimension: 'owner.created_at', dateRange: ['2026-01-01', '2026-02-01'] }], + }, + { native: true }, + ); + + expect(dimensions).toEqual(['owner.created_at']); + expect(sqls[0]).toContain('LEFT JOIN "owner" ON "crm_account"."owner" = "owner"."id"'); + expect(sqls[0]).toContain('"owner"."created_at" >= $1'); + expect(sqls[0]).not.toMatch(/WHERE \(created_at >=/); + }); + + it('declines a cross-object BUCKET loudly on ObjectQL', async () => { + const { error, calls } = await run({ + cube: 'crm_account', + measures: ['count'], + timeDimensions: [{ dimension: 'owner.created_at', granularity: 'month' }], + }); + + expect(error?.message).toMatch(/cannot bucket a cross-object time dimension \("owner\.created_at"\)/); + expect(calls).toEqual([]); + }); +}); + +// ── 5. The protected surface: every rejection that already existed ─────────── + +describe('[#5739] the source-field gates keep every rejection they already made', () => { + it('still refuses a bogus BARE dimension with the #5520 envelope', async () => { + const { error } = await run( + { cube: 'crm_account', measures: ['count'], dimensions: ['bogus_dim'] }, + { native: true }, + ); + + expect(error?.code).toBe('INVALID_FIELD'); + expect(error?.status).toBe(400); + expect(error?.field).toBe('bogus_dim'); + expect(error?.message).toMatch(/groups by field 'bogus_dim'/); + }); + + it('still refuses a bogus BARE filter member with the #5669 envelope', async () => { + const { error } = await run( + { cube: 'crm_account', measures: ['count'], where: { bogus_col: 'x' } }, + { native: true }, + ); + + expect(error?.code).toBe('INVALID_FIELD'); + expect(error?.status).toBe(400); + expect(error?.field).toBe('bogus_col'); + expect(error?.message).toMatch(/constrains field 'bogus_col'/); + }); + + it('still refuses a dotted MEASURE through #4437, naming what it stripped to', async () => { + // Measures deliberately keep the blanket strip — `lookupMember`'s synthetic + // traversal tier is dimension-only, so a dotted measure has no traversal to + // converge with, and minting it verbatim would trade this `400 INVALID_FIELD` + // for ObjectQL's uncoded "cross-object measure" throw. See the mint loop, and + // #5918 for the residue that leaves on the `measures` key. + const { error, sqls } = await run( + { cube: 'crm_account', measures: ['owner.score_sum'] }, + { native: true }, + ); + + expect(error?.code).toBe('INVALID_FIELD'); + expect(error?.status).toBe(400); + expect(error?.field).toBe('score'); + expect(sqls).toEqual([]); + }); + + it('leaves a NESTED relation object reading exactly as it did', async () => { + // `{owner: {region: 'NA'}}`'s top-level key is the bare `owner`, so the mint + // is unchanged by the ruling — and the LEAF `owner.region` reaches the + // strategies through `lookupMember`'s synthetic tier, as it always has. + const { sqls, dimensions } = await run( + { cube: 'crm_account', measures: ['count'], where: { owner: { region: 'NA' } } }, + { native: true }, + ); + + expect(dimensions).toEqual(['owner']); + expect(sqls[0]).toContain('WHERE "owner"."region" = '); + }); +}); diff --git a/packages/services/service-analytics/src/__tests__/infer-cube-where-spelling-parity.test.ts b/packages/services/service-analytics/src/__tests__/infer-cube-where-spelling-parity.test.ts index ee1ab977f6..eececb464e 100644 --- a/packages/services/service-analytics/src/__tests__/infer-cube-where-spelling-parity.test.ts +++ b/packages/services/service-analytics/src/__tests__/infer-cube-where-spelling-parity.test.ts @@ -44,14 +44,24 @@ * `conjunctFieldKeys` deliberately does not descend `$or`. Those two cases pin a * deliberate NON-change; reading the table as "13 red" would be wrong. * - * Block 3 is GREEN IN BOTH DIRECTIONS, and that is the point rather than a gap. - * It pins the DOTTED residue — the one shape #5353 left answering per spelling — - * so restoring the guard changes nothing there. Reading block 3 as part of the - * fix would be wrong; it is the fence around what the fix could not decide, and - * it is what makes a future `collectFilterLeaves` refactor (which would flatten - * `{owner: {region: 'NA'}}` to the leaf `owner.region` and mint `region`) fail - * loudly instead of quietly changing a verdict #5740 shares with the `dimensions` - * request key. + * Block 3 was #5353's fence around the one shape it could not decide: a DOTTED + * `where` key, which answered per spelling — the object spelling minting the + * stripped tail as a base column (a `400 INVALID_FIELD` naming `region`, or a + * silent wrong-column filter where the base had one), the array spelling minting + * nothing and compiling the traversal. + * + * [#5739] The maintainer ruled on 2026-08-06: mint the traversal VERBATIM. Block + * 3 is therefore no longer a fence but the parity's dotted half — both spellings + * mint `owner.region` and compile the same `LEFT JOIN`. It is still green under + * the `!Array.isArray` reverse verification above (that guard is not what decided + * the dotted answer); what turns it red is restoring the blanket `stripPrefix` in + * `inferCubeFromQuery`, which is #5739's own reverse direction and is measured in + * `infer-cube-relation-traversal.test.ts`. + * + * The case that still holds the line against a `collectFilterLeaves` refactor is + * `a nested relation object seeds its RELATION key, not the tail`: flattening + * `{owner: {region: 'NA'}}` to the leaf `owner.region` would change which key + * this bag mints, and that must fail loudly rather than drift. */ import { describe, it, expect, vi } from 'vitest'; @@ -345,44 +355,60 @@ describe('[#5353] the seeded dimensions change no verdict and no statement', () }); }); -// ── 3. The #5739 residue: dotted keys, NOT unified, and why ────────────────── +// ── 3. Dotted keys, unified by #5739 ───────────────────────────────────────── /** - * These pin what #5353 deliberately did NOT fix. A dotted `where` key still - * answers per spelling, because unifying it means choosing a direction that - * belongs to #5739 — and both directions are measured here so the choice is made - * on facts rather than on which spelling someone tried first. + * #5353 left a dotted `where` key answering per SPELLING, because unifying it + * meant choosing a direction that belonged to #5739. The maintainer ruled on + * 2026-08-06: mint the traversal VERBATIM, so the object spelling converges on + * the JOIN the array spelling already compiled. These cases were the residue's + * fence; they are now the parity's dotted half, and they carry the ruling's + * weight — both spellings must produce the SAME statement, and it must be the + * traversal. + * + * The traversal's own coverage (all four strategy × request-key combinations, + * the base-column-shadowing case that made the defect silent, and the + * `.`-qualifier cases that must NOT move) lives in + * `infer-cube-relation-traversal.test.ts`. What stays here is what this file is + * about: one filter, two spellings, one answer. */ -describe('[#5353] a dotted `where` key keeps its per-spelling answer (#5739 owns it)', () => { +describe('[#5353/#5739] a dotted `where` key is unified too — as a traversal', () => { /** No base `region` column — the shape a relation filter is normally written against. */ const NO_REGION = { fields: ['id', 'stage', 'owner', 'amount', 'closed_at'], native: true }; - it('the OBJECT spelling still mints the stripped tail as a base-table dimension', async () => { - // `origin/main`'s behaviour, reproduced verbatim by the residue loop. The - // minted `region` is then found by `declaredMemberEntry`'s dotted tail lookup, - // so #5669's gate resolves the member `owner.region` to a base column `deal` - // does not have and refuses — which #5740 pinned as the honest answer given - // what actually reaches the driver on this path. - const err = await rejection( - makeService(NO_REGION).service.query({ - cube: 'deal', - measures: ['count'], - where: { 'owner.region': 'NA' }, - } as never), - ); - expect((err as { code?: string }).code).toBe('INVALID_FIELD'); - expect(err.message).toMatch(/constrains field 'region'/); + it('mints the traversal verbatim and compiles the JOIN — both spellings, one statement', async () => { + // The flip. Before the ruling the OBJECT spelling minted the stripped tail as + // a base-table dimension, `declaredMemberEntry`'s dotted tail lookup found it, + // and #5669's gate refused `owner.region` by naming a base column `deal` does + // not have (`constrains field 'region'`) — while the ARRAY spelling of the + // same filter minted nothing and compiled the traversal. One filter, two + // spellings, a 400 and a JOIN. + const object = await inferredDimensions({ 'owner.region': 'NA' }, NO_REGION); + const array = await inferredDimensions([['owner.region', '=', 'NA']], NO_REGION); + + // Load-bearing in both halves: the same cube AND the same statement, pinned + // to the JOIN so the pair cannot agree by both producing nothing. + expect(object.dimensions).toEqual(['owner.region']); + expect(array.dimensions).toEqual(object.dimensions); + expect(array.sqls).toEqual(object.sqls); + expect(object.sqls[0]).toContain('LEFT JOIN "owner" ON "deal"."owner" = "owner"."id"'); + expect(object.sqls[0]).toContain('WHERE "owner"."region" = '); + // The mis-cast's own spellings, named so a regression cannot hide behind the + // parity: neither the stripped dimension nor the base-column predicate. + expect(object.dimensions).not.toContain('region'); + expect(object.sqls[0]).not.toContain('WHERE region = '); }); - it('the ARRAY spelling still mints nothing, and compiles the traversal', async () => { - // The other half of the residue. Propagating the mint to this spelling is the - // measured REGRESSION #5353 refused: this query runs today and would become - // either a base-column filter over different rows or — as the case above - // shows — a 400. - const { dimensions, sqls } = await inferredDimensions([['owner.region', '=', 'NA']], NO_REGION); + it('runs the query the object spelling used to be refused for', async () => { + // The other half of the flip, stated as a verdict rather than a statement: + // `deal` has no `region` column, so before the ruling this exact query was a + // `400 INVALID_FIELD` naming a field the caller never wrote. The traversal + // does not need one. + const { service, sqls } = makeService(NO_REGION); - expect(dimensions).toEqual([]); - expect(sqls[0]).toContain('LEFT JOIN "owner" ON "deal"."owner" = "owner"."id"'); + await expect( + service.query({ cube: 'deal', measures: ['count'], where: { 'owner.region': 'NA' } } as never), + ).resolves.toBeTruthy(); expect(sqls[0]).toContain('WHERE "owner"."region" = '); }); @@ -396,14 +422,23 @@ describe('[#5353] a dotted `where` key keeps its per-spelling answer (#5739 owns expect(sqls[0]).toContain('WHERE "owner"."region" = '); }); - it('bare keys reach parity even when a dotted key rides along', async () => { - // The residue is scoped to the dotted key alone: `stage` is unified, and the - // whole query is still refused for `region` — one rejection at a time, naming - // a real mistake either way. - const { dimensions } = await inferredDimensions( - [['stage', '=', 'won'], ['owner.region', '=', 'NA']], + it('bare and dotted keys reach parity together when both ride along', async () => { + // Before the ruling only `stage` was unified and the whole query was refused + // for `region`; now both keys are minted, on both spellings, and the query + // runs. The bare column stays BARE in the statement — `qualifyAndRegisterJoin` + // qualifies plain identifiers only for a cube declaring `joins`, and minting a + // dotted dimension does not give an inferred cube one (block 2's rule, still + // holding with a traversal in the same filter). + const both = [['stage', '=', 'won'], ['owner.region', '=', 'NA']]; + const array = await inferredDimensions(both, NO_REGION); + const object = await inferredDimensions( + { $and: [{ stage: 'won' }, { 'owner.region': 'NA' }] }, NO_REGION, ); - expect(dimensions).toEqual(['stage']); + + expect(array.dimensions).toEqual(['owner.region', 'stage']); + expect(object.dimensions).toEqual(array.dimensions); + expect(object.sqls).toEqual(array.sqls); + expect(array.sqls[0]).toContain('WHERE (stage = $1 AND "owner"."region" = $2)'); }); }); diff --git a/packages/services/service-analytics/src/__tests__/where-source-field-gate.test.ts b/packages/services/service-analytics/src/__tests__/where-source-field-gate.test.ts index 291ee34926..6936aa78fb 100644 --- a/packages/services/service-analytics/src/__tests__/where-source-field-gate.test.ts +++ b/packages/services/service-analytics/src/__tests__/where-source-field-gate.test.ts @@ -39,7 +39,8 @@ * `??`-chain order was touched — the one thing that could have inverted here is * `resolveMemberSource`'s bag order, and `kind` is a NEW parameter, not a * reordered one. **Predicted 15 red / 16 green, naming the 15; measured exactly - * that set.** + * that set** — as of #5669. #5739 later moved ONE case out of the red set; see + * the third bullet. * * Two departures from "block 1+2 red, block 3 green" are deliberate, and are * named here rather than left for the next reader to trip over: @@ -47,14 +48,18 @@ * - In block 1, "is answered about its MEASURE first" stays GREEN — the #4437 * gate produces that rejection, and the case exists to pin the ORDER, not the * `where` verdict. - * - In block 3, "answers a dotted member on the INFERENCE path exactly as the - * shipped dimension gate does" goes RED. It sits in the must-NOT-do block - * because it bounds the gate's reach, but what it pins is a VERDICT (and its - * agreement with #5520's), not a stand-down. Reading it as green-before/red- - * after is correct; reading the block heading as a promise of greenness is not. * - In block 2, "the pre-fix driver error carried the statement" stays GREEN by * design: it asserts the OLD behaviour on a cube the gate stands down for, the * control proving this harness can still produce the leak the gate removes. + * + * [#5739] The third departure has since gone away, and the count with it. Block + * 3's inference-path case pinned a VERDICT under #5669 (dotted member → the same + * `INVALID_FIELD` the #5520 dimension gate gave), so it read green-before / + * red-after and belonged to the red set. #5739 ruled that the ad-hoc path serves + * the traversal, so the cube that case reads now mints `{sql: 'owner.region'}`, + * `resolveMemberSource` answers `source: null`, and the case pins a STAND-DOWN + * like the rest of block 3 — green in both directions. The gate's own code did + * not move; **14 red / 17 green from #5739 onward**. */ import { describe, it, expect, vi } from 'vitest'; @@ -625,23 +630,27 @@ describe('#5669 — what the gate must NOT do', () => { expect(settled.message).toMatch(/cross-object filter \("owner\.region"\)/); }); - it('answers a dotted member on the INFERENCE path exactly as the shipped dimension gate does', async () => { - // Not a stand-down, and reported rather than papered over. On the - // auto-inference path `inferCubeFromQuery` mints `stripPrefix(member)` — - // `region` — as a dimension, and `lookupMember`'s legacy second-segment - // lookup then finds it, so the member resolves to BASE column `region`. - // That is what actually reaches the engine there (measured on - // `origin/main`: `executeAggregate` received `{region: 'NA'}`, because the - // ad-hoc cube is single-table and `planCrossObject` never sees a dotted - // name to classify), so naming `region` as missing is the honest answer. + it('stands down for a dotted member on the INFERENCE path, exactly as the shipped dimension gate does', async () => { + // The invariant this case exists for is unchanged: ONE dotted member gets + // ONE answer across the `where` and `dimensions` request keys, because both + // resolve through `resolveMemberSource`. What the answer IS changed once, + // for both keys at the same time — which is precisely what the shared + // resolution was built to guarantee. // - // The point of pinning it: `dimensions: ['owner.region']` on the same path - // ALREADY answers the identical `INVALID_FIELD`/`region` on `main` (#5520), - // so this is one behaviour shared through `resolveMemberSource`, not a new - // over-reach invented by the `where` gate. If that reading is ever judged - // wrong it must change for both keys at once — which is exactly why the - // resolution lives in one function. - const { service } = makeService(); + // Until #5739, `inferCubeFromQuery` minted `stripPrefix(member)` — `region` + // — as a dimension, `lookupMember`'s legacy second-segment lookup found it, + // and the member resolved to BASE column `region`; both keys answered + // `INVALID_FIELD` naming a field the caller never wrote (and, where the + // base HAD a `region` column, filtered/grouped it silently). The maintainer + // ruled on 2026-08-06 that the ad-hoc path serves the traversal, so the + // mint is now verbatim (`{sql: 'owner.region'}`), `BARE_IDENTIFIER` rejects + // the dotted `sql`, and `resolveMemberSource` answers `source: null` — this + // gate stands down on both keys, as it always has for a relation traversal + // on an AUTHORED cube (the two cases above). + // + // The gate's own code is untouched by #5739; what moved is the cube it + // reads. Both keys are asserted here for that reason. + const { service, aggregated } = makeService({ native: true }); const viaWhere = await settle( service.query({ cube: 'crm_account', measures: ['count'], where: { 'owner.region': 'NA' } } as any), @@ -650,11 +659,14 @@ describe('#5669 — what the gate must NOT do', () => { service.query({ cube: 'crm_account', measures: ['count'], dimensions: ['owner.region'] } as any), ); - expect(viaWhere.code).toBe('INVALID_FIELD'); - expect(viaWhere.message).toMatch(/constrains field 'region'/); - // The #5520 half, unchanged by this PR — the two keys agree. - expect(viaDimension.code).toBe('INVALID_FIELD'); - expect(viaDimension.message).toMatch(/groups by field 'region'/); + expect(viaWhere.code).not.toBe('INVALID_FIELD'); + expect(viaDimension.code).not.toBe('INVALID_FIELD'); + // Load-bearing rather than a bare "not refused": the queries reached the + // driver, and what they compiled to is the traversal — not the base column + // the gate used to report as missing. + expect(aggregated).toEqual(['crm_account', 'crm_account']); + expect(viaWhere.message).toBeUndefined(); + expect(viaDimension.message).toBeUndefined(); }); it('leaves a declared dimension whose `sql` is an expression alone', async () => { diff --git a/packages/services/service-analytics/src/analytics-service.ts b/packages/services/service-analytics/src/analytics-service.ts index ae55e50592..ef948c3985 100644 --- a/packages/services/service-analytics/src/analytics-service.ts +++ b/packages/services/service-analytics/src/analytics-service.ts @@ -1569,20 +1569,65 @@ export class AnalyticsService implements IAnalyticsService { const measures: Record = {}; const dimensions: Record = {}; - const stripPrefix = (m: string) => (m.includes('.') ? m.split('.').slice(1).join('.') : m); + // [#5739] Strip the `.` QUALIFIER, and nothing else. + // + // The predecessor (`stripPrefix`) dropped the first segment of ANY dotted + // member, which conflated two different facts wearing the same punctuation: + // + // `deal.stage` — the canonical analytics QUALIFIER. `getMeta` hands + // members out cube-prefixed and callers echo them back, + // so the prefix is noise and stripping it is right. + // `owner.region` — a relation TRAVERSAL. Stripping it minted + // `dimensions.region = {sql: 'region'}`, a BASE-TABLE + // column, and `lookupMember`'s "plain second-segment" + // tier then found it BEFORE its synthetic-traversal tier + // could hand the dotted path to the JOIN machinery. Where + // the base table happened to carry a same-named column + // that filtered/grouped the WRONG column with no error to + // read; where it did not, the 400 named `region` for a + // caller who wrote `owner.region`. + // + // Only the first is a qualifier, and only the first is stripped. Everything + // else is minted VERBATIM (`{sql: 'owner.region'}`), which is precisely what + // `lookupMember`'s synthetic tier already hands the strategies for an + // undeclared dotted member — so the ad-hoc path now compiles the traversal + // the array `where` spelling has compiled all along, and the two spellings + // converge instead of disagreeing. Maintainer ruling, 2026-08-06 (#5739). + // + // Scope, measured rather than assumed: this governs the DIMENSION-shaped + // mints (`dimensions`, the `where`'s field keys, `timeDimensions`) and NOT + // `measures`, which keeps the old blanket strip below. See that loop. + const stripCubeQualifier = (m: string): string => { + const dot = m.indexOf('.'); + if (dot < 0) return m; + return m.slice(0, dot) === cubeName ? m.slice(dot + 1) : m; + }; // Always provide a default `count` measure measures.count = { name: 'count', label: 'Count', type: 'count', sql: '*' }; for (const m of query.measures || []) { - const key = stripPrefix(m); + // [#5739] MEASURES keep the blanket strip, deliberately and on measurement. + // `lookupMember`'s synthetic relation-traversal tier is DIMENSION-ONLY + // (`if (kind === 'dimension')`), so a dotted measure has no traversal + // answer to converge with — minting `measures['total.sum']` verbatim does + // not join anything, it only re-routes the member into ObjectQL's + // "cannot evaluate a cross-object measure" throw, which carries no + // `code`/`status`. Measured on this tree: `measures: ['total.sum']` (a + // `total_sum` typo, not a traversal) would go from #4437's + // `400 INVALID_FIELD` naming `sum` to that uncoded 5xx-class error. A + // worse envelope and a wrong diagnosis, for a spelling that is not what + // this issue is about — so the strip stays until a dotted MEASURE is ruled + // on in its own right. Its own residue (`owner.region_count_distinct` + // silently aggregating the BASE `region`, measured) is filed as #5918. + const key = m.includes('.') ? m.split('.').slice(1).join('.') : m; if (measures[key]) continue; const inferred = inferMeasure(key); measures[key] = inferred; } for (const d of query.dimensions || []) { - const key = stripPrefix(d); + const key = stripCubeQualifier(d); if (dimensions[key]) continue; dimensions[key] = { name: key, label: key, type: 'string', sql: key }; } @@ -1621,57 +1666,23 @@ export class AnalyticsService implements IAnalyticsService { // genuinely differ — `{stage: {$in: []}}` lowers to the boolean constant // FALSE, binding nothing while still naming `stage`. for (const key of conjunctFieldKeys(lowered)) { - // BARE keys only — a dotted one is a relation traversal, and #5353 cannot - // unify those. See the residue loop below for why, and for the ONE case - // that still answers per-spelling. - if (key.includes('.')) continue; - if (dimensions[key] || measures[key]) continue; - dimensions[key] = { name: key, label: key, type: 'string', sql: key }; - } - } - - // ── #5739 residue: dotted keys, still answered per SPELLING ─────────────── - // - // A dotted key names a relation traversal, and an ad-hoc single-table cube - // has nothing to declare it as. `stripPrefix` mints the TAIL as a BASE-TABLE - // dimension, which is #5739's mis-cast, and today only the OBJECT spelling - // reaches that mint. #5353 can go neither way on its own: - // - // - PROPAGATE it to the array spelling (`stripPrefix` in the loop above) is - // a measured REGRESSION. On cube `deal`, filter `owner.region = 'NA'`: - // {'owner.region': 'NA'} → WHERE region = $1 (both, after) - // [['owner.region','=','NA']] → LEFT JOIN "owner" ON "deal"."owner" = - // "owner"."id" WHERE "owner"."region" = $1 - // (before) - // …i.e. a working traversal becomes a different-rows base-column filter — - // and where the base has no `region` column, a 400 INVALID_FIELD, because - // `declaredMemberEntry`'s dotted tail lookup resolves `owner.region` to - // the minted dimension and hands #5669's gate a column that is absent. - // - WITHDRAW it from the object spelling (skip dotted keys entirely) breaks - // an invariant #5740 pinned deliberately: one dotted member gets one - // answer across the `where` and `dimensions` request keys, sharing - // `resolveMemberSource`, and "if that reading is ever judged wrong it must - // change for BOTH keys at once". Withdrawing here alone would leave - // `where: {'owner.region': …}` standing down while - // `dimensions: ['owner.region']` still 400s on `region`. - // - // Both directions are #5739's to choose, so this loop reproduces `origin/main` - // verbatim — the OBJECT `where`'s own top-level dotted keys, `stripPrefix`ed — - // and the asymmetry #5353 is about survives for dotted keys alone. Delete the - // loop (or fold it into the one above) when #5739 rules; the parity test names - // the case that then flips. - const rawWhere = (query as { where?: unknown }).where; - if (rawWhere && typeof rawWhere === 'object' && !Array.isArray(rawWhere)) { - for (const key of Object.keys(rawWhere as Record)) { - if (key.startsWith('$') || !key.includes('.')) continue; - const stripped = stripPrefix(key); - if (dimensions[stripped] || measures[stripped]) continue; - dimensions[stripped] = { name: stripped, label: stripped, type: 'string', sql: stripped }; + // [#5739] Dotted keys ride this loop too, and that is the FOLD #5353 left + // for this issue. Until the ruling, a dotted key was skipped here and + // re-minted from the RAW object `where` by a separate residue loop — + // stripped to its tail, so one filter got one answer per spelling: the + // object spelling mis-cast `owner.region` to base `region`, the array + // spelling minted nothing and compiled the traversal. One loop over the + // LOWERED condition mints both spellings identically, and + // `stripCubeQualifier` keeps them a traversal instead of a base column, + // so the cube AND the compiled SQL now match on either spelling. + const minted = stripCubeQualifier(key); + if (dimensions[minted] || measures[minted]) continue; + dimensions[minted] = { name: minted, label: minted, type: 'string', sql: minted }; } } for (const td of query.timeDimensions || []) { - const key = stripPrefix(td.dimension); + const key = stripCubeQualifier(td.dimension); if (dimensions[key]) continue; dimensions[key] = { name: key, label: key, type: 'time', sql: key,