chore(schema-compiler): Make cube joins to be an array instead of hashmap (#9800)

* more types in CubeSymbols

* change joins schema in Validator

* specify type for accessPolicy

* make prepareJoins() aware of arrays

* make joins in CubeSymbols / CubeEvaluator as array instead of hashmap

* update JoinGraph to treat joins as array instead of hashmap

* fix CubePropContextTranspiler to convert joins hashmap into array

* add @types/js-yaml

* make YamlCompiler aware of joins as array

* fix/update snapshots for schema tests

* fix transform yaml joins

* add schema tests for joins as array/object

* lint:fix

* update snapshots

* add a fallback workaround for joins as object in js models

Author: KSDaemon

packages/cubejs-schema-compiler/package.json

@@ -67,6 +67,7 @@
     "@types/babel__traverse": "^7.20.5",
     "@types/inflection": "^1.5.28",
     "@types/jest": "^29",
+    "@types/js-yaml": "^4.0.9",
     "@types/node": "^20",
     "@types/node-dijkstra": "^2.5.6",
     "@types/ramda": "^0.27.34",

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

@@ -2,10 +2,13 @@
 import R from 'ramda';
 
 import {
+  AccessPolicyDefinition,
   CubeDefinitionExtended,
   CubeSymbols,
-  HierarchyDefinition, JoinDefinition,
-  PreAggregationDefinition, PreAggregationDefinitionRollup,
+  HierarchyDefinition,
+  JoinDefinition,
+  PreAggregationDefinition,
+  PreAggregationDefinitionRollup,
   type ToString
 } from './CubeSymbols';
 import { UserError } from './UserError';
@@ -110,30 +113,6 @@ export type EvaluatedHierarchy = {
   [key: string]: any;
 };
 
-export type Filter =
-  | {
-      member: string;
-      memberReference?: string;
-      [key: string]: any;
-    }
-  | {
-      and?: Filter[];
-      or?: Filter[];
-      [key: string]: any;
-    };
-
-export type AccessPolicy = {
-  rowLevel?: {
-    filters: Filter[];
-  };
-  memberLevel?: {
-    includes?: string | string[];
-    excludes?: string | string[];
-    includesMembers?: string[];
-    excludesMembers?: string[];
-  };
-};
-
 export type EvaluatedFolder = {
   name: string;
   includes: (EvaluatedFolder | DimensionDefinition | MeasureDefinition)[];
@@ -145,15 +124,15 @@ export type EvaluatedCube = {
   measures: Record<string, MeasureDefinition>;
   dimensions: Record<string, DimensionDefinition>;
   segments: Record<string, SegmentDefinition>;
-  joins: Record<string, JoinDefinition>;
+  joins: JoinDefinition[];
   hierarchies: Record<string, HierarchyDefinition>;
   evaluatedHierarchies: EvaluatedHierarchy[];
   preAggregations: Record<string, PreAggregationDefinitionExtended>;
   dataSource?: string;
   folders: EvaluatedFolder[];
   sql?: (...args: any[]) => string;
   sqlTable?: (...args: any[]) => string;
-  accessPolicy?: AccessPolicy[];
+  accessPolicy?: AccessPolicyDefinition[];
 };
 
 export class CubeEvaluator extends CubeSymbols {
@@ -200,7 +179,7 @@ export class CubeEvaluator extends CubeSymbols {
     );
   }
 
-  protected prepareCube(cube, errorReporter: ErrorReporter) {
+  protected prepareCube(cube, errorReporter: ErrorReporter): EvaluatedCube {
     this.prepareJoins(cube, errorReporter);
     this.preparePreAggregations(cube, errorReporter);
     this.prepareMembers(cube.measures, cube, errorReporter);
@@ -443,36 +422,55 @@ export class CubeEvaluator extends CubeSymbols {
     }
   }
 
-  protected prepareJoins(cube: any, _errorReporter: ErrorReporter) {
-    if (cube.joins) {
-      // eslint-disable-next-line no-restricted-syntax
-      for (const join of Object.values(cube.joins) as any[]) {
-        // eslint-disable-next-line default-case
-        switch (join.relationship) {
-          case 'belongs_to':
-          case 'many_to_one':
-          case 'manyToOne':
-            join.relationship = 'belongsTo';
-            break;
-          case 'has_many':
-          case 'one_to_many':
-          case 'oneToMany':
-            join.relationship = 'hasMany';
-            break;
-          case 'has_one':
-          case 'one_to_one':
-          case 'oneToOne':
-            join.relationship = 'hasOne';
-            break;
-        }
+  protected prepareJoins(cube: any, errorReporter: ErrorReporter) {
+    if (!cube.joins) {
+      return;
+    }
+
+    const transformRelationship = (relationship: string): string => {
+      switch (relationship) {
+        case 'belongs_to':
+        case 'many_to_one':
+        case 'manyToOne':
+          return 'belongsTo';
+        case 'has_many':
+        case 'one_to_many':
+        case 'oneToMany':
+          return 'hasMany';
+        case 'has_one':
+        case 'one_to_one':
+        case 'oneToOne':
+          return 'hasOne';
+        default:
+          return relationship;
       }
+    };
+
+    let joins: JoinDefinition[] = [];
+
+    if (Array.isArray(cube.joins)) {
+      joins = cube.joins.map((join: JoinDefinition) => {
+        join.relationship = transformRelationship(join.relationship);
+        return join;
+      });
+    } else if (typeof cube.joins === 'object') {
+      joins = Object.entries(cube.joins).map(([name, join]: [string, any]) => {
+        join.relationship = transformRelationship(join.relationship);
+        join.name = name;
+        return join as JoinDefinition;
+      });
+    } else {
+      errorReporter.error(`Invalid joins definition for cube '${cube.name}': expected an array or an object.`);
     }
+
+    cube.joins = joins;
   }
 
   protected preparePreAggregations(cube: any, errorReporter: ErrorReporter) {
     if (cube.preAggregations) {
       // eslint-disable-next-line no-restricted-syntax
       for (const preAggregation of Object.values(cube.preAggregations) as any) {
+        // preAggregation is actually (PreAggregationDefinitionRollup | PreAggregationDefinitionOriginalSql)
         if (preAggregation.timeDimension) {
           preAggregation.timeDimensionReference = preAggregation.timeDimension;
           delete preAggregation.timeDimension;
@@ -574,7 +572,7 @@ export class CubeEvaluator extends CubeSymbols {
     }
   }
 
-  public cubesByFileName(fileName) {
+  public cubesByFileName(fileName): CubeDefinitionExtended[] {
     return this.byFileName[fileName] || [];
   }
 
@@ -691,7 +689,7 @@ export class CubeEvaluator extends CubeSymbols {
     return this.preAggregations({ scheduled: true });
   }
 
-  public cubeNames() {
+  public cubeNames(): string[] {
     return Object.keys(this.evaluatedCubes);
   }
 

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

@@ -100,10 +100,35 @@ export type PreAggregationDefinitionRollup = BasePreAggregationDefinition & {
 export type PreAggregationDefinition = PreAggregationDefinitionRollup;
 
 export type JoinDefinition = {
+  name: string,
   relationship: string,
   sql: (...args: any[]) => string,
 };
 
+export type Filter =
+  | {
+      member: string;
+      memberReference?: string;
+      [key: string]: any;
+    }
+  | {
+      and?: Filter[];
+      or?: Filter[];
+      [key: string]: any;
+    };
+
+export type AccessPolicyDefinition = {
+  rowLevel?: {
+    filters: Filter[];
+  };
+  memberLevel?: {
+    includes?: string | string[];
+    excludes?: string | string[];
+    includesMembers?: string[];
+    excludesMembers?: string[];
+  };
+};
+
 export interface CubeDefinition {
   name: string;
   extends?: (...args: Array<unknown>) => { __cubeName: string };
@@ -119,8 +144,8 @@ export interface CubeDefinition {
   preAggregations?: Record<string, PreAggregationDefinitionRollup | PreAggregationDefinitionOriginalSql>;
   // eslint-disable-next-line camelcase
   pre_aggregations?: Record<string, PreAggregationDefinitionRollup | PreAggregationDefinitionOriginalSql>;
-  joins?: Record<string, JoinDefinition>;
-  accessPolicy?: any[];
+  joins?: JoinDefinition[];
+  accessPolicy?: AccessPolicyDefinition[];
   // eslint-disable-next-line camelcase
   access_policy?: any[];
   folders?: any[];
@@ -221,17 +246,17 @@ export class CubeSymbols {
   }
 
   public createCube(cubeDefinition: CubeDefinition): CubeDefinitionExtended {
-    let preAggregations: any;
-    let joins: any;
-    let measures: any;
-    let dimensions: any;
-    let segments: any;
-    let hierarchies: any;
-    let accessPolicy: any;
-    let folders: any;
-    let cubes: any;
-
-    const cubeObject = Object.assign({
+    let preAggregations: CubeDefinition['preAggregations'];
+    let joins: CubeDefinition['joins'];
+    let measures: CubeDefinition['measures'];
+    let dimensions: CubeDefinition['dimensions'];
+    let segments: CubeDefinition['segments'];
+    let hierarchies: CubeDefinition['hierarchies'];
+    let accessPolicy: CubeDefinition['accessPolicy'];
+    let folders: CubeDefinition['folders'];
+    let cubes: CubeDefinition['cubes'];
+
+    const cubeObject: CubeDefinitionExtended = Object.assign({
       allDefinitions(type: string) {
         if (cubeDefinition.extends) {
           return {
@@ -297,7 +322,8 @@ export class CubeSymbols {
 
       get joins() {
         if (!joins) {
-          joins = this.allDefinitions('joins');
+          const parentJoins = cubeDefinition.extends ? super.joins : [];
+          joins = [...parentJoins, ...(cubeDefinition.joins || [])];
         }
         return joins;
       },
@@ -383,9 +409,10 @@ export class CubeSymbols {
     return cubeObject;
   }
 
-  protected transform(cubeName: string, errorReporter: ErrorReporter, splitViews: SplitViews) {
+  protected transform(cubeName: string, errorReporter: ErrorReporter, splitViews: SplitViews): CubeSymbolsDefinition {
     const cube = this.getCubeDefinition(cubeName);
-    const duplicateNames = R.compose(
+    // @ts-ignore
+    const duplicateNames: string[] = R.compose(
       R.map((nameToDefinitions: any) => nameToDefinitions[0]),
       R.toPairs,
       R.filter((definitionsByName: any) => definitionsByName.length > 1),
@@ -397,9 +424,7 @@ export class CubeSymbols {
       // @ts-ignore
     )([cube.measures, cube.dimensions, cube.segments, cube.preAggregations, cube.hierarchies]);
 
-    // @ts-ignore
     if (duplicateNames.length > 0) {
-      // @ts-ignore
       errorReporter.error(`${duplicateNames.join(', ')} defined more than once`);
     }
 
@@ -427,23 +452,24 @@ export class CubeSymbols {
       ...cube.dimensions || {},
       ...cube.segments || {},
       ...cube.preAggregations || {}
-    };
+    } as CubeSymbolsDefinition;
   }
 
-  private camelCaseTypes(obj: Object | undefined) {
+  private camelCaseTypes(obj: Object | Array<any> | undefined) {
     if (!obj) {
       return;
     }
 
-    // eslint-disable-next-line no-restricted-syntax
-    for (const member of Object.values(obj)) {
+    const members = Array.isArray(obj) ? obj : Object.values(obj);
+
+    members.forEach(member => {
       if (member.type && member.type.indexOf('_') !== -1) {
         member.type = camelize(member.type, true);
       }
       if (member.relationship && member.relationship.indexOf('_') !== -1) {
         member.relationship = camelize(member.relationship, true);
       }
-    }
+    });
   }
 
   protected transformPreAggregations(preAggregations: Object) {
@@ -548,7 +574,7 @@ export class CubeSymbols {
         }
       }
 
-      const includeMembers = this.generateIncludeMembers(cubeIncludes, cube.name, type);
+      const includeMembers = this.generateIncludeMembers(cubeIncludes, type);
       this.applyIncludeMembers(includeMembers, cube, type, errorReporter);
 
       const existing = cube.includedMembers ?? [];
@@ -699,11 +725,7 @@ export class CubeSymbols {
           splitViewDef = splitViews[viewName];
         }
 
-        const includeMembers = this.generateIncludeMembers(
-          finalIncludes,
-          parentCube.name,
-          type
-        );
+        const includeMembers = this.generateIncludeMembers(finalIncludes, type);
         this.applyIncludeMembers(includeMembers, splitViewDef, type, errorReporter);
       } else {
         for (const member of finalIncludes) {
@@ -733,7 +755,7 @@ export class CubeSymbols {
     return this.symbols[cubeName]?.cubeObj()?.[type]?.[memberName];
   }
 
-  protected generateIncludeMembers(members: any[], cubeName: string, type: string) {
+  protected generateIncludeMembers(members: any[], type: string) {
     return members.map(memberRef => {
       const path = memberRef.member.split('.');
       const resolvedMember = this.getResolvedMember(type, path[path.length - 2], path[path.length - 1]);
@@ -870,9 +892,8 @@ export class CubeSymbols {
 
   /**
    * 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 static joinHintFromPath(path: string): { path: string, joinHint: Array<string> } {
+  public static joinHintFromPath(path: string): { path: string, joinHint: string[] } {
     const parts = path.split('.');
     if (parts.length > 2) {
       // Path contains join path
@@ -908,7 +929,7 @@ export class CubeSymbols {
     }
   }
 
-  protected withSymbolsCallContext(func, context) {
+  protected withSymbolsCallContext(func: Function, context) {
     const oldContext = this.resolveSymbolsCallContext;
     this.resolveSymbolsCallContext = context;
     try {
@@ -933,7 +954,7 @@ export class CubeSymbols {
     return this.funcArgumentsValues[funcDefinition];
   }
 
-  protected joinHints() {
+  protected joinHints(): string | string[] | undefined {
     const { joinHints } = this.resolveSymbolsCallContext || {};
     if (Array.isArray(joinHints)) {
       return R.uniq(joinHints);
@@ -1014,7 +1035,7 @@ export class CubeSymbols {
     return (...filterParamArgs) => '';
   }
 
-  public resolveSymbol(cubeName, name) {
+  public resolveSymbol(cubeName, name: string) {
     const { sqlResolveFn, contextSymbols, collectJoinHints, depsResolveFn, currResolveIndexFn } = this.resolveSymbolsCallContext || {};
     if (name === 'USER_CONTEXT') {
       throw new Error('Support for USER_CONTEXT was removed, please migrate to SECURITY_CONTEXT.');