feat(schema-compiler): Use views join maps and hints for query joins construction (#10039)

* add tests

* enable view join tests for tesseract

* fix test

* temp comment out tests

* correct additional hints

predecessors comparator

OMG! Is it really working?!

everything works besides loop detection

correct additional hints: everything works besides loop detection

* trying to adopt loop test

fix actually incorrect tests

* linter fix

* small fix in error handling

* add test case for join maps test

* implement join maps

* update snapshot

* refactor tests

* add more tests

* return back loop for join resolution

* fix typo

* join map in tesseract

* enable view join tests for tesseract

* cargo fmt

* uncomment test

* remove println!

* remove unneeded rootOfJoin

* refactor tests

* fix linter warning

* refactor allJoinHints()

* refactor: extract inlined isJoinTreesEqual()

* remove unused

* add support for transitive joins in tesseract

* uncomment transitive joins tests for tesseract

* cargo fmt

Author: KSDaemon

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

@@ -22,7 +22,6 @@ import {
   localTimestampToUtc,
   timeSeries as timeSeriesBase,
   timeSeriesFromCustomInterval,
-  parseSqlInterval,
   findMinGranularityDimension
 } from '@cubejs-backend/shared';
 
@@ -387,12 +386,59 @@ export class BaseQuery {
     }
   }
 
