PR feedback

src/4.3_to_5.0/_cleanupDescendants.test.ts

@@ -54,7 +54,7 @@ describe("_cleanupDescendants", () => {
     `);
   });
 
-  it("replaces a useless Descendants function when the second argument is a level index", () => {
+  it("replaces a useless Descendants function when the first argument is a set of members from the leaf level, and the second argument is a distance", () => {
     const mdx = parse<MdxSelect>(`                                 
     SELECT                                                                                                                                                              
       NON EMPTY Hierarchize(
@@ -78,6 +78,22 @@ describe("_cleanupDescendants", () => {
     `);
   });
 
+  it("does not replace a Descendants function when the first argument is a set of members from a non-leaf level, and the second argument is a distance", () => {
+    const mdx = parse<MdxSelect>(`                                 
+    SELECT                                                                                                                                                              
+      NON EMPTY Hierarchize(
+            Descendants(
+              {
+                [Booking].[Desk].[ALL].[AllMember].[LegalEntityA].[BusinessUnitA]
+              },
+              1
+            )
+          ) ON ROWS
+        FROM [EquityDerivativesCube]`);
+    const cleanMdx = _cleanupDescendants(mdx, cube);
+    expect(cleanMdx).toStrictEqual(mdx);
+  });
+
   it("replaces a useless Descendants function called on a set with members from the leaf level, without a second argument", () => {
     const mdx = parse<MdxSelect>(`                                 
     SELECT                                                                                                                                                              

src/4.3_to_5.0/_cleanupDrilldownLevel.test.ts

@@ -6,7 +6,7 @@ import { _cleanupDrilldownLevel } from "./_cleanupDrilldownLevel";
 const cube = dataModelsForTests.sandbox.catalogs[0].cubes[0];
 
 describe("_cleanupDrilldownLevel", () => {
-  it("replaces a DrilldownLevel function by its input set if the deepest level expressed in the set is a leaf level", () => {
+  it("replaces a DrilldownLevel function by its input set if it is the only argument, and if the deepest level expressed within it is a leaf level", () => {
     const mdx = parse(`                                 
     SELECT                                                                                                                                                              
       NON EMPTY Hierarchize(
@@ -28,4 +28,41 @@ describe("_cleanupDrilldownLevel", () => {
         FROM [EquityDerivativesCube]"
     `);
   });
+
+  it("does not replace a DrilldownLevel function if it has several arguments", () => {
+    // See the function doc: https://learn.microsoft.com/en-us/sql/mdx/drilldownlevel-mdx?view=sql-server-ver16
+    // Extra arguments can include a level expression or a hierarchy index to specify where to drilldown to.
+    // These more complex cases are not covered by _cleanupDrilldownLevel: better to keep suboptimal MDX than break it.
+    const mdx = parse(`                                 
+    SELECT                                                                                                                                                              
+      NON EMPTY Hierarchize(
+            DrilldownLevel(
+              {
+                (
+                  [Geography].[City].[ALL].[AllMember].[Berlin],
+                  [Currency].[Currency].[ALL].[AllMember].[EUR]
+                )
+              },
+              1
+            )
+          ) ON ROWS
+        FROM [EquityDerivativesCube]`);
+    const cleanMdx = _cleanupDrilldownLevel(mdx, cube);
+    expect(cleanMdx).toStrictEqual(mdx);
+  });
+
+  it("does not replace a DrilldownLevel function if the deepest level expressed in the input set is not a leaf level", () => {
+    const mdx = parse(`                                 
+    SELECT                                                                                                                                                              
+      NON EMPTY Hierarchize(
+            DrilldownLevel(
+              {
+                [Booking].[Desk].[ALL].[AllMember].[LegalEntityA].[BusinessUnitA]
+              }
+            )
+          ) ON ROWS
+        FROM [EquityDerivativesCube]`);
+    const cleanMdx = _cleanupDrilldownLevel(mdx, cube);
+    expect(cleanMdx).toStrictEqual(mdx);
+  });
 });

src/4.3_to_5.0/_cleanupDrilldownLevel.ts

@@ -15,7 +15,9 @@ import { produce } from "immer";
 import _set from "lodash/set";
 
 /**
- * If the input set holds members of the leaf level, then DrilldownLevel can be replaced by it.
+ * A DrilldownLevel can be replaced by its first argument if both the following conditions are met:
+ * - It has a single argument
+ * - In this set, the deepest level expressed in the first hierarchy (which is the one being implicitly drilled) is a leaf level
  */
 const canDrilldownLevelBeReplacedByItsFirstArgument = (
   drilldownLevelNode: MdxFunction,
@@ -32,8 +34,8 @@ const canDrilldownLevelBeReplacedByItsFirstArgument = (
 
   const levelsInSet = getLevels(set, {
     cube,
-  }).filter((levelCorodinates) =>
-    areHierarchiesEqual(levelCorodinates, hierarchyCoordinates),
+  }).filter((levelCoordinates) =>
+    areHierarchiesEqual(levelCoordinates, hierarchyCoordinates),
   );
   const deepestLevelIndexInSet = Math.max(
     ...levelsInSet.map((levelCoordinates) =>