fix: Multiplied measures rollup select case and leaf measure additive exact match

Author: paveltiunov

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

@@ -330,26 +330,7 @@ class BaseQuery {
   }
 
   fullKeyQueryAggregate() {
-    const measureToHierarchy = this.collectRootMeasureToHieararchy();
-
-    const measuresToRender = (multiplied, cumulative) => R.pipe(
-      R.values,
-      R.flatten,
-      R.filter(
-        m => m.multiplied === multiplied && this.newMeasure(m.measure).isCumulative() === cumulative
-      ),
-      R.map(m => m.measure),
-      R.uniq,
-      R.map(m => this.newMeasure(m))
-    );
-
-    const multipliedMeasures = measuresToRender(true, false)(measureToHierarchy);
-    const regularMeasures = measuresToRender(false, false)(measureToHierarchy);
-    const cumulativeMeasures =
-      R.pipe(
-        R.map(multiplied => R.xprod([multiplied], measuresToRender(multiplied, true)(measureToHierarchy))),
-        R.unnest
-      )([false, true]);
+    const { multipliedMeasures, regularMeasures, cumulativeMeasures } = this.fullKeyQueryAggregateMeasures();
 
     if (!multipliedMeasures.length && !cumulativeMeasures.length) {
       return this.simpleQuery();
@@ -410,6 +391,30 @@ class BaseQuery {
     return `SELECT ${this.topLimit()}${columnsToSelect} FROM (${toJoin[0]}) as q_0 ${join}${havingFilters}${this.orderBy()}${this.groupByDimensionLimit()}`;
   }
 
+  fullKeyQueryAggregateMeasures() {
+    const measureToHierarchy = this.collectRootMeasureToHieararchy();
+
+    const measuresToRender = (multiplied, cumulative) => R.pipe(
+      R.values,
+      R.flatten,
+      R.filter(
+        m => m.multiplied === multiplied && this.newMeasure(m.measure).isCumulative() === cumulative
+      ),
+      R.map(m => m.measure),
+      R.uniq,
+      R.map(m => this.newMeasure(m))
+    );
+
+    const multipliedMeasures = measuresToRender(true, false)(measureToHierarchy);
+    const regularMeasures = measuresToRender(false, false)(measureToHierarchy);
+    const cumulativeMeasures =
+      R.pipe(
+        R.map(multiplied => R.xprod([multiplied], measuresToRender(multiplied, true)(measureToHierarchy))),
+        R.unnest
+      )([false, true]);
+    return { multipliedMeasures, regularMeasures, cumulativeMeasures };
+  }
+
   dimensionsJoinCondition(leftAlias, rightAlias) {
     const dimensionAliases = this.dimensionAliasNames();
     if (!dimensionAliases.length) {

packages/cubejs-schema-compiler/adapter/PreAggregations.js

@@ -165,6 +165,7 @@ class PreAggregations {
     const isAdditive = R.all(m => m.isAdditive(), query.measures);
     const leafMeasureAdditive = R.all(path => query.newMeasure(path).isAdditive(), leafMeasurePaths);
     const granularityHierarchies = query.granularityHierarchies();
+    const hasMultipliedMeasures = query.fullKeyQueryAggregateMeasures().multipliedMeasures.length > 0;
 
     return {
       sortedDimensions,
@@ -175,7 +176,8 @@ class PreAggregations {
       hasNoTimeDimensionsWithoutGranularity,
       allFiltersWithinSelectedDimensions,
       isAdditive,
-      granularityHierarchies
+      granularityHierarchies,
+      hasMultipliedMeasures
     };
   }
 
@@ -232,8 +234,7 @@ class PreAggregations {
     // [[TimeDimension]]
     const queryTimeDimensionsList = transformedQuery.sortedTimeDimensions.map(expandTimeDimension);
 
-    const canUsePreAggregationNotAdditive = (references) =>
-      transformedQuery.hasNoTimeDimensionsWithoutGranularity &&
+    const canUsePreAggregationNotAdditive = (references) => transformedQuery.hasNoTimeDimensionsWithoutGranularity &&
       transformedQuery.allFiltersWithinSelectedDimensions &&
       R.equals(references.sortedDimensions || references.dimensions, transformedQuery.sortedDimensions) &&
       (
@@ -245,35 +246,18 @@ class PreAggregations {
         references.sortedTimeDimensions || sortTimeDimensions(references.timeDimensions)
       );
 
-    const canUsePreAggregationLeafMeasureAdditive = (references) =>
-      R.all(
-        d => (references.sortedDimensions || references.dimensions).indexOf(d) !== -1,
-        transformedQuery.sortedDimensions
-      ) &&
+    const canUsePreAggregationLeafMeasureAdditive = (references) => R.all(
+      d => (references.sortedDimensions || references.dimensions).indexOf(d) !== -1,
+      transformedQuery.sortedDimensions
+    ) &&
       R.all(m => references.measures.indexOf(m) !== -1, transformedQuery.leafMeasures) &&
       R.allPass(
         queryTimeDimensionsList.map(tds => R.anyPass(tds.map(td => R.contains(td))))
       )(references.sortedTimeDimensions || sortTimeDimensions(references.timeDimensions));
 
-    const canUsePreAggregationAdditive = (references) =>
-      R.all(
-        d => (references.sortedDimensions || references.dimensions).indexOf(d) !== -1,
-        transformedQuery.sortedDimensions
-      ) &&
-      (
-        R.all(m => references.measures.indexOf(m) !== -1, transformedQuery.measures) ||
-        R.all(m => references.measures.indexOf(m) !== -1, transformedQuery.leafMeasures)
-      ) &&
-      R.allPass(
-        queryTimeDimensionsList.map(tds => R.anyPass(tds.map(td => R.contains(td))))
-      )(references.sortedTimeDimensions || sortTimeDimensions(references.timeDimensions));
-
-
     let canUseFn;
-    if (transformedQuery.isAdditive) {
-      canUseFn = canUsePreAggregationAdditive;
-    } else if (transformedQuery.leafMeasureAdditive) {
-      canUseFn = canUsePreAggregationLeafMeasureAdditive;
+    if (transformedQuery.leafMeasureAdditive && !transformedQuery.hasMultipliedMeasures) {
+      canUseFn = (r) => canUsePreAggregationLeafMeasureAdditive(r) || canUsePreAggregationNotAdditive(r);
     } else {
       canUseFn = canUsePreAggregationNotAdditive;
     }

packages/cubejs-schema-compiler/test/SQLGenerationTest.js

@@ -1138,8 +1138,10 @@ describe('SQL Generation', function test() {
         ],
         timeDimensions: [{
           dimension: 'visitors.created_at',
+          granularity: 'day',
           dateRange: ['2017-01-01', '2017-01-30']
         }],
+        dimensions: ['visitor_checkins.source'],
         timezone: 'America/Los_Angeles',
         order: [],
         filters: [{
@@ -1164,11 +1166,11 @@ describe('SQL Generation', function test() {
       ])).then(res => {
         console.log(JSON.stringify(res));
         res.should.be.deepEqual(
-          [
-            {
-              "visitors__visitor_revenue": "100"
-            }
-          ]
+          [{
+            "visitor_checkins__source": "google",
+            "visitors__created_at_day": "2017-01-02T00:00:00.000Z",
+            "visitors__visitor_revenue": "100"
+          }]
         );
       });
     });