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

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

@@ -22,7 +22,6 @@ import {
   localTimestampToUtc,
   timeSeries as timeSeriesBase,
   timeSeriesFromCustomInterval,
-  parseSqlInterval,
   findMinGranularityDimension
 } from '@cubejs-backend/shared';
 
@@ -436,9 +435,11 @@ export class BaseQuery {
    */
   get allJoinHints() {
     if (!this.collectedJoinHints) {
-      const [rootOfJoin, ...allMembersJoinHints] = this.collectJoinHintsFromMembers(this.allMembersConcat(false));
+      const allMembersJoinHints = this.collectJoinHintsFromMembers(this.allMembersConcat(false));
+      const explicitJoinHintMembers = new Set(allMembersJoinHints.filter(j => Array.isArray(j)).flat());
+      const queryJoinMaps = this.queryJoinMap();
       const customSubQueryJoinHints = this.collectJoinHintsFromMembers(this.joinMembersFromCustomSubQuery());
-      let joinMembersJoinHints = this.collectJoinHintsFromMembers(this.joinMembersFromJoin(this.join));
+      const newCollectedHints = [];
 
       // One cube may join the other cube via transitive joined cubes,
       // members from which are referenced in the join `on` clauses.
@@ -447,27 +448,16 @@ export class BaseQuery {
       // 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 constructJH = () => R.uniq(this.enrichHintsWithJoinMap([
+        ...this.queryLevelJoinHints,
+        ...newCollectedHints,
+        ...allMembersJoinHints,
+        ...customSubQueryJoinHints,
+      ],
+      queryJoinMaps));
 
-      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]);
-      };
+      let prevJoin = null;
+      let newJoin = null;
 
       const isJoinTreesEqual = (a, b) => {
         if (!a || !b || a.root !== b.root || a.joins.length !== b.joins.length) {
@@ -494,27 +484,77 @@ export class BaseQuery {
 
       // Safeguard against infinite loop in case of cyclic joins somehow managed to slip through
       let cnt = 0;
+      let newJoinHintsCollectedCnt;
 
-      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;
+      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 && !isJoinTreesEqual(prevJoin, newJoin) && cnt < 10000 && newJoinHintsCollectedCnt > 0);
 
       if (cnt >= 10000) {
         throw new UserError('Can not construct joins for the query, potential loop detected');
       }
 
-      this.collectedJoinHints = R.uniq(constructJH());
+      this.collectedJoinHints = constructJH();
     }
     return this.collectedJoinHints;
   }
 
