MONDRIAN: Partial fix for bug MONDRIAN-751, "Drill SQL does not inclu…

…de slicer

    members in WHERE clause". If there are calculated members,
    Cell.canDrillThrough returns false, but if you choose to drill through,
    Cell.drillThrough will generate a query that ignores those members.

    Most of this has been the behavior since MONDRIAN-180, but new in this
    change, we check for calculated members in hierarchies other than measures,
    and we recognise 'Aggregate({m})' as a trivial calculated member equivalent
    to 'm'.

    Also remove 'throws Exception' from a few test cases. Checked exceptions are
    ugly.

    Also, don't be so eager to wrap the argument to the 'Aggregate' function in
    a call to the 'Cache' function. If the argument is simple, it is faster to
    evaluate without caching.

[git-p4: depot-paths = "//open/mondrian-release/3.2/": change = 13660]

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

@@ -40,11 +40,14 @@ protected Exp validateArg(
         if (i == 0) {
             if (MondrianProperties.instance().EnableExpCache.get()) {
                 Exp arg = args[0];
-                final Exp cacheCall = new UnresolvedFunCall(
-                        CacheFunDef.NAME,
-                        Syntax.Function,
-                        new Exp[] {arg});
-                return validator.validate(cacheCall, false);
+                if (FunUtil.worthCaching(arg)) {
+                    final Exp cacheCall =
+                        new UnresolvedFunCall(
+                            CacheFunDef.NAME,
+                            Syntax.Function,
+                            new Exp[] {arg});
+                    return validator.validate(cacheCall, false);
+                }
             }
         }
         return super.validateArg(validator, args, i, category);

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

@@ -2333,6 +2333,42 @@ static Member parseMember(
         return member;
     }
 
+    /**
+     * Returns whether an expression is worth wrapping in "Cache( ... )".
+     *
+     * @param exp Expression
+     * @return Whether worth caching
+     */
+    public static boolean worthCaching(Exp exp) {
+        // Literal is not worth caching.
+        if (exp instanceof Literal) {
+            return false;
+        }
+        // Member, hierarchy, level, or dimension expression is not worth
+        // caching.
+        if (exp instanceof MemberExpr
+            || exp instanceof LevelExpr
+            || exp instanceof HierarchyExpr
+            || exp instanceof DimensionExpr)
+        {
+            return false;
+        }
+        if (exp instanceof ResolvedFunCall) {
+            ResolvedFunCall call = (ResolvedFunCall) exp;
+
+            // A set of literals is not worth caching.
+            if (call.getFunDef() instanceof SetFunDef) {
+                for (Exp setArg : call.getArgs()) {
+                    if (worthCaching(setArg)) {
+                        return true;
+                    }
+                }
+                return false;
+            }
+        }
+        return true;
+    }
+
     // ~ Inner classes ---------------------------------------------------------
 
     /**

src/main/mondrian/rolap/RolapCell.java

@@ -13,6 +13,8 @@
 import mondrian.olap.*;
 import mondrian.olap.Cell;
 import mondrian.olap.Connection;
+import mondrian.olap.fun.AggregateFunDef;
+import mondrian.olap.fun.SetFunDef;
 import mondrian.rolap.agg.AggregationManager;
 import mondrian.rolap.agg.CellRequest;
 import mondrian.spi.Dialect;
@@ -142,10 +144,28 @@ public int getDrillThroughCount() {
     public boolean canDrillThrough() {
         // get current members
         final Member[] currentMembers = getMembersForDrillThrough();
+        if (containsCalcMembers(currentMembers)) {
+            return false;
+        }
         Cube x = chooseDrillThroughCube(currentMembers, result.getCube());
         return x != null;
     }
 
+    private boolean containsCalcMembers(Member[] currentMembers) {
+        // Any calculated members which are not measures, we can't drill
+        // through. Trivial calculated members should have been converted
+        // already. We allow simple calculated measures such as
+        // [Measures].[Unit Sales] / [Measures].[Store Sales] provided that both
+        // are from the same cube.
+        for (int i = 1; i < currentMembers.length; i++) {
+            final Member currentMember = currentMembers[i];
+            if (currentMember.isCalculated()) {
+                return true;
+            }
+        }
+        return false;
+    }
+
     public static RolapCube chooseDrillThroughCube(
         Member[] currentMembers,
         RolapCube defaultCube)
@@ -189,20 +209,55 @@ private Member[] getMembersForDrillThrough() {
         final Member[] currentMembers = result.getCellMembers(pos);
 
         // replace member if we're dealing with a trivial formula
-        if (currentMembers[0]
-            instanceof RolapHierarchy.RolapCalculatedMeasure)
-        {
-            RolapHierarchy.RolapCalculatedMeasure measure =
-                (RolapHierarchy.RolapCalculatedMeasure)currentMembers[0];
-            if (measure.getFormula().getExpression() instanceof MemberExpr) {
-                currentMembers[0] =
-                    ((MemberExpr) measure.getFormula().getExpression())
-                    .getMember();
-            }
+        List<Member> memberList = Arrays.asList(currentMembers);
+        for (int i = 0; i < currentMembers.length; i++) {
+            replaceTrivialCalcMember(i, memberList);
         }
         return currentMembers;
     }
 
+    private void replaceTrivialCalcMember(int i, List<Member> members) {
+        Member member = members.get(i);
+        if (!member.isCalculated()) {
+            return;
+        }
+        if (member instanceof RolapCubeMember) {
+            RolapCubeMember rolapCubeMember = (RolapCubeMember) member;
+            member = rolapCubeMember.getRolapMember();
+        }
+        // if "cm" is a calc member defined by
+        // "with member cm as m" then
+        // "cm" is equivalent to "m"
+        RolapCalculatedMember measure = (RolapCalculatedMember) member;
+        final Exp expr = measure.getFormula().getExpression();
+        if (expr instanceof MemberExpr) {
+            members.set(
+                i,
+                ((MemberExpr) expr).getMember());
+            return;
+        }
+        // "Aggregate({m})" is equivalent to "m"
+        if (expr instanceof ResolvedFunCall) {
+            ResolvedFunCall call = (ResolvedFunCall) expr;
+            if (call.getFunDef() instanceof AggregateFunDef) {
+                final Exp[] args = call.getArgs();
+                if (args[0] instanceof ResolvedFunCall) {
+                    final ResolvedFunCall arg0 = (ResolvedFunCall) args[0];
+                    if (arg0.getFunDef() instanceof SetFunDef) {
+                        if (arg0.getArgCount() == 1
+                            && arg0.getArg(0) instanceof MemberExpr)
+                        {
+                            final MemberExpr memberExpr =
+                                (MemberExpr) arg0.getArg(0);
+                            members.set(i, memberExpr.getMember());
+                            return;
+                        }
+                    }
+                }
+            }
+        }
+    }
+
     /**
      * Generates an executes a SQL statement to drill through this cell.
      *

src/main/mondrian/rolap/RolapNamedSetEvaluator.java

@@ -191,6 +191,7 @@ public Member currentMember() {
      *
      * @param <T> Element type
      */
+    // TODO: should also implement List -- save a copy
     private class IterableCollection<T> implements Collection<T>, Iterable<T> {
         public Iterator<T> iterator() {
             return new Iterator<T>() {