You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2018/08/27 05:00:27 UTC

[GitHub] fjy closed pull request #6240: [Backport] SQL: Fix precision of TIMESTAMP types.

fjy closed pull request #6240: [Backport] SQL: Fix precision of TIMESTAMP types.
URL: https://github.com/apache/incubator-druid/pull/6240
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/sql/src/main/java/io/druid/sql/calcite/expression/OperatorConversions.java b/sql/src/main/java/io/druid/sql/calcite/expression/OperatorConversions.java
index c129388d2a8..f75186d4ab5 100644
--- a/sql/src/main/java/io/druid/sql/calcite/expression/OperatorConversions.java
+++ b/sql/src/main/java/io/druid/sql/calcite/expression/OperatorConversions.java
@@ -20,6 +20,7 @@
 package io.druid.sql.calcite.expression;
 
 import com.google.common.base.Preconditions;
+import io.druid.sql.calcite.planner.Calcites;
 import io.druid.sql.calcite.planner.PlannerContext;
 import io.druid.sql.calcite.table.RowSignature;
 import org.apache.calcite.rex.RexCall;
@@ -131,18 +132,16 @@ public OperatorBuilder kind(final SqlKind kind)
 
     public OperatorBuilder returnType(final SqlTypeName typeName)
     {
-      this.returnTypeInference = ReturnTypes.explicit(typeName);
+      this.returnTypeInference = ReturnTypes.explicit(
+          factory -> Calcites.createSqlType(factory, typeName)
+      );
       return this;
     }
 
     public OperatorBuilder nullableReturnType(final SqlTypeName typeName)
     {
       this.returnTypeInference = ReturnTypes.explicit(
-          factory ->
-              factory.createTypeWithNullability(
-                  factory.createSqlType(typeName),
-                  true
-              )
+          factory -> Calcites.createSqlTypeWithNullability(factory, typeName, true)
       );
       return this;
     }
diff --git a/sql/src/main/java/io/druid/sql/calcite/planner/Calcites.java b/sql/src/main/java/io/druid/sql/calcite/planner/Calcites.java
index bf39def562b..dfcfad96868 100644
--- a/sql/src/main/java/io/druid/sql/calcite/planner/Calcites.java
+++ b/sql/src/main/java/io/druid/sql/calcite/planner/Calcites.java
@@ -32,10 +32,13 @@
 import io.druid.sql.calcite.schema.DruidSchema;
 import io.druid.sql.calcite.schema.InformationSchema;
 import org.apache.calcite.jdbc.CalciteSchema;
+import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.rel.type.RelDataTypeFactory;
 import org.apache.calcite.rex.RexLiteral;
 import org.apache.calcite.rex.RexNode;
 import org.apache.calcite.schema.Schema;
 import org.apache.calcite.schema.SchemaPlus;
+import org.apache.calcite.sql.SqlCollation;
 import org.apache.calcite.sql.SqlKind;
 import org.apache.calcite.sql.type.SqlTypeName;
 import org.apache.calcite.util.ConversionUtil;
@@ -163,6 +166,46 @@ public static StringComparator getStringComparatorForValueType(ValueType valueTy
     }
   }
 
