fix(schema-compiler): Fix inherited drill members for views (#9966)

* Add drillMembers and drillMembersGrouped to inhereted properties by views

* feat: Implement drill member inheritance for view cubes and enhance error reporting

# Conflicts:
#	packages/cubejs-schema-compiler/src/compiler/ErrorReporter.ts

* PR comments

* Move filtering logic to generateIncludeMembers

* more types in CubeSymbols

* Simplified drillMembers filtering for views

* fix/correct test comments

* update snapshot

* fix drill members inheritance (cases of non-owned members)

* fix tests

---------

Co-authored-by: Paco Valdez <paco@cube.dev>

Author: 2 people

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

@@ -187,6 +187,20 @@ export interface CubeSymbolsBase {
 
 export type CubeSymbolsDefinition = CubeSymbolsBase & Record<string, CubeSymbolDefinition>;
 
+type MemberSets = {
+  resolvedMembers: Set<string>;
+  allMembers: Set<string>;
+};
+
+type ViewResolvedMember = {
+  member: string;
+  name: string;
+};
+
+type ViewExcludedMember = {
+  member: string;
+};
+
 const FunctionRegex = /function\s+\w+\(([A-Za-z0-9_,]*)|\(([\s\S]*?)\)\s*=>|\(?(\w+)\)?\s*=>/;
 export const CONTEXT_SYMBOLS = {
   SECURITY_CONTEXT: 'securityContext',
@@ -560,19 +574,23 @@ export class CubeSymbols implements TranspilerSymbolResolver {
       return;
     }
 
-    const memberSets = {
+    const memberSets: MemberSets = {
       resolvedMembers: new Set<string>(),
       allMembers: new Set<string>(),
     };
 
     const autoIncludeMembers = new Set<string>();
     // `hierarchies` must be processed first
-    const types = ['hierarchies', 'measures', 'dimensions', 'segments'];
+    // It's also important `dimensions` to be processed BEFORE `measures`
+    // because drillMembers processing for views in generateIncludeMembers() relies on this
+    const types = ['hierarchies', 'dimensions', 'measures', 'segments'];
 
     const joinMap: string[][] = [];
 
+    const viewAllMembers: ViewResolvedMember[] = [];
+
     for (const type of types) {
-      let cubeIncludes: any[] = [];
+      let cubeIncludes: ViewResolvedMember[] = [];
 
       // If the hierarchy is included all members from it should be included as well
       // Extend `includes` with members from hierarchies that should be auto-included
@@ -603,6 +621,7 @@ export class CubeSymbols implements TranspilerSymbolResolver {
       }) : includedCubes;
 
       cubeIncludes = this.membersFromCubes(cube, cubes, type, errorReporter, splitViews, memberSets) || [];
+      viewAllMembers.push(...cubeIncludes);
 
       if (type === 'hierarchies') {
         for (const member of cubeIncludes) {
@@ -620,7 +639,7 @@ export class CubeSymbols implements TranspilerSymbolResolver {
         }
       }
 
-      const includeMembers = this.generateIncludeMembers(cubeIncludes, type);
+      const includeMembers = this.generateIncludeMembers(cubeIncludes, type, cube, viewAllMembers);
       this.applyIncludeMembers(includeMembers, cube, type, errorReporter);
 
       const existing = cube.includedMembers ?? [];
@@ -669,9 +688,9 @@ export class CubeSymbols implements TranspilerSymbolResolver {
     type: string,
     errorReporter: ErrorReporter,
     splitViews: SplitViews,
-    memberSets: any
-  ) {
-    const result: any[] = [];
+    memberSets: MemberSets
+  ): ViewResolvedMember[] {
+    const result: ViewResolvedMember[] = [];
     const seen = new Set<string>();
 
     for (const cubeInclude of cubes) {
@@ -769,7 +788,8 @@ export class CubeSymbols implements TranspilerSymbolResolver {
           splitViewDef = splitViews[viewName];
         }
 
-        const includeMembers = this.generateIncludeMembers(finalIncludes, type);
+        const viewAllMembers: ViewResolvedMember[] = [];
+        const includeMembers = this.generateIncludeMembers(finalIncludes, type, splitViewDef, viewAllMembers);
         this.applyIncludeMembers(includeMembers, splitViewDef, type, errorReporter);
       } else {
         for (const member of finalIncludes) {
@@ -785,7 +805,7 @@ export class CubeSymbols implements TranspilerSymbolResolver {
     return result;
   }
 
-  protected diffByMember(includes: any[], excludes: any[]) {
+  protected diffByMember(includes: ViewResolvedMember[], excludes: ViewExcludedMember[]) {
     const excludesMap = new Map();
 
     for (const exclude of excludes) {
@@ -799,14 +819,42 @@ export class CubeSymbols implements TranspilerSymbolResolver {
     return this.symbols[cubeName]?.cubeObj()?.[type]?.[memberName];
   }
 
-  protected generateIncludeMembers(members: any[], type: string) {
+  protected generateIncludeMembers(members: any[], type: string, targetCube: CubeDefinitionExtended, viewAllMembers: ViewResolvedMember[]) {
     return members.map(memberRef => {
       const path = memberRef.member.split('.');
       const resolvedMember = this.getResolvedMember(type, path[path.length - 2], path[path.length - 1]);
       if (!resolvedMember) {
         throw new Error(`Can't resolve '${memberRef.member}' while generating include members`);
       }
 
+      let processedDrillMembers = resolvedMember.drillMembers;
+
+      // We need to filter only included drillMembers for views
+      if (type === 'measures' && resolvedMember.drillMembers && targetCube.isView) {
+        const sourceCubeName = path[path.length - 2];
+
+        const evaluatedDrillMembers = this.evaluateReferences(
+          sourceCubeName,
+          resolvedMember.drillMembers,
+          { originalSorting: true }
+        );
+
+        const drillMembersArray = (Array.isArray(evaluatedDrillMembers)
+          ? evaluatedDrillMembers
+          : [evaluatedDrillMembers]);
+
+        const filteredDrillMembers = drillMembersArray.flatMap(member => {
+          const found = viewAllMembers.find(v => v.member.endsWith(member));
+          if (!found) {
+            return [];
+          }
+
+          return [`${targetCube.name}.${found.name}`];
+        });
+
+        processedDrillMembers = () => filteredDrillMembers;
+      }
+
       // eslint-disable-next-line no-new-func
       const sql = new Function(path[0], `return \`\${${memberRef.member}}\`;`);
       let memberDefinition;
@@ -822,6 +870,8 @@ export class CubeSymbols implements TranspilerSymbolResolver {
           ...(resolvedMember.multiStage && { multiStage: resolvedMember.multiStage }),
           ...(resolvedMember.timeShift && { timeShift: resolvedMember.timeShift }),
           ...(resolvedMember.orderBy && { orderBy: resolvedMember.orderBy }),
+          ...(processedDrillMembers && { drillMembers: processedDrillMembers }),
+          ...(resolvedMember.drillMembersGrouped && { drillMembersGrouped: resolvedMember.drillMembersGrouped }),
         };
       } else if (type === 'dimensions') {
         memberDefinition = {
@@ -904,8 +954,7 @@ export class CubeSymbols implements TranspilerSymbolResolver {
           name
         );
       // eslint-disable-next-line no-underscore-dangle
-      // if (resolvedSymbol && resolvedSymbol._objectWithResolvedProperties) {
-      if (resolvedSymbol._objectWithResolvedProperties) {
+      if (resolvedSymbol?._objectWithResolvedProperties) {
         return resolvedSymbol;
       }
       return cubeEvaluator.pathFromArray(fullPath(cubeEvaluator.joinHints(), [referencedCube, name]));
@@ -1015,7 +1064,7 @@ export class CubeSymbols implements TranspilerSymbolResolver {
           cubeName,
           name
         );
-        if (resolvedSymbol._objectWithResolvedProperties) {
+        if (resolvedSymbol?._objectWithResolvedProperties) {
           return resolvedSymbol;
         }
         return '';

packages/cubejs-schema-compiler/test/integration/postgres/cube-views.test.ts

@@ -47,7 +47,7 @@ cube(\`Orders\`, {
   measures: {
     count: {
       type: \`count\`,
-      //drillMembers: [id, createdAt]
+      drillMembers: [id, createdAt, Products.ProductCategories.name]
     },
 
     runningTotal: {
@@ -183,6 +183,10 @@ cube(\`ProductCategories\`, {
   measures: {
     count: {
       type: \`count\`,
+    },
+    count2: {
+      type: \`count\`,
+      drillMembers: [id, name]
     }
   },
 
@@ -255,7 +259,35 @@ view(\`OrdersView3\`, {
     split: true
   }]
 });
-    `);
+
+view(\`OrdersSimpleView\`, {
+  cubes: [{
+    join_path: Orders,
+    includes: ['createdAt', 'count']
+  }]
+});
+
+view(\`OrdersViewDrillMembers\`, {
+  cubes: [{
+    join_path: Orders,
+    includes: ['createdAt', 'count']
+  }, {
+    join_path: Orders.Products.ProductCategories,
+    includes: ['name', 'count2']
+  }]
+});
+
+view(\`OrdersViewDrillMembersWithPrefix\`, {
+  cubes: [{
+    join_path: Orders,
+    includes: ['createdAt', 'count']
+  }, {
+    join_path: Orders.Products.ProductCategories,
+    includes: ['name', 'count2'],
+    prefix: true
+  }]
+});
+  `);
 
   async function runQueryTest(q: any, expectedResult: any, additionalTest?: (query: BaseQuery) => any) {
     await compiler.compile();
@@ -429,4 +461,104 @@ view(\`OrdersView3\`, {
     orders_view3__count: '2',
     orders_view3__product_categories__name: 'Groceries',
   }]));
+
+  it('check drillMembers are inherited in views', async () => {
+    await compiler.compile();
+    const cube = metaTransformer.cubes.find(c => c.config.name === 'OrdersView');
+    const countMeasure = cube.config.measures.find((m) => m.name === 'OrdersView.count');
+    expect(countMeasure.drillMembers).toEqual(['OrdersView.id', 'OrdersView.ProductCategories_name']);
+    expect(countMeasure.drillMembersGrouped).toEqual({
+      measures: [],
+      dimensions: ['OrdersView.id', 'OrdersView.ProductCategories_name']
+    });
+  });
+
+  it('verify drill member inheritance functionality', async () => {
+    await compiler.compile();
+
+    // Check that the source Orders cube has drill members
+    const sourceOrdersCube = metaTransformer.cubes.find(c => c.config.name === 'Orders');
+    const sourceCountMeasure = sourceOrdersCube.config.measures.find((m) => m.name === 'Orders.count');
+    expect(sourceCountMeasure.drillMembers).toEqual(['Orders.id', 'Orders.createdAt', 'ProductCategories.name']);
+
+    // Check that the OrdersView cube inherits these drill members with correct naming
+    const viewCube = metaTransformer.cubes.find(c => c.config.name === 'OrdersView');
+    const viewCountMeasure = viewCube.config.measures.find((m) => m.name === 'OrdersView.count');
+
+    expect(viewCountMeasure.drillMembers).toBeDefined();
+    expect(Array.isArray(viewCountMeasure.drillMembers)).toBe(true);
+    expect(viewCountMeasure.drillMembers.length).toBeGreaterThan(0);
+    expect(viewCountMeasure.drillMembers).toContain('OrdersView.id');
+    expect(viewCountMeasure.drillMembersGrouped).toBeDefined();
+  });
+
+  it('check drill member inheritance with limited includes in OrdersSimpleView', async () => {
+    await compiler.compile();
+    const cube = metaTransformer.cubes.find(c => c.config.name === 'OrdersSimpleView');
+
+    if (!cube) {
+      throw new Error('OrdersSimpleView not found in compiled cubes');
+    }
+
+    const countMeasure = cube.config.measures.find((m) => m.name === 'OrdersSimpleView.count');
+
+    if (!countMeasure) {
+      throw new Error('OrdersSimpleView.count measure not found');
+    }
+
+    // Check what dimensions are actually available in this limited view
+    const availableDimensions = cube.config.dimensions?.map(d => d.name) || [];
+
+    // This view only includes 'createdAt' dimension and should not include id
+    expect(availableDimensions).not.toContain('OrdersSimpleView.id');
+    expect(availableDimensions).toContain('OrdersSimpleView.createdAt');
+
+    // The source measure has drillMembers: ['Orders.id', 'Orders.createdAt']
+    // Both should be available in this view since we explicitly included them
+    expect(countMeasure.drillMembers).toBeDefined();
+    // Verify drill members are inherited and correctly transformed to use View naming
+    expect(countMeasure.drillMembers).toEqual(['OrdersSimpleView.createdAt']);
+    expect(countMeasure.drillMembersGrouped).toEqual({
+      measures: [],
+      dimensions: ['OrdersSimpleView.createdAt']
+    });
+  });
+
+  it('verify drill member inheritance functionality (with transitive joins)', async () => {
+    await compiler.compile();
+
+    // Check that the OrdersView cube inherits these drill members with correct naming
+    const viewCube = metaTransformer.cubes.find(c => c.config.name === 'OrdersViewDrillMembers');
+
+    const viewCountMeasure = viewCube.config.measures.find((m) => m.name === 'OrdersViewDrillMembers.count');
+    expect(viewCountMeasure.drillMembers).toBeDefined();
+    expect(Array.isArray(viewCountMeasure.drillMembers)).toBe(true);
+    expect(viewCountMeasure.drillMembers.length).toEqual(2);
+    expect(viewCountMeasure.drillMembers).toEqual(['OrdersViewDrillMembers.createdAt', 'OrdersViewDrillMembers.name']);
+
+    const viewCount2Measure = viewCube.config.measures.find((m) => m.name === 'OrdersViewDrillMembers.count2');
+    expect(viewCount2Measure.drillMembers).toBeDefined();
+    expect(Array.isArray(viewCount2Measure.drillMembers)).toBe(true);
+    expect(viewCount2Measure.drillMembers.length).toEqual(1);
+    expect(viewCount2Measure.drillMembers).toContain('OrdersViewDrillMembers.name');
+  });
+
+  it('verify drill member inheritance functionality (with transitive joins + prefix)', async () => {
+    await compiler.compile();
+
+    // Check that the OrdersView cube inherits these drill members with correct naming
+    const viewCube = metaTransformer.cubes.find(c => c.config.name === 'OrdersViewDrillMembersWithPrefix');
+
+    const viewCountMeasure = viewCube.config.measures.find((m) => m.name === 'OrdersViewDrillMembersWithPrefix.count');
+    expect(viewCountMeasure.drillMembers).toBeDefined();
+    expect(Array.isArray(viewCountMeasure.drillMembers)).toBe(true);
+    expect(viewCountMeasure.drillMembers.length).toEqual(2);
+    expect(viewCountMeasure.drillMembers).toEqual(['OrdersViewDrillMembersWithPrefix.createdAt', 'OrdersViewDrillMembersWithPrefix.ProductCategories_name']);
+
+    const viewCount2Measure = viewCube.config.measures.find((m) => m.name === 'OrdersViewDrillMembersWithPrefix.ProductCategories_count2');
+    expect(viewCount2Measure.drillMembers).toBeDefined();
+    expect(Array.isArray(viewCount2Measure.drillMembers)).toBe(true);
+    expect(viewCount2Measure.drillMembers.length).toEqual(1);
+    expect(viewCount2Measure.drillMembers).toContain('OrdersViewDrillMembersWithPrefix.ProductCategories_name');
+  });
 });

packages/cubejs-schema-compiler/test/unit/__snapshots__/schema.test.ts.snap

@@ -1848,11 +1848,6 @@ Object {
       "name": "hello",
       "type": "hierarchies",
     },
-    Object {
-      "memberPath": "orders.count",
-      "name": "count",
-      "type": "measures",
-    },
     Object {
       "memberPath": "orders.status",
       "name": "my_beloved_status",
@@ -1863,6 +1858,11 @@ Object {
       "name": "my_beloved_created_at",
       "type": "dimensions",
     },
+    Object {
+      "memberPath": "orders.count",
+      "name": "count",
+      "type": "measures",
+    },
   ],
   "isView": true,
   "joinMap": Array [],