Skip to content

Commit 2f7a128

Browse files
authored
fix(schema-compiler): Handle measures with dimension-only member expressions (#9335)
`fullKeyQueryAggregate` is using measure.cube() a lot, like for grouping measures to key cube This can be wrong for measure targeting view: we want to attach measure to subquery for its definition cube, and there should be no subquery for view itself It's expected that `fullKeyQueryAggregateMeasures` would resolve and prepare all measures from its original form in query to actual leaf measures in cubes, then build subqueries with those, and then `joinFullKeyQueryAggregate` would use `renderedReference` to point to these leaf measures Hard case is a measure that: * targeting view * is a member expression * references only dimensions from that view Measure like that are "leaf" - there's nowhere to push it, member expression has to be directly in leaf subquery If measure references dimension from a single cube it can be multiplied (think `SUM(${view.deep_dimension})`) So, three points: * it must be accounted for in `multipliedMeasures` * it must be attached to a proper cube subquery * it must use `renderedReference` correctly `collectRootMeasureToHieararchy` will not drop such measure completely. Now it will check is there's 0 measures collected, try to collect all referenced members to gather used cubes and detect multiplication, and add new measure in hierarchy. Because new returned measure is patched, it will be attached to correct cube subquery in `fullKeyQueryAggregate` Then `outerMeasuresJoinFullKeyQueryAggregate` needs to generate proper alias for it, so outer unpatched measure would pick up alias from inner patched one Another special case of those measures are zero reference measures. It's member expression has no reference to any member at all, and it NOT a `COUNT(*)`. Like a `SUM(1)`. Measures like that are considered regular, and are calculated on top of join tree.
1 parent b5701e3 commit 2f7a128

File tree

2 files changed

+791
-6
lines changed

2 files changed

+791
-6
lines changed

packages/cubejs-schema-compiler/src/adapter/BaseQuery.js

Lines changed: 90 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1055,9 +1055,14 @@ export class BaseQuery {
10551055
outerMeasuresJoinFullKeyQueryAggregate(innerMembers, outerMembers, toJoin) {
10561056
const renderedReferenceContext = {
10571057
renderedReference: R.pipe(
1058-
R.map(m => [m.measure || m.dimension, m.aliasName()]),
1058+
R.map(m => {
1059+
const member = m.measure ? m.measure : m.dimension;
1060+
const memberPath = typeof member === 'string'
1061+
? member
1062+
: this.cubeEvaluator.pathFromArray([m.measure?.originalCubeName ?? m.expressionCubeName, m.expressionName]);
1063+
return [memberPath, m.aliasName()];
1064+
}),
10591065
R.fromPairs,
1060-
// eslint-disable-next-line @typescript-eslint/no-unused-vars
10611066
)(innerMembers),
10621067
};
10631068

@@ -1692,6 +1697,55 @@ export class BaseQuery {
16921697
${this.query()}`;
16931698
}
16941699

1700+
dimensionOnlyMeasureToHierarchy(context, m) {
1701+
const measureName = typeof m.measure === 'string' ? m.measure : `${m.measure.cubeName}.${m.measure.name}`;
1702+
const memberNamesForMeasure = this.collectFrom(
1703+
[m],
1704+
this.collectMemberNamesFor.bind(this),
1705+
context ? ['collectMemberNamesFor', JSON.stringify(context)] : 'collectMemberNamesFor',
1706+
this.queryCache
1707+
);
1708+
const cubeNamesForMeasure = R.pipe(
1709+
R.map(member => this.memberInstanceByPath(member)),
1710+
// collectMemberNamesFor can return both view.dim and cube.dim
1711+
R.filter(member => member.definition().ownedByCube),
1712+
R.map(member => member.cube().name),
1713+
// Single member expression can reference multiple dimensions from same cube
1714+
R.uniq,
1715+
)(
1716+
memberNamesForMeasure
1717+
);
1718+
1719+
let cubeNameToAttach;
1720+
switch (cubeNamesForMeasure.length) {
1721+
case 0:
1722+
// For zero reference measure there's nothing to derive info about measure from
1723+
// So it assume that it's a regular measure, and it will be evaluated on top of join tree
1724+
return [measureName, [{
1725+
multiplied: false,
1726+
measure: m.measure,
1727+
}]];
1728+
case 1:
1729+
[cubeNameToAttach] = cubeNamesForMeasure;
1730+
break;
1731+
default:
1732+
throw new Error(`Expected single cube for dimension-only measure ${measureName}, got ${cubeNamesForMeasure}`);
1733+
}
1734+
1735+
const multiplied = this.multipliedJoinRowResult(cubeNameToAttach) || false;
1736+
1737+
const attachedMeasure = {
1738+
...m.measure,
1739+
originalCubeName: m.measure.cubeName,
1740+
cubeName: cubeNameToAttach
1741+
};
1742+
1743+
return [measureName, [{
1744+
multiplied,
1745+
measure: attachedMeasure,
1746+
}]];
1747+
}
1748+
16951749
collectRootMeasureToHieararchy(context) {
16961750
const notAddedMeasureFilters = R.flatten(this.measureFilters.map(f => f.getMembers()))
16971751
.filter(f => R.none(m => m.measure === f.measure, this.measures));
@@ -1710,7 +1764,18 @@ export class BaseQuery {
17101764
const cubeName = m.expressionCubeName ? `\`${m.expressionCubeName}\` ` : '';
17111765
throw new UserError(`The query contains \`COUNT(*)\` expression but cube/view ${cubeName}is missing \`count\` measure`);
17121766
}
1713-
return [typeof m.measure === 'string' ? m.measure : `${m.measure.cubeName}.${m.measure.name}`, collectedMeasures];
1767+
if (collectedMeasures.length === 0 && m.isMemberExpression) {
1768+
// `m` is member expression measure, but does not reference any other measure
1769+
// Consider this dimensions-only measure. This can happen at least in 2 cases:
1770+
// 1. Ad-hoc aggregation over dimension: SELECT MAX(dim) FROM cube
1771+
// 2. Ungrouped query with SQL pushdown will render every column as measure: SELECT dim1 FROM cube WHERE LOWER(dim2) = 'foo';
1772+
// Measures like this needs a special treatment to attach them to cube and decide if they are multiplied or not
1773+
// This would return measure object in `measure`, not path
1774+
// TODO return measure object for every measure
1775+
return this.dimensionOnlyMeasureToHierarchy(context, m);
1776+
}
1777+
const measureName = typeof m.measure === 'string' ? m.measure : `${m.measure.cubeName}.${m.measure.name}`;
1778+
return [measureName, collectedMeasures];
17141779
}));
17151780
}
17161781