+  /**
+   * Like RelDataTypeFactory.createSqlType, but creates types that align best with how Druid represents them.
+   */
+  public static RelDataType createSqlType(final RelDataTypeFactory typeFactory, final SqlTypeName typeName)
+  {
+    return createSqlTypeWithNullability(typeFactory, typeName, false);
+  }
+
+  /**
+   * Like RelDataTypeFactory.createSqlTypeWithNullability, but creates types that align best with how Druid
+   * represents them.
+   */
+  public static RelDataType createSqlTypeWithNullability(
+      final RelDataTypeFactory typeFactory,
+      final SqlTypeName typeName,
+      final boolean nullable
+  )
+  {
+    final RelDataType dataType;
+
+    switch (typeName) {
+      case TIMESTAMP:
+        // Our timestamps are down to the millisecond (precision = 3).
+        dataType = typeFactory.createSqlType(typeName, 3);
+        break;
+      case CHAR:
+      case VARCHAR:
+        dataType = typeFactory.createTypeWithCharsetAndCollation(
+            typeFactory.createSqlType(typeName),
+            Calcites.defaultCharset(),
+            SqlCollation.IMPLICIT
+        );
+        break;
+      default:
+        dataType = typeFactory.createSqlType(typeName);
+    }
+
+    return typeFactory.createTypeWithNullability(dataType, nullable);
+  }
+
   /**
    * Calcite expects "TIMESTAMP" types to be an instant that has the expected local time fields if printed as UTC.
    *
diff --git a/sql/src/main/java/io/druid/sql/calcite/planner/DruidPlanner.java b/sql/src/main/java/io/druid/sql/calcite/planner/DruidPlanner.java
index 8779d793e06..4a9feae0cd2 100644
--- a/sql/src/main/java/io/druid/sql/calcite/planner/DruidPlanner.java
+++ b/sql/src/main/java/io/druid/sql/calcite/planner/DruidPlanner.java
@@ -321,7 +321,7 @@ private PlannerResult planExplanation(
     return new PlannerResult(
         resultsSupplier,
         typeFactory.createStructType(
-            ImmutableList.of(typeFactory.createSqlType(SqlTypeName.VARCHAR)),
+            ImmutableList.of(Calcites.createSqlType(typeFactory, SqlTypeName.VARCHAR)),
             ImmutableList.of("PLAN")
         )
     );
diff --git a/sql/src/main/java/io/druid/sql/calcite/planner/DruidTypeSystem.java b/sql/src/main/java/io/druid/sql/calcite/planner/DruidTypeSystem.java
index cdeff82aa3f..fced0096e44 100644
--- a/sql/src/main/java/io/druid/sql/calcite/planner/DruidTypeSystem.java
+++ b/sql/src/main/java/io/druid/sql/calcite/planner/DruidTypeSystem.java
@@ -93,9 +93,9 @@ public RelDataType deriveSumType(final RelDataTypeFactory typeFactory, final Rel
     // Widen all sums to 64-bits regardless of the size of the inputs.
 
     if (SqlTypeName.INT_TYPES.contains(argumentType.getSqlTypeName())) {
-      return typeFactory.createSqlType(SqlTypeName.BIGINT);
+      return Calcites.createSqlType(typeFactory, SqlTypeName.BIGINT);
     } else {
-      return typeFactory.createSqlType(SqlTypeName.DOUBLE);
+      return Calcites.createSqlType(typeFactory, SqlTypeName.DOUBLE);
     }
   }
 
diff --git a/sql/src/main/java/io/druid/sql/calcite/rule/CaseFilteredAggregatorRule.java b/sql/src/main/java/io/druid/sql/calcite/rule/CaseFilteredAggregatorRule.java
index 4635addcbd3..2f4841eded9 100644
--- a/sql/src/main/java/io/druid/sql/calcite/rule/CaseFilteredAggregatorRule.java
+++ b/sql/src/main/java/io/druid/sql/calcite/rule/CaseFilteredAggregatorRule.java
@@ -111,7 +111,7 @@ public void onMatch(RelOptRuleCall call)
 
           // Operand 1: Filter
           final RexNode filter;
-          final RelDataType booleanType = typeFactory.createSqlType(SqlTypeName.BOOLEAN);
+          final RelDataType booleanType = Calcites.createSqlType(typeFactory, SqlTypeName.BOOLEAN);
           final RexNode filterFromCase = rexBuilder.makeCall(
               booleanType,
               flip ? SqlStdOperatorTable.IS_FALSE : SqlStdOperatorTable.IS_TRUE,
@@ -177,7 +177,7 @@ public void onMatch(RelOptRuleCall call)
                   false,
                   ImmutableList.of(),
                   newProjects.size() - 1,
-                  typeFactory.createSqlType(SqlTypeName.BIGINT),
+                  Calcites.createSqlType(typeFactory, SqlTypeName.BIGINT),
                   aggregateCall.getName()
               );
             } else if (RexLiteral.isNullLiteral(arg2) /* Case A1 */
diff --git a/sql/src/main/java/io/druid/sql/calcite/table/RowSignature.java b/sql/src/main/java/io/druid/sql/calcite/table/RowSignature.java
index 5f0a8e071ed..1e4b779170e 100644
--- a/sql/src/main/java/io/druid/sql/calcite/table/RowSignature.java
+++ b/sql/src/main/java/io/druid/sql/calcite/table/RowSignature.java
@@ -36,7 +36,6 @@
 import org.apache.calcite.rel.type.RelDataType;
 import org.apache.calcite.rel.type.RelDataTypeFactory;
 import org.apache.calcite.rel.type.RelDataTypeField;
