MONDRIAN: Fix bug MONDRIAN-902, "mondrian populating the same members…

… on both axes".

    Fix a test exception NativeSetFunDefTest on JDK 1.4, and a checkFile exception.

[git-p4: depot-paths = "//open/mondrian/": change = 14151]

src/main/mondrian/olap/fun/FunUtil.java

@@ -621,9 +621,6 @@ public static TupleList sortTuples(
             comparator =
                 new HierarchicalTupleComparator(evaluator, exp, arity, desc)
                 .wrap();
-
-// new HierarchizeTupleComparator(arity, false).wrap();
-
         }
 
         Arrays.sort(tuples, comparator);

src/main/mondrian/olap/fun/NativizeSetFunDef.java

@@ -94,35 +94,36 @@ public Calc compileCall(ResolvedFunCall call, ExpCompiler compiler) {
             return funArg.accept(compiler);
         }
 
-        final Calc[] calcs = new Calc[] {compiler.compileList(funArg, true)};
+        final Calc[] calcs = {compiler.compileList(funArg, true)};
 
         final int arity = calcs[0].getType().getArity();
-        if (arity < 1) {
-            throw new IllegalArgumentException(
-                "unexpected value for .getArity() - " + arity);
-        } else if (arity == 1 || substitutionMap.isEmpty()) {
+        assert arity >= 0;
+        if (arity == 1 || substitutionMap.isEmpty()) {
             IterCalc calc = (IterCalc) funArg.accept(compiler);
             final boolean highCardinality =
                 arity == 1
                 && isHighCardinality(funArg, compiler.getEvaluator());
-            if (calc instanceof ListCalc) {
+            if (calc == null) {
+                // This can happen under JDK1.4: caller wants iterator
+                // implementation, but compiler can only provide list.
+                // Fall through and use native.
+            } else if (calc instanceof ListCalc) {
                 return new NonNativeListCalc((ListCalc) calc, highCardinality);
             } else {
                 return new NonNativeIterCalc(calc, highCardinality);
             }
-        } else {
-            if (isFirstCompileCall) {
-                isFirstCompileCall = false;
-                originalExp = funArg.clone();
-                Query query = compiler.getEvaluator().getQuery();
-                call.accept(
-                    new AddFormulasVisitor(query, substitutionMap, dimensions));
-                call.accept(new TransformToFormulasVisitor(query));
-                query.resolve();
-            }
-            return new NativeListCalc(
-                call, calcs, compiler, substitutionMap, originalExp);
         }
+        if (isFirstCompileCall) {
+            isFirstCompileCall = false;
+            originalExp = funArg.clone();
+            Query query = compiler.getEvaluator().getQuery();
+            call.accept(
+                new AddFormulasVisitor(query, substitutionMap, dimensions));
+            call.accept(new TransformToFormulasVisitor(query));
+            query.resolve();
+        }
+        return new NativeListCalc(
+            call, calcs, compiler, substitutionMap, originalExp);
     }
 
     private boolean isHighCardinality(Exp funArg, Evaluator evaluator) {
@@ -169,6 +170,7 @@ static class NonNativeCalc implements Calc {
         final boolean nativeEnabled;
 
         protected NonNativeCalc(Calc parent, final boolean nativeEnabled) {
+            assert parent != null;
             this.parent = parent;
             this.nativeEnabled = nativeEnabled;
         }

src/main/mondrian/rolap/RolapAxis.java

@@ -440,6 +440,11 @@ public int hashCode() {
             return hashCode;
         }
 
+        @Override
+        public String toString() {
+            return getList().toString();
+        }
+
         public ListIterator<Member> listIterator() {
             return new ListItr(0);
         }

src/main/mondrian/rolap/RolapNativeSet.java

@@ -30,7 +30,7 @@
  * Supports crossjoin, member.children, level.members and member.descendants -
  * all in non empty mode, i.e. there is a join to the fact table.<p/>
  *
- * TODO: the order of the result is different from the order of the
+ * <p>TODO: the order of the result is different from the order of the
  * enumeration. Should sort.
  *
  * @author av
@@ -45,17 +45,17 @@ public abstract class RolapNativeSet extends RolapNative {
         new SoftSmartCache<Object, TupleList>();
 
     /**
-     * Returns whether certain member types(e.g. calculated members) should
+     * Returns whether certain member types (e.g. calculated members) should
      * disable native SQL evaluation for expressions containing them.
      *
-     * <p>
-     * If true, expressions containing calculated members will be evaluated by
-     * the interpreter, instead of using SQL.
+     * <p>If true, expressions containing calculated members will be evaluated
+     * by the interpreter, instead of using SQL.
      *
-     * If false, calc members will be ignored and the computation will be done
-     * in SQL, returning more members than requested.  This is ok, if
+     * <p>If false, calc members will be ignored and the computation will be
+     * done in SQL, returning more members than requested.  This is ok, if
      * the superflous members are filtered out in java code afterwards.
-     * </p>
+     *
+     * @return whether certain member types should disable native SQL evaluation
      */
     protected abstract boolean restrictMemberTypes();
 
@@ -190,12 +190,21 @@ protected TupleList executeList(final SqlTupleReader tr) {
                 addLevel(tr, arg);
             }
 
-            // lookup the result in cache; we can't return the cached
+            // Look up the result in cache; we can't return the cached
             // result if the tuple reader contains a target with calculated
             // members because the cached result does not include those
             // members; so we still need to cross join the cached result
-            // with those enumerated members
-            Object key = tr.getCacheKey();
+            // with those enumerated members.
+            //
+            // The key needs to include the arguments (projection) as well as
+            // the constraint, because it's possible (see bug MONDRIAN-902)
+            // that independent axes have identical constraints but different
+            // args (i.e. projections). REVIEW: In this case, should we use the
+            // same cached result and project different columns?
+            List<Object> key = new ArrayList<Object>();
+            key.add(tr.getCacheKey());
+            key.addAll(Arrays.asList(args));
+
             TupleList result = cache.get(key);
             boolean hasEnumTargets = (tr.getEnumTargetCount() > 0);
             if (result != null && !hasEnumTargets) {

testsrc/main/mondrian/rolap/VirtualCubeTest.java

@@ -3,7 +3,7 @@
 // This software is subject to the terms of the Eclipse Public License v1.0
 // Agreement, available at the following URL:
 // http://www.eclipse.org/legal/epl-v10.html.
-// Copyright (C) 2003-2010 Julian Hyde
+// Copyright (C) 2003-2011 Julian Hyde
 // All Rights Reserved.
 // You must accept the terms of that agreement to use this software.
 */
@@ -17,7 +17,7 @@
 import java.util.List;
 
 /**
- * <code>VirtualCubeTest</code> shows virtual cube tests.
+ * Unit tests for virtual cubes.
  *
  * @author remberson
  * @since Feb 14, 2003
@@ -1334,6 +1334,33 @@ public void testNonEmptyConstraintOnVirtualCubeWithCalcMeasure() {
         assertQuerySql(query, mysqlPattern, true);
         assertQueryReturns(query, result);
     }
+
+    /**
+     * Test case for bug <a href="http://jira.pentaho.com/browse/MONDRIAN-902">
+     * MONDRIAN-902, "mondrian populating the same members on both axes"</a>.
+     */
+    public void testBugMondrian902() {
+        Result result = executeQuery(
+            "SELECT\n"
+            + "NON EMPTY CrossJoin(\n"
+            + "  [Education Level].[Education Level].Members,\n"
+            + "  CrossJoin(\n"
+            + "    [Product].[Product Family].Members,\n"
+            + "    [Store].[Store State].Members)) ON COLUMNS,\n"
+            + "NON EMPTY CrossJoin(\n"
+            + "  [Promotions].[Promotion Name].Members,\n"
+            + "  [Marital Status].[Marital Status].Members) ON ROWS\n"
+            + "FROM [Warehouse and Sales]");
+        assertEquals(
+            "[[Education Level].[Bachelors Degree], [Product].[Drink], [Store].[USA].[CA]]",
+            result.getAxes()[0].getPositions().get(0).toString());
+        assertEquals(45, result.getAxes()[0].getPositions().size());
+        // With bug MONDRIAN-902, this gave the same result as for axis #0:
+        assertEquals(
+            "[[Promotions].[Bag Stuffers], [Marital Status].[M]]",
+            result.getAxes()[1].getPositions().get(0).toString());
+        assertEquals(96, result.getAxes()[1].getPositions().size());
+    }
 }
 
 // End VirtualCubeTest.java