You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by jc...@apache.org on 2017/02/28 20:16:22 UTC

calcite git commit: [CALCITE-1661] Support aggregation functions on DECIMAL in DruidAdapter

Repository: calcite
Updated Branches:
  refs/heads/master 49888a6c5 -> 0372d23b8


[CALCITE-1661] Support aggregation functions on DECIMAL in DruidAdapter

Close apache/calcite#385


Project: http://git-wip-us.apache.org/repos/asf/calcite/repo
Commit: http://git-wip-us.apache.org/repos/asf/calcite/commit/0372d23b
Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/0372d23b
Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/0372d23b

Branch: refs/heads/master
Commit: 0372d23b847d4d145917dd786d1c9e3570cb8041
Parents: 49888a6
Author: Jesus Camacho Rodriguez <jc...@apache.org>
Authored: Tue Feb 28 17:46:53 2017 +0000
Committer: Jesus Camacho Rodriguez <jc...@apache.org>
Committed: Tue Feb 28 20:15:20 2017 +0000

----------------------------------------------------------------------
 .../calcite/config/CalciteConnectionConfig.java |  2 +
 .../config/CalciteConnectionConfigImpl.java     |  5 ++
 .../config/CalciteConnectionProperty.java       |  4 ++
 .../calcite/adapter/druid/DruidQuery.java       | 29 +++++++++--
 .../calcite/adapter/druid/DruidRules.java       | 54 ++++++++++++++------
 site/_docs/adapter.md                           |  1 +
 6 files changed, 76 insertions(+), 19 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/calcite/blob/0372d23b/core/src/main/java/org/apache/calcite/config/CalciteConnectionConfig.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/config/CalciteConnectionConfig.java b/core/src/main/java/org/apache/calcite/config/CalciteConnectionConfig.java
index b96f8a9..5dd6a1e 100644
--- a/core/src/main/java/org/apache/calcite/config/CalciteConnectionConfig.java
+++ b/core/src/main/java/org/apache/calcite/config/CalciteConnectionConfig.java
@@ -30,6 +30,8 @@ public interface CalciteConnectionConfig extends ConnectionConfig {
   boolean approximateDistinctCount();
   /** @see CalciteConnectionProperty#APPROXIMATE_TOP_N */
   boolean approximateTopN();
+  /** @see CalciteConnectionProperty#APPROXIMATE_DECIMAL */
+  boolean approximateDecimal();
   /** @see CalciteConnectionProperty#AUTO_TEMP */
   boolean autoTemp();
   /** @see CalciteConnectionProperty#MATERIALIZATIONS_ENABLED */

http://git-wip-us.apache.org/repos/asf/calcite/blob/0372d23b/core/src/main/java/org/apache/calcite/config/CalciteConnectionConfigImpl.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/config/CalciteConnectionConfigImpl.java b/core/src/main/java/org/apache/calcite/config/CalciteConnectionConfigImpl.java
index 5527752..c2a3239 100644
--- a/core/src/main/java/org/apache/calcite/config/CalciteConnectionConfigImpl.java
+++ b/core/src/main/java/org/apache/calcite/config/CalciteConnectionConfigImpl.java
@@ -56,6 +56,11 @@ public class CalciteConnectionConfigImpl extends ConnectionConfigImpl
         .getBoolean();
   }
 
+  public boolean approximateDecimal() {
+    return CalciteConnectionProperty.APPROXIMATE_DECIMAL.wrap(properties)
+        .getBoolean();
+  }
+
   public boolean autoTemp() {
     return CalciteConnectionProperty.AUTO_TEMP.wrap(properties).getBoolean();
   }

