fix(schema-compiler): Use join paths from pre-aggregation declaration instead of building join tree from scratch

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

@@ -10,6 +10,8 @@ export class BaseDimension {
 
   public readonly isMemberExpression: boolean = false;
 
+  public readonly joinHint: Array<string> = [];
+
   public constructor(
     protected readonly query: BaseQuery,
     public readonly dimension: any
@@ -20,6 +22,14 @@ export class BaseDimension {
       // In case of SQL push down expressionName doesn't contain cube name. It's just a column name.
       this.expressionName = dimension.expressionName || `${dimension.cubeName}.${dimension.name}`;
       this.isMemberExpression = !!dimension.definition;
+    } else {
+      // TODO move this `as` to static types
+      const dimensionPath = dimension as string | null;
+      if (dimensionPath !== null) {
+        const { path, joinHint } = this.query.cubeEvaluator.joinHintFromPath(dimensionPath);
+        this.dimension = path;
+        this.joinHint = joinHint;
+      }
     }
   }
 

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

@@ -13,6 +13,8 @@ export class BaseMeasure {
 
   protected readonly patchedMeasure: MeasureDefinition | null = null;
 
+  public readonly joinHint: Array<string> = [];
+
   protected preparePatchedMeasure(sourceMeasure: string, newMeasureType: string | null, addFilters: Array<{sql: Function}>): MeasureDefinition {
     const source = this.query.cubeEvaluator.measureByPath(sourceMeasure);
 
@@ -123,6 +125,12 @@ export class BaseMeasure {
           measure.expression.addFilters,
         );
       }
+    } else {
+      // TODO move this `as` to static types
+      const measurePath = measure as string;
+      const { path, joinHint } = this.query.cubeEvaluator.joinHintFromPath(measurePath);
+      this.measure = path;
+      this.joinHint = joinHint;
     }
   }
 

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

@@ -307,16 +307,16 @@
   }
 
   prebuildJoin() {
-    if (this.useNativeSqlPlanner) {
-      // Tesseract doesn't require join to be prebuilt and there's a case where single join can't be built for multi-fact query
-      // But we need this join for a fallback when using pre-aggregations. So we’ll try to obtain the join but ignore any errors (which may occur if the query is a multi-fact one).
-      try {
-        this.join = this.joinGraph.buildJoin(this.allJoinHints);
-      } catch (e) {
-        // Ignore
-      }
-    } else {
+    try {
+      // TODO allJoinHints should contain join hints form pre-agg
       this.join = this.joinGraph.buildJoin(this.allJoinHints);
+    } catch (e) {
+      if (this.useNativeSqlPlanner) {
+        // Tesseract doesn't require join to be prebuilt and there's a case where single join can't be built for multi-fact query
+        // But we need this join for a fallback when using pre-aggregations. So we’ll try to obtain the join but ignore any errors (which may occur if the query is a multi-fact one).
+      } else {
+        throw e;
+      }
     }
   }
 
@@ -363,6 +363,10 @@
     return this.collectedCubeNames;
   }
 
