Skip to content

Commit b9c394e

Browse files
committed
fix(service-analytics): an undeclared base member in a filter takes the same qualification; fixtures that pinned a bare base column beside a join
Three fixtures asserted a bare base column in a statement that joins: the shape the qualification rule now corrects. Their expectations name the qualified column, and the comments that described the old rule describe the new one. Claude-Session: https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ Co-authored-by: Claude <noreply@anthropic.com>
1 parent 51368f7 commit b9c394e

4 files changed

Lines changed: 25 additions & 17 deletions

File tree

‎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
});

‎packages/services/service-analytics/src/strategies/native-sql-strategy.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1402,7 +1402,10 @@ export class NativeSQLStrategy implements AnalyticsStrategy {
14021402
const measure = this.lookupMember(cube, member, 'measure');
14031403
if (measure) return this.qualifyAndRegisterJoin(measure.sql, parentTable, joins, cube);
14041404
const fieldName = member.includes('.') ? member.split('.')[1] : member;
1405-
return fieldName;
1405+
// [#21249] A member the cube does not declare is a base-table column too
1406+
// (`where: { id }`), so it takes the same qualification as a declared one:
1407+
// returned bare it sat beside a joined target's own `id`, ambiguous.
1408+
return this.qualifyAndRegisterJoin(fieldName, parentTable, joins, cube);
14061409
}
14071410

14081411
/**

0 commit comments

Comments
 (0)