+  /**
+   * Is used by native
+   * This function follows the same logic as in this.collectJoinHints()
+   * @private
+   * @param {Array<(Array<string> | string)>} hints
+   * @return {import('../compiler/JoinGraph').FinishedJoinTree}
+   */
+  joinTreeForHints(hints) {
+    const explicitJoinHintMembers = new Set(hints.filter(j => Array.isArray(j)).flat());
+    const queryJoinMaps = this.queryJoinMap();
+    const newCollectedHints = [];
+
+    const constructJH = () => R.uniq(this.enrichHintsWithJoinMap([
+      ...newCollectedHints,
+      ...hints,
+    ],
+    queryJoinMaps));
+
+    let prevJoin = null;
+    let newJoin = null;
+
+    // Safeguard against infinite loop in case of cyclic joins somehow managed to slip through
+    let cnt = 0;
+    let newJoinHintsCollectedCnt;
+
+    do {
+      const allJoinHints = constructJH();
+      prevJoin = newJoin;
+      newJoin = this.joinGraph.buildJoin(allJoinHints);
+      const allJoinHintsFlatten = new Set(allJoinHints.flat());
+      const joinMembersJoinHints = this.collectJoinHintsFromMembers(this.joinMembersFromJoin(newJoin));
+
+      const iterationCollectedHints = joinMembersJoinHints.filter(j => !allJoinHintsFlatten.has(j));
+      newJoinHintsCollectedCnt = iterationCollectedHints.length;
+      cnt++;
+      if (newJoin) {
+        newCollectedHints.push(...joinMembersJoinHints.filter(j => !explicitJoinHintMembers.has(j)));
+      }
+    } while (newJoin?.joins.length > 0 && !this.isJoinTreesEqual(prevJoin, newJoin) && cnt < 10000 && newJoinHintsCollectedCnt > 0);
+
+    if (cnt >= 10000) {
+      throw new UserError('Can not construct joins for the query, potential loop detected');
+    }
+
+    return newJoin;
+  }
+
   cacheValue(key, fn, { contextPropNames, inputProps, cache } = {}) {
     const currentContext = this.safeEvaluateSymbolContext();
     if (contextPropNames) {
       const contextKey = {};
-      for (let i = 0; i < contextPropNames.length; i++) {
-        contextKey[contextPropNames[i]] = currentContext[contextPropNames[i]];
+      for (const element of contextPropNames) {
+        contextKey[element] = currentContext[element];
       }
       key = key.concat([JSON.stringify(contextKey)]);
     }
@@ -436,83 +482,54 @@ export class BaseQuery {
    */
   get allJoinHints() {
     if (!this.collectedJoinHints) {
-      const [rootOfJoin, ...allMembersJoinHints] = this.collectJoinHintsFromMembers(this.allMembersConcat(false));
-      const customSubQueryJoinHints = this.collectJoinHintsFromMembers(this.joinMembersFromCustomSubQuery());
-      let joinMembersJoinHints = this.collectJoinHintsFromMembers(this.joinMembersFromJoin(this.join));
-
-      // One cube may join the other cube via transitive joined cubes,
-      // members from which are referenced in the join `on` clauses.
-      // We need to collect such join hints and push them upfront of the joining one
-      // but only if they don't exist yet. Cause in other case we might affect what
-      // join path will be constructed in join graph.
-      // It is important to use queryLevelJoinHints during the calculation if it is set.
-
-      const constructJH = () => {
-        const filteredJoinMembersJoinHints = joinMembersJoinHints.filter(m => !allMembersJoinHints.includes(m));
-        return [
-          ...this.queryLevelJoinHints,
-          ...(rootOfJoin ? [rootOfJoin] : []),
-          ...filteredJoinMembersJoinHints,
-          ...allMembersJoinHints,
-          ...customSubQueryJoinHints,
-        ];
-      };
-
-      let prevJoins = this.join;
-      let prevJoinMembersJoinHints = joinMembersJoinHints;
-      let newJoin = this.joinGraph.buildJoin(constructJH());
-
-      const isOrderPreserved = (base, updated) => {
-        const common = base.filter(value => updated.includes(value));
-        const bFiltered = updated.filter(value => common.includes(value));
-
-        return common.every((x, i) => x === bFiltered[i]);
-      };
-
-      const isJoinTreesEqual = (a, b) => {
-        if (!a || !b || a.root !== b.root || a.joins.length !== b.joins.length) {
-          return false;
-        }
-
-        // We don't care about the order of joins on the same level, so
-        // we can compare them as sets.
-        const aJoinsSet = new Set(a.joins.map(j => `${j.originalFrom}->${j.originalTo}`));
-        const bJoinsSet = new Set(b.joins.map(j => `${j.originalFrom}->${j.originalTo}`));
-
-        if (aJoinsSet.size !== bJoinsSet.size) {
-          return false;
-        }
-
-        for (const val of aJoinsSet) {
-          if (!bJoinsSet.has(val)) {
-            return false;
-          }
-        }
+      this.collectedJoinHints = this.collectJoinHints();
+    }
+    return this.collectedJoinHints;
+  }
 
-        return true;
-      };
+  /**
+   * @private
+   * @return { Record<string, string[][]>}
+   */
+  queryJoinMap() {
+    const queryMembers = this.allMembersConcat(false);
+    const joinMaps = {};
+
+    for (const member of queryMembers) {
+      const memberCube = member.cube?.();
+      if (memberCube?.isView && !joinMaps[memberCube.name] && memberCube.joinMap) {
+        joinMaps[memberCube.name] = memberCube.joinMap;
+      }
+    }
 
-      // Safeguard against infinite loop in case of cyclic joins somehow managed to slip through
-      let cnt = 0;
+    return joinMaps;
+  }
 
-      while (newJoin?.joins.length > 0 && !isJoinTreesEqual(prevJoins, newJoin) && cnt < 10000) {
-        prevJoins = newJoin;
-        joinMembersJoinHints = this.collectJoinHintsFromMembers(this.joinMembersFromJoin(newJoin));
-        if (!isOrderPreserved(prevJoinMembersJoinHints, joinMembersJoinHints)) {
-          throw new UserError(`Can not construct joins for the query, potential loop detected: ${prevJoinMembersJoinHints.join('->')} vs ${joinMembersJoinHints.join('->')}`);
-        }
-        newJoin = this.joinGraph.buildJoin(constructJH());
-        prevJoinMembersJoinHints = joinMembersJoinHints;
-        cnt++;
+  /**
+   * @private
+   * @param { (string|string[])[] } hints
+   * @param { Record<string, string[][]>} joinMap
+   * @return {(string|string[])[]}
+   */
+  enrichHintsWithJoinMap(hints, joinMap) {
+    // Potentially, if joins between views would take place, we need to distinguish
+    // join maps on per view basis.
+    const allPaths = Object.values(joinMap).flat();
+
+    return hints.map(hint => {
+      if (Array.isArray(hint)) {
+        return hint;
       }
 
-      if (cnt >= 10000) {
-        throw new UserError('Can not construct joins for the query, potential loop detected');
+      for (const path of allPaths) {
+        const hintIndex = path.indexOf(hint);
+        if (hintIndex !== -1) {
+          return path.slice(0, hintIndex + 1);
+        }
       }
 
-      this.collectedJoinHints = R.uniq(constructJH());
-    }
-    return this.collectedJoinHints;
+      return hint;
+    });
   }
 
   get dataSource() {
@@ -2613,18 +2630,89 @@ export class BaseQuery {
   }
 
   /**
-   *
+   * Just a helper to avoid copy/paste
+   * @private
+   * @param {import('../compiler/JoinGraph').FinishedJoinTree} a
+   * @param {import('../compiler/JoinGraph').FinishedJoinTree} b
+   * @return {boolean}
+   */
+  isJoinTreesEqual(a, b) {
+    if (!a || !b || a.root !== b.root || a.joins.length !== b.joins.length) {
+      return false;
+    }
+
+    // We don't care about the order of joins on the same level, so
+    // we can compare them as sets.
+    const aJoinsSet = new Set(a.joins.map(j => `${j.originalFrom}->${j.originalTo}`));
+    const bJoinsSet = new Set(b.joins.map(j => `${j.originalFrom}->${j.originalTo}`));
+
+    if (aJoinsSet.size !== bJoinsSet.size) {
+      return false;
+    }
+
+    for (const val of aJoinsSet) {
+      if (!bJoinsSet.has(val)) {
+        return false;
+      }
+    }
+
+    return true;
+  }
+
+  /**
+   * @private
    * @param {boolean} [excludeTimeDimensions=false]
-   * @returns {Array<Array<string>>}
+   * @returns {Array<(Array<string> | string)>}
    */
   collectJoinHints(excludeTimeDimensions = false) {
-    const membersToCollectFrom = [
-      ...this.allMembersConcat(excludeTimeDimensions),
-      ...this.joinMembersFromJoin(this.join),
-      ...this.joinMembersFromCustomSubQuery(),
-    ];
+    const allMembersJoinHints = this.collectJoinHintsFromMembers(this.allMembersConcat(excludeTimeDimensions));
+    const explicitJoinHintMembers = new Set(allMembersJoinHints.filter(j => Array.isArray(j)).flat());
+    const queryJoinMaps = this.queryJoinMap();
+    const customSubQueryJoinHints = this.collectJoinHintsFromMembers(this.joinMembersFromCustomSubQuery());
+    const newCollectedHints = [];
+
+    // One cube may join the other cube via transitive joined cubes,
+    // members from which are referenced in the join `on` clauses.
+    // We need to collect such join hints and push them upfront of the joining one
+    // but only if they don't exist yet. Cause in other case we might affect what
+    // join path will be constructed in join graph.
+    // It is important to use queryLevelJoinHints during the calculation if it is set.
+
+    const constructJH = () => R.uniq(this.enrichHintsWithJoinMap([
+      ...this.queryLevelJoinHints,
+      ...newCollectedHints,
+      ...allMembersJoinHints,
+      ...customSubQueryJoinHints,
+    ],
+    queryJoinMaps));
+
+    let prevJoin = null;
+    let newJoin = null;
+
+    // Safeguard against infinite loop in case of cyclic joins somehow managed to slip through
+    let cnt = 0;
+    let newJoinHintsCollectedCnt;
+
+    do {
+      const allJoinHints = constructJH();
+      prevJoin = newJoin;
+      newJoin = this.joinGraph.buildJoin(allJoinHints);
+      const allJoinHintsFlatten = new Set(allJoinHints.flat());
+      const joinMembersJoinHints = this.collectJoinHintsFromMembers(this.joinMembersFromJoin(newJoin));
+
+      const iterationCollectedHints = joinMembersJoinHints.filter(j => !allJoinHintsFlatten.has(j));
+      newJoinHintsCollectedCnt = iterationCollectedHints.length;
+      cnt++;
+      if (newJoin) {
+        newCollectedHints.push(...joinMembersJoinHints.filter(j => !explicitJoinHintMembers.has(j)));
+      }
+    } while (newJoin?.joins.length > 0 && !this.isJoinTreesEqual(prevJoin, newJoin) && cnt < 10000 && newJoinHintsCollectedCnt > 0);
+
+    if (cnt >= 10000) {
+      throw new UserError('Can not construct joins for the query, potential loop detected');
+    }
 
-    return this.collectJoinHintsFromMembers(membersToCollectFrom);
+    return constructJH();
   }
 
   joinMembersFromCustomSubQuery() {

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

@@ -133,6 +133,12 @@ export type AccessPolicyDefinition = {
   };
 };
 
+export type ViewIncludedMember = {
+  type: string;
+  memberPath: string;
+  name: string;
+};
+
 export interface CubeDefinition {
   name: string;
   extends?: (...args: Array<unknown>) => { __cubeName: string };
@@ -159,7 +165,8 @@ export interface CubeDefinition {
   isView?: boolean;
   calendar?: boolean;
   isSplitView?: boolean;
-  includedMembers?: any[];
+  includedMembers?: ViewIncludedMember[];
+  joinMap?: string[][];
   fileName?: string;
 }
 
@@ -562,6 +569,8 @@ export class CubeSymbols implements TranspilerSymbolResolver {
     // `hierarchies` must be processed first
     const types = ['hierarchies', 'measures', 'dimensions', 'segments'];
 
+    const joinMap: string[][] = [];
+
     for (const type of types) {
       let cubeIncludes: any[] = [];
 
@@ -573,6 +582,11 @@ export class CubeSymbols implements TranspilerSymbolResolver {
         const split = fullPath.split('.');
         const cubeRef = split[split.length - 1];
 
+        // No need to keep a simple direct cube joins in join map
+        if (split.length > 1) {
+          joinMap.push(split);
+        }
+
         if (it.includes === '*') {
           return it;
         }
@@ -614,11 +628,7 @@ export class CubeSymbols implements TranspilerSymbolResolver {
         existing.map(({ type: t, memberPath, name }) => `${t}|${memberPath}|${name}`)
       );
 
-      const additions: {
-        type: string;
-        memberPath: string;
-        name: string;
-      }[] = [];
+      const additions: ViewIncludedMember[] = [];
 
       for (const { member, name } of cubeIncludes) {
         const parts = member.split('.');
@@ -636,6 +646,8 @@ export class CubeSymbols implements TranspilerSymbolResolver {
       }
     }
 
+    cube.joinMap = joinMap;
+
     [...memberSets.allMembers].filter(it => !memberSets.resolvedMembers.has(it)).forEach(it => {
       errorReporter.error(`Member '${it}' is included in '${cube.name}' but not defined in any cube`);
     });

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

@@ -736,7 +736,7 @@ export class DataSchemaCompiler {
       if (e.toString().indexOf('SyntaxError') !== -1) {
         const err = e as SyntaxErrorInterface;
         const line = file.content.split('\n')[(err.loc?.start?.line || 1) - 1];
-        const spaces = Array(err.loc?.start.column).fill(' ').join('');
+        const spaces = Array(err.loc?.start?.column).fill(' ').join('') || '';
         errorsReport.error(`Syntax error during parsing: ${err.message}:\n${line}\n${spaces}^`, file.fileName);
       } else {
         errorsReport.error(e);