fix(schema-compiler): Fix rendered references for preaggregations with join paths

Author: mcheshkov

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

@@ -1443,76 +1443,72 @@ export class PreAggregations {
   }
 
   private measuresRenderedReference(preAggregationForQuery: PreAggregationForQuery): Record<string, string> {
-    return R.pipe<
-      string[],
-      Array<[string, string]>,
-      Record<string, string>
-    >(
-      R.map((path: string): [string, string] => {
+    const measures = this.rollupMeasures(preAggregationForQuery);
+
+    return Object.fromEntries(measures
+      .flatMap(path => {
         const measure = this.query.newMeasure(path);
-        return [
+        const memberPath = this.query.cubeEvaluator.pathFromArray(measure.path());
+        const column = this.query.ungrouped ? measure.aliasName() : (this.query.aggregateOnGroupedColumn(
+          measure.measureDefinition(),
+          measure.aliasName(),
+          !this.query.safeEvaluateSymbolContext().overTimeSeriesAggregate,
           path,
-          this.query.ungrouped ? measure.aliasName() : (this.query.aggregateOnGroupedColumn(
-            measure.measureDefinition(),
-            measure.aliasName(),
-            !this.query.safeEvaluateSymbolContext().overTimeSeriesAggregate,
-            path,
-          ) || `sum(${measure.aliasName()})`),
+        ) || `sum(${measure.aliasName()})`);
+        // Return both full join path and measure path
+        return [
+          [path, column],
+          [memberPath, column],
         ];
-      }),
-      R.fromPairs,
-    )(this.rollupMeasures(preAggregationForQuery));
+      }));
   }
 
   private measureAliasesRenderedReference(preAggregationForQuery: PreAggregationForQuery): Record<string, string> {
-    return R.pipe<
-      string[],
-      Array<[string, string]>,
-      Record<string, string>
-    >(
-      R.map((path: string): [string, string] => {
+    const measures = this.rollupMeasures(preAggregationForQuery);
+
+    return Object.fromEntries(measures
+      .flatMap(path => {
         const measure = this.query.newMeasure(path);
+        const memberPath = this.query.cubeEvaluator.pathFromArray(measure.path());
+        const alias = measure.aliasName();
+        // Return both full join path and measure path
         return [
-          path,
-          measure.aliasName(),
+          [path, alias],
+          [memberPath, alias],
         ];
-      }),
-      R.fromPairs,
-    )(this.rollupMeasures(preAggregationForQuery));
+      }));
   }
 
   private dimensionsRenderedReference(preAggregationForQuery: PreAggregationForQuery): Record<string, string> {
-    return R.pipe<
-      string[],
-      Array<[string, string]>,
-      Record<string, string>
-    >(
-      R.map((path: string): [string, string] => {
+    const dimensions = this.rollupDimensions(preAggregationForQuery);
+
+    return Object.fromEntries(dimensions
+      .flatMap(path => {
         const dimension = this.query.newDimension(path);
+        const column = this.query.escapeColumnName(dimension.unescapedAliasName());
+        const memberPath = this.query.cubeEvaluator.pathFromArray(dimension.path());
+        // Return both full join path and dimension path
         return [
-          path,
-          this.query.escapeColumnName(dimension.unescapedAliasName()),
+          [path, column],
+          [memberPath, column],
         ];
-      }),
-      R.fromPairs,
-    )(this.rollupDimensions(preAggregationForQuery));
+      }));
   }
 
   private timeDimensionsRenderedReference(rollupGranularity: string, preAggregationForQuery: PreAggregationForQuery): Record<string, string> {
-    return R.pipe<
-      PreAggregationTimeDimensionReference[],
-      Array<[string, string]>,
-      Record<string, string>
-    >(
-      R.map((td: PreAggregationTimeDimensionReference): [string, string] => {
+    const timeDimensions = this.rollupTimeDimensions(preAggregationForQuery);
+
+    return Object.fromEntries(timeDimensions
+      .flatMap(td => {
         const timeDimension = this.query.newTimeDimension(td);
+        const column = this.query.escapeColumnName(timeDimension.unescapedAliasName(rollupGranularity));
+        const memberPath = this.query.cubeEvaluator.pathFromArray(timeDimension.path());
+        // Return both full join path and dimension path
         return [
-          td.dimension,
-          this.query.escapeColumnName(timeDimension.unescapedAliasName(rollupGranularity)),
+          [td.dimension, column],
+          [memberPath, column],
         ];
-      }),
-      R.fromPairs,
-    )(this.rollupTimeDimensions(preAggregationForQuery));
+      }));
   }
 
   private rollupMembers<T extends 'measures' | 'dimensions' | 'timeDimensions'>(preAggregationForQuery: PreAggregationForQuery, type: T): PreAggregationReferences[T] {

packages/cubejs-schema-compiler/test/integration/postgres/multiple-join-paths.test.ts

@@ -3,6 +3,7 @@ import { prepareJsCompiler } from '../../unit/PrepareCompiler';
 import { DataSchemaCompiler } from '../../../src/compiler/DataSchemaCompiler';
 import { JoinGraph } from '../../../src/compiler/JoinGraph';
 import { CubeEvaluator } from '../../../src/compiler/CubeEvaluator';
+import { testWithPreAggregation } from './pre-aggregation-utils';
 
 describe('Multiple join paths', () => {
   jest.setTimeout(200000);
@@ -20,7 +21,8 @@ describe('Multiple join paths', () => {
     // └-->F-------┘
     // View, pre-aggregations and all interesting parts should use ADEX path
     // It should NOT be the shortest one from A to X (that's AFX), nor first in join edges declaration (that's ABCX)
-    // All join conditions would be essentially `FALSE`, but with different syntax, to be able to test SQL generation
+    // All join conditions would be essentially `TRUE` for ADEX joins and `FALSE` for everything else
+    // But they would use different syntax, to be able to test SQL generation
     // Also, there should be only one way to cover cubes A and D with joins: A->D join
 
     // TODO in this model queries like [A.a_id, X.x_id] become ambiguous, probably we want to handle this better
@@ -37,7 +39,7 @@ describe('Multiple join paths', () => {
           },
           D: {
             relationship: 'many_to_one',
-            sql: "'A' = 'D'",
+            sql: "'A' = 'D' OR TRUE",
           },
           F: {
             relationship: 'many_to_one',
@@ -78,7 +80,6 @@ describe('Multiple join paths', () => {
               a_id,
               A.D.d_id,
               A.D.d_name_for_join_paths,
-              // D.d_id,
               A.D.E.X.x_id,
             ],
             measures: [
@@ -93,6 +94,19 @@ describe('Multiple join paths', () => {
             granularity: 'day',
           },
 
+          adex_cumulative_with_join_paths: {
+            type: 'rollup',
+            dimensions: [
+              a_id,
+              A.D.E.X.x_id,
+            ],
+            measures: [
+              A.D.E.X.x_cumulative_sum,
+            ],
+            timeDimension: A.D.E.X.x_time,
+            granularity: 'day',
+          },
+
           ad_without_join_paths: {
             type: 'rollup',
             dimensions: [
@@ -183,7 +197,7 @@ describe('Multiple join paths', () => {
         joins: {
           E: {
             relationship: 'many_to_one',
-            sql: "'D' = 'E'",
+            sql: "'D' = 'E' OR TRUE",
           },
         },
 
@@ -228,7 +242,7 @@ describe('Multiple join paths', () => {
         joins: {
           X: {
             relationship: 'many_to_one',
-            sql: "'E' = 'X'",
+            sql: "'E' = 'X' OR TRUE",
           },
         },
 
@@ -319,6 +333,13 @@ describe('Multiple join paths', () => {
             sql: 'x_value',
             type: 'sum',
           },
+          x_cumulative_sum: {
+            sql: 'x_value',
+            type: 'sum',
+            rolling_window: {
+                trailing: 'unbounded',
+            },
+          },
         },
 
         segments: {
@@ -515,33 +536,108 @@ describe('Multiple join paths', () => {
       expect(preAggregation).toBeDefined();
     });
 
-    const preAggregationIds = [
-      'A.adex_with_join_paths',
-      'A.ad_without_join_paths',
+    function makeReferenceQueryFor(preAggregationId: string, withDateRange: boolean = false): PostgresQuery {
+      const preAggregations = cubeEvaluator.preAggregations({
+        preAggregationIds: [preAggregationId]
+      });
+
+      expect(preAggregations.length).toBe(1);
+      const preAggregation = preAggregations[0];
+
+      if (withDateRange) {
+        preAggregation.references.timeDimensions = preAggregation.references.timeDimensions.map(td => ({
+          ...td,
+          dateRange: ['1970-01-01', '1970-01-02'],
+        }));
+      }
+
+      return new PostgresQuery({ joinGraph, cubeEvaluator, compiler }, {
+        ...preAggregation.references,
+        preAggregationId: preAggregation.id,
+        preAggregationsSchema: '',
+        timezone: 'UTC',
+      });
+    }
+
+    const preAggregationTests = [
+      {
+        preAggregationId: 'A.adex_with_join_paths',
+        addTimeRange: false,
+        expectedData: [
+          {
+            a__a_id: 1,
+            a__a_seg: false,
+            a__a_sum: '100',
+            d__d_id: 1,
+            d__d_name_for_join_paths: 'foo',
+            d__d_seg: false,
+            x__x_id: 1,
+            x__x_seg: false,
+            x__x_time_day: '1970-01-01T00:00:00.000Z',
+          },
+        ],
+      },
+      {
+        preAggregationId: 'A.adex_cumulative_with_join_paths',
+        addTimeRange: true,
+        expectedData: [
+          {
+            a__a_id: 1,
+            x__x_cumulative_sum: '100',
+            x__x_id: 1,
+            x__x_time_day: '1970-01-01T00:00:00.000Z',
+          },
+          {
+            a__a_id: 1,
+            x__x_cumulative_sum: '100',
+            x__x_id: 1,
+            x__x_time_day: '1970-01-02T00:00:00.000Z',
+          },
+        ],
+      },
+      {
+        preAggregationId: 'A.ad_without_join_paths',
+        addTimeRange: false,
+        expectedData: [
+          {
+            a__a_id: 1,
+            a__a_seg: false,
+            a__a_sum: '100',
+            d__d_id: 1,
+            d__d_name_for_no_join_paths: 'foo',
+            d__d_seg: false,
+            d__d_time_day: '1970-01-01T00:00:00.000Z',
+          },
+        ],
+      },
     ];
-    for (const preAggregationId of preAggregationIds) {
+    for (const { preAggregationId, addTimeRange, expectedData } of preAggregationTests) {
       // eslint-disable-next-line no-loop-func
       it(`pre-aggregation ${preAggregationId} should match its own references`, async () => {
-        const preAggregations = cubeEvaluator.preAggregations({});
+        // Always not using range, because reference query would have no range to start from
+        // but should match pre-aggregation anyway
+        const query = makeReferenceQueryFor(preAggregationId);
 
-        const preAggregation = preAggregations
-          .find(p => p.id === preAggregationId);
-        if (preAggregation === undefined) {
-          throw expect(preAggregation).toBeDefined();
+        const preAggregationsDescription: any = query.preAggregations?.preAggregationsDescription();
+        const preAggregationFromQuery = preAggregationsDescription.find(p => p.preAggregationId === preAggregationId);
+        if (preAggregationFromQuery === undefined) {
+          throw expect(preAggregationFromQuery).toBeDefined();
         }
+      });
 
-        const query = new PostgresQuery({ joinGraph, cubeEvaluator, compiler }, {
-          ...preAggregation.references,
-          preAggregationId: preAggregation.id,
-        });
+      // eslint-disable-next-line no-loop-func
+      it(`pre-aggregation ${preAggregationId} reference query should be executable`, async () => {
+        // Adding date range for rolling window measure
+        const query = makeReferenceQueryFor(preAggregationId, addTimeRange);
 
         const preAggregationsDescription: any = query.preAggregations?.preAggregationsDescription();
-        const preAggregationFromQuery = preAggregationsDescription.find(p => p.preAggregationId === preAggregation.id);
+        const preAggregationFromQuery = preAggregationsDescription.find(p => p.preAggregationId === preAggregationId);
         if (preAggregationFromQuery === undefined) {
           throw expect(preAggregationFromQuery).toBeDefined();
         }
 
-        expect(preAggregationFromQuery.preAggregationId).toBe(preAggregationId);
+        const res = await testWithPreAggregation(preAggregationFromQuery, query);
+        expect(res).toEqual(expectedData);
       });
     }
   });