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: us...@infra.apache.org With regards, Apache Git Services --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org