Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(schema-compiler): Handle measures with dimension-only member expressions #9335

Merged
merged 7 commits into from
Mar 18, 2025
96 changes: 90 additions & 6 deletions packages/cubejs-schema-compiler/src/adapter/BaseQuery.js
Original file line number Diff line number Diff line change
Expand Up @@ -1055,9 +1055,14 @@ export class BaseQuery {
outerMeasuresJoinFullKeyQueryAggregate(innerMembers, outerMembers, toJoin) {
const renderedReferenceContext = {
renderedReference: R.pipe(
R.map(m => [m.measure || m.dimension, m.aliasName()]),
R.map(m => {
const member = m.measure ? m.measure : m.dimension;
const memberPath = typeof member === 'string'
? member
: this.cubeEvaluator.pathFromArray([m.measure?.originalCubeName ?? m.expressionCubeName, m.expressionName]);
return [memberPath, m.aliasName()];
}),
R.fromPairs,
// eslint-disable-next-line @typescript-eslint/no-unused-vars
)(innerMembers),
};

Expand Down Expand Up @@ -1692,6 +1697,55 @@ export class BaseQuery {
${this.query()}`;
}

dimensionOnlyMeasureToHierarchy(context, m) {
const measureName = typeof m.measure === 'string' ? m.measure : `${m.measure.cubeName}.${m.measure.name}`;
const memberNamesForMeasure = this.collectFrom(
[m],
this.collectMemberNamesFor.bind(this),
context ? ['collectMemberNamesFor', JSON.stringify(context)] : 'collectMemberNamesFor',
this.queryCache
);
const cubeNamesForMeasure = R.pipe(
R.map(member => this.memberInstanceByPath(member)),
// collectMemberNamesFor can return both view.dim and cube.dim
R.filter(member => member.definition().ownedByCube),
R.map(member => member.cube().name),
// Single member expression can reference multiple dimensions from same cube
R.uniq,
)(
memberNamesForMeasure
);

let cubeNameToAttach;
switch (cubeNamesForMeasure.length) {
case 0:
// For zero reference measure there's nothing to derive info about measure from
// So it assume that it's a regular measure, and it will be evaluated on top of join tree
return [measureName, [{
multiplied: false,
measure: m.measure,
}]];
case 1:
[cubeNameToAttach] = cubeNamesForMeasure;
break;
default:
throw new Error(`Expected single cube for dimension-only measure ${measureName}, got ${cubeNamesForMeasure}`);
}

const multiplied = this.multipliedJoinRowResult(cubeNameToAttach) || false;

const attachedMeasure = {
...m.measure,
originalCubeName: m.measure.cubeName,
cubeName: cubeNameToAttach
};

return [measureName, [{
multiplied,
measure: attachedMeasure,
}]];
}

collectRootMeasureToHieararchy(context) {
const notAddedMeasureFilters = R.flatten(this.measureFilters.map(f => f.getMembers()))
.filter(f => R.none(m => m.measure === f.measure, this.measures));
Expand All @@ -1710,7 +1764,18 @@ export class BaseQuery {
const cubeName = m.expressionCubeName ? `\`${m.expressionCubeName}\` ` : '';
throw new UserError(`The query contains \`COUNT(*)\` expression but cube/view ${cubeName}is missing \`count\` measure`);
}
return [typeof m.measure === 'string' ? m.measure : `${m.measure.cubeName}.${m.measure.name}`, collectedMeasures];
if (collectedMeasures.length === 0 && m.isMemberExpression) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should put this part related to the member expression over dimension into a separate function and just call it here? There is a lot of code inside the lambda function - it's quite hard to read.

// `m` is member expression measure, but does not reference any other measure
// Consider this dimensions-only measure. This can happen at least in 2 cases:
// 1. Ad-hoc aggregation over dimension: SELECT MAX(dim) FROM cube
// 2. Ungrouped query with SQL pushdown will render every column as measure: SELECT dim1 FROM cube WHERE LOWER(dim2) = 'foo';
// Measures like this needs a special treatment to attach them to cube and decide if they are multiplied or not
// This would return measure object in `measure`, not path
// TODO return measure object for every measure
return this.dimensionOnlyMeasureToHierarchy(context, m);
}
const measureName = typeof m.measure === 'string' ? m.measure : `${m.measure.cubeName}.${m.measure.name}`;
return [measureName, collectedMeasures];
}));
}

Expand Down Expand Up @@ -1977,14 +2042,33 @@ export class BaseQuery {
}

checkShouldBuildJoinForMeasureSelect(measures, keyCubeName) {
// When member expression references view, it would have to collect join hints from view
// Consider join A->B, as many-to-one, so B is multiplied and A is not, and member expression like SUM(AB_view.dimB)
// Both `collectCubeNamesFor` and `collectJoinHintsFor` would return too many cubes here
// They both walk join hints, and gather every cube present there
// For view we would get both A and B, because join hints would go from join tree root
// Even though expression references only B, and should be OK to use it with B as keyCube
// So this check would build new join tree from both A and B, B will be multiplied, and that would break check

return measures.map(measure => {
const cubes = this.collectFrom([measure], this.collectCubeNamesFor.bind(this), 'collectCubeNamesFor');
const joinHints = this.collectFrom([measure], this.collectJoinHintsFor.bind(this), 'collectJoinHintsFor');
const memberNamesForMeasure = this.collectFrom(
[measure],
this.collectMemberNamesFor.bind(this),
'collectMemberNamesFor',
);

const nonViewMembers = memberNamesForMeasure
.map(member => this.memberInstanceByPath(member))
.filter(member => member.definition().ownedByCube);

const cubes = this.collectFrom(nonViewMembers, this.collectCubeNamesFor.bind(this), 'collectCubeNamesFor');
const joinHints = this.collectFrom(nonViewMembers, this.collectJoinHintsFor.bind(this), 'collectJoinHintsFor');
if (R.any(cubeName => keyCubeName !== cubeName, cubes)) {
const measuresJoin = this.joinGraph.buildJoin(joinHints);
if (measuresJoin.multiplicationFactor[keyCubeName]) {
const measureName = measure.isMemberExpression ? measure.expressionName : measure.measure;
throw new UserError(
`'${measure.measure}' references cubes that lead to row multiplication. Please rewrite it using sub query.`
`'${measureName}' references cubes that lead to row multiplication. Please rewrite it using sub query.`
);
}
return true;
Expand Down
Loading
Loading