You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by fj...@apache.org on 2018/08/27 05:00:27 UTC
[incubator-druid] branch 0.12.3 updated: SQL: Fix precision of
TIMESTAMP types. (#5464) (#6240)
This is an automated email from the ASF dual-hosted git repository.
fjy pushed a commit to branch 0.12.3
in repository https://gitbox.apache.org/repos/asf/incubator-druid.git
The following commit(s) were added to refs/heads/0.12.3 by this push:
new 3a63540 SQL: Fix precision of TIMESTAMP types. (#5464) (#6240)
3a63540 is described below
commit 3a63540e9e6c1201e8a355bc22da733a8542753d
Author: Gian Merlino <gi...@gmail.com>
AuthorDate: Sun Aug 26 22:00:25 2018 -0700
SQL: Fix precision of TIMESTAMP types. (#5464) (#6240)
Druid stores timestamps down to the millisecond, so we should use
precision = 3. Setting this wrong sometimes caused milliseconds
to be ignored in timestamp literals.
Fixes #5337.
---
.../calcite/expression/OperatorConversions.java | 11 +++---
.../io/druid/sql/calcite/planner/Calcites.java | 43 ++++++++++++++++++++++
.../io/druid/sql/calcite/planner/DruidPlanner.java | 2 +-
.../druid/sql/calcite/planner/DruidTypeSystem.java | 4 +-
.../calcite/rule/CaseFilteredAggregatorRule.java | 4 +-
.../io/druid/sql/calcite/table/RowSignature.java | 23 +++---------
.../io/druid/sql/calcite/CalciteQueryTest.java | 27 ++++++++++++++
7 files changed, 86 insertions(+), 28 deletions(-)
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 c129388..f75186d 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 class OperatorConversions
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 bf39def..dfcfad9 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.server.security.AuthorizerMapper;
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;
@@ -164,6 +167,46 @@ public class Calcites
}
/**
+ * 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.
*
* @param dateTime joda timestamp
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 8779d79..4a9feae 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 @@ public class DruidPlanner implements Closeable
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 cdeff82..fced009 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 class DruidTypeSystem implements RelDataTypeSystem
// 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 4635add..2f4841e 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 class CaseFilteredAggregatorRule extends RelOptRule
// 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 class CaseFilteredAggregatorRule extends RelOptRule
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 5f0a8e0..1e4b779 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 io.druid.sql.calcite.planner.Calcites;
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 class RowSignature
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 9778596..842a76a 100644
--- a/sql/src/test/java/io/druid/sql/calcite/CalciteQueryTest.java
+++ b/sql/src/test/java/io/druid/sql/calcite/CalciteQueryTest.java
@@ -3001,6 +3001,33 @@ public class CalciteQueryTest extends CalciteTestBase
}
@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
{
// Strings are implicitly cast to timestamps. Test a few different forms.
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org