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/06/19 16:07:39 UTC

[1/2] calcite git commit: [CALCITE-1819] Druid Adapter does not push the boolean operator "<>" as a filter correctly (Zain Humayun) [Forced Update!]

Repository: calcite
Updated Branches:
  refs/heads/master e162df1cf -> d47191e8c (forced update)


[CALCITE-1819] Druid Adapter does not push the boolean operator "<>" as a filter correctly (Zain Humayun)

Close apache/calcite#464


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

Branch: refs/heads/master
Commit: d47191e8c48932d888cfc79574fe1ca075eb4eeb
Parents: a088aab
Author: Zain Humayun <zh...@yahoo-inc.com>
Authored: Wed May 31 14:59:55 2017 -0700
Committer: Jesus Camacho Rodriguez <jc...@apache.org>
Committed: Mon Jun 19 17:03:15 2017 +0100

----------------------------------------------------------------------
 .../calcite/adapter/druid/DruidQuery.java       | 67 ++++++++++++--------
 .../org/apache/calcite/test/DruidAdapterIT.java | 16 ++++-
 2 files changed, 56 insertions(+), 27 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/calcite/blob/d47191e8/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 c25262e..39ce4a8 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
@@ -77,6 +77,7 @@ import java.io.IOException;
 import java.io.StringWriter;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Locale;
 import java.util.Objects;
 import java.util.Properties;
 import java.util.Set;
