Skip to content

Commit 032865c

Browse files
committed
Merge remote-tracking branch 'origin/main' into claude/issue-21229-object-grid-export-options
2 parents 39fef70 + b91e40b commit 032865c

8 files changed

Lines changed: 494 additions & 117 deletions
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
---
2+
"@objectstack/service-analytics": patch
3+
---
4+
5+
fix(service-analytics): on the native-SQL strategy, a base-table column is qualified with its table whenever the statement joins a related object, not only when the cube declares a join
6+
7+
Clause-②: no
8+
9+
A cube that declares no join still joins a lookup's declared `reference` when a query names a relationship path through it (`owner.email`). The native-SQL strategy qualified base-table columns only for a cube that declares a join, so it wrote them bare beside the joined object. When that object declares a column of the same name, the database refused the statement as ambiguous, and `POST /api/v1/analytics/query` answered `500 DATABASE_ERROR` on SQLite and on PostgreSQL. The ObjectQL strategy answered `200` for the same query.
10+
11+
**Before and after**, measured on a configured cube over a `deal` object that declares no join, whose lookup `owner` points at a person object that also declares `note`, `amount`, `closed_on` and `id`:
12+
13+
- Dimensions `note` and `owner.email`, with or without a `where` on `note` and an `order` by it: `500` → `200`, one group per (deal note, owner email).
14+
- A `sum` over `amount`, a `timeDimensions` window on `closed_on`, or a `where` on `id`, each grouped by `owner.email`: `500` → `200`.
15+
- An ad-hoc query over the object, whose inferred cube never declares a join: the same.
16+
17+
The strategy now reads what the statement actually joins, from the one relationship-path resolver, and qualifies every base column in the select list, the grouping, the filters, the measures and the time windows. A statement that joins nothing keeps bare columns. That is now also true on a cube that declares a join when the query uses none of it: the statement it shows on `POST /api/v1/analytics/sql` reads `note` where it read `"deal"."note"`, and the answer is the same.
18+
19+
**Unchanged.** The ObjectQL strategy; every query on a cube that declares the join it uses; every statement that joins nothing on a cube that declares no join; the refusals.

‎packages/qa/dogfood/test/authz-conformance.matrix.ts‎

Lines changed: 43 additions & 43 deletions
Large diffs are not rendered by default.

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,9 @@ describe('NativeSQLStrategy', () => {
247247

248248
expect(sql).toContain('LEFT JOIN "account" ON "opportunity"."account" = "account"."id"');
249249
expect(sql).toContain('"account"."industry" AS "account.industry"');
250-
expect(sql).toContain('SUM(amount)');
250+
// [#21249] The cube declares no join, but the statement joins `account`, so
251+
// the base column the measure sums is qualified against the base table.
252+
expect(sql).toContain('SUM("opportunity"."amount")');
251253
});
252254

253255
it('should execute query and return structured result', async () => {

‎packages/services/service-analytics/src/__tests__/infer-cube-relation-traversal.test.ts‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -352,10 +352,11 @@ describe('[#5739] the object and array `where` spellings converge on one travers
352352
);
353353

354354
expect(dimensions).toEqual(['industry', 'owner.region']);
355-
// The bare column stays BARE: `qualifyAndRegisterJoin` qualifies plain
356-
// identifiers only when the cube declares `joins`, and an inferred cube never
357-
// does — minting a dotted dimension does not change that (#5353's block 2).
358-
expect(sqls[0]).toContain('WHERE (industry = $1 AND "owner"."region" = $2)');
355+
// [#21249] The base column is qualified: the statement joins `owner`, so
356+
// `industry` beside it is written against the base table, whether or not
357+
// the cube declares `joins` — an inferred cube never does. Left bare, a
358+
// joined target that also declares `industry` makes it ambiguous.
359+
expect(sqls[0]).toContain('WHERE ("crm_account"."industry" = $1 AND "owner"."region" = $2)');
359360
});
360361
});
361362

‎packages/services/service-analytics/src/__tests__/infer-cube-where-spelling-parity.test.ts‎

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,10 @@
2020
*
2121
* ## Why this had no user-visible symptom (the issue's own observation class)
2222
*
23-
* `NativeSQLStrategy.resolveFieldSql` falls back to the bare column name for a
24-
* member the cube does not declare, and `qualifyAndRegisterJoin` leaves bare
25-
* columns bare on a cube with no `joins` — which an ad-hoc cube never has. So
23+
* `NativeSQLStrategy.resolveFieldSql` falls back to the column the member names
24+
* for a member the cube does not declare, and `qualifyAndRegisterJoin` leaves
25+
* that column bare in a statement that joins nothing ([#21249]: what the
26+
* statement joins, not the cube's `joins`, which an ad-hoc cube never has). So
2627
* both spellings compiled the same SQL before the fix and still do; block 2
2728
* measures that rather than asserting it. The divergence was confined to the
2829
* dimension VOCABULARY, which is why this was filed as an observation and fixed
@@ -317,9 +318,9 @@ describe('[#5353] the seeded dimensions change no verdict and no statement', ()
317318

318319
expect(arraySpelling.sqls).toEqual(objectSpelling.sqls);
319320
// A bare column stays bare: `qualifyAndRegisterJoin` only qualifies when the
320-
// cube declares `joins`, and an inferred cube never does. This is the whole
321-
// reason #5353 was an observation rather than a defect — and the assertion
322-
// that keeps a newly-DECLARED dimension from starting to qualify.
321+
// statement joins something ([#21249]), and this one joins nothing. This is
322+
// the whole reason #5353 was an observation rather than a defect — and the
323+
// assertion that keeps a newly-DECLARED dimension from starting to qualify.
323324
expect(objectSpelling.sqls[0]).toContain('WHERE stage = ');
324325
expect(objectSpelling.sqls[0]).not.toContain('"deal"."stage"');
325326
});
@@ -459,10 +460,11 @@ describe('[#5353/#5739] a dotted `where` key is unified too — as a traversal',
459460
it('bare and dotted keys reach parity together when both ride along', async () => {
460461
// Before the ruling only `stage` was unified and the whole query was refused
461462
// for `region`; now both keys are minted, on both spellings, and the query
462-
// runs. The bare column stays BARE in the statement — `qualifyAndRegisterJoin`
463-
// qualifies plain identifiers only for a cube declaring `joins`, and minting a
464-
// dotted dimension does not give an inferred cube one (block 2's rule, still
465-
// holding with a traversal in the same filter).
463+
// runs. [#21249] The traversal makes the statement join `owner`, so the base
464+
// column is qualified against the base table on both spellings alike —
465+
// `qualifyAndRegisterJoin` reads what the statement joins, not whether the
466+
// cube declares `joins` (block 2's bare column is the statement that joins
467+
// nothing).
466468
const both = [['stage', '=', 'won'], ['owner.region', '=', 'NA']];
467469
const array = await inferredDimensions(both, NO_REGION);
468470
const object = await inferredDimensions(
@@ -473,6 +475,6 @@ describe('[#5353/#5739] a dotted `where` key is unified too — as a traversal',
473475
expect(array.dimensions).toEqual(['owner.region', 'stage']);
474476
expect(object.dimensions).toEqual(array.dimensions);
475477
expect(object.sqls).toEqual(array.sqls);
476-
expect(array.sqls[0]).toContain('WHERE (stage = $1 AND "owner"."region" = $2)');
478+
expect(array.sqls[0]).toContain('WHERE ("deal"."stage" = $1 AND "owner"."region" = $2)');
477479
});
478480
});

0 commit comments

Comments
 (0)