Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions .changeset/21249-native-sql-base-column-qualify.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
---
"@objectstack/service-analytics": patch
---

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

Clause-②: no

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.

**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`:

- 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).
- A `sum` over `amount`, a `timeDimensions` window on `closed_on`, or a `where` on `id`, each grouped by `owner.email`: `500` → `200`.
- An ad-hoc query over the object, whose inferred cube never declares a join: the same.

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.

**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.
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,9 @@ describe('NativeSQLStrategy', () => {

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

it('should execute query and return structured result', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -352,10 +352,11 @@ describe('[#5739] the object and array `where` spellings converge on one travers
);

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)');
// [#21249] The base column is qualified: the statement joins `owner`, so
// `industry` beside it is written against the base table, whether or not
// the cube declares `joins` — an inferred cube never does. Left bare, a
// joined target that also declares `industry` makes it ambiguous.
expect(sqls[0]).toContain('WHERE ("crm_account"."industry" = $1 AND "owner"."region" = $2)');
});
});

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

expect(arraySpelling.sqls).toEqual(objectSpelling.sqls);
// A bare column stays bare: `qualifyAndRegisterJoin` only qualifies when the
// cube declares `joins`, and an inferred cube never does. This is the whole
// reason #5353 was an observation rather than a defect — and the assertion
// that keeps a newly-DECLARED dimension from starting to qualify.
// statement joins something ([#21249]), and this one joins nothing. This is
// the whole reason #5353 was an observation rather than a defect — and the
// assertion that keeps a newly-DECLARED dimension from starting to qualify.
expect(objectSpelling.sqls[0]).toContain('WHERE stage = ');
expect(objectSpelling.sqls[0]).not.toContain('"deal"."stage"');
});
Expand Down Expand Up @@ -459,10 +460,11 @@ describe('[#5353/#5739] a dotted `where` key is unified too — as a traversal',
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).
// runs. [#21249] The traversal makes the statement join `owner`, so the base
// column is qualified against the base table on both spellings alike —
// `qualifyAndRegisterJoin` reads what the statement joins, not whether the
// cube declares `joins` (block 2's bare column is the statement that joins
// nothing).
const both = [['stage', '=', 'won'], ['owner.region', '=', 'NA']];
const array = await inferredDimensions(both, NO_REGION);
const object = await inferredDimensions(
Expand All @@ -473,6 +475,6 @@ describe('[#5353/#5739] a dotted `where` key is unified too — as a traversal',
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)');
expect(array.sqls[0]).toContain('WHERE ("deal"."stage" = $1 AND "owner"."region" = $2)');
});
});
Loading
Loading