MONDRIAN: Fix bug in DESCENDANTS(..., -1, LEAVES).

[git-p4: depot-paths = "//open/mondrian/": change = 5977]
pentaho · Mar 25, 2006 · c8934c4 · c8934c4
1 parent 69aad52
commit c8934c4
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 7 deletions.
diff --git a/src/main/mondrian/olap/fun/DescendantsFunDef.java b/src/main/mondrian/olap/fun/DescendantsFunDef.java
@@ -71,10 +71,10 @@ public List evaluateList(Evaluator evaluator) {
                     final Member member = memberCalc.evaluateMember(evaluator);
                     List result = new ArrayList();
                     int depth = depthCalc.evaluateInteger(evaluator);
-                    final SchemaReader schemaReader = evaluator.getSchemaReader();
                     if (depth < 0) {
-                        depth = Integer.MAX_VALUE;
+                        depth = -1; // no limit
                     }
+                    final SchemaReader schemaReader = evaluator.getSchemaReader();
                     descendantsLeavesByDepth(
                             member, result, schemaReader, depth);
                     hierarchize(result, false);
@@ -159,9 +159,14 @@ private static void descendantsByDepth(
         }
     }
 
+    /**
+     * Populates 'result' with the descendants at the leaf level at depth
+     * 'depthLimit' or less. If 'depthLimit' is -1, does not apply a depth
+     * constraint.
+     */
     private static void descendantsLeavesByDepth(
-            Member member,
-            List result,
+            final Member member,
+            final List result,
             final SchemaReader schemaReader,
             final int depthLimit) {
         if (!schemaReader.isDrillable(member)) {
@@ -171,22 +176,26 @@ private static void descendantsLeavesByDepth(
             return;
         }
         Member[] children = {member};
-        for (int depth = 0; depth <= depthLimit; ++depth) {
+        for (int depth = 0; depthLimit == -1 || depth <= depthLimit; ++depth) {
             children = schemaReader.getMemberChildren(children);
             if (children.length == 0) {
                 throw Util.newInternal("drillable member must have children");
             }
             List nextChildren = new ArrayList();
             for (int i = 0; i < children.length; i++) {
                 Member child = children[i];
+                // TODO: Implement this more efficiently. The current
+                // implementation of isDrillable for a parent-child hierarchy
+                // simply retrieves the children sees whether there are any,
+                // so we end up fetching each member's children twice.
                 if (schemaReader.isDrillable(child)) {
                     nextChildren.add(child);
                 } else {
                     result.add(child);
                 }
             }
             if (nextChildren.isEmpty()) {
-                break;
+                return;
             }
             children = (Member[])
                     nextChildren.toArray(new Member[nextChildren.size()]);

diff --git a/testsrc/main/mondrian/olap/fun/FunctionTest.java b/testsrc/main/mondrian/olap/fun/FunctionTest.java
@@ -2682,8 +2682,12 @@ public void testDescendantsParentChildLeaves() {
         testContext.assertExprReturns("Count(Descendants([Employees], 3, LEAVES))", "16");
         testContext.assertExprReturns("Count(Descendants([Employees], 4, LEAVES))", "63");
         testContext.assertExprReturns("Count(Descendants([Employees], 999, LEAVES))", "1,044");
+
         // negative depth acts like +infinity (per MSAS)
-        testContext.assertExprReturns("Count(Descendants([Employees], -1, LEAVES))", "1,044");
+        // Run the test several times because we had a non-deterministic bug here.
+        for (int i = 0; i < 100; ++i) {
+            testContext.assertExprReturns("Count(Descendants([Employees], -1, LEAVES))", "1,044");
+        }
     }
 
     public void testDescendantsSBA() throws Exception {