You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by jh...@apache.org on 2014/09/27 00:29:28 UTC

[1/2] git commit: [OPTIQ-426] Pool JDBC data sources, to make it easier to pool connections

Repository: incubator-optiq
Updated Branches:
  refs/heads/master a5665df97 -> 5d95c5fd1


[OPTIQ-426] Pool JDBC data sources, to make it easier to pool connections

Also pool OptiqConnection objects in OptiqSqlOperatorTest.


Project: http://git-wip-us.apache.org/repos/asf/incubator-optiq/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-optiq/commit/a1539e31
Tree: http://git-wip-us.apache.org/repos/asf/incubator-optiq/tree/a1539e31
Diff: http://git-wip-us.apache.org/repos/asf/incubator-optiq/diff/a1539e31

Branch: refs/heads/master
Commit: a1539e319c1d2443a7c0965c5081eb6fd25fdd25
Parents: a5665df
Author: Julian Hyde <jh...@apache.org>
Authored: Fri Sep 26 14:27:15 2014 -0700
Committer: Julian Hyde <jh...@apache.org>
Committed: Fri Sep 26 14:27:15 2014 -0700

----------------------------------------------------------------------
 .../hydromatic/optiq/impl/jdbc/JdbcSchema.java  | 10 +----
 .../hydromatic/optiq/impl/jdbc/JdbcUtils.java   | 46 ++++++++++++++++++++
 .../optiq/test/OptiqSqlOperatorTest.java        | 18 +++++---
 3 files changed, 60 insertions(+), 14 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-optiq/blob/a1539e31/core/src/main/java/net/hydromatic/optiq/impl/jdbc/JdbcSchema.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/net/hydromatic/optiq/impl/jdbc/JdbcSchema.java b/core/src/main/java/net/hydromatic/optiq/impl/jdbc/JdbcSchema.java
index 0301489..655fd87 100644
--- a/core/src/main/java/net/hydromatic/optiq/impl/jdbc/JdbcSchema.java
+++ b/core/src/main/java/net/hydromatic/optiq/impl/jdbc/JdbcSchema.java
@@ -29,8 +29,6 @@ import org.eigenbase.util.Util;
 
 import com.google.common.collect.*;
 
-import org.apache.commons.dbcp.BasicDataSource;
-
 import java.sql.*;
 import java.util.*;
 import javax.sql.DataSource;
@@ -131,12 +129,8 @@ public class JdbcSchema implements Schema {
       // Prevent hsqldb from screwing up java.util.logging.
       System.setProperty("hsqldb.reconfig_logging", "false");
     }
