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

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

@@ -10,8 +10,6 @@ export class BaseDimension {
 
   public readonly isMemberExpression: boolean = false;
 
-  public readonly joinHint: Array<string> = [];
-
   public constructor(
     protected readonly query: BaseQuery,
     public readonly dimension: any
@@ -22,14 +20,6 @@ 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,8 +13,6 @@ 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);
 
@@ -125,12 +123,6 @@ 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() {
-    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;
+    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 {
+      this.join = this.joinGraph.buildJoin(this.allJoinHints);
     }
   }
 
@@ -363,10 +363,6 @@
     return this.collectedCubeNames;
   }
 
-  /**
-   *
-   * @returns {Array<Array<string>>}
-   */
   get allJoinHints() {
     if (!this.collectedJoinHints) {
       this.collectedJoinHints = this.collectJoinHints();
@@ -1207,16 +1203,7 @@
 
   collectAllMultiStageMembers(allMemberChildren) {
     const allMembers = R.uniq(R.flatten(Object.keys(allMemberChildren).map(k => [k].concat(allMemberChildren[k]))));
-    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()];
-    }));
+    return R.fromPairs(allMembers.map(m => ([m, this.memberInstanceByPath(m).isMultiStage()])));
   }
 
   memberInstanceByPath(m) {
@@ -2005,7 +1992,7 @@
     );
 
     if (shouldBuildJoinForMeasureSelect) {
-      const joinHints = this.collectJoinHintsFromMembers(measures);
+      const joinHints = this.collectFrom(measures, this.collectJoinHintsFor.bind(this), 'collectJoinHintsFor');
       const measuresJoin = this.joinGraph.buildJoin(joinHints);
       if (measuresJoin.multiplicationFactor[keyCubeName]) {
         throw new UserError(
@@ -2059,11 +2046,6 @@
       (!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)
@@ -2085,11 +2067,7 @@
         .filter(member => member.definition().ownedByCube);
 
       const cubes = this.collectFrom(nonViewMembers, this.collectCubeNamesFor.bind(this), 'collectCubeNamesFor');
-      // Not using `collectJoinHintsFromMembers([measure])` because it would collect too many join hints from view
-      const joinHints = [
-        measure.joinHint,
-        ...this.collectJoinHintsFromMembers(nonViewMembers),
-      ];
+      const joinHints = this.collectFrom(nonViewMembers, this.collectJoinHintsFor.bind(this), 'collectJoinHintsFor');
       if (R.any(cubeName => keyCubeName !== cubeName, cubes)) {
         const measuresJoin = this.joinGraph.buildJoin(joinHints);
         if (measuresJoin.multiplicationFactor[keyCubeName]) {
@@ -2208,29 +2186,12 @@
     );
   }
 
-  /**
-   *
-   * @param {boolean} [excludeTimeDimensions=false]
-   * @returns {Array<Array<string>>}
-   */
   collectJoinHints(excludeTimeDimensions = false) {
-    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'),
-    ];
+    return this.collectFromMembers(
+      excludeTimeDimensions,
+      this.collectJoinHintsFor.bind(this),
+      'collectJoinHintsFor'
+    );
   }
 
   collectFromMembers(excludeTimeDimensions, fn, methodName) {
@@ -2245,11 +2206,6 @@
     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,8 +9,6 @@ export class BaseSegment {
 
   public readonly isMemberExpression: boolean = false;
 
-  public readonly joinHint: Array<string> = [];
-
   public constructor(
     protected readonly query: BaseQuery,
     public readonly segment: string | any
@@ -21,12 +19,6 @@ 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,18 +169,13 @@
         const timeDimensionsReference =
           foundPreAggregation.preAggregation.rollupLambdaTimeDimensionsReference ||
           foundPreAggregation.references.timeDimensions;
-        const timeDimensionReference = timeDimensionsReference[0];
 
-        // 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) {
+        if (td.dimension === timeDimensionsReference[0].dimension) {
           return true;
         }
 
         // Handling for views
-        return td.dimension === allBackAliasMembers[timeDimensionReferenceDimension];
+        return td.dimension === allBackAliasMembers[timeDimensionsReference[0].dimension];
       });
 
     const filters = preAggregation.partitionGranularity && this.query.filters.filter(td => {
@@ -490,10 +485,6 @@
    * @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.
@@ -1070,35 +1061,22 @@
   }
 
   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) => {
-          // `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;
-        }),
-    });
+    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)
+      }
+    );
   }
 
   autoRollupPreAggregationQuery(cube, aggregation) {
@@ -1123,7 +1101,6 @@
     }
     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;
@@ -1142,13 +1119,6 @@
       .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,7 +82,6 @@ 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>,
@@ -744,14 +743,14 @@ export class CubeEvaluator extends CubeSymbols {
 
     if (aggregation.timeDimensionReference) {
       timeDimensions.push({
-        dimension: this.evaluateReferences(cube, aggregation.timeDimensionReference, { collectJoinHints: true }),
+        dimension: this.evaluateReferences(cube, aggregation.timeDimensionReference),
         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, { collectJoinHints: true }),
+          dimension: this.evaluateReferences(cube, timeDimensionReference.dimension),
           granularity: timeDimensionReference.granularity
         });
       }
@@ -760,12 +759,12 @@ export class CubeEvaluator extends CubeSymbols {
     return {
       allowNonStrictDateRangeMatch: aggregation.allowNonStrictDateRangeMatch,
       dimensions:
-        (aggregation.dimensionReferences && this.evaluateReferences(cube, aggregation.dimensionReferences, { collectJoinHints: true }) || [])
+        (aggregation.dimensionReferences && this.evaluateReferences(cube, aggregation.dimensionReferences) || [])
           .concat(
-            aggregation.segmentReferences && this.evaluateReferences(cube, aggregation.segmentReferences, { collectJoinHints: true }) || []
+            aggregation.segmentReferences && this.evaluateReferences(cube, aggregation.segmentReferences) || []
           ),
       measures:
-        (aggregation.measureReferences && this.evaluateReferences(cube, aggregation.measureReferences, { collectJoinHints: true }) || []),
+        aggregation.measureReferences && this.evaluateReferences(cube, aggregation.measureReferences) || [],
       timeDimensions,
       rollups:
         aggregation.rollupReferences && this.evaluateReferences(cube, aggregation.rollupReferences, { originalSorting: true }) || [],

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

@@ -609,27 +609,6 @@ export class CubeSymbols {
     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,