@@ -1977,14 +2042,33 @@ export class BaseQuery {
19772042
}
19782043

19792044
checkShouldBuildJoinForMeasureSelect(measures, keyCubeName) {
2045+
// When member expression references view, it would have to collect join hints from view
2046+
// Consider join A->B, as many-to-one, so B is multiplied and A is not, and member expression like SUM(AB_view.dimB)
2047+
// Both `collectCubeNamesFor` and `collectJoinHintsFor` would return too many cubes here
2048+
// They both walk join hints, and gather every cube present there
2049+
// For view we would get both A and B, because join hints would go from join tree root
2050+
// Even though expression references only B, and should be OK to use it with B as keyCube
2051+
// So this check would build new join tree from both A and B, B will be multiplied, and that would break check
2052+
19802053
return measures.map(measure => {
1981-
const cubes = this.collectFrom([measure], this.collectCubeNamesFor.bind(this), 'collectCubeNamesFor');
1982-
const joinHints = this.collectFrom([measure], this.collectJoinHintsFor.bind(this), 'collectJoinHintsFor');
2054+
const memberNamesForMeasure = this.collectFrom(
2055+
[measure],
2056+
this.collectMemberNamesFor.bind(this),
2057+
'collectMemberNamesFor',
2058+
);
2059+
2060+
const nonViewMembers = memberNamesForMeasure
2061+
.map(member => this.memberInstanceByPath(member))
2062+
.filter(member => member.definition().ownedByCube);
2063+
2064+
const cubes = this.collectFrom(nonViewMembers, this.collectCubeNamesFor.bind(this), 'collectCubeNamesFor');
2065+
const joinHints = this.collectFrom(nonViewMembers, this.collectJoinHintsFor.bind(this), 'collectJoinHintsFor');
19832066
if (R.any(cubeName => keyCubeName !== cubeName, cubes)) {
19842067
const measuresJoin = this.joinGraph.buildJoin(joinHints);
19852068
if (measuresJoin.multiplicationFactor[keyCubeName]) {
2069+
const measureName = measure.isMemberExpression ? measure.expressionName : measure.measure;
19862070
throw new UserError(
1987-
`'${measure.measure}' references cubes that lead to row multiplication. Please rewrite it using sub query.`
2071+
`'${measureName}' references cubes that lead to row multiplication. Please rewrite it using sub query.`
19882072
);
19892073
}
19902074
return true;

0 commit comments

Comments
 (0)