MONDRIAN-845: Have RolapNamedSetEvaluator#IterableCollection

implement List, to prevent a copy from iterable -> list when calling 
.evaluateList() on an expression derived from a NamedSet. This has 2 
consequences in the unit test suite: 
1) the list returned from .evaluateList() is no longer guaranteed 
to be mutable. Since there's a ResultStyle.IMMUTABLE_LIST, though, 
seems like this guarantee was more based on implementation than on 
the contract? 
2) NamedSetTest#testCurrentAndCurrentOrdinal modified to actually 
return what looks like the desired behavior anyway. Don't entirely 
understand why the context is now available but wasn't before, but 
this behavior seems more correct. 
Contributed by Joe Barnett

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

src/main/mondrian/rolap/RolapNamedSetEvaluator.java

@@ -186,31 +186,16 @@ public Member currentMember() {
      * {@code isEmpty}.
      *
      * <p>Implements {@link Iterable} explicitly because Collection does not
-     * implement Iterable until JDK1.5. This way, we don't have to use a wrapper
-     * that hides the size method.
+     * implement Iterable until JDK1.5. This way, we don't have to use a
+     * wrapper that hides the size method.
      *
      * @param <T> Element type
      */
-    // TODO: should also implement List -- save a copy
-    private class IterableCollection<T> implements Collection<T>, Iterable<T> {
+    private class IterableCollection<T>
+            implements Collection<T>, Iterable<T>, List<T>
+    {
         public Iterator<T> iterator() {
-            return new Iterator<T>() {
-                int i = -1;
-
-                public boolean hasNext() {
-                    return i < list.size() - 1;
-                }
-
-                public T next() {
-                    currentOrdinal = ++i;
-                    //noinspection unchecked
-                    return (T) list.get(i);
-                }
-
-                public void remove() {
-                    throw new UnsupportedOperationException();
-                }
-            };
+            return listIterator();
         }
 
         // The following are included to fill out the Collection
@@ -257,12 +242,100 @@ public int size() {
         }
 
         public Object[] toArray() {
-            throw new UnsupportedOperationException();
+            return list.toArray();
         }
 
         public <T> T[] toArray(T[] a) {
+            return (T[]) list.toArray(a);
+        }
+
+        // the rest fill out the List interface, again mostly unsupported
+        public void add(int index, T element) {
+            throw new UnsupportedOperationException();
+        }
+
+        public boolean addAll(int index, Collection<? extends T> c) {
             throw new UnsupportedOperationException();
         }
+
+        public T get(int index) {
+            return (T)list.get(index);
+        }
+
+        public int indexOf(Object o) {
+            return list.indexOf(o);
+        }
+
+        public int lastIndexOf(Object o) {
+            return list.lastIndexOf(o);
+        }
+
+        public ListIterator<T> listIterator() {
+            return new ListIterator<T>() {
+                int i = -1;
+
+                public boolean hasNext() {
+                    return i < list.size() - 1;
+                }
+
+                public boolean hasPrevious() {
+                    return i > 0;
+                }
+
+                public T next() {
+                    currentOrdinal = ++i;
+                    //noinspection unchecked
+                    return (T) list.get(i);
+                }
+
+                public T previous() {
+                    currentOrdinal = --i;
+                    return (T)list.get(i);
+                }
+
+                public int nextIndex() {
+                    return i + 1;
+                }
+
+                public int previousIndex() {
+                    return i;
+                }
+
+                public void set(T o) {
+                    throw new UnsupportedOperationException();
+                }
+
+                public void remove() {
+                    throw new UnsupportedOperationException();
+                }
+
+                public void add(T o) {
+                    throw new UnsupportedOperationException();
+                }
+            };
+        }
+
+        public ListIterator<T> listIterator(int index) {
+            // TODO: should actually implement a bidirectional
+            //       iterator here, have listIterator() call
+            //       this, and have iterator() call listIterator().
+            //       for now, nothing seems to call listIterator(),
+            //       so this works, and saves a copy from iterable
+            //       to list higher up in the call stack
+            throw new UnsupportedOperationException();
+        }
+
+        public T remove(int index) {
+            throw new UnsupportedOperationException();
+        }
+
+        public T set(int index, T element) {
+            return (T)list.set(index, element);
+        }
+
+        public List<T> subList(int fromIndex, int toIndex) {
+            return list.subList(fromIndex, toIndex);
+        }
     }
 }
 

testsrc/main/mondrian/test/NamedSetTest.java

@@ -1052,11 +1052,8 @@ public void testHierarchizeNamedSetImmutable() {
 
     public void testCurrentAndCurrentOrdinal() {
         // This test checks that <Named Set>.Current and
-        // <Named Set>.CurrentOrdinal basically work - that is, don't give an
-        // error - but the results are currently off. Note that every cell says
-        // that CurrentOrdinal=8. This is because by the time cells
-        // are evaluated, the evaluator context that produced the axes has
-        // already been lost. Could do better.
+        // <Named Set>.CurrentOrdinal work, and return the
+        // correct value
         assertQueryReturns(
             "with set [Gender Marital Status] as\n"
             + " [Gender].members * [Marital Status].members\n"
@@ -1087,29 +1084,29 @@ public void testCurrentAndCurrentOrdinal() {
             + "{[Gender].[M], [Marital Status].[M]}\n"
             + "{[Gender].[M], [Marital Status].[S]}\n"
             + "Row #0: 266,773\n"
-            + "Row #0: 8\n"
-            + "Row #0: ([Gender].[M], [Marital Status].[S])\n"
+            + "Row #0: 0\n"
+            + "Row #0: ([Gender].[All Gender], [Marital Status].[All Marital Status])\n"
             + "Row #1: 131,796\n"
-            + "Row #1: 8\n"
-            + "Row #1: ([Gender].[M], [Marital Status].[S])\n"
+            + "Row #1: 1\n"
+            + "Row #1: ([Gender].[All Gender], [Marital Status].[M])\n"
             + "Row #2: 134,977\n"
-            + "Row #2: 8\n"
-            + "Row #2: ([Gender].[M], [Marital Status].[S])\n"
+            + "Row #2: 2\n"
+            + "Row #2: ([Gender].[All Gender], [Marital Status].[S])\n"
             + "Row #3: 131,558\n"
-            + "Row #3: 8\n"
-            + "Row #3: ([Gender].[M], [Marital Status].[S])\n"
+            + "Row #3: 3\n"
+            + "Row #3: ([Gender].[F], [Marital Status].[All Marital Status])\n"
             + "Row #4: 65,336\n"
-            + "Row #4: 8\n"
-            + "Row #4: ([Gender].[M], [Marital Status].[S])\n"
+            + "Row #4: 4\n"
+            + "Row #4: ([Gender].[F], [Marital Status].[M])\n"
             + "Row #5: 66,222\n"
-            + "Row #5: 8\n"
-            + "Row #5: ([Gender].[M], [Marital Status].[S])\n"
+            + "Row #5: 5\n"
+            + "Row #5: ([Gender].[F], [Marital Status].[S])\n"
             + "Row #6: 135,215\n"
-            + "Row #6: 8\n"
-            + "Row #6: ([Gender].[M], [Marital Status].[S])\n"
+            + "Row #6: 6\n"
+            + "Row #6: ([Gender].[M], [Marital Status].[All Marital Status])\n"
             + "Row #7: 66,460\n"
-            + "Row #7: 8\n"
-            + "Row #7: ([Gender].[M], [Marital Status].[S])\n"
+            + "Row #7: 7\n"
+            + "Row #7: ([Gender].[M], [Marital Status].[M])\n"
             + "Row #8: 68,755\n"
             + "Row #8: 8\n"
             + "Row #8: ([Gender].[M], [Marital Status].[S])\n");

testsrc/main/mondrian/test/UdfTest.java

@@ -1074,8 +1074,9 @@ public Object execute(Evaluator eval, Argument[] args) {
             // Note: must call Argument.evaluateList. If we call
             // Argument.evaluate we may get an Iterable.
             List<?> list = args[0].evaluateList(eval);
-            // We do not need to copy before we reverse. The list is guaranteed
+            // We do need to copy before we reverse. The list is not guaranteed
             // to be mutable.
+            list = new ArrayList(list);
             Collections.reverse(list);
             return list;
         }