-import org.apache.calcite.sql.SqlCollation;
 import org.apache.calcite.sql.type.SqlTypeName;
 
 import javax.annotation.Nonnull;
@@ -152,35 +151,25 @@ public RelDataType getRelDataType(final RelDataTypeFactory typeFactory)
       final RelDataType type;
 
       if (Column.TIME_COLUMN_NAME.equals(columnName)) {
-        type = typeFactory.createSqlType(SqlTypeName.TIMESTAMP);
+        type = Calcites.createSqlType(typeFactory, SqlTypeName.TIMESTAMP);
       } else {
         switch (columnType) {
           case STRING:
             // Note that there is no attempt here to handle multi-value in any special way. Maybe one day...
-            type = typeFactory.createTypeWithNullability(
-                typeFactory.createTypeWithCharsetAndCollation(
-                    typeFactory.createSqlType(SqlTypeName.VARCHAR),
-                    Calcites.defaultCharset(),
-                    SqlCollation.IMPLICIT
-                ),
-                true
-            );
+            type = Calcites.createSqlTypeWithNullability(typeFactory, SqlTypeName.VARCHAR, true);
             break;
           case LONG:
-            type = typeFactory.createSqlType(SqlTypeName.BIGINT);
+            type = Calcites.createSqlType(typeFactory, SqlTypeName.BIGINT);
             break;
           case FLOAT:
-            type = typeFactory.createSqlType(SqlTypeName.FLOAT);
+            type = Calcites.createSqlType(typeFactory, SqlTypeName.FLOAT);
             break;
           case DOUBLE:
-            type = typeFactory.createSqlType(SqlTypeName.DOUBLE);
+            type = Calcites.createSqlType(typeFactory, SqlTypeName.DOUBLE);
             break;
           case COMPLEX:
             // Loses information about exactly what kind of complex column this is.
-            type = typeFactory.createTypeWithNullability(
-                typeFactory.createSqlType(SqlTypeName.OTHER),
-                true
-            );
+            type = Calcites.createSqlTypeWithNullability(typeFactory, SqlTypeName.OTHER, true);
             break;
           default:
             throw new ISE("WTF?! valueType[%s] not translatable?", columnType);
diff --git a/sql/src/test/java/io/druid/sql/calcite/CalciteQueryTest.java b/sql/src/test/java/io/druid/sql/calcite/CalciteQueryTest.java
index 97785962710..842a76a8aa5 100644
--- a/sql/src/test/java/io/druid/sql/calcite/CalciteQueryTest.java
+++ b/sql/src/test/java/io/druid/sql/calcite/CalciteQueryTest.java
@@ -3000,6 +3000,33 @@ public void testRemoveUselessCaseWhen() throws Exception
     );
   }
 
+  @Test
+  public void testCountStarWithTimeMillisecondFilters() throws Exception
+  {
+    testQuery(
+        "SELECT COUNT(*) FROM druid.foo\n"
+        + "WHERE __time = TIMESTAMP '2000-01-01 00:00:00.111'\n"
+        + "OR (__time >= TIMESTAMP '2000-01-01 00:00:00.888' AND __time < TIMESTAMP '2000-01-02 00:00:00.222')",
+        ImmutableList.of(
+            Druids.newTimeseriesQueryBuilder()
+                  .dataSource(CalciteTests.DATASOURCE1)
+                  .intervals(
+                      QSS(
+                          Intervals.of("2000-01-01T00:00:00.111/2000-01-01T00:00:00.112"),
+                          Intervals.of("2000-01-01T00:00:00.888/2000-01-02T00:00:00.222")
+                      )
+                  )
+                  .granularity(Granularities.ALL)
+                  .aggregators(AGGS(new CountAggregatorFactory("a0")))
+                  .context(TIMESERIES_CONTEXT_DEFAULT)
+                  .build()
+        ),
+        ImmutableList.of(
+            new Object[]{1L}
+        )
+    );
+  }
+
   @Test
   public void testCountStarWithTimeFilterUsingStringLiterals() throws Exception
   {


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org