http://git-wip-us.apache.org/repos/asf/calcite/blob/0372d23b/core/src/main/java/org/apache/calcite/config/CalciteConnectionProperty.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/config/CalciteConnectionProperty.java b/core/src/main/java/org/apache/calcite/config/CalciteConnectionProperty.java
index c2e1027..86fbb6a 100644
--- a/core/src/main/java/org/apache/calcite/config/CalciteConnectionProperty.java
+++ b/core/src/main/java/org/apache/calcite/config/CalciteConnectionProperty.java
@@ -42,6 +42,10 @@ public enum CalciteConnectionProperty implements ConnectionProperty {
    * ({@code ORDER BY aggFun DESC LIMIT n}) are acceptable. */
   APPROXIMATE_TOP_N("approximateTopN", Type.BOOLEAN, false, false),
 
+  /** Whether approximate results from aggregate functions on
+   * DECIMAL types are acceptable. */
+  APPROXIMATE_DECIMAL("approximateDecimal", Type.BOOLEAN, false, false),
+
   /** Whether to store query results in temporary tables. */
   AUTO_TEMP("autoTemp", Type.BOOLEAN, false, false),
 

http://git-wip-us.apache.org/repos/asf/calcite/blob/0372d23b/druid/src/main/java/org/apache/calcite/adapter/druid/DruidQuery.java
----------------------------------------------------------------------
diff --git a/druid/src/main/java/org/apache/calcite/adapter/druid/DruidQuery.java b/druid/src/main/java/org/apache/calcite/adapter/druid/DruidQuery.java
index 9539326..c0a8a84 100644
--- a/druid/src/main/java/org/apache/calcite/adapter/druid/DruidQuery.java
+++ b/druid/src/main/java/org/apache/calcite/adapter/druid/DruidQuery.java
@@ -696,7 +696,28 @@ public class DruidQuery extends AbstractRelNode implements BindableRel {
       list.add(fieldNames.get(arg));
     }
     final String only = Iterables.getFirst(list, null);
-    final boolean b = aggCall.getType().getSqlTypeName() == SqlTypeName.DOUBLE;
+    final boolean fractional;
+    switch (aggCall.getType().getSqlTypeName().getFamily()) {
+    case APPROXIMATE_NUMERIC:
+      fractional = true;
+      break;
+    case INTEGER:
+      fractional = false;
+      break;
+    case EXACT_NUMERIC:
+      // Decimal
+      RelDataType type = aggCall.getType();
+      assert type.getSqlTypeName() == SqlTypeName.DECIMAL;
+      if (type.getScale() == 0) {
+        fractional = false;
+      } else {
+        fractional = true;
+      }
+      break;
+    default:
+      // Cannot handle this aggregate function type
+      throw new AssertionError("unknown aggregate type " + aggCall.getType());
+    }
     switch (aggCall.getAggregation().getKind()) {
     case COUNT:
       if (aggCall.isDistinct()) {
@@ -705,11 +726,11 @@ public class DruidQuery extends AbstractRelNode implements BindableRel {
       return new JsonAggregation("count", name, only);
     case SUM:
     case SUM0:
-      return new JsonAggregation(b ? "doubleSum" : "longSum", name, only);
+      return new JsonAggregation(fractional ? "doubleSum" : "longSum", name, only);
     case MIN:
-      return new JsonAggregation(b ? "doubleMin" : "longMin", name, only);
+      return new JsonAggregation(fractional ? "doubleMin" : "longMin", name, only);
     case MAX:
-      return new JsonAggregation(b ? "doubleMax" : "longMax", name, only);
+      return new JsonAggregation(fractional ? "doubleMax" : "longMax", name, only);
     default:
       throw new AssertionError("unknown aggregate " + aggCall);
     }

http://git-wip-us.apache.org/repos/asf/calcite/blob/0372d23b/druid/src/main/java/org/apache/calcite/adapter/druid/DruidRules.java
----------------------------------------------------------------------
diff --git a/druid/src/main/java/org/apache/calcite/adapter/druid/DruidRules.java b/druid/src/main/java/org/apache/calcite/adapter/druid/DruidRules.java
index dc002c2..0f3099f 100644
--- a/druid/src/main/java/org/apache/calcite/adapter/druid/DruidRules.java
+++ b/druid/src/main/java/org/apache/calcite/adapter/druid/DruidRules.java
@@ -16,6 +16,7 @@
  */
 package org.apache.calcite.adapter.druid;
 
+import org.apache.calcite.config.CalciteConnectionConfig;
 import org.apache.calcite.plan.RelOptCluster;
 import org.apache.calcite.plan.RelOptRule;
 import org.apache.calcite.plan.RelOptRuleCall;
@@ -39,6 +40,7 @@ import org.apache.calcite.rex.RexNode;
 import org.apache.calcite.rex.RexShuttle;
 import org.apache.calcite.rex.RexUtil;
 import org.apache.calcite.runtime.PredicateImpl;
+import org.apache.calcite.sql.type.SqlTypeName;
 import org.apache.calcite.util.ImmutableBitSet;
 import org.apache.calcite.util.Pair;
 import org.apache.calcite.util.Util;
@@ -46,7 +48,6 @@ import org.apache.calcite.util.trace.CalciteTrace;
 
 import com.google.common.base.Predicate;
 import com.google.common.collect.ImmutableList;
-import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
 
 import org.slf4j.Logger;
@@ -74,19 +75,42 @@ public class DruidRules {
       PROJECT, AGGREGATE, PROJECT_SORT, SORT, SORT_PROJECT);
 
   /** Predicate that returns whether Druid can not handle an aggregate. */
-  private static final Predicate<AggregateCall> BAD_AGG =
-      new PredicateImpl<AggregateCall>() {
-        public boolean test(AggregateCall aggregateCall) {
-          switch (aggregateCall.getAggregation().getKind()) {
-          case COUNT:
-          case SUM:
-          case SUM0:
-          case MIN:
-          case MAX:
-            return false;
-          default:
-            return true;
+  private static final Predicate<Aggregate> BAD_AGG =
+      new PredicateImpl<Aggregate>() {
+        public boolean test(Aggregate aggregate) {
+          final CalciteConnectionConfig config =
+                  aggregate.getCluster().getPlanner().getContext()
+                      .unwrap(CalciteConnectionConfig.class);
+          for (AggregateCall aggregateCall : aggregate.getAggCallList()) {
+            switch (aggregateCall.getAggregation().getKind()) {
+            case COUNT:
+            case SUM:
+            case SUM0:
+            case MIN:
+            case MAX:
+              switch (aggregateCall.getType().getSqlTypeName().getFamily()) {
+              case APPROXIMATE_NUMERIC:
+              case INTEGER:
+                continue;
+              case EXACT_NUMERIC:
+                // Decimal
+                RelDataType type = aggregateCall.getType();
+                assert type.getSqlTypeName() == SqlTypeName.DECIMAL;
+                if (type.getScale() == 0 || config.approximateDecimal()) {
+                  // If scale is zero or we allow approximating decimal, we can proceed
+                  continue;
+                }
+                return true;
+              default:
+                // Cannot handle this aggregate function
+                return true;
+              }
+            default:
+              // Cannot handle this aggregate function
+              return true;
+            }
           }
+          return false;
         }
       };
 
@@ -292,7 +316,7 @@ public class DruidRules {
       }
       if (aggregate.indicator
               || aggregate.getGroupSets().size() != 1
-              || Iterables.any(aggregate.getAggCallList(), BAD_AGG)
+              || BAD_AGG.apply(aggregate)
               || !validAggregate(aggregate, query)) {
         return;
       }
@@ -333,7 +357,7 @@ public class DruidRules {
       }
       if (aggregate.indicator
               || aggregate.getGroupSets().size() != 1
-              || Iterables.any(aggregate.getAggCallList(), BAD_AGG)
+              || BAD_AGG.apply(aggregate)
               || !validAggregate(aggregate, timestampIdx)) {
         return;
       }

http://git-wip-us.apache.org/repos/asf/calcite/blob/0372d23b/site/_docs/adapter.md
----------------------------------------------------------------------
diff --git a/site/_docs/adapter.md b/site/_docs/adapter.md
index a11f53c..819740b 100644
--- a/site/_docs/adapter.md
+++ b/site/_docs/adapter.md
@@ -70,6 +70,7 @@ as implemented by Avatica's
 
 | Property | Description |
 |:-------- |:------------|
+| <a href="{{ site.apiRoot }}/org/apache/calcite/config/CalciteConnectionProperty.html#APPROXIMATE_DECIMAL">approximateDecimal</a> | Whether approximate results from aggregate functions on `DECIMAL` types are acceptable
 | <a href="{{ site.apiRoot }}/org/apache/calcite/config/CalciteConnectionProperty.html#APPROXIMATE_DISTINCT_COUNT">approximateDistinctCount</a> | Whether approximate results from `COUNT(DISTINCT ...)` aggregate functions are acceptable
 | <a href="{{ site.apiRoot }}/org/apache/calcite/config/CalciteConnectionProperty.html#APPROXIMATE_TOP_N">approximateTopN</a> | Whether approximate results from "Top N" queries * (`ORDER BY aggFun() DESC LIMIT n`) are acceptable
 | <a href="{{ site.apiRoot }}/org/apache/calcite/config/CalciteConnectionProperty.html#CASE_SENSITIVE">caseSensitive</a> | Whether identifiers are matched case-sensitively. If not specified, value from `lex` is used.