@@ -1023,21 +1024,21 @@ public class DruidQuery extends AbstractRelNode implements BindableRel {
 
         switch (e.getKind()) {
         case EQUALS:
-          return new JsonSelector("selector", dimName, tr(e, posConstant), extractionFunction);
+          return new JsonSelector(dimName, tr(e, posConstant), extractionFunction);
         case NOT_EQUALS:
-          return new JsonCompositeFilter("not",
-              new JsonSelector("selector", dimName, tr(e, posConstant), extractionFunction));
+          return new JsonCompositeFilter(JsonFilter.Type.NOT,
+              new JsonSelector(dimName, tr(e, posConstant), extractionFunction));
         case GREATER_THAN:
-          return new JsonBound("bound", dimName, tr(e, posConstant),
+          return new JsonBound(dimName, tr(e, posConstant),
               true, null, false, numeric, extractionFunction);
         case GREATER_THAN_OR_EQUAL:
-          return new JsonBound("bound", dimName, tr(e, posConstant),
+          return new JsonBound(dimName, tr(e, posConstant),
               false, null, false, numeric, extractionFunction);
         case LESS_THAN:
-          return new JsonBound("bound", dimName, null, false,
+          return new JsonBound(dimName, null, false,
               tr(e, posConstant), true, numeric, extractionFunction);
         case LESS_THAN_OR_EQUAL:
-          return new JsonBound("bound", dimName, null, false,
+          return new JsonBound(dimName, null, false,
               tr(e, posConstant), false, numeric, extractionFunction);
         case IN:
           ImmutableList.Builder<String> listBuilder = ImmutableList.builder();
@@ -1046,9 +1047,9 @@ public class DruidQuery extends AbstractRelNode implements BindableRel {
               listBuilder.add(((RexLiteral) rexNode).getValue3().toString());
             }
           }
-          return new JsonInFilter("in", dimName, listBuilder.build(), extractionFunction);
+          return new JsonInFilter(dimName, listBuilder.build(), extractionFunction);
         case BETWEEN:
-          return new JsonBound("bound", dimName, tr(e, 2), false,
+          return new JsonBound(dimName, tr(e, 2), false,
               tr(e, 3), false, numeric, extractionFunction);
         default:
           throw new AssertionError();
@@ -1057,7 +1058,7 @@ public class DruidQuery extends AbstractRelNode implements BindableRel {
       case OR:
       case NOT:
         call = (RexCall) e;
-        return new JsonCompositeFilter(e.getKind().lowerName,
+        return new JsonCompositeFilter(JsonFilter.Type.valueOf(e.getKind().name()),
             translateFilters(call.getOperands()));
       default:
         throw new AssertionError("cannot translate filter: " + e);
@@ -1234,9 +1235,25 @@ public class DruidQuery extends AbstractRelNode implements BindableRel {
 
   /** Filter element of a Druid "groupBy" or "topN" query. */
   private abstract static class JsonFilter implements Json {
-    final String type;
+    /**
+     * Supported filter types
+     * */
+    protected enum Type {
+      AND,
+      OR,
+      NOT,
+      SELECTOR,
+      IN,
+      BOUND;
+
+      public String lowercase() {
+        return name().toLowerCase(Locale.ROOT);
+      }
+    }
 
-    private JsonFilter(String type) {
+    final Type type;
+
+    private JsonFilter(Type type) {
       this.type = type;
     }
   }
@@ -1247,9 +1264,9 @@ public class DruidQuery extends AbstractRelNode implements BindableRel {
     private final String value;
     private final ExtractionFunction extractionFunction;
 
-    private JsonSelector(String type, String dimension, String value,
+    private JsonSelector(String dimension, String value,
         ExtractionFunction extractionFunction) {
-      super(type);
+      super(Type.SELECTOR);
       this.dimension = dimension;
       this.value = value;
       this.extractionFunction = extractionFunction;
@@ -1257,7 +1274,7 @@ public class DruidQuery extends AbstractRelNode implements BindableRel {
 
     public void write(JsonGenerator generator) throws IOException {
       generator.writeStartObject();
-      generator.writeStringField("type", type);
+      generator.writeStringField("type", type.lowercase());
       generator.writeStringField("dimension", dimension);
       generator.writeStringField("value", value);
       writeFieldIf(generator, "extractionFn", extractionFunction);
@@ -1276,10 +1293,10 @@ public class DruidQuery extends AbstractRelNode implements BindableRel {
     private final boolean alphaNumeric;
     private final ExtractionFunction extractionFunction;
 
-    private JsonBound(String type, String dimension, String lower,
+    private JsonBound(String dimension, String lower,
         boolean lowerStrict, String upper, boolean upperStrict,
         boolean alphaNumeric, ExtractionFunction extractionFunction) {
-      super(type);
+      super(Type.BOUND);
       this.dimension = dimension;
       this.lower = lower;
       this.lowerStrict = lowerStrict;
@@ -1291,7 +1308,7 @@ public class DruidQuery extends AbstractRelNode implements BindableRel {
 
     public void write(JsonGenerator generator) throws IOException {
       generator.writeStartObject();
-      generator.writeStringField("type", type);
+      generator.writeStringField("type", type.lowercase());
       generator.writeStringField("dimension", dimension);
       if (lower != null) {
         generator.writeStringField("lower", lower);
@@ -1315,21 +1332,21 @@ public class DruidQuery extends AbstractRelNode implements BindableRel {
   private static class JsonCompositeFilter extends JsonFilter {
     private final List<? extends JsonFilter> fields;
 
-    private JsonCompositeFilter(String type,
+    private JsonCompositeFilter(Type type,
         Iterable<? extends JsonFilter> fields) {
       super(type);
       this.fields = ImmutableList.copyOf(fields);
     }
 
-    private JsonCompositeFilter(String type, JsonFilter... fields) {
+    private JsonCompositeFilter(Type type, JsonFilter... fields) {
       this(type, ImmutableList.copyOf(fields));
     }
 
     public void write(JsonGenerator generator) throws IOException {
       generator.writeStartObject();
-      generator.writeStringField("type", type);
+      generator.writeStringField("type", type.lowercase());
       switch (type) {
-      case "NOT":
+      case NOT:
         writeField(generator, "field", fields.get(0));
         break;
       default:
@@ -1345,9 +1362,9 @@ public class DruidQuery extends AbstractRelNode implements BindableRel {
     private final List<String> values;
     private final ExtractionFunction extractionFunction;
 
-    private JsonInFilter(String type, String dimension, List<String> values,
+    private JsonInFilter(String dimension, List<String> values,
         ExtractionFunction extractionFunction) {
-      super(type);
+      super(Type.IN);
       this.dimension = dimension;
       this.values = values;
       this.extractionFunction = extractionFunction;
@@ -1355,7 +1372,7 @@ public class DruidQuery extends AbstractRelNode implements BindableRel {
 
     public void write(JsonGenerator generator) throws IOException {
       generator.writeStartObject();
-      generator.writeStringField("type", type);
+      generator.writeStringField("type", type.lowercase());
       generator.writeStringField("dimension", dimension);
       writeField(generator, "values", values);
       writeFieldIf(generator, "extractionFn", extractionFunction);

http://git-wip-us.apache.org/repos/asf/calcite/blob/d47191e8/druid/src/test/java/org/apache/calcite/test/DruidAdapterIT.java
----------------------------------------------------------------------
diff --git a/druid/src/test/java/org/apache/calcite/test/DruidAdapterIT.java b/druid/src/test/java/org/apache/calcite/test/DruidAdapterIT.java
index 719d3bd..da905b2 100644
--- a/druid/src/test/java/org/apache/calcite/test/DruidAdapterIT.java
+++ b/druid/src/test/java/org/apache/calcite/test/DruidAdapterIT.java
@@ -2155,10 +2155,9 @@ public class DruidAdapterIT {
         .queryContains(druidChecker("'queryType':'timeseries'"));
   }
 
-
   /**
    * <a href="https://issues.apache.org/jira/browse/CALCITE-1805">[CALCITE-1805]
-   * Druid adapter can not handel count column without adding support for nested queries</a>.
+   * Druid adapter cannot handle count column without adding support for nested queries</a>.
    */
   @Test public void testCountColumn() {
 
@@ -2175,6 +2174,19 @@ public class DruidAdapterIT {
         + "intervals=[[1900-01-01T00:00:00.000/3000-01-01T00:00:00.000]], projects=[[$7]])");
   }
 
+  /**
+   * Test to make sure the "not" filter has only 1 field, rather than an array of fields.
+   */
+  @Test public void testNotFilterForm() {
+    String sql = "select count(distinct \"the_month\") from "
+            + "\"foodmart\" where \"the_month\" <> \'October\'";
+    String druidFilter = "'filter':{'type':'not',"
+            + "'field':{'type':'selector','dimension':'the_month','value':'October'}}";
+    // Check that the filter actually worked, and that druid was responsible for the filter
+    sql(sql, FOODMART)
+            .queryContains(druidChecker(druidFilter))
+            .returnsOrdered("EXPR$0=11");
+  }
 }
 
 // End DruidAdapterIT.java


[2/2] calcite git commit: [CALCITE-1805] Druid adapter incorrectly pushes down "COUNT(c)"; Druid only supports "COUNT(*)"

Posted by jc...@apache.org.
[CALCITE-1805] Druid adapter incorrectly pushes down "COUNT(c)"; Druid only supports "COUNT(*)"

Fixes DruidAdapterIT.testGroupByAvgSumCount and DruidAdapterIT.testTimeExtractThatCannotBePushed


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

Branch: refs/heads/master
Commit: a088aab78655e138353f40b57bebaf3cf3aec939
Parents: 12e020e
Author: Jesus Camacho Rodriguez <jc...@apache.org>
Authored: Mon Jun 19 15:29:11 2017 +0100
Committer: Jesus Camacho Rodriguez <jc...@apache.org>
Committed: Mon Jun 19 17:03:15 2017 +0100

----------------------------------------------------------------------
 .../calcite/interpreter/AggregateNode.java      | 22 ++++++++++++++++----
 .../org/apache/calcite/test/DruidAdapterIT.java | 15 ++++---------
 2 files changed, 22 insertions(+), 15 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/calcite/blob/a088aab7/core/src/main/java/org/apache/calcite/interpreter/AggregateNode.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/interpreter/AggregateNode.java b/core/src/main/java/org/apache/calcite/interpreter/AggregateNode.java
index 5a5d265..bc488e3 100644
--- a/core/src/main/java/org/apache/calcite/interpreter/AggregateNode.java
+++ b/core/src/main/java/org/apache/calcite/interpreter/AggregateNode.java
@@ -118,7 +118,8 @@ public class AggregateNode extends AbstractSingleNode<Aggregate> {
           return new CountAccumulator(call);
         }
       };
-    } else if (call.getAggregation() == SqlStdOperatorTable.SUM) {
+    } else if (call.getAggregation() == SqlStdOperatorTable.SUM
+        || call.getAggregation() == SqlStdOperatorTable.SUM0) {
       final Class<?> clazz;
       switch (call.type.getSqlTypeName()) {
       case DOUBLE:
@@ -134,8 +135,13 @@ public class AggregateNode extends AbstractSingleNode<Aggregate> {
         clazz = LongSum.class;
         break;
       }
-      return new UdaAccumulatorFactory(
-          AggregateFunctionImpl.create(clazz), call);
+      if (call.getAggregation() == SqlStdOperatorTable.SUM) {
+        return new UdaAccumulatorFactory(
+            AggregateFunctionImpl.create(clazz), call, true);
+      } else {
+        return new UdaAccumulatorFactory(
+            AggregateFunctionImpl.create(clazz), call, false);
+      }
     } else {
       final JavaTypeFactory typeFactory =
           (JavaTypeFactory) rel.getCluster().getTypeFactory();
@@ -436,9 +442,10 @@ public class AggregateNode extends AbstractSingleNode<Aggregate> {
     final AggregateFunctionImpl aggFunction;
     final int argOrdinal;
     public final Object instance;
+    public final boolean nullIfEmpty;
 
     UdaAccumulatorFactory(AggregateFunctionImpl aggFunction,
-        AggregateCall call) {
+        AggregateCall call, boolean nullIfEmpty) {
       this.aggFunction = aggFunction;
       if (call.getArgList().size() != 1) {
         throw new UnsupportedOperationException("in current implementation, "
@@ -457,6 +464,7 @@ public class AggregateNode extends AbstractSingleNode<Aggregate> {
           throw new RuntimeException(e);
         }
       }
+      this.nullIfEmpty = nullIfEmpty;
     }
 
     public Accumulator get() {
@@ -468,6 +476,7 @@ public class AggregateNode extends AbstractSingleNode<Aggregate> {
   private static class UdaAccumulator implements Accumulator {
     private final UdaAccumulatorFactory factory;
     private Object value;
+    private boolean empty;
 
     UdaAccumulator(UdaAccumulatorFactory factory) {
       this.factory = factory;
@@ -476,6 +485,7 @@ public class AggregateNode extends AbstractSingleNode<Aggregate> {
       } catch (IllegalAccessException | InvocationTargetException e) {
         throw new RuntimeException(e);
       }
+      this.empty = true;
     }
 
     public void send(Row row) {
@@ -490,9 +500,13 @@ public class AggregateNode extends AbstractSingleNode<Aggregate> {
       } catch (IllegalAccessException | InvocationTargetException e) {
         throw new RuntimeException(e);
       }
+      empty = false;
     }
 
     public Object end() {
+      if (factory.nullIfEmpty && empty) {
+        return null;
+      }
       final Object[] args = {value};
       try {
         return factory.aggFunction.resultMethod.invoke(factory.instance, args);

http://git-wip-us.apache.org/repos/asf/calcite/blob/a088aab7/druid/src/test/java/org/apache/calcite/test/DruidAdapterIT.java
----------------------------------------------------------------------
diff --git a/druid/src/test/java/org/apache/calcite/test/DruidAdapterIT.java b/druid/src/test/java/org/apache/calcite/test/DruidAdapterIT.java
index 0cd7a4b..719d3bd 100644
--- a/druid/src/test/java/org/apache/calcite/test/DruidAdapterIT.java
+++ b/druid/src/test/java/org/apache/calcite/test/DruidAdapterIT.java
@@ -1071,17 +1071,11 @@ public class DruidAdapterIT {
         + "from \"foodmart\"\n"
         + "group by \"state_province\"\n"
         + "order by 1";
-    String druidQuery = "'aggregations':["
-        + "{'type':'longSum','name':'$f1','fieldName':'unit_sales'},"
-        + "{'type':'count','name':'$f2','fieldName':'unit_sales'},"
-        + "{'type':'count','name':'C','fieldName':'store_sqft'},"
-        + "{'type':'count','name':'C0'}],"
-        + "'intervals':['1900-01-09T00:00:00.000/2992-01-10T00:00:00.000']}";
     sql(sql)
         .limit(2)
-        .returnsUnordered("state_province=CA; A=3; S=74748; C=24441; C0=24441",
+        .returnsUnordered("state_province=CA; A=3; S=74748; C=16347; C0=24441",
             "state_province=OR; A=3; S=67659; C=21610; C0=21610")
-        .queryContains(druidChecker(druidQuery));
+        .queryContains(druidChecker("'queryType':'select'"));
   }
 
   @Test public void testGroupByMonthGranularity() {
@@ -2075,13 +2069,12 @@ public class DruidAdapterIT {
         + "\"product_id\" = 1558 group by extract(CENTURY from \"timestamp\")";
     final String plan = "PLAN=EnumerableInterpreter\n"
         + "  BindableAggregate(group=[{0}])\n"
-        + "    BindableProject(EXPR$0=[/INT(EXTRACT_DATE(FLAG(YEAR), /INT(Reinterpret($0), "
-        + "86400000)), 100)])\n"
+        + "    BindableProject(EXPR$0=[EXTRACT_DATE(FLAG(CENTURY), /INT(Reinterpret($0), 86400000))])\n"
         + "      DruidQuery(table=[[foodmart, foodmart]], "
         + "intervals=[[1900-01-09T00:00:00.000/2992-01-10T00:00:00.000]], filter=[=($1, 1558)], "
         + "projects=[[$0]])";
     sql(sql).explainContains(plan).queryContains(druidChecker("'queryType':'select'"))
-        .returnsUnordered("EXPR$0=19");
+        .returnsUnordered("EXPR$0=20");
   }
 
   /** Test case for