+  /**
+   * @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]) {
+        joinMaps[memberCube.name] = memberCube.joinMap;
+      }
+    }
+
+    return joinMaps;
+  }
+
+  /**
+   * @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;
+      }
+
+      for (const path of allPaths) {
+        const hintIndex = path.indexOf(hint);
+        if (hintIndex !== -1) {
+          return path.slice(0, hintIndex + 1);
+        }
+      }
+
+      return hint;
+    });
+  }
+
   get dataSource() {
     const dataSources = R.uniq(this.allCubeNames.map(c => this.cubeDataSource(c)));
     if (dataSources.length > 1 && !this.externalPreAggregationQuery()) {

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);

packages/cubejs-schema-compiler/test/integration/postgres/sql-generation.test.ts

@@ -5198,7 +5198,7 @@ cubes:
         sql: amount
         type: sum
 
-# Join loop for testing transitive joins
+# Model for testing multiple joins to the same cube via transitive joins
   - name: alpha_facts
     sql: |
       (
@@ -5215,7 +5215,7 @@ cubes:
         sql: "{CUBE}.b_id = {gamma_dims.b_id}"
       - name: delta_bridge
         relationship: many_to_one
-        sql: "{beta_dims.a_name} = {delta_bridge.a_name} AND {gamma_dims.b_name} = {delta_bridge.b_name}"
+        sql: "{beta_dims.a_name} = {delta_bridge.a_name} AND {gamma_dims.b_name} = {delta_bridge.c_name}"
     dimensions:
       - name: reporting_date
         sql: reporting_date
@@ -5256,9 +5256,9 @@ cubes:
   - name: gamma_dims
     sql: |
       (
-        SELECT 10 AS b_id, 'Beta1' AS b_name
+        SELECT 10 AS b_id, 'Beta1' AS b_name, 'Gamma1' AS c_name
         UNION ALL
-        SELECT 20 AS b_id, 'Beta2' AS b_name
+        SELECT 20 AS b_id, 'Beta2' AS b_name, 'Gamma2' AS c_name
       )
     dimensions:
       - name: b_id
@@ -5272,16 +5272,16 @@ cubes:
   - name: delta_bridge
     sql: |
       (
-        SELECT 'Alpha1' AS a_name, 'Beta1' AS b_name, 'Organic' AS channel
+        SELECT 'Alpha1' AS a_name, 'Beta1' AS b_name, 'Beta1' AS c_name, 'Organic' AS channel
         UNION ALL
-        SELECT 'Alpha1' AS a_name, 'Beta2' AS b_name, 'Paid' AS channel
+        SELECT 'Alpha2' AS a_name, 'Beta2' AS b_name, 'Beta2' AS c_name, 'Paid' AS channel
         UNION ALL
-        SELECT 'Alpha2' AS a_name, 'Beta1' AS b_name, 'Referral' AS channel
+        SELECT 'Alpha1' AS a_name, 'Beta1' AS b_name, 'Beta3' AS c_name, 'Referral' AS channel
       )
     joins:
       - name: gamma_dims
         relationship: many_to_one
-        sql: "{CUBE}.b_name = {gamma_dims.b_name}"
+        sql: "{CUBE}.c_name = {gamma_dims.c_name}"
     dimensions:
       - name: a_name
         sql: a_name
@@ -5290,7 +5290,9 @@ cubes:
       - name: b_name
         sql: "{gamma_dims.b_name}"
         type: string
-        primary_key: true
+      - name: c_name
+        sql: c_name
+        type: string
       - name: channel
         sql: channel
         type: string
@@ -5344,31 +5346,40 @@ cubes:
     }
 
     if (!getEnv('nativeSqlPlanner')) {
-      it('querying cube with transitive joins with loop', async () => {
+      it('querying cube with transitive joins with a few joins to the same cube', async () => {
         await compiler.compile();
 
-        try {
-          const query = new PostgresQuery({ joinGraph, cubeEvaluator, compiler }, {
-            measures: [],
-            dimensions: [
-              'alpha_facts.reporting_date',
-              'delta_bridge.b_name',
-              'alpha_facts.channel'
-            ],
-            order: [{
-              id: 'alpha_facts.reporting_date'
-            }],
-            timezone: 'America/Los_Angeles'
-          });
-
-          await dbRunner.testQuery(query.buildSqlAndParams());
-          throw new Error('Should have thrown an error');
-        } catch (err: any) {
-          expect(err.message).toContain('Can not construct joins for the query, potential loop detected');
-        }
+        const query = new PostgresQuery({ joinGraph, cubeEvaluator, compiler }, {
+          measures: [],
+          dimensions: [
+            'alpha_facts.reporting_date',
+            'delta_bridge.b_name',
+            'alpha_facts.channel'
+          ],
+          order: [{
+            id: 'alpha_facts.reporting_date'
+          }],
+          timezone: 'America/Los_Angeles'
+        });
+
+        const res = await dbRunner.testQuery(query.buildSqlAndParams());
+        console.log(JSON.stringify(res));
+
+        expect(res).toEqual([
+          {
+            alpha_facts__channel: 'Organic',
+            alpha_facts__reporting_date: '2023-01-01T00:00:00.000Z',
+            delta_bridge__b_name: 'Beta1',
+          },
+          {
+            alpha_facts__channel: 'Paid',
+            alpha_facts__reporting_date: '2023-01-02T00:00:00.000Z',
+            delta_bridge__b_name: 'Beta2',
+          },
+        ]);
       });
     } else {
-      it.skip('FIXME(tesseract): querying cube dimension that require transitive joins', async () => {
+      it.skip('FIXME(tesseract): querying cube with transitive joins with a few joins to the same cube', async () => {
         // FIXME should be implemented in Tesseract
       });
     }