fix(schema-compiler): Reject pre-agg if measure is unmultiplied in query but multiplied in pre-agg

Author: KSDaemon

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

@@ -3873,11 +3873,11 @@ export class BaseQuery {
   }
 
   // eslint-disable-next-line consistent-return
-  preAggregationQueryForSqlEvaluation(cube, preAggregation) {
+  preAggregationQueryForSqlEvaluation(cube, preAggregation, context = {}) {
     if (preAggregation.type === 'autoRollup') {
       return this.preAggregations.autoRollupPreAggregationQuery(cube, preAggregation);
     } else if (preAggregation.type === 'rollup') {
-      return this.preAggregations.rollupPreAggregationQuery(cube, preAggregation);
+      return this.preAggregations.rollupPreAggregationQuery(cube, preAggregation, context);
     } else if (preAggregation.type === 'originalSql') {
       return this;
     }

packages/cubejs-schema-compiler/src/adapter/PreAggregations.ts

@@ -50,6 +50,10 @@ export type PreAggregationForCube = {
   references: PreAggregationReferences;
 };
 
+export type EvaluateReferencesContext = {
+  inPreAggEvaluation?: boolean;
+};
+
 export type BaseMember = BaseDimension | BaseMeasure | BaseFilter | BaseGroupFilter | BaseSegment;
 
 export type CanUsePreAggregationFn = (references: PreAggregationReferences) => boolean;
@@ -583,12 +587,15 @@ export class PreAggregations {
       const dimensionsTrimmed = references
         .dimensions
         .map(d => CubeSymbols.joinHintFromPath(d).path);
+      const multipliedMeasuresTrimmed = references
+        .multipliedMeasures?.map(m => CubeSymbols.joinHintFromPath(m).path);
 
       return {
         ...references,
         dimensions: dimensionsTrimmed,
         measures: measuresTrimmed,
         timeDimensions: timeDimensionsTrimmed,
+        multipliedMeasures: multipliedMeasuresTrimmed,
       };
     }
 
@@ -723,6 +730,19 @@ export class PreAggregations {
       // TODO remove this in favor of matching with join path
       const referencesTrimmed = trimmedReferences(references);
 
+      // Even if there are no multiplied measures in the query (because no multiplier dimensions are requested)
+      // but the same measures are multiplied in the pre-aggregation, we can't use pre-aggregation
+      // for such queries.
+      if (referencesTrimmed.multipliedMeasures) {
+        const backAliasMultipliedMeasures = backAlias(referencesTrimmed.multipliedMeasures);
+
+        if (transformedQuery.leafMeasures.some(m => referencesTrimmed.multipliedMeasures.includes(m)) ||
+          transformedQuery.measures.some(m => backAliasMultipliedMeasures.includes(m))
+        ) {
+          return false;
+        }
+      }
+
       const dimensionsMatch = (dimensions, doBackAlias) => R.all(
         d => (
           doBackAlias ?
@@ -1194,11 +1214,11 @@ export class PreAggregations {
     );
   }
 
-  public rollupPreAggregationQuery(cube: string, aggregation): BaseQuery {
+  public rollupPreAggregationQuery(cube: string, aggregation: PreAggregationDefinition, context: EvaluateReferencesContext = {}): BaseQuery {
     // `this.evaluateAllReferences` will retain not only members, but their join path as well, and pass join hints
     // to subquery. Otherwise, members in subquery would regenerate new join tree from clean state,
     // and it can be different from expected by join path in pre-aggregation declaration
-    const references = this.evaluateAllReferences(cube, aggregation);
+    const references = this.evaluateAllReferences(cube, aggregation, null, context);
     const cubeQuery = this.query.newSubQueryForCube(cube, {});
     return this.query.newSubQueryForCube(cube, {
       rowLimit: null,
@@ -1226,7 +1246,7 @@ export class PreAggregations {
     });
   }
 
-  public autoRollupPreAggregationQuery(cube: string, aggregation): BaseQuery {
+  public autoRollupPreAggregationQuery(cube: string, aggregation: PreAggregationDefinition): BaseQuery {
     return this.query.newSubQueryForCube(
       cube,
       {
@@ -1267,13 +1287,18 @@ export class PreAggregations {
       .toLowerCase();
   }
 
-  private evaluateAllReferences(cube: string, aggregation: PreAggregationDefinition, preAggregationName: string | null = null): PreAggregationReferences {
+  private evaluateAllReferences(cube: string, aggregation: PreAggregationDefinition, preAggregationName: string | null = null, context: EvaluateReferencesOptions = {}): PreAggregationReferences {
     // TODO build a join tree for all references, so they would always include full join path
     //  Even for preaggregation references without join path
     //  It is necessary to be able to match query and preaggregation based on full join tree
 
     const evaluateReferences = () => {
       const references = this.query.cubeEvaluator.evaluatePreAggregationReferences(cube, aggregation);
+      if (!context.inPreAggEvaluation) {
+        const preAggQuery = this.query.preAggregationQueryForSqlEvaluation(cube, aggregation, { inPreAggEvaluation: true });
+        const aggregateMeasures = preAggQuery?.fullKeyQueryAggregateMeasures({ hasMultipliedForPreAggregation: true });
+        references.multipliedMeasures = aggregateMeasures?.multipliedMeasures?.map(m => m.measure);
+      }
       if (aggregation.type === 'rollupLambda') {
         if (references.rollups.length > 0) {
           const [firstLambdaCube] = this.query.cubeEvaluator.parsePath('preAggregations', references.rollups[0]);

packages/cubejs-schema-compiler/src/compiler/CubeEvaluator.ts

@@ -121,6 +121,7 @@ export type PreAggregationReferences = {
   measures: Array<string>,
   timeDimensions: Array<PreAggregationTimeDimensionReference>,
   rollups: Array<string>,
+  multipliedMeasures?: Array<string>,
 };
 
 export type PreAggregationInfo = {