-    BasicDataSource dataSource = new BasicDataSource();
-    dataSource.setUrl(url);
-    dataSource.setUsername(username);
-    dataSource.setPassword(password);
-    dataSource.setDriverClassName(driverClassName);
-    return dataSource;
+    return JdbcUtils.DataSourcePool.INSTANCE.get(url, driverClassName, username,
+        password);
   }
 
   public boolean isMutable() {

http://git-wip-us.apache.org/repos/asf/incubator-optiq/blob/a1539e31/core/src/main/java/net/hydromatic/optiq/impl/jdbc/JdbcUtils.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/net/hydromatic/optiq/impl/jdbc/JdbcUtils.java b/core/src/main/java/net/hydromatic/optiq/impl/jdbc/JdbcUtils.java
index 18a399a..ed0975f 100644
--- a/core/src/main/java/net/hydromatic/optiq/impl/jdbc/JdbcUtils.java
+++ b/core/src/main/java/net/hydromatic/optiq/impl/jdbc/JdbcUtils.java
@@ -20,12 +20,18 @@ import net.hydromatic.linq4j.expressions.Primitive;
 import net.hydromatic.linq4j.function.*;
 
 import org.eigenbase.sql.SqlDialect;
+import org.eigenbase.util.ImmutableNullableList;
 import org.eigenbase.util.IntList;
 import org.eigenbase.util.Pair;
 import org.eigenbase.util14.DateTimeUtil;
 
+import com.google.common.cache.CacheBuilder;
+import com.google.common.cache.CacheLoader;
+import com.google.common.cache.LoadingCache;
 import com.google.common.collect.ImmutableList;
 
+import org.apache.commons.dbcp.BasicDataSource;
+
 import java.sql.*;
 import java.sql.Date;
 import java.util.*;
@@ -41,11 +47,17 @@ final class JdbcUtils {
 
   /** Pool of dialects. */
   public static class DialectPool {
+    final Map<DataSource, SqlDialect> map0 =
+        new IdentityHashMap<DataSource, SqlDialect>();
     final Map<List, SqlDialect> map = new HashMap<List, SqlDialect>();
 
     public static final DialectPool INSTANCE = new DialectPool();
 
     SqlDialect get(DataSource dataSource) {
+      final SqlDialect sqlDialect = map0.get(dataSource);
+      if (sqlDialect != null) {
+        return sqlDialect;
+      }
       Connection connection = null;
       try {
         connection = dataSource.getConnection();
@@ -63,6 +75,7 @@ final class JdbcUtils {
                   productName,
                   metaData.getIdentifierQuoteString());
           map.put(key, dialect);
+          map0.put(dataSource, dialect);
         }
         connection.close();
         connection = null;
@@ -174,6 +187,39 @@ final class JdbcUtils {
       return new Date(time + offset);
     }
   }
+
+  /** Ensures that if two data sources have the same definition, they will use
+   * the same object.
+   *
+   * <p>This in turn makes it easier to cache
+   * {@link org.eigenbase.sql.SqlDialect} objects. Otherwise, each time we
+   * see a new data source, we have to open a connection to find out what
+   * database product and version it is. */
+  public static class DataSourcePool {
+    public static final DataSourcePool INSTANCE = new DataSourcePool();
+
+    private final LoadingCache<List<String>, BasicDataSource> cache =
+        CacheBuilder.newBuilder().softValues().build(
+            new CacheLoader<List<String>, BasicDataSource>() {
+              @Override public BasicDataSource load(List<String> key) {
+                BasicDataSource dataSource = new BasicDataSource();
+                dataSource.setUrl(key.get(0));
+                dataSource.setUsername(key.get(1));
+                dataSource.setPassword(key.get(2));
+                dataSource.setDriverClassName(key.get(3));
+                return dataSource;
+              }
+            });
+
+    public DataSource get(String url, String driverClassName,
+        String username, String password) {
+      // Get data source objects from a cache, so that we don't have to sniff
+      // out what kind of database they are quite as often.
+      final List<String> key =
+          ImmutableNullableList.of(url, username, password, driverClassName);
+      return cache.apply(key);
+    }
+  }
 }
 
 // End JdbcUtils.java

http://git-wip-us.apache.org/repos/asf/incubator-optiq/blob/a1539e31/core/src/test/java/net/hydromatic/optiq/test/OptiqSqlOperatorTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/net/hydromatic/optiq/test/OptiqSqlOperatorTest.java b/core/src/test/java/net/hydromatic/optiq/test/OptiqSqlOperatorTest.java
index bb7678b..fe50255 100644
--- a/core/src/test/java/net/hydromatic/optiq/test/OptiqSqlOperatorTest.java
+++ b/core/src/test/java/net/hydromatic/optiq/test/OptiqSqlOperatorTest.java
@@ -26,13 +26,19 @@ import org.eigenbase.sql.test.SqlTester;
  * that generates SQL statements and executes them using Optiq.
  */
 public class OptiqSqlOperatorTest extends SqlOperatorBaseTest {
+  private static final ThreadLocal<OptiqConnection> LOCAL =
+      new ThreadLocal<OptiqConnection>() {
+        @Override protected OptiqConnection initialValue() {
+          try {
+            return OptiqAssert.getConnection("hr");
+          } catch (Exception e) {
+            throw new RuntimeException(e);
+          }
+        }
+      };
+
   private static SqlTester getHrTester() {
-    try {
-      OptiqConnection connection = OptiqAssert.getConnection("hr");
-      return tester(connection);
-    } catch (Exception e) {
-      throw new RuntimeException(e);
-    }
+    return tester(LOCAL.get());
   }
 
   public OptiqSqlOperatorTest() {


[2/2] git commit: [OPTIQ-427] Off-by-one issues in RemoveDistinctAggregateRule, AggregateFilterTransposeRule

Posted by jh...@apache.org.
[OPTIQ-427] Off-by-one issues in RemoveDistinctAggregateRule, AggregateFilterTransposeRule


Project: http://git-wip-us.apache.org/repos/asf/incubator-optiq/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-optiq/commit/5d95c5fd
Tree: http://git-wip-us.apache.org/repos/asf/incubator-optiq/tree/5d95c5fd
Diff: http://git-wip-us.apache.org/repos/asf/incubator-optiq/diff/5d95c5fd

Branch: refs/heads/master
Commit: 5d95c5fd1a809f07c82f968e48d46cb4f1f3e100
Parents: a1539e3
Author: Julian Hyde <jh...@apache.org>
Authored: Fri Sep 26 14:47:20 2014 -0700
Committer: Julian Hyde <jh...@apache.org>
Committed: Fri Sep 26 14:57:16 2014 -0700

----------------------------------------------------------------------
 .../org/eigenbase/rel/AggregateRelBase.java     |  1 +
 .../java/org/eigenbase/rel/FilterRelBase.java   | 15 +++++-
 .../rel/rules/AggregateFilterTransposeRule.java | 13 ++---
 .../rel/rules/AggregateStarTableRule.java       | 16 +-----
 .../rel/rules/RemoveDistinctAggregateRule.java  | 50 ++++++------------
 .../eigenbase/relopt/RelOptMaterialization.java | 10 ++--
 .../java/org/eigenbase/relopt/RelOptUtil.java   | 18 +++----
 .../eigenbase/relopt/SubstitutionVisitor.java   | 13 +++--
 .../org/eigenbase/sql2rel/RelDecorrelator.java  |  8 +--
 core/src/main/java/org/eigenbase/util/Util.java | 12 +++++
 .../net/hydromatic/optiq/test/FoodmartTest.java | 20 ++++++-
 .../net/hydromatic/optiq/test/LatticeTest.java  | 55 ++++++++++++++++++++
 .../optiq/test/MaterializationTest.java         |  5 +-
 .../org/eigenbase/test/RelOptRulesTest.java     | 11 ++++
 .../org/eigenbase/test/RelOptRulesTest.xml      | 19 +++++++
 15 files changed, 183 insertions(+), 83 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-optiq/blob/5d95c5fd/core/src/main/java/org/eigenbase/rel/AggregateRelBase.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/eigenbase/rel/AggregateRelBase.java b/core/src/main/java/org/eigenbase/rel/AggregateRelBase.java
index 3aac21c..6de37b3 100644
--- a/core/src/main/java/org/eigenbase/rel/AggregateRelBase.java
+++ b/core/src/main/java/org/eigenbase/rel/AggregateRelBase.java
@@ -68,6 +68,7 @@ public abstract class AggregateRelBase extends SingleRel {
     assert groupSet.isEmpty() == (groupSet.cardinality() == 0)
         : "See https://bugs.openjdk.java.net/browse/JDK-6222207, "
         + "BitSet internal invariants may be violated";
+    assert groupSet.length() <= child.getRowType().getFieldCount();
     for (AggregateCall aggCall : aggCalls) {
       assert typeMatchesInferred(aggCall, true);
     }

http://git-wip-us.apache.org/repos/asf/incubator-optiq/blob/5d95c5fd/core/src/main/java/org/eigenbase/rel/FilterRelBase.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/eigenbase/rel/FilterRelBase.java b/core/src/main/java/org/eigenbase/rel/FilterRelBase.java
index 1737470..7468a7e 100644
--- a/core/src/main/java/org/eigenbase/rel/FilterRelBase.java
+++ b/core/src/main/java/org/eigenbase/rel/FilterRelBase.java
@@ -54,14 +54,15 @@ public abstract class FilterRelBase extends SingleRel {
     assert condition != null;
     assert RexUtil.isFlat(condition) : condition;
     this.condition = condition;
+    // Too expensive for everyday use:
+    // assert isValid(true);
   }
 
   /**
    * Creates a FilterRelBase by parsing serialized output.
    */
   protected FilterRelBase(RelInput input) {
-    this(
-        input.getCluster(), input.getTraitSet(), input.getInput(),
+    this(input.getCluster(), input.getTraitSet(), input.getInput(),
         input.getExpression("condition"));
   }
 
@@ -84,6 +85,16 @@ public abstract class FilterRelBase extends SingleRel {
     return condition;
   }
 
+  @Override public boolean isValid(boolean fail) {
+    final RexChecker checker = new RexChecker(getChild().getRowType(), fail);
+    condition.accept(checker);
+    if (checker.getFailureCount() > 0) {
+      assert !fail;
+      return false;
+    }
+    return true;
+  }
+
   public RelOptCost computeSelfCost(RelOptPlanner planner) {
     double dRows = RelMetadataQuery.getRowCount(this);
     double dCpu = RelMetadataQuery.getRowCount(getChild());

http://git-wip-us.apache.org/repos/asf/incubator-optiq/blob/5d95c5fd/core/src/main/java/org/eigenbase/rel/rules/AggregateFilterTransposeRule.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/eigenbase/rel/rules/AggregateFilterTransposeRule.java b/core/src/main/java/org/eigenbase/rel/rules/AggregateFilterTransposeRule.java
index 06693fa..c7b4f73 100644
--- a/core/src/main/java/org/eigenbase/rel/rules/AggregateFilterTransposeRule.java
+++ b/core/src/main/java/org/eigenbase/rel/rules/AggregateFilterTransposeRule.java
@@ -36,6 +36,7 @@ import org.eigenbase.util.mapping.Mappings;
 import net.hydromatic.optiq.util.BitSets;
 
 import com.google.common.base.Function;
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Lists;
 
 /**
@@ -108,14 +109,8 @@ public class AggregateFilterTransposeRule extends RelOptRule {
         topGroupSet.set(BitSets.toList(newGroupSet).indexOf(c));
       }
       final List<AggregateCall> topAggCallList = Lists.newArrayList();
-      final int offset = newGroupSet.cardinality()
-          - aggregate.getGroupSet().cardinality();
-      assert offset > 0;
+      int i = newGroupSet.cardinality();
       for (AggregateCall aggregateCall : aggregate.getAggCallList()) {
-        final List<Integer> args = Lists.newArrayList();
-        for (int arg : aggregateCall.getArgList()) {
-          args.add(arg + offset);
-        }
         final Aggregation rollup =
             SubstitutionVisitor.getRollup(aggregateCall.getAggregation());
         if (rollup == null) {
@@ -127,8 +122,8 @@ public class AggregateFilterTransposeRule extends RelOptRule {
           return;
         }
         topAggCallList.add(
-            new AggregateCall(rollup, aggregateCall.isDistinct(), args,
-                aggregateCall.type, aggregateCall.name));
+            new AggregateCall(rollup, aggregateCall.isDistinct(),
+                ImmutableList.of(i++), aggregateCall.type, aggregateCall.name));
       }
       final AggregateRelBase topAggregate =
           aggregate.copy(aggregate.getTraitSet(), newFilter, topGroupSet,

http://git-wip-us.apache.org/repos/asf/incubator-optiq/blob/5d95c5fd/core/src/main/java/org/eigenbase/rel/rules/AggregateStarTableRule.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/eigenbase/rel/rules/AggregateStarTableRule.java b/core/src/main/java/org/eigenbase/rel/rules/AggregateStarTableRule.java
index ac659ea..7f866ab 100644
--- a/core/src/main/java/org/eigenbase/rel/rules/AggregateStarTableRule.java
+++ b/core/src/main/java/org/eigenbase/rel/rules/AggregateStarTableRule.java
@@ -31,8 +31,8 @@ import org.eigenbase.relopt.RelOptRuleCall;
 import org.eigenbase.relopt.RelOptRuleOperand;
 import org.eigenbase.relopt.RelOptTable;
 import org.eigenbase.relopt.RelOptUtil;
+import org.eigenbase.relopt.SubstitutionVisitor;
 import org.eigenbase.reltype.RelDataType;
-import org.eigenbase.sql.fun.SqlStdOperatorTable;
 import org.eigenbase.util.Pair;
 import org.eigenbase.util.mapping.AbstractSourceMapping;
 
@@ -194,7 +194,7 @@ public class AggregateStarTableRule extends RelOptRule {
     final int i = find(measures, seek);
   tryRoll:
     if (i >= 0) {
-      final Aggregation roll = getRollup(aggregation);
+      final Aggregation roll = SubstitutionVisitor.getRollup(aggregation);
       if (roll == null) {
         break tryRoll;
       }
@@ -221,18 +221,6 @@ public class AggregateStarTableRule extends RelOptRule {
     return null;
   }
 
-  private static Aggregation getRollup(Aggregation aggregation) {
-    if (aggregation == SqlStdOperatorTable.SUM
-        || aggregation == SqlStdOperatorTable.MIN
-        || aggregation == SqlStdOperatorTable.MAX) {
-      return aggregation;
-    } else if (aggregation == SqlStdOperatorTable.COUNT) {
-      return SqlStdOperatorTable.SUM;
-    } else {
-      return null;
-    }
-  }
-
   private static int find(ImmutableList<Lattice.Measure> measures,
       Pair<Aggregation, List<Integer>> seek) {
     for (int i = 0; i < measures.size(); i++) {

http://git-wip-us.apache.org/repos/asf/incubator-optiq/blob/5d95c5fd/core/src/main/java/org/eigenbase/rel/rules/RemoveDistinctAggregateRule.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/eigenbase/rel/rules/RemoveDistinctAggregateRule.java b/core/src/main/java/org/eigenbase/rel/rules/RemoveDistinctAggregateRule.java
index 560f13d..39928cd 100644
--- a/core/src/main/java/org/eigenbase/rel/rules/RemoveDistinctAggregateRule.java
+++ b/core/src/main/java/org/eigenbase/rel/rules/RemoveDistinctAggregateRule.java
@@ -29,6 +29,7 @@ import net.hydromatic.optiq.util.BitSets;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Lists;
 
 /**
  * Rule to remove distinct aggregates from a {@link AggregateRel}.
@@ -94,7 +95,7 @@ public final class RemoveDistinctAggregateRule extends RelOptRule {
     final List<RexInputRef> refs = new ArrayList<RexInputRef>();
     final List<String> fieldNames = aggregate.getRowType().getFieldNames();
     final BitSet groupSet = aggregate.getGroupSet();
-    for (int i : BitSets.toIter(groupSet)) {
+    for (int i : Util.range(groupSet.cardinality())) {
       refs.add(RexInputRef.of(i, aggFields));
     }
 
@@ -176,7 +177,7 @@ public final class RemoveDistinctAggregateRule extends RelOptRule {
     return aggregate.copy(
         aggregate.getTraitSet(),
         distinct,
-        aggregate.getGroupSet(),
+        BitSets.range(aggregate.getGroupSet().cardinality()),
         newAggCalls);
   }
 
@@ -300,16 +301,12 @@ public final class RemoveDistinctAggregateRule extends RelOptRule {
               aggCall.getName());
       assert refs.get(i) == null;
       if (left == null) {
-        refs.set(
-            i,
-            new RexInputRef(
-                groupCount + aggCallList.size(),
+        refs.set(i,
+            new RexInputRef(groupCount + aggCallList.size(),
                 newAggCall.getType()));
       } else {
-        refs.set(
-            i,
-            new RexInputRef(
-                leftFields.size() + groupCount + aggCallList.size(),
+        refs.set(i,
+            new RexInputRef(leftFields.size() + groupCount + aggCallList.size(),
                 newAggCall.getType()));
       }
       aggCallList.add(newAggCall);
@@ -319,7 +316,7 @@ public final class RemoveDistinctAggregateRule extends RelOptRule {
         aggregate.copy(
             aggregate.getTraitSet(),
             distinct,
-            aggregate.getGroupSet(),
+            BitSets.range(aggregate.getGroupSet().cardinality()),
             aggCallList);
 
     // If there's no left child yet, no need to create the join
@@ -332,37 +329,22 @@ public final class RemoveDistinctAggregateRule extends RelOptRule {
     // where {f0, f1, ...} are the GROUP BY fields.
     final List<RelDataTypeField> distinctFields =
         distinctAgg.getRowType().getFieldList();
-    RexNode condition = rexBuilder.makeLiteral(true);
+    final List<RexNode> conditions = Lists.newArrayList();
     for (i = 0; i < groupCount; ++i) {
-      final int leftOrdinal = i;
-      final int rightOrdinal = sourceOf.get(i);
-
       // null values form its own group
       // use "is not distinct from" so that the join condition
       // allows null values to match.
-      RexNode equi =
-          rexBuilder.makeCall(
-              SqlStdOperatorTable.IS_NOT_DISTINCT_FROM,
-              RexInputRef.of(leftOrdinal, leftFields),
-              new RexInputRef(
-                  leftFields.size() + rightOrdinal,
-                  distinctFields.get(rightOrdinal).getType()));
-      if (i == 0) {
-        condition = equi;
-      } else {
-        condition =
-            rexBuilder.makeCall(
-                SqlStdOperatorTable.AND,
-                condition,
-                equi);
-      }
+      conditions.add(
+          rexBuilder.makeCall(SqlStdOperatorTable.IS_NOT_DISTINCT_FROM,
+              RexInputRef.of(i, leftFields),
+              new RexInputRef(leftFields.size() + i,
+                  distinctFields.get(i).getType())));
     }
 
     // Join in the new 'select distinct' relation.
-    return joinFactory.createJoin(
-        left,
+    return joinFactory.createJoin(left,
         distinctAgg,
-        condition,
+        RexUtil.composeConjunction(rexBuilder, conditions, false),
         JoinRelType.INNER,
         ImmutableSet.<String>of(),
         false);

http://git-wip-us.apache.org/repos/asf/incubator-optiq/blob/5d95c5fd/core/src/main/java/org/eigenbase/relopt/RelOptMaterialization.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/eigenbase/relopt/RelOptMaterialization.java b/core/src/main/java/org/eigenbase/relopt/RelOptMaterialization.java
index 0b0686d..77883cc 100644
--- a/core/src/main/java/org/eigenbase/relopt/RelOptMaterialization.java
+++ b/core/src/main/java/org/eigenbase/relopt/RelOptMaterialization.java
@@ -150,7 +150,7 @@ public class RelOptMaterialization {
                   Mappings.asList(mapping.inverse()));
               final List<RexNode> conditions = Lists.newArrayList();
               if (left.condition != null) {
-                conditions.add(RexUtil.apply(mapping, left.condition));
+                conditions.add(left.condition);
               }
               if (right.condition != null) {
                 conditions.add(
@@ -174,12 +174,12 @@ public class RelOptMaterialization {
                   Mappings.asList(mapping.inverse()));
               final List<RexNode> conditions = Lists.newArrayList();
               if (left.condition != null) {
-                conditions.add(RexUtil.apply(mapping, left.condition));
-              }
-              if (right.condition != null) {
                 conditions.add(
                     RexUtil.apply(mapping,
-                        RexUtil.shift(right.condition, offset)));
+                        RexUtil.shift(left.condition, offset)));
+              }
+              if (right.condition != null) {
+                conditions.add(RexUtil.apply(mapping, right.condition));
               }
               final RelNode filter =
                   RelOptUtil.createFilter(project, conditions);

http://git-wip-us.apache.org/repos/asf/incubator-optiq/blob/5d95c5fd/core/src/main/java/org/eigenbase/relopt/RelOptUtil.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/eigenbase/relopt/RelOptUtil.java b/core/src/main/java/org/eigenbase/relopt/RelOptUtil.java
index aee380d..bd9c306 100644
--- a/core/src/main/java/org/eigenbase/relopt/RelOptUtil.java
+++ b/core/src/main/java/org/eigenbase/relopt/RelOptUtil.java
@@ -34,6 +34,7 @@ import net.hydromatic.linq4j.Ord;
 
 import net.hydromatic.optiq.util.BitSets;
 
+import com.google.common.base.Function;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Lists;
 
@@ -46,6 +47,13 @@ public abstract class RelOptUtil {
 
   public static final double EPSILON = 1.0e-5;
 
+  private static final Function<RelDataTypeField, RelDataType> GET_TYPE =
+      new Function<RelDataTypeField, RelDataType>() {
+        public RelDataType apply(RelDataTypeField field) {
+          return field.getType();
+        }
+      };
+
   //~ Methods ----------------------------------------------------------------
 
   /**
@@ -131,15 +139,7 @@ public abstract class RelOptUtil {
    * @see org.eigenbase.reltype.RelDataType#getFieldNames()
    */
   public static List<RelDataType> getFieldTypeList(final RelDataType type) {
-    return new AbstractList<RelDataType>() {
-      public RelDataType get(int index) {
-        return type.getFieldList().get(index).getType();
-      }
-
-      public int size() {
-        return type.getFieldCount();
-      }
-    };
+    return Lists.transform(type.getFieldList(), GET_TYPE);
   }
 
   public static boolean areRowTypesEqual(

http://git-wip-us.apache.org/repos/asf/incubator-optiq/blob/5d95c5fd/core/src/main/java/org/eigenbase/relopt/SubstitutionVisitor.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/eigenbase/relopt/SubstitutionVisitor.java b/core/src/main/java/org/eigenbase/relopt/SubstitutionVisitor.java
index adac55a..7a08d7a 100644
--- a/core/src/main/java/org/eigenbase/relopt/SubstitutionVisitor.java
+++ b/core/src/main/java/org/eigenbase/relopt/SubstitutionVisitor.java
@@ -1166,7 +1166,7 @@ public class SubstitutionVisitor {
         aggregateCalls.add(
             new AggregateCall(getRollup(aggregateCall.getAggregation()),
                 aggregateCall.isDistinct(),
-                ImmutableList.of(groupSet.cardinality() + i),
+                ImmutableList.of(target.groupSet.cardinality() + i),
                 aggregateCall.type, aggregateCall.name));
       }
       result = MutableAggregate.of(target, groupSet, aggregateCalls);
@@ -1215,8 +1215,15 @@ public class SubstitutionVisitor {
   }
 
   public static Aggregation getRollup(Aggregation aggregation) {
-    // TODO: count rolls up using sum; etc.
-    return aggregation;
+    if (aggregation == SqlStdOperatorTable.SUM
+        || aggregation == SqlStdOperatorTable.MIN
+        || aggregation == SqlStdOperatorTable.MAX) {
+      return aggregation;
+    } else if (aggregation == SqlStdOperatorTable.COUNT) {
+      return SqlStdOperatorTable.SUM;
+    } else {
+      return null;
+    }
   }
 
   /** Builds a shuttle that stores a list of expressions, and can map incoming

http://git-wip-us.apache.org/repos/asf/incubator-optiq/blob/5d95c5fd/core/src/main/java/org/eigenbase/sql2rel/RelDecorrelator.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/eigenbase/sql2rel/RelDecorrelator.java b/core/src/main/java/org/eigenbase/sql2rel/RelDecorrelator.java
index a818f48..6965175 100644
--- a/core/src/main/java/org/eigenbase/sql2rel/RelDecorrelator.java
+++ b/core/src/main/java/org/eigenbase/sql2rel/RelDecorrelator.java
@@ -2204,8 +2204,9 @@ public class RelDecorrelator implements ReflectiveVisitor {
 
       rightInputRel =
           createProjectWithAdditionalExprs(rightInputRel,
-              ImmutableList.of(Pair.<RexNode, String>of(rexBuilder.makeLiteral(
-                      true), "nullIndicator")));
+              ImmutableList.of(
+                  Pair.<RexNode, String>of(rexBuilder.makeLiteral(true),
+                      "nullIndicator")));
 
       JoinRel joinRel =
           new JoinRel(
@@ -2279,7 +2280,8 @@ public class RelDecorrelator implements ReflectiveVisitor {
           }
         }
 
-        newAggCalls.add(aggCall.adaptTo(joinOutputProjRel, newAggArgs,
+        newAggCalls.add(
+            aggCall.adaptTo(joinOutputProjRel, newAggArgs,
                 aggRel.getGroupCount(), groupCount));
       }
 

http://git-wip-us.apache.org/repos/asf/incubator-optiq/blob/5d95c5fd/core/src/main/java/org/eigenbase/util/Util.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/eigenbase/util/Util.java b/core/src/main/java/org/eigenbase/util/Util.java
index 25b5837..41587a6 100644
--- a/core/src/main/java/org/eigenbase/util/Util.java
+++ b/core/src/main/java/org/eigenbase/util/Util.java
@@ -1990,6 +1990,18 @@ public class Util {
     return list.subList(fromIndex, list.size());
   }
 
+  public static List<Integer> range(final int end) {
+    return new AbstractList<Integer>() {
+      public int size() {
+        return end;
+      }
+
+      public Integer get(int index) {
+        return index;
+      }
+    };
+  }
+
   public static List<Integer> range(final int start, final int end) {
     return new AbstractList<Integer>() {
       public int size() {

http://git-wip-us.apache.org/repos/asf/incubator-optiq/blob/5d95c5fd/core/src/test/java/net/hydromatic/optiq/test/FoodmartTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/net/hydromatic/optiq/test/FoodmartTest.java b/core/src/test/java/net/hydromatic/optiq/test/FoodmartTest.java
index 852a106..ca02d54 100644
--- a/core/src/test/java/net/hydromatic/optiq/test/FoodmartTest.java
+++ b/core/src/test/java/net/hydromatic/optiq/test/FoodmartTest.java
@@ -25,6 +25,7 @@ import com.fasterxml.jackson.databind.ObjectMapper;
 
 import com.google.common.collect.ImmutableList;
 
+import org.junit.Ignore;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.Parameterized;
@@ -151,10 +152,8 @@ public class FoodmartTest {
   public void test() {
     try {
       OptiqAssert.that()
-//          .withModel(JdbcTest.FOODMART_MODEL)
           .with(OptiqAssert.Config.FOODMART_CLONE)
           .pooled()
-//          .withSchema("foodmart")
           .query(query.sql)
           .runs();
     } catch (Throwable e) {
@@ -163,6 +162,23 @@ public class FoodmartTest {
     }
   }
 
+  @Test(timeout = 60000)
+  @Ignore
+  public void testWithLattice() {
+    try {
+      OptiqAssert.that()
+          .with(OptiqAssert.Config.JDBC_FOODMART_WITH_LATTICE)
+          .pooled()
+          .withSchema("foodmart")
+          .query(query.sql)
+          .enableMaterializations(true)
+          .runs();
+    } catch (Throwable e) {
+      throw new RuntimeException("Test failed, id=" + query.id + ", sql="
+          + query.sql, e);
+    }
+  }
+
   public static class FoodMartQuerySet {
     private static SoftReference<FoodMartQuerySet> ref;
 

http://git-wip-us.apache.org/repos/asf/incubator-optiq/blob/5d95c5fd/core/src/test/java/net/hydromatic/optiq/test/LatticeTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/net/hydromatic/optiq/test/LatticeTest.java b/core/src/test/java/net/hydromatic/optiq/test/LatticeTest.java
index 162992b..5a10401 100644
--- a/core/src/test/java/net/hydromatic/optiq/test/LatticeTest.java
+++ b/core/src/test/java/net/hydromatic/optiq/test/LatticeTest.java
@@ -24,10 +24,14 @@ import org.eigenbase.util.TestUtil;
 import org.eigenbase.util.Util;
 
 import com.google.common.base.Function;
+import com.google.common.collect.ImmutableList;
 
+import org.junit.Ignore;
 import org.junit.Test;
 
+import java.io.IOException;
 import java.util.Arrays;
+import java.util.List;
 import java.util.concurrent.atomic.AtomicInteger;
 
 import static org.hamcrest.CoreMatchers.anyOf;
@@ -335,6 +339,57 @@ public class LatticeTest {
       .sameResultWithMaterializationsDisabled();
   }
 
+  /** Runs all queries against the Foodmart schema, using a lattice.
+   *
+   * <p>Disabled for normal runs, because it is slow. */
+  @Ignore
+  @Test public void testAllFoodmartQueries() throws IOException {
+    // Test ids that had bugs in them until recently. Useful for a sanity check.
+    final List<Integer> fixed = ImmutableList.of(13, 24, 28, 30, 61, 76, 79, 81,
+        85, 98, 101, 107, 128, 129, 130, 131);
+    // Test ids that still have bugs
+    final List<Integer> bad = ImmutableList.of(382, 423);
+    for (int i = 1; i < 1000; i++) {
+      System.out.println("i=" + i);
+      try {
+        if (bad.contains(i)) {
+          continue;
+        }
+        check(i);
+      } catch (Throwable e) {
+        throw new RuntimeException("error in " + i, e);
+      }
+    }
+  }
+
+  private void check(int n) throws IOException {
+    final FoodmartTest.FoodmartQuery query =
+        FoodmartTest.FoodMartQuerySet.instance().queries.get(n);
+    if (query == null) {
+      return;
+    }
+    foodmartModel(
+        " auto: false,\n"
+        + "  defaultMeasures: [ {\n"
+        + "    agg: 'count'\n"
+        + "  } ],\n"
+        + "  tiles: [ {\n"
+        + "    dimensions: [ 'the_year', ['t', 'quarter'] ],\n"
+        + "    measures: [ {\n"
+        + "      agg: 'sum',\n"
+        + "      args: 'unit_sales'\n"
+        + "    }, {\n"
+        + "      agg: 'sum',\n"
+        + "      args: 'store_sales'\n"
+        + "    }, {\n"
+        + "      agg: 'count'\n"
+        + "    } ]\n"
+        + "  } ]\n")
+        .withSchema("foodmart")
+        .query(query.sql)
+      .sameResultWithMaterializationsDisabled();
+  }
+
   /** A tile with no measures should inherit default measure list from the
    * lattice. */
   @Test public void testTileWithNoMeasures() {

http://git-wip-us.apache.org/repos/asf/incubator-optiq/blob/5d95c5fd/core/src/test/java/net/hydromatic/optiq/test/MaterializationTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/net/hydromatic/optiq/test/MaterializationTest.java b/core/src/test/java/net/hydromatic/optiq/test/MaterializationTest.java
index 3b4629d..86bf601 100644
--- a/core/src/test/java/net/hydromatic/optiq/test/MaterializationTest.java
+++ b/core/src/test/java/net/hydromatic/optiq/test/MaterializationTest.java
@@ -238,7 +238,8 @@ public class MaterializationTest {
   }
 
   /** Aggregation query at coarser level of aggregation than aggregation
-   * materialization. Requires an additional AggregateRel to roll up. */
+   * materialization. Requires an additional AggregateRel to roll up. Note that
+   * COUNT is rolled up using SUM. */
   @Test public void testAggregateRollUp() {
     checkMaterialize(
         "select \"empid\", \"deptno\", count(*) as c, sum(\"empid\") as s from \"emps\" group by \"empid\", \"deptno\"",
@@ -246,7 +247,7 @@ public class MaterializationTest {
         JdbcTest.HR_MODEL,
         OptiqAssert.checkResultContains(
             "EnumerableCalcRel(expr#0..1=[{inputs}], expr#2=[1], expr#3=[+($t1, $t2)], C=[$t3], deptno=[$t0])\n"
-            + "  EnumerableAggregateRel(group=[{1}], agg#0=[COUNT($1)])\n"
+            + "  EnumerableAggregateRel(group=[{1}], agg#0=[SUM($2)])\n"
             + "    EnumerableTableAccessRel(table=[[hr, m0]])"));
   }
 

http://git-wip-us.apache.org/repos/asf/incubator-optiq/blob/5d95c5fd/core/src/test/java/org/eigenbase/test/RelOptRulesTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/eigenbase/test/RelOptRulesTest.java b/core/src/test/java/org/eigenbase/test/RelOptRulesTest.java
index 095d7d4..a38317d 100644
--- a/core/src/test/java/org/eigenbase/test/RelOptRulesTest.java
+++ b/core/src/test/java/org/eigenbase/test/RelOptRulesTest.java
@@ -49,6 +49,7 @@ import org.eigenbase.rel.rules.PushSemiJoinPastProjectRule;
 import org.eigenbase.rel.rules.ReduceAggregatesRule;
 import org.eigenbase.rel.rules.ReduceExpressionsRule;
 import org.eigenbase.rel.rules.ReduceValuesRule;
+import org.eigenbase.rel.rules.RemoveDistinctAggregateRule;
 import org.eigenbase.rel.rules.RemoveEmptyRules;
 import org.eigenbase.rel.rules.RemoveSemiJoinRule;
 import org.eigenbase.rel.rules.RemoveTrivialProjectRule;
@@ -251,6 +252,16 @@ public class RelOptRulesTest extends RelOptTestBase {
         + " from sales.dept group by name");
   }
 
+  @Test public void testDistinctCount() {
+    final HepProgram program = HepProgram.builder()
+        .addRuleInstance(RemoveDistinctAggregateRule.INSTANCE)
+        .addRuleInstance(AggregateProjectMergeRule.INSTANCE)
+        .build();
+    checkPlanning(program,
+        "select deptno, count(distinct ename)"
+        + " from sales.emp group by deptno");
+  }
+
   @Test public void testPushProjectPastFilter() {
     checkPlanning(
         PushProjectPastFilterRule.INSTANCE,

http://git-wip-us.apache.org/repos/asf/incubator-optiq/blob/5d95c5fd/core/src/test/resources/org/eigenbase/test/RelOptRulesTest.xml
----------------------------------------------------------------------
diff --git a/core/src/test/resources/org/eigenbase/test/RelOptRulesTest.xml b/core/src/test/resources/org/eigenbase/test/RelOptRulesTest.xml
index 7016804..a4d2d31 100644
--- a/core/src/test/resources/org/eigenbase/test/RelOptRulesTest.xml
+++ b/core/src/test/resources/org/eigenbase/test/RelOptRulesTest.xml
@@ -2229,4 +2229,23 @@ ProjectRel(ENAME=[$0], R=[$1])
 ]]>
         </Resource>
     </TestCase>
+    <TestCase name="testDistinctCount">
+        <Resource name="sql">
+            <![CDATA[select deptno, count(distinct ename) from sales.emp group by deptno]]>
+        </Resource>
+        <Resource name="planBefore">
+            <![CDATA[
+AggregateRel(group=[{0}], EXPR$1=[COUNT(DISTINCT $1)])
+  ProjectRel(DEPTNO=[$7], ENAME=[$1])
+    TableAccessRel(table=[[CATALOG, SALES, EMP]])
+]]>
+        </Resource>
+        <Resource name="planAfter">
+            <![CDATA[
+AggregateRel(group=[{1}], EXPR$1=[COUNT($0)])
+  AggregateRel(group=[{1, 7}])
+    TableAccessRel(table=[[CATALOG, SALES, EMP]])
+]]>
+        </Resource>
+    </TestCase>
 </Root>