fix(schema-compiler): Fix queries with time dimensions without granul…

…arities don't hit pre-aggregations with allow_non_strict_date_range_match=true
cube-js · Jan 15, 2025 · 62c542f · 62c542f
1 parent dcd283a
commit 62c542f
Show file tree

Hide file tree

Showing 3 changed files with 58 additions and 12 deletions.
diff --git a/packages/cubejs-schema-compiler/src/adapter/BaseTimeDimension.ts b/packages/cubejs-schema-compiler/src/adapter/BaseTimeDimension.ts
@@ -39,7 +39,7 @@ export class BaseTimeDimension extends BaseFilter {
   }
 
   // TODO: find and fix all hidden references to granularity to rely on granularityObj instead?
-  public get granularity(): string | undefined {
+  public get granularity(): string | null | undefined {
     return this.granularityObj?.granularity;
   }
 
@@ -217,7 +217,7 @@ export class BaseTimeDimension extends BaseFilter {
     return this.query.dateTimeCast(this.query.paramAllocator.allocateParam(this.dateRange ? this.dateToFormatted() : BUILD_RANGE_END_LOCAL));
   }
 
-  public dateRangeGranularity() {
+  public dateRangeGranularity(): string | null {
     if (!this.dateRange) {
       return null;
     }
@@ -262,7 +262,7 @@ export class BaseTimeDimension extends BaseFilter {
   }
 
   public resolvedGranularity() {
-    return this.granularityObj?.resolvedGranularity();
+    return this.granularityObj ? this.granularityObj.resolvedGranularity() : this.dateRangeGranularity();
   }
 
   public isPredefinedGranularity(): boolean {

diff --git a/packages/cubejs-schema-compiler/src/adapter/PreAggregations.js b/packages/cubejs-schema-compiler/src/adapter/PreAggregations.js
@@ -439,18 +439,34 @@ export class PreAggregations {
   static sortTimeDimensionsWithRollupGranularity(timeDimensions) {
     return timeDimensions && R.sortBy(
       R.prop(0),
-      timeDimensions.map(d => (d.isPredefinedGranularity() ?
-        [d.expressionPath(), d.rollupGranularity(), null] :
-        // For custom granularities we need to add its name to the list (for exact matches)
-        [d.expressionPath(), d.rollupGranularity(), d.granularity]
-      ))
+      timeDimensions.map(d => {
+        const res = [d.expressionPath(), d.rollupGranularity()];
+        if (d.isPredefinedGranularity()) {
+          res.push(null);
+        } else if (d.granularity && d.granularity !== res[1]) {
+          // For custom granularities we need to add its name to the list (for exact matches)
+          res.push(d.granularity);
+        }
+        return res;
+      })
     ) || [];
   }
 
   static timeDimensionsAsIs(timeDimensions) {
     return timeDimensions && R.sortBy(
       R.prop(0),
-      timeDimensions.map(d => [d.expressionPath(), d.granularity]),
+      timeDimensions.map(d => {
+        const res = [d.expressionPath()];
+        const resolvedGranularity = d.resolvedGranularity();
+        if (d.granularity && d.granularity !== resolvedGranularity) {
+          // For custom granularities we need to add its name to the list (for exact matches)
+          res.push(...[resolvedGranularity, d.granularity]);
+        } else {
+          // For query timeDimension without granularity we pass the resolved from dataRange as fallback
+          res.push(resolvedGranularity);
+        }
+        return res;
+      }),
     ) || [];
   }
 
@@ -628,13 +644,15 @@ export class PreAggregations {
      * @returns {Array<Array<string>>}
      */
     const expandTimeDimension = (timeDimension) => {
-      const [dimension, granularity, customGranularity] = timeDimension;
+      const [dimension, granularity, customOrResolvedGranularity] = timeDimension;
       const res = expandGranularity(granularity)
         .map((newGranularity) => [dimension, newGranularity]);
 
-      if (customGranularity) {
+      if (customOrResolvedGranularity) {
         // For custom granularities we add it upfront to the list (for exact matches)
-        res.unshift([dimension, customGranularity]);
+        // For queries with timeDimension but without granularity specified we use resolved
+        // granularity from date range
+        res.unshift([dimension, customOrResolvedGranularity]);
       }
 
       return res;

diff --git a/packages/cubejs-schema-compiler/test/integration/postgres/pre-aggregations.test.ts b/packages/cubejs-schema-compiler/test/integration/postgres/pre-aggregations.test.ts
@@ -229,6 +229,7 @@ describe('PreAggregations', () => {
           dimensions: [sourceAndId, source],
           timeDimension: createdAt,
           granularity: 'hour',
+          allowNonStrictDateRangeMatch: true
         },
         visitorsMultiplied: {
           measures: [count],
@@ -546,6 +547,33 @@ describe('PreAggregations', () => {
     });
   }));
 
+  it('simple pre-aggregation (with no granularity in query)', () => compiler.compile().then(() => {
+    const query = new PostgresQuery({ joinGraph, cubeEvaluator, compiler }, {
+      measures: [
+        'visitors.count'
+      ],
+      timeDimensions: [{
+        dimension: 'visitors.createdAt',
+        dateRange: ['2017-01-01 00:00:00.000', '2017-01-29 22:59:59.999']
+      }],
+      timezone: 'America/Los_Angeles',
+      preAggregationsSchema: ''
+    });
+
+    const queryAndParams = query.buildSqlAndParams();
+    console.log(queryAndParams);
+    expect(query.preAggregations?.preAggregationForQuery?.canUsePreAggregation).toEqual(true);
+    expect(queryAndParams[0]).toMatch(/visitors_source_and_id_rollup/);
+
+    return dbRunner.evaluateQueryWithPreAggregations(query).then(res => {
+      expect(res).toEqual(
+        [{
+          visitors__count: '5'
+        }]
+      );
+    });
+  }));
+
   it('leaf measure pre-aggregation', () => compiler.compile().then(() => {
     const query = new PostgresQuery({ joinGraph, cubeEvaluator, compiler }, {
       measures: [