+  /**
+   *
+   * @returns {Array<Array<string>>}
+   */
   get allJoinHints() {
     if (!this.collectedJoinHints) {
       this.collectedJoinHints = this.collectJoinHints();
@@ -1203,7 +1207,16 @@
 
   collectAllMultiStageMembers(allMemberChildren) {
     const allMembers = R.uniq(R.flatten(Object.keys(allMemberChildren).map(k => [k].concat(allMemberChildren[k]))));
-    return R.fromPairs(allMembers.map(m => ([m, this.memberInstanceByPath(m).isMultiStage()])));
+    return R.fromPairs(allMembers.map(m => {
+      // When `m` is coming from `collectAllMemberChildren`, it can contain `granularities.customGranularityName` in path
+      // And it would mess up with join hints detection
+      const trimmedPath = this
+        .cubeEvaluator
+        .parsePathAnyType(m)
+        .slice(0, 2)
+        .join('.');
+      return [m, this.memberInstanceByPath(trimmedPath).isMultiStage()];
+    }));
   }
 
   memberInstanceByPath(m) {
@@ -1992,7 +2005,7 @@
     );
 
     if (shouldBuildJoinForMeasureSelect) {
-      const joinHints = this.collectFrom(measures, this.collectJoinHintsFor.bind(this), 'collectJoinHintsFor');
+      const joinHints = this.collectJoinHintsFromMembers(measures);
       const measuresJoin = this.joinGraph.buildJoin(joinHints);
       if (measuresJoin.multiplicationFactor[keyCubeName]) {
         throw new UserError(
@@ -2046,6 +2059,11 @@
       (!this.safeEvaluateSymbolContext().ungrouped && this.aggregateSubQueryGroupByClause() || '');
   }
 
+  /**
+   * @param {Array<BaseMeasure>} measures
+   * @param {string} keyCubeName
+   * @returns {boolean}
+   */
   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)
@@ -2067,7 +2085,11 @@
         .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');
+      // Not using `collectJoinHintsFromMembers([measure])` because it would collect too many join hints from view
+      const joinHints = [
+        measure.joinHint,
+        ...this.collectJoinHintsFromMembers(nonViewMembers),
+      ];
       if (R.any(cubeName => keyCubeName !== cubeName, cubes)) {
         const measuresJoin = this.joinGraph.buildJoin(joinHints);
         if (measuresJoin.multiplicationFactor[keyCubeName]) {
@@ -2186,12 +2208,29 @@
     );
   }
 
+  /**
+   *
+   * @param {boolean} [excludeTimeDimensions=false]
+   * @returns {Array<Array<string>>}
+   */
   collectJoinHints(excludeTimeDimensions = false) {
-    return this.collectFromMembers(
-      excludeTimeDimensions,
-      this.collectJoinHintsFor.bind(this),
-      'collectJoinHintsFor'
-    );
+    const membersToCollectFrom = this.allMembersConcat(excludeTimeDimensions)
+      .concat(this.join ? this.join.joins.map(j => ({
+        getMembers: () => [{
+          path: () => null,
+          cube: () => this.cubeEvaluator.cubeFromPath(j.originalFrom),
+          definition: () => j.join,
+        }]
+      })) : []);
+
+    return this.collectJoinHintsFromMembers(membersToCollectFrom);
+  }
+
+  collectJoinHintsFromMembers(members) {
+    return [
+      ...members.map(m => m.joinHint).filter(h => h?.length > 0),
+      ...this.collectFrom(members, this.collectJoinHintsFor.bind(this), 'collectJoinHintsFromMembers'),
+    ];
   }
 
   collectFromMembers(excludeTimeDimensions, fn, methodName) {
@@ -2206,6 +2245,11 @@
     return this.collectFrom(membersToCollectFrom, fn, methodName);
   }
 
+  /**
+   *
+   * @param {boolean} excludeTimeDimensions
+   * @returns {Array<BaseMeasure | BaseDimension | BaseSegment>}
+   */
   allMembersConcat(excludeTimeDimensions) {
     return this.measures
       .concat(this.dimensions)

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

@@ -9,6 +9,8 @@ export class BaseSegment {
 
   public readonly isMemberExpression: boolean = false;
 
+  public readonly joinHint: Array<string> = [];
+
   public constructor(
     protected readonly query: BaseQuery,
     public readonly segment: string | any
@@ -19,6 +21,12 @@ export class BaseSegment {
       // In case of SQL push down expressionName doesn't contain cube name. It's just a column name.
       this.expressionName = segment.expressionName || `${segment.cubeName}.${segment.name}`;
       this.isMemberExpression = !!segment.definition;
+    } else {
+      // TODO move this `as` to static types
+      const segmentPath = segment as string;
+      const { path, joinHint } = this.query.cubeEvaluator.joinHintFromPath(segmentPath);
+      this.segment = path;
+      this.joinHint = joinHint;
     }
   }
 

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

@@ -169,13 +169,18 @@
         const timeDimensionsReference =
           foundPreAggregation.preAggregation.rollupLambdaTimeDimensionsReference ||
           foundPreAggregation.references.timeDimensions;
+        const timeDimensionReference = timeDimensionsReference[0];
 
-        if (td.dimension === timeDimensionsReference[0].dimension) {
+        // timeDimensionsReference[*].dimension can contain full join path, so we should trim it
+        // TODO check full join path match here
+        const timeDimensionReferenceDimension = this.query.cubeEvaluator.joinHintFromPath(timeDimensionReference.dimension).path;
+
+        if (td.dimension === timeDimensionReferenceDimension) {
           return true;
         }
 
         // Handling for views
-        return td.dimension === allBackAliasMembers[timeDimensionsReference[0].dimension];
+        return td.dimension === allBackAliasMembers[timeDimensionReferenceDimension];
       });
 
     const filters = preAggregation.partitionGranularity && this.query.filters.filter(td => {
@@ -485,6 +490,10 @@
    * @returns {function(preagg: Object): boolean}
    */
   static canUsePreAggregationForTransformedQueryFn(transformedQuery, refs) {
+    // TODO this needs to check not only members list, but their join paths as well:
+    //  query can have same members as pre-agg, but different calculated join path
+    //  `refs` will come from preagg references, and would contain full join paths
+
     /**
      * Returns an array of 2-elements arrays with the dimension and granularity
      * sorted by the concatenated dimension + granularity key.
@@ -1061,22 +1070,35 @@
   }
 
   rollupPreAggregationQuery(cube, aggregation) {
+    // `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 cubeQuery = this.query.newSubQueryForCube(cube, {});
-    return this.query.newSubQueryForCube(
-      cube,
-      {
-        rowLimit: null,
-        offset: null,
-        measures: references.measures,
-        dimensions: references.dimensions,
-        timeDimensions: this.mergePartitionTimeDimensions(references, aggregation.partitionTimeDimensions),
-        preAggregationQuery: true,
-        useOriginalSqlPreAggregationsInPreAggregation: aggregation.useOriginalSqlPreAggregations,
-        ungrouped: cubeQuery.preAggregationAllowUngroupingWithPrimaryKey(cube, aggregation) &&
-          !!references.dimensions.find(d => this.query.cubeEvaluator.dimensionByPath(d).primaryKey)
-      }
-    );
+    return this.query.newSubQueryForCube(cube, {
+      rowLimit: null,
+      offset: null,
+      measures: references.measures,
+      dimensions: references.dimensions,
+      timeDimensions: this.mergePartitionTimeDimensions(
+        references,
+        aggregation.partitionTimeDimensions
+      ),
+      preAggregationQuery: true,
+      useOriginalSqlPreAggregationsInPreAggregation:
+        aggregation.useOriginalSqlPreAggregations,
+      ungrouped:
+        cubeQuery.preAggregationAllowUngroupingWithPrimaryKey(
+          cube,
+          aggregation
+        ) &&
+        !!references.dimensions.find((d) => {
+          // `d` can contain full join path, so we should trim it
+          // TODO check full join path match here
+          const trimmedDimension = this.query.cubeEvaluator.joinHintFromPath(d).path;
+          return this.query.cubeEvaluator.dimensionByPath(trimmedDimension).primaryKey;
+        }),
+    });
   }
 
   autoRollupPreAggregationQuery(cube, aggregation) {
@@ -1101,6 +1123,7 @@
     }
     return aggregation.timeDimensions.map(d => {
       const toMerge = partitionTimeDimensions.find(
+        // Both qd and d comes from PreaggregationReferences
         qd => qd.dimension === d.dimension
       );
       return toMerge ? { ...d, dateRange: toMerge.dateRange, boundaryDateRange: toMerge.boundaryDateRange } : d;
@@ -1119,6 +1142,13 @@
       .toLowerCase();
   }
 
+  /**
+   *
+   * @param {string} cube
+   * @param aggregation
+   * @param {string} [preAggregationName]
+   * @returns {PreAggregationReferences}
+   */
   evaluateAllReferences(cube, aggregation, preAggregationName) {
     const evaluateReferences = () => {
       const references = this.query.cubeEvaluator.evaluatePreAggregationReferences(cube, aggregation);

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

@@ -82,6 +82,7 @@ type PreAggregationTimeDimensionReference = {
   granularity: string,
 };
 
+/// Strings in `dimensions`, `measures` and `timeDimensions[*].dimension` can contain full join path, not just `cube.member`
 type PreAggregationReferences = {
   allowNonStrictDateRangeMatch?: boolean,
   dimensions: Array<string>,
@@ -743,14 +744,14 @@ export class CubeEvaluator extends CubeSymbols {
 
     if (aggregation.timeDimensionReference) {
       timeDimensions.push({
-        dimension: this.evaluateReferences(cube, aggregation.timeDimensionReference),
+        dimension: this.evaluateReferences(cube, aggregation.timeDimensionReference, { collectJoinHints: true }),
         granularity: aggregation.granularity
       });
     } else if (aggregation.timeDimensionReferences) {
       // eslint-disable-next-line guard-for-in
       for (const timeDimensionReference of aggregation.timeDimensionReferences) {
         timeDimensions.push({
-          dimension: this.evaluateReferences(cube, timeDimensionReference.dimension),
+          dimension: this.evaluateReferences(cube, timeDimensionReference.dimension, { collectJoinHints: true }),
           granularity: timeDimensionReference.granularity
         });
       }
@@ -759,12 +760,12 @@ export class CubeEvaluator extends CubeSymbols {
     return {
       allowNonStrictDateRangeMatch: aggregation.allowNonStrictDateRangeMatch,
       dimensions:
-        (aggregation.dimensionReferences && this.evaluateReferences(cube, aggregation.dimensionReferences) || [])
+        (aggregation.dimensionReferences && this.evaluateReferences(cube, aggregation.dimensionReferences, { collectJoinHints: true }) || [])
           .concat(
-            aggregation.segmentReferences && this.evaluateReferences(cube, aggregation.segmentReferences) || []
+            aggregation.segmentReferences && this.evaluateReferences(cube, aggregation.segmentReferences, { collectJoinHints: true }) || []
           ),
       measures:
-        aggregation.measureReferences && this.evaluateReferences(cube, aggregation.measureReferences) || [],
+        (aggregation.measureReferences && this.evaluateReferences(cube, aggregation.measureReferences, { collectJoinHints: true }) || []),
       timeDimensions,
       rollups:
         aggregation.rollupReferences && this.evaluateReferences(cube, aggregation.rollupReferences, { originalSorting: true }) || [],

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

@@ -609,6 +609,27 @@
     return array.join('.');
   }
 
+  /**
+   * Split join path to member to join hint and member path: `A.B.C.D.E.dim` => `[A, B, C, D, E]` + `E.dim`
+   * @param path
+   */
+  public joinHintFromPath(path: string): { path: string, joinHint: Array<string> } {
+    const parts = path.split('.');
+    if (parts.length > 2) {
+      // Path contains join path
+      const joinHint = parts.slice(0, -1);
+      return {
+        path: parts.slice(-2).join('.'),
+        joinHint,
+      };
+    } else {
+      return {
+        path,
+        joinHint: [],
+      };
+    }
+  }
+
   protected resolveSymbolsCall<T>(
     func: (...args: Array<unknown>) => T | DynamicReference<T>,
     nameResolver: (id: string) => unknown,