This is an automated email from the ASF dual-hosted git repository. dzamo pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/drill.git
The following commit(s) were added to refs/heads/master by this push: new ab7f9e999d DRILL-8136: Overhaul implict type casting logic (#2638) ab7f9e999d is described below commit ab7f9e999d97eff7565055aa190d54bccdd6b6b6 Author: James Turton <9107319+jntur...@users.noreply.github.com> AuthorDate: Tue Sep 20 06:42:53 2022 +0200 DRILL-8136: Overhaul implict type casting logic (#2638) --- .../src/main/codegen/data/NumericTypes.tdd | 6 + .../templates/NumericFunctionsTemplates.java | 6 + .../exec/expr/ExpressionTreeMaterializer.java | 3 +- .../exec/expr/fn/impl/ByteArrayFunctions.java | 102 ++++++ .../drill/exec/expr/fn/impl/ByteSubstring.java | 81 ----- .../drill/exec/expr/fn/impl/SchemaFunctions.java | 6 +- .../drill/exec/physical/impl/join/JoinUtils.java | 5 +- .../physical/impl/union/UnionAllRecordBatch.java | 17 +- .../drill/exec/planner/common/DrillRelOptUtil.java | 10 +- .../sql/DrillCalciteSqlBetweenOperatorWrapper.java | 5 +- .../drill/exec/planner/sql/TypeInferenceUtils.java | 149 +++++---- .../exec/resolver/DefaultFunctionResolver.java | 58 ++-- .../drill/exec/resolver/ExactFunctionResolver.java | 4 +- .../exec/resolver/ResolverTypePrecedence.java | 345 +++++++++++++-------- .../apache/drill/exec/resolver/TypeCastRules.java | 182 +++++------ .../store/parquet/ParquetTableMetadataUtils.java | 6 +- .../java/org/apache/drill/TestImplicitCasting.java | 217 ++++++++++++- .../physical/impl/TestReverseImplicitCast.java | 4 +- .../drill/exec/store/json/TestJsonReaderFns.java | 20 +- .../functions/cast/two_way_implicit_cast.json | 4 +- .../org/apache/drill/common/types/TypeProtos.java | 4 +- protocol/src/main/protobuf/Types.proto | 2 +- 22 files changed, 769 insertions(+), 467 deletions(-) diff --git a/exec/java-exec/src/main/codegen/data/NumericTypes.tdd b/exec/java-exec/src/main/codegen/data/NumericTypes.tdd index 6b28c1a1f0..8cca678df6 100644 --- a/exec/java-exec/src/main/codegen/data/NumericTypes.tdd +++ b/exec/java-exec/src/main/codegen/data/NumericTypes.tdd @@ -37,6 +37,12 @@ {inputType: "NullableUInt4", intype: "numeric"}, {inputType: "UInt8", intype: "numeric"}, {inputType: "NullableUInt8", intype: "numeric"}, + {inputType: "Float4", intype: "numeric"}, + {inputType: "NullableFloat4", intype: "numeric"}, + {inputType: "Float8", intype: "numeric"}, + {inputType: "NullableFloat8", intype: "numeric"}, + {inputType: "VarDecimal", intype: "numeric"}, + {inputType: "NullableVarDecimal", intype: "numeric"}, {inputType: "VarChar", intype: "char"}, {inputType: "NullableVarChar", intype: "char"} ] diff --git a/exec/java-exec/src/main/codegen/templates/NumericFunctionsTemplates.java b/exec/java-exec/src/main/codegen/templates/NumericFunctionsTemplates.java index b26e9eee18..5ad467ab0a 100644 --- a/exec/java-exec/src/main/codegen/templates/NumericFunctionsTemplates.java +++ b/exec/java-exec/src/main/codegen/templates/NumericFunctionsTemplates.java @@ -55,6 +55,12 @@ import org.apache.drill.exec.expr.holders.UInt4Holder; import org.apache.drill.exec.expr.holders.NullableUInt4Holder; import org.apache.drill.exec.expr.holders.UInt8Holder; import org.apache.drill.exec.expr.holders.NullableUInt8Holder; +import org.apache.drill.exec.expr.holders.Float4Holder; +import org.apache.drill.exec.expr.holders.NullableFloat4Holder; +import org.apache.drill.exec.expr.holders.Float8Holder; +import org.apache.drill.exec.expr.holders.NullableFloat8Holder; +import org.apache.drill.exec.expr.holders.VarDecimalHolder; +import org.apache.drill.exec.expr.holders.NullableVarDecimalHolder; import org.apache.drill.exec.expr.holders.VarCharHolder; import org.apache.drill.exec.expr.holders.NullableVarCharHolder; import org.apache.drill.exec.record.RecordBatch; diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java index b242feec6d..c5b3443ba2 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java @@ -19,7 +19,6 @@ package org.apache.drill.exec.expr; import java.util.ArrayDeque; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collections; import java.util.Deque; import java.util.List; @@ -721,7 +720,7 @@ public class ExpressionTreeMaterializer { // Check if we need a cast if (thenType != elseType && !(thenType == MinorType.NULL || elseType == MinorType.NULL)) { - MinorType leastRestrictive = TypeCastRules.getLeastRestrictiveType((Arrays.asList(thenType, elseType))); + MinorType leastRestrictive = TypeCastRules.getLeastRestrictiveType(thenType, elseType); if (leastRestrictive != thenType) { // Implicitly cast the then expression conditions = new IfExpression.IfCondition(newCondition, diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/ByteArrayFunctions.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/ByteArrayFunctions.java new file mode 100644 index 0000000000..0142fc33ed --- /dev/null +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/ByteArrayFunctions.java @@ -0,0 +1,102 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.drill.exec.expr.fn.impl; + +import org.apache.drill.exec.expr.DrillSimpleFunc; +import org.apache.drill.exec.expr.annotations.FunctionTemplate; +import org.apache.drill.exec.expr.annotations.Output; +import org.apache.drill.exec.expr.annotations.Param; +import org.apache.drill.exec.expr.holders.BigIntHolder; +import org.apache.drill.exec.expr.holders.VarBinaryHolder; + +public class ByteArrayFunctions { + private ByteArrayFunctions() { + } + + // TODO: implement optional length parameter + /** + * Evaluate a substring expression for a given value; specifying the start + * position, and optionally the end position. + * + * - If the start position is negative, start from abs(start) characters from + * the end of the buffer. + * + * - If no length is specified, continue to the end of the string. + * + * - If the substring expression's length exceeds the value's upward bound, the + * value's length will be used. + * + * - If the substring is invalid, return an empty string. + */ + @FunctionTemplate(names = {"bytesubstring", "byte_substr"}, + scope = FunctionTemplate.FunctionScope.SIMPLE, + nulls = FunctionTemplate.NullHandling.NULL_IF_NULL, + outputWidthCalculatorType = FunctionTemplate.OutputWidthCalculatorType.CUSTOM_CLONE_DEFAULT) + public static class ByteSubstring implements DrillSimpleFunc { + + @Param VarBinaryHolder in; + @Param BigIntHolder offset; + @Param BigIntHolder length; + @Output VarBinaryHolder out; + + @Override + public void setup() { } + + @Override + public void eval() { + out.buffer = in.buffer; + + // handle invalid values; e.g. SUBSTRING(value, 0, x) or SUBSTRING(value, x, 0) + if (offset.value == 0 || length.value <= 0) { + out.start = 0; + out.end = 0; + } else { + // handle negative and positive offset values + if (offset.value < 0) { + out.start = in.end + (int)offset.value; + } else { + out.start = in.start + (int)offset.value - 1; + } + // calculate end position from length and truncate to upper value bounds + if (out.start + length.value > in.end) { + out.end = in.end; + } else { + out.end = out.start + (int)length.value; + } + } + } + } + + @FunctionTemplate( + name = "length", + scope = FunctionTemplate.FunctionScope.SIMPLE, + nulls = FunctionTemplate.NullHandling.NULL_IF_NULL + ) + public static class ByteArrLength implements DrillSimpleFunc { + @Param VarBinaryHolder input; + @Output BigIntHolder out; + + @Override + public void setup() {} + + @Override + public void eval() { + out.value = input.end - input.start; + } + } +} diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/ByteSubstring.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/ByteSubstring.java deleted file mode 100644 index e1d02584bf..0000000000 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/ByteSubstring.java +++ /dev/null @@ -1,81 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.apache.drill.exec.expr.fn.impl; - -import org.apache.drill.exec.expr.DrillSimpleFunc; -import org.apache.drill.exec.expr.annotations.FunctionTemplate; -import org.apache.drill.exec.expr.annotations.Output; -import org.apache.drill.exec.expr.annotations.Param; -import org.apache.drill.exec.expr.holders.BigIntHolder; -import org.apache.drill.exec.expr.holders.VarBinaryHolder; - -// TODO: implement optional length parameter - -/** - * Evaluate a substring expression for a given value; specifying the start - * position, and optionally the end position. - * - * - If the start position is negative, start from abs(start) characters from - * the end of the buffer. - * - * - If no length is specified, continue to the end of the string. - * - * - If the substring expression's length exceeds the value's upward bound, the - * value's length will be used. - * - * - If the substring is invalid, return an empty string. - */ -@FunctionTemplate(names = {"bytesubstring", "byte_substr"}, - scope = FunctionTemplate.FunctionScope.SIMPLE, - nulls = FunctionTemplate.NullHandling.NULL_IF_NULL, - outputWidthCalculatorType = FunctionTemplate.OutputWidthCalculatorType.CUSTOM_CLONE_DEFAULT) -public class ByteSubstring implements DrillSimpleFunc { - - @Param VarBinaryHolder in; - @Param BigIntHolder offset; - @Param BigIntHolder length; - @Output VarBinaryHolder out; - - @Override - public void setup() { } - - @Override - public void eval() { - out.buffer = in.buffer; - - // handle invalid values; e.g. SUBSTRING(value, 0, x) or SUBSTRING(value, x, 0) - if (offset.value == 0 || length.value <= 0) { - out.start = 0; - out.end = 0; - } else { - // handle negative and positive offset values - if (offset.value < 0) { - out.start = in.end + (int)offset.value; - } else { - out.start = in.start + (int)offset.value - 1; - } - // calculate end position from length and truncate to upper value bounds - if (out.start + length.value > in.end) { - out.end = in.end; - } else { - out.end = out.start + (int)length.value; - } - } - } - -} diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/SchemaFunctions.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/SchemaFunctions.java index 7f747bb6c3..bfcbd2d183 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/SchemaFunctions.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/SchemaFunctions.java @@ -72,10 +72,12 @@ public class SchemaFunctions { if (materializedField != null && !materializedField.getType().equals(type)) { org.apache.drill.common.types.TypeProtos.MinorType leastRestrictiveType = org.apache.drill.exec.resolver.TypeCastRules.getLeastRestrictiveType( - java.util.Arrays.asList(materializedField.getType().getMinorType(), type.getMinorType())); + materializedField.getType().getMinorType(), type.getMinorType()); org.apache.drill.common.types.TypeProtos.DataMode leastRestrictiveMode = org.apache.drill.exec.resolver.TypeCastRules.getLeastRestrictiveDataMode( - java.util.Arrays.asList(materializedField.getType().getMode(), type.getMode())); + materializedField.getType().getMode(), + type.getMode() + ); org.apache.drill.exec.record.MaterializedField clone = materializedField.clone(); clone.replaceType(materializedField.getType().toBuilder() diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/JoinUtils.java b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/JoinUtils.java index fc5ca527e3..5cb279d864 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/JoinUtils.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/JoinUtils.java @@ -212,10 +212,7 @@ public class JoinUtils { } // We need to add a cast to one of the expressions - List<TypeProtos.MinorType> types = new LinkedList<>(); - types.add(rightType); - types.add(leftType); - TypeProtos.MinorType result = TypeCastRules.getLeastRestrictiveType(types); + TypeProtos.MinorType result = TypeCastRules.getLeastRestrictiveType(leftType, rightType); ErrorCollector errorCollector = new ErrorCollectorImpl(); if (result == null) { diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/union/UnionAllRecordBatch.java b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/union/UnionAllRecordBatch.java index c8b9879d1f..07d916d586 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/union/UnionAllRecordBatch.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/union/UnionAllRecordBatch.java @@ -60,7 +60,6 @@ import org.apache.drill.exec.vector.FixedWidthVector; import org.apache.drill.exec.vector.SchemaChangeCallBack; import org.apache.drill.exec.vector.ValueVector; import org.apache.drill.shaded.guava.com.google.common.base.Preconditions; -import org.apache.drill.shaded.guava.com.google.common.collect.Lists; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -302,10 +301,10 @@ public class UnionAllRecordBatch extends AbstractBinaryRecordBatch<UnionAll> { builder.setMinorType(leftField.getType().getMinorType()); builder = Types.calculateTypePrecisionAndScale(leftField.getType(), rightField.getType(), builder); } else { - List<TypeProtos.MinorType> types = Lists.newLinkedList(); - types.add(leftField.getType().getMinorType()); - types.add(rightField.getType().getMinorType()); - TypeProtos.MinorType outputMinorType = TypeCastRules.getLeastRestrictiveType(types); + TypeProtos.MinorType outputMinorType = TypeCastRules.getLeastRestrictiveType( + leftField.getType().getMinorType(), + rightField.getType().getMinorType() + ); if (outputMinorType == null) { throw new DrillRuntimeException("Type mismatch between " + leftField.getType().getMinorType().toString() + " on the left side and " + rightField.getType().getMinorType().toString() + @@ -315,10 +314,10 @@ public class UnionAllRecordBatch extends AbstractBinaryRecordBatch<UnionAll> { } // The output data mode should be as flexible as the more flexible one from the two input tables - List<TypeProtos.DataMode> dataModes = Lists.newLinkedList(); - dataModes.add(leftField.getType().getMode()); - dataModes.add(rightField.getType().getMode()); - builder.setMode(TypeCastRules.getLeastRestrictiveDataMode(dataModes)); + builder.setMode(TypeCastRules.getLeastRestrictiveDataMode( + leftField.getType().getMode(), + rightField.getType().getMode() + )); container.addOrGet(MaterializedField.create(leftField.getName(), builder.build()), callBack); } diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillRelOptUtil.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillRelOptUtil.java index bf1fd6f789..134554ce41 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillRelOptUtil.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillRelOptUtil.java @@ -56,7 +56,6 @@ import org.apache.calcite.util.Pair; import org.apache.calcite.util.Util; import org.apache.drill.common.expression.PathSegment; import org.apache.drill.common.expression.SchemaPath; -import org.apache.drill.common.types.TypeProtos; import org.apache.drill.common.types.Types; import org.apache.drill.exec.planner.logical.DrillRelFactories; import org.apache.drill.exec.planner.logical.DrillTable; @@ -67,7 +66,6 @@ import org.apache.drill.exec.resolver.TypeCastRules; import org.apache.drill.exec.util.Utilities; import org.apache.drill.shaded.guava.com.google.common.collect.ImmutableList; import org.apache.drill.shaded.guava.com.google.common.collect.ImmutableMap; -import org.apache.drill.shaded.guava.com.google.common.collect.Lists; import org.apache.drill.shaded.guava.com.google.common.collect.Sets; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -118,10 +116,10 @@ public abstract class DrillRelOptUtil { } // Check if Drill implicit casting can resolve the incompatibility - List<TypeProtos.MinorType> types = Lists.newArrayListWithCapacity(2); - types.add(Types.getMinorTypeFromName(type1.getSqlTypeName().getName())); - types.add(Types.getMinorTypeFromName(type2.getSqlTypeName().getName())); - return TypeCastRules.getLeastRestrictiveType(types) != null; + return TypeCastRules.getLeastRestrictiveType( + Types.getMinorTypeFromName(type1.getSqlTypeName().getName()), + Types.getMinorTypeFromName(type2.getSqlTypeName().getName()) + ) != null; } } return true; diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillCalciteSqlBetweenOperatorWrapper.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillCalciteSqlBetweenOperatorWrapper.java index f112e57908..6252405d54 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillCalciteSqlBetweenOperatorWrapper.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillCalciteSqlBetweenOperatorWrapper.java @@ -67,7 +67,10 @@ public class DrillCalciteSqlBetweenOperatorWrapper extends SqlBetweenOperator im types.add(inMinorType); } - final boolean isCompatible = TypeCastRules.getLeastRestrictiveType(types) != null; + final boolean isCompatible = TypeCastRules.getLeastRestrictiveType( + types.toArray(new TypeProtos.MinorType[0]) + ) != null; + if (!isCompatible && throwOnFailure) { throw callBinding.newValidationSignatureError(); } diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/TypeInferenceUtils.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/TypeInferenceUtils.java index ced54a49b8..44e46bde45 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/TypeInferenceUtils.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/TypeInferenceUtils.java @@ -56,6 +56,7 @@ import org.apache.drill.exec.resolver.FunctionResolverFactory; import org.apache.drill.exec.resolver.TypeCastRules; import java.util.List; +import java.util.Optional; import java.util.Set; @SuppressWarnings("WeakerAccess") @@ -447,47 +448,57 @@ public class TypeInferenceUtils { } // Determines SqlTypeName of the result. - // For the case when input may be implicitly casted to BIGINT, the type of result is BIGINT. - // Else for the case when input may be implicitly casted to FLOAT4, the type of result is DOUBLE. - // Else for the case when input may be implicitly casted to VARDECIMAL, the type of result is DECIMAL - // with the same scale as input and max allowed numeric precision. - // Else for the case when input may be implicitly casted to FLOAT8, the type of result is DOUBLE. - // When none of these conditions is satisfied, error is thrown. - // This order of checks is caused by the order of types in ResolverTypePrecedence.precedenceMap + // When the cheapest cast is is to + // - BIGINT then the result is a BIGINT + // - FLOAT8 then the result is DOUBLE + // - VARDECIMAL then the result is a DECIMAL with the same scale as the input and + // the max allowed numeric precision. + // When none of these conditions are satisfied an error is thrown. final RelDataType operandType = opBinding.getOperandType(0); final TypeProtos.MinorType inputMinorType = getDrillTypeFromCalciteType(operandType); - if (TypeCastRules.getLeastRestrictiveType(Lists.newArrayList(inputMinorType, TypeProtos.MinorType.BIGINT)) - == TypeProtos.MinorType.BIGINT) { - return createCalciteTypeWithNullability( - factory, - SqlTypeName.BIGINT, - isNullable); - } else if (TypeCastRules.getLeastRestrictiveType(Lists.newArrayList(inputMinorType, TypeProtos.MinorType.FLOAT4)) - == TypeProtos.MinorType.FLOAT4) { - return createCalciteTypeWithNullability( - factory, - SqlTypeName.DOUBLE, - isNullable); - } else if (TypeCastRules.getLeastRestrictiveType(Lists.newArrayList(inputMinorType, TypeProtos.MinorType.VARDECIMAL)) - == TypeProtos.MinorType.VARDECIMAL) { - RelDataType sqlType = factory.createSqlType(SqlTypeName.DECIMAL, - DrillRelDataTypeSystem.DRILL_REL_DATATYPE_SYSTEM.getMaxNumericPrecision(), - Math.min(operandType.getScale(), - DrillRelDataTypeSystem.DRILL_REL_DATATYPE_SYSTEM.getMaxNumericScale())); - return factory.createTypeWithNullability(sqlType, isNullable); - } else if (TypeCastRules.getLeastRestrictiveType(Lists.newArrayList(inputMinorType, TypeProtos.MinorType.FLOAT8)) - == TypeProtos.MinorType.FLOAT8) { - return createCalciteTypeWithNullability( - factory, - SqlTypeName.DOUBLE, - isNullable); - } else { + + Optional<TypeProtos.MinorType> targetType = TypeCastRules.getCheapestCast( + inputMinorType, + TypeProtos.MinorType.BIGINT, + TypeProtos.MinorType.FLOAT8, + TypeProtos.MinorType.VARDECIMAL + ); + + if (!targetType.isPresent()) { throw UserException - .functionError() - .message(String.format("%s does not support operand types (%s)", - opBinding.getOperator().getName(), - opBinding.getOperandType(0).getSqlTypeName())) - .build(logger); + .functionError() + .message(String.format("%s does not support operand types (%s)", + opBinding.getOperator().getName(), + opBinding.getOperandType(0).getSqlTypeName())) + .build(logger); + } + + switch (targetType.get()) { + case BIGINT: return createCalciteTypeWithNullability( + factory, + SqlTypeName.BIGINT, + isNullable + ); + case FLOAT8: return createCalciteTypeWithNullability( + factory, + SqlTypeName.DOUBLE, + isNullable + ); + case VARDECIMAL: + RelDataType sqlType = factory.createSqlType( + SqlTypeName.DECIMAL, + DrillRelDataTypeSystem.DRILL_REL_DATATYPE_SYSTEM.getMaxNumericPrecision(), + Math.min( + operandType.getScale(), + DrillRelDataTypeSystem.DRILL_REL_DATATYPE_SYSTEM.getMaxNumericScale() + ) + ); + return factory.createTypeWithNullability(sqlType, isNullable); + default: + throw new IllegalStateException(String.format( + "Internal error: a minor type of %s should not occur here.", + targetType.get() + )); } } } @@ -853,34 +864,21 @@ public class TypeInferenceUtils { } // Determines SqlTypeName of the result. - // For the case when input may be implicitly casted to FLOAT4, the type of result is DOUBLE. - // Else for the case when input may be implicitly casted to VARDECIMAL, the type of result is DECIMAL - // with scale max(6, input) and max allowed numeric precision. - // Else for the case when input may be implicitly casted to FLOAT8, the type of result is DOUBLE. - // When none of these conditions is satisfied, error is thrown. - // This order of checks is caused by the order of types in ResolverTypePrecedence.precedenceMap + // When the cheapest cast is to + // - FLOAT8 then the result is a DOUBLE + // - VARDECIMAL then the result is a DECIMAL with scale max(6, input) and + // the max allowed numeric precision. + // When none of these conditions are satisfied an error is thrown. final RelDataType operandType = opBinding.getOperandType(0); final TypeProtos.MinorType inputMinorType = getDrillTypeFromCalciteType(operandType); - if (TypeCastRules.getLeastRestrictiveType(Lists.newArrayList(inputMinorType, TypeProtos.MinorType.FLOAT4)) - == TypeProtos.MinorType.FLOAT4) { - return createCalciteTypeWithNullability( - factory, - SqlTypeName.DOUBLE, - isNullable); - } else if (TypeCastRules.getLeastRestrictiveType(Lists.newArrayList(inputMinorType, TypeProtos.MinorType.VARDECIMAL)) - == TypeProtos.MinorType.VARDECIMAL) { - RelDataType sqlType = factory.createSqlType(SqlTypeName.DECIMAL, - DrillRelDataTypeSystem.DRILL_REL_DATATYPE_SYSTEM.getMaxNumericPrecision(), - Math.min(Math.max(6, operandType.getScale()), - DrillRelDataTypeSystem.DRILL_REL_DATATYPE_SYSTEM.getMaxNumericScale())); - return factory.createTypeWithNullability(sqlType, isNullable); - } else if (TypeCastRules.getLeastRestrictiveType(Lists.newArrayList(inputMinorType, TypeProtos.MinorType.FLOAT8)) - == TypeProtos.MinorType.FLOAT8) { - return createCalciteTypeWithNullability( - factory, - SqlTypeName.DOUBLE, - isNullable); - } else { + + Optional<TypeProtos.MinorType> targetType = TypeCastRules.getCheapestCast( + inputMinorType, + TypeProtos.MinorType.FLOAT8, + TypeProtos.MinorType.VARDECIMAL + ); + + if (!targetType.isPresent()) { throw UserException .functionError() .message(String.format("%s does not support operand types (%s)", @@ -888,6 +886,29 @@ public class TypeInferenceUtils { opBinding.getOperandType(0).getSqlTypeName())) .build(logger); } + + switch (targetType.get()) { + case FLOAT8: return createCalciteTypeWithNullability( + factory, + SqlTypeName.DOUBLE, + isNullable + ); + case VARDECIMAL: + RelDataType sqlType = factory.createSqlType( + SqlTypeName.DECIMAL, + DrillRelDataTypeSystem.DRILL_REL_DATATYPE_SYSTEM.getMaxNumericPrecision(), + Math.min( + Math.max(6, operandType.getScale()), + DrillRelDataTypeSystem.DRILL_REL_DATATYPE_SYSTEM.getMaxNumericScale() + ) + ); + return factory.createTypeWithNullability(sqlType, isNullable); + default: + throw new IllegalStateException(String.format( + "Internal error: a minor type of %s should not occur here.", + targetType.get() + )); + } } } diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/resolver/DefaultFunctionResolver.java b/exec/java-exec/src/main/java/org/apache/drill/exec/resolver/DefaultFunctionResolver.java index ae3c68b767..66ddd8c99a 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/resolver/DefaultFunctionResolver.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/resolver/DefaultFunctionResolver.java @@ -20,11 +20,12 @@ package org.apache.drill.exec.resolver; import java.util.LinkedList; import java.util.List; import java.util.stream.Collectors; + +import org.apache.drill.common.exceptions.UserException; import org.apache.drill.common.expression.FunctionCall; import org.apache.drill.common.expression.LogicalExpression; import org.apache.drill.common.types.TypeProtos; import org.apache.drill.exec.expr.fn.DrillFuncHolder; -import org.apache.drill.exec.util.AssertionUtil; public class DefaultFunctionResolver implements FunctionResolver { @@ -33,52 +34,49 @@ public class DefaultFunctionResolver implements FunctionResolver { @Override public DrillFuncHolder getBestMatch(List<DrillFuncHolder> methods, FunctionCall call) { - int bestcost = Integer.MAX_VALUE; - int currcost = Integer.MAX_VALUE; - DrillFuncHolder bestmatch = null; + float bestCost = Float.POSITIVE_INFINITY, currCost = Float.POSITIVE_INFINITY; + DrillFuncHolder bestMatch = null; final List<DrillFuncHolder> bestMatchAlternatives = new LinkedList<>(); List<TypeProtos.MajorType> argumentTypes = call.args().stream() .map(LogicalExpression::getMajorType) .collect(Collectors.toList()); - for (DrillFuncHolder h : methods) { - currcost = TypeCastRules.getCost(argumentTypes, h); - // if cost is lower than 0, func implementation is not matched, either w/ or w/o implicit casts - if (currcost < 0 ) { - continue; - } + for (DrillFuncHolder h : methods) { + currCost = TypeCastRules.getCost(argumentTypes, h); - if (currcost < bestcost) { - bestcost = currcost; - bestmatch = h; + if (currCost < bestCost) { + bestCost = currCost; + bestMatch = h; bestMatchAlternatives.clear(); - } else if (currcost == bestcost) { + } else if (currCost == bestCost && currCost < Float.POSITIVE_INFINITY) { // keep log of different function implementations that have the same best cost bestMatchAlternatives.add(h); } } - if (bestcost < 0) { + if (bestCost == Float.POSITIVE_INFINITY) { //did not find a matched func implementation, either w/ or w/o implicit casts //TODO: raise exception here? return null; - } else { - if (AssertionUtil.isAssertionsEnabled() && bestMatchAlternatives.size() > 0) { - /* - * There are other alternatives to the best match function which could have been selected - * Log the possible functions and the chose implementation and raise an exception - */ - logger.error("Chosen function impl: " + bestmatch.toString()); - - // printing the possible matches - logger.error("Printing all the possible functions that could have matched: "); - for (DrillFuncHolder holder: bestMatchAlternatives) { - logger.error(holder.toString()); - } + } + if (bestMatchAlternatives.size() > 0) { + logger.info("Multiple functions with best cost found, query processing will be aborted."); - throw new AssertionError("Multiple functions with best cost found"); + // printing the possible matches + logger.debug("Printing all the possible functions that could have matched: "); + for (DrillFuncHolder holder : bestMatchAlternatives) { + logger.debug(holder.toString()); } - return bestmatch; + + throw UserException.functionError() + .message( + "There are %d function definitions with the same casting cost for " + + "%s, please write explicit casts disambiguate your function call.", + 1+bestMatchAlternatives.size(), + call + ) + .build(logger); } + return bestMatch; } } diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/resolver/ExactFunctionResolver.java b/exec/java-exec/src/main/java/org/apache/drill/exec/resolver/ExactFunctionResolver.java index 59c248891a..478aeee18c 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/resolver/ExactFunctionResolver.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/resolver/ExactFunctionResolver.java @@ -36,7 +36,7 @@ public class ExactFunctionResolver implements FunctionResolver { @Override public DrillFuncHolder getBestMatch(List<DrillFuncHolder> methods, FunctionCall call) { - int currCost; + float currCost; final List<TypeProtos.MajorType> argumentTypes = Lists.newArrayList(); for (LogicalExpression expression : call.args()) { @@ -46,7 +46,7 @@ public class ExactFunctionResolver implements FunctionResolver { for (DrillFuncHolder h : methods) { currCost = TypeCastRules.getCost(argumentTypes, h); // Return if we found a function that has an exact match with the input arguments - if (currCost == 0) { + if (currCost == 0f) { return h; } } diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/resolver/ResolverTypePrecedence.java b/exec/java-exec/src/main/java/org/apache/drill/exec/resolver/ResolverTypePrecedence.java index bf0bb22485..abc181402f 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/resolver/ResolverTypePrecedence.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/resolver/ResolverTypePrecedence.java @@ -21,148 +21,219 @@ import java.util.HashMap; import java.util.HashSet; import java.util.Map; import java.util.Set; +import java.util.TreeSet; import org.apache.drill.common.types.TypeProtos.MinorType; +import org.apache.drill.shaded.guava.com.google.common.graph.ImmutableValueGraph; +import org.apache.drill.shaded.guava.com.google.common.graph.ValueGraphBuilder; public class ResolverTypePrecedence { + // A weighted directed graph that represents the cost of casting between + // pairs of data types. The edge weights represent casting preferences and + // it is important to note that only some of these preferences can be + // understood in terms of factors like computational cost or loss of + // precision. The others are derived from the expected behaviour of the query + // engine in the face of various data types and queries as expressed by the + // test suite. + // + // Bear in mind that this class only establishes which casts will be tried + // automatically and how they're ranked. See + // {@link org.apache.drill.exec.resolver.TypeCastRules} for listings of which + // casts are possible at all. + public static final ImmutableValueGraph<MinorType, Float> CAST_GRAPH = ValueGraphBuilder + .directed() + .<MinorType, Float>immutable() + + // null type source vertex (null is castable to any type) + // prefer to cast NULL to a non-NULL over any non-NULL to another non-NULL + .putEdgeValue(MinorType.NULL, MinorType.VARCHAR, 1f) + .putEdgeValue(MinorType.NULL, MinorType.BIT, 1.1f) + .putEdgeValue(MinorType.NULL, MinorType.INT, 1.2f) + .putEdgeValue(MinorType.NULL, MinorType.FLOAT4, 1.3f) + .putEdgeValue(MinorType.NULL, MinorType.DECIMAL9, 1.4f) + .putEdgeValue(MinorType.NULL, MinorType.DATE, 1.5f) + .putEdgeValue(MinorType.NULL, MinorType.INTERVALDAY, 1.6f) + .putEdgeValue(MinorType.NULL, MinorType.MONEY, 1.7f) + .putEdgeValue(MinorType.NULL, MinorType.LIST, 1.8f) + .putEdgeValue(MinorType.NULL, MinorType.DICT, 1.9f) + + // Apart from NULL, casts between differing types have a starting cost + // of 10f so that when they're summed, a smaller number of casts gets + // preferred over a larger number. + + // bit conversions + // prefer to cast VARCHAR to BIT than BIT to numerics + .putEdgeValue(MinorType.BIT, MinorType.TINYINT, 100f) + .putEdgeValue(MinorType.BIT, MinorType.UINT1, 100f) + + // unsigned int widening + .putEdgeValue(MinorType.UINT1, MinorType.UINT2, 10f) + .putEdgeValue(MinorType.UINT2, MinorType.UINT4, 10f) + .putEdgeValue(MinorType.UINT4, MinorType.UINT8, 10f) + .putEdgeValue(MinorType.UINT8, MinorType.VARDECIMAL, 10f) + // unsigned int conversions + // prefer to cast UINTs to BIGINT over FLOAT4 + .putEdgeValue(MinorType.UINT4, MinorType.BIGINT, 10f) + .putEdgeValue(MinorType.UINT4, MinorType.FLOAT4, 11f) + .putEdgeValue(MinorType.UINT8, MinorType.FLOAT4, 12f) + + // int widening + .putEdgeValue(MinorType.TINYINT, MinorType.SMALLINT, 10f) + .putEdgeValue(MinorType.SMALLINT, MinorType.INT, 10f) + .putEdgeValue(MinorType.INT, MinorType.BIGINT, 10f) + // int conversions + .putEdgeValue(MinorType.BIGINT, MinorType.FLOAT4, 10f) + // prefer to cast INTs to FLOATs over VARDECIMALs + .putEdgeValue(MinorType.BIGINT, MinorType.VARDECIMAL, 100f) + + // float widening + .putEdgeValue(MinorType.FLOAT4, MinorType.FLOAT8, 10f) + // float conversion + // FLOATs are not currently castable to VARDECIMAL (see TypeCastRules) + // but it is not possible to avoid some path between them here since + // FLOATs must ultimately be implicitly castable to VARCHAR, and VARCHAR + // to VARDECIMAL. + // prefer the cast in the opposite direction + .putEdgeValue(MinorType.FLOAT8, MinorType.VARDECIMAL, 10_000f) + + // decimal widening + .putEdgeValue(MinorType.DECIMAL9, MinorType.DECIMAL18, 10f) + .putEdgeValue(MinorType.DECIMAL18, MinorType.DECIMAL28DENSE, 10f) + .putEdgeValue(MinorType.DECIMAL28DENSE, MinorType.DECIMAL28SPARSE, 10f) + .putEdgeValue(MinorType.DECIMAL28SPARSE, MinorType.DECIMAL38DENSE, 10f) + .putEdgeValue(MinorType.DECIMAL38DENSE, MinorType.DECIMAL38SPARSE, 10f) + .putEdgeValue(MinorType.DECIMAL38SPARSE, MinorType.VARDECIMAL, 10f) + .putEdgeValue(MinorType.MONEY, MinorType.VARDECIMAL, 10f) + // decimal conversions + // prefer to cast INTs to VARDECIMALs over VARDECIMALs to FLOATs + .putEdgeValue(MinorType.VARDECIMAL, MinorType.FLOAT8, 1_000f) + .putEdgeValue(MinorType.VARDECIMAL, MinorType.FLOAT4, 1_001f) + // prefer the casts in the opposite directions + .putEdgeValue(MinorType.VARDECIMAL, MinorType.INT, 1_002f) + .putEdgeValue(MinorType.VARDECIMAL, MinorType.VARCHAR, 1_003f) + + // interval widening + .putEdgeValue(MinorType.INTERVALDAY, MinorType.INTERVALYEAR, 10f) + .putEdgeValue(MinorType.INTERVALYEAR, MinorType.INTERVAL, 10f) + // interval conversions + // prefer the cast in the opposite direction + .putEdgeValue(MinorType.INTERVAL, MinorType.VARCHAR, 100f) + + // dict widening + .putEdgeValue(MinorType.DICT, MinorType.MAP, 10f) + + // timestamp widening + .putEdgeValue(MinorType.DATE, MinorType.TIMESTAMP, 10f) + .putEdgeValue(MinorType.TIMESTAMP, MinorType.TIMESTAMPTZ, 10f) + .putEdgeValue(MinorType.TIME, MinorType.TIMETZ, 10f) + // timestamp conversions + // prefer the casts in the opposite directions + .putEdgeValue(MinorType.TIMESTAMP, MinorType.DATE, 100f) + .putEdgeValue(MinorType.TIMESTAMP, MinorType.TIME, 101f) + .putEdgeValue(MinorType.TIMESTAMPTZ, MinorType.VARCHAR, 1_000f) + .putEdgeValue(MinorType.TIMETZ, MinorType.VARCHAR, 1_000f) + + // char and binary widening + .putEdgeValue(MinorType.FIXEDCHAR, MinorType.VARCHAR, 10f) + .putEdgeValue(MinorType.FIXEDBINARY, MinorType.VARBINARY, 10f) + // char and binary conversions + .putEdgeValue(MinorType.VARCHAR, MinorType.INT, 10f) + .putEdgeValue(MinorType.VARCHAR, MinorType.FLOAT8, 20f) + .putEdgeValue(MinorType.VARCHAR, MinorType.FLOAT4, 21f) + .putEdgeValue(MinorType.VARCHAR, MinorType.VARDECIMAL, 30f) + .putEdgeValue(MinorType.VARCHAR, MinorType.TIMESTAMP, 40f) + .putEdgeValue(MinorType.VARCHAR, MinorType.INTERVALDAY, 50f) + .putEdgeValue(MinorType.VARCHAR, MinorType.BIT, 60f) + .putEdgeValue(MinorType.VARCHAR, MinorType.VARBINARY, 70f) + .putEdgeValue(MinorType.VARBINARY, MinorType.VARCHAR, 80f) + + // union type sink vertex + .putEdgeValue(MinorType.LIST, MinorType.UNION, 10f) + .putEdgeValue(MinorType.MAP, MinorType.UNION, 10f) + .putEdgeValue(MinorType.VARBINARY, MinorType.UNION, 10f) + .putEdgeValue(MinorType.UNION, MinorType.LATE, 10f) + + .build(); + + /** + * Searches the implicit casting graph for the path of least total cost using + * Dijkstra's algorithm. + * @param fromType type to cast from + * @param toType type to cast to + * @return a positive float path cost or +∞ if no path exists + */ + public static float computeCost(MinorType fromType, MinorType toType) { + TreeSet<VertexDatum> remaining = new TreeSet<>(); + Map<MinorType, VertexDatum> vertexData = new HashMap<>(); + Set<MinorType> shortestPath = new HashSet<>(); + + VertexDatum sourceDatum = new VertexDatum(fromType, 0, null); + remaining.add(sourceDatum); + vertexData.put(fromType, sourceDatum); + + while (!remaining.isEmpty()) { + // Poll the datum with lowest totalDistance in remaining + VertexDatum vertexDatum = remaining.pollFirst(); + MinorType vertex = vertexDatum.vertex; + shortestPath.add(vertex); + + if (vertex.equals(toType)) { + // Goal found. We only wanted to calculate the path distance so we + // don't need to go on to backtrace it. + return vertexDatum.totalDistance; + } + + Set<MinorType> successors = CAST_GRAPH.successors(vertex); + for (MinorType successor : successors) { + if (shortestPath.contains(successor)) { + continue; + } + + float distance = CAST_GRAPH.edgeValue(vertex, successor).orElseThrow(IllegalStateException::new); + float totalDistance = vertexDatum.totalDistance + distance; + + VertexDatum successorDatum = vertexData.get(successor); + if (successorDatum == null) { + successorDatum = new VertexDatum(successor, totalDistance, vertexDatum); + vertexData.put(successor, successorDatum); + remaining.add(successorDatum); + } else if (totalDistance < successorDatum.totalDistance) { + successorDatum.totalDistance = totalDistance; + successorDatum.predecessor = vertexDatum; + } + } + } + + return Float.POSITIVE_INFINITY; + } - public static final Map<MinorType, Integer> precedenceMap; - public static final Map<MinorType, Set<MinorType>> secondaryImplicitCastRules; - public static int MAX_IMPLICIT_CAST_COST; - - static { - /* The precedenceMap is used to decide whether it's allowed to implicitly "promote" - * one type to another type. - * - * The order that each type is inserted into HASHMAP decides its precedence. - * First in ==> lowest precedence. - * A type of lower precedence can be implicitly "promoted" to type of higher precedence. - * For instance, NULL could be promoted to any other type; - * tinyint could be promoted into int; but int could NOT be promoted into tinyint (due to possible precision loss). - */ - int i = 0; - precedenceMap = new HashMap<>(); - precedenceMap.put(MinorType.NULL, i += 2); // NULL is legal to implicitly be promoted to any other type - precedenceMap.put(MinorType.FIXEDBINARY, i += 2); // Fixed-length is promoted to var length - precedenceMap.put(MinorType.VARBINARY, i += 2); - precedenceMap.put(MinorType.FIXEDCHAR, i += 2); - precedenceMap.put(MinorType.VARCHAR, i += 2); - precedenceMap.put(MinorType.FIXED16CHAR, i += 2); - precedenceMap.put(MinorType.VAR16CHAR, i += 2); - precedenceMap.put(MinorType.BIT, i += 2); - precedenceMap.put(MinorType.TINYINT, i += 2); //type with few bytes is promoted to type with more bytes ==> no data loss. - precedenceMap.put(MinorType.UINT1, i += 2); //signed is legal to implicitly be promoted to unsigned. - precedenceMap.put(MinorType.SMALLINT, i += 2); - precedenceMap.put(MinorType.UINT2, i += 2); - precedenceMap.put(MinorType.INT, i += 2); - precedenceMap.put(MinorType.UINT4, i += 2); - precedenceMap.put(MinorType.BIGINT, i += 2); - precedenceMap.put(MinorType.UINT8, i += 2); - precedenceMap.put(MinorType.MONEY, i += 2); - precedenceMap.put(MinorType.FLOAT4, i += 2); - precedenceMap.put(MinorType.DECIMAL9, i += 2); - precedenceMap.put(MinorType.DECIMAL18, i += 2); - precedenceMap.put(MinorType.DECIMAL28DENSE, i += 2); - precedenceMap.put(MinorType.DECIMAL28SPARSE, i += 2); - precedenceMap.put(MinorType.DECIMAL38DENSE, i += 2); - precedenceMap.put(MinorType.DECIMAL38SPARSE, i += 2); - precedenceMap.put(MinorType.VARDECIMAL, i += 2); - precedenceMap.put(MinorType.FLOAT8, i += 2); - precedenceMap.put(MinorType.DATE, i += 2); - precedenceMap.put(MinorType.TIMESTAMP, i += 2); - precedenceMap.put(MinorType.TIMETZ, i += 2); - precedenceMap.put(MinorType.TIMESTAMPTZ, i += 2); - precedenceMap.put(MinorType.TIME, i += 2); - precedenceMap.put(MinorType.INTERVALDAY, i+= 2); - precedenceMap.put(MinorType.INTERVALYEAR, i+= 2); - precedenceMap.put(MinorType.INTERVAL, i+= 2); - precedenceMap.put(MinorType.MAP, i += 2); - precedenceMap.put(MinorType.DICT, i += 2); - precedenceMap.put(MinorType.LIST, i += 2); - precedenceMap.put(MinorType.UNION, i += 2); - - MAX_IMPLICIT_CAST_COST = i; - - /* Currently implicit cast follows the precedence rules. - * It may be useful to perform an implicit cast in - * the opposite direction as specified by the precedence rules. - * - * For example: As per the precedence rules we can implicitly cast - * from VARCHAR ---> BIGINT , but based upon some functions (eg: substr, concat) - * it may be useful to implicitly cast from BIGINT ---> VARCHAR. - * - * To allow for such cases we have a secondary set of rules which will allow the reverse - * implicit casts. Currently we only allow the reverse implicit cast to VARCHAR so we don't - * need any cost associated with it, if we add more of these that may collide we can add costs. - */ - secondaryImplicitCastRules = new HashMap<>(); - HashSet<MinorType> rule = new HashSet<>(); - - // Following cast functions should exist - rule.add(MinorType.TINYINT); - rule.add(MinorType.SMALLINT); - rule.add(MinorType.INT); - rule.add(MinorType.BIGINT); - rule.add(MinorType.UINT1); - rule.add(MinorType.UINT2); - rule.add(MinorType.UINT4); - rule.add(MinorType.UINT8); - rule.add(MinorType.VARDECIMAL); - rule.add(MinorType.MONEY); - rule.add(MinorType.FLOAT4); - rule.add(MinorType.FLOAT8); - rule.add(MinorType.BIT); - rule.add(MinorType.FIXEDCHAR); - rule.add(MinorType.FIXED16CHAR); - rule.add(MinorType.VARCHAR); - rule.add(MinorType.DATE); - rule.add(MinorType.TIME); - rule.add(MinorType.TIMESTAMP); - rule.add(MinorType.TIMESTAMPTZ); - rule.add(MinorType.INTERVAL); - rule.add(MinorType.INTERVALYEAR); - rule.add(MinorType.INTERVALDAY); - secondaryImplicitCastRules.put(MinorType.VARCHAR, rule); - - rule = new HashSet<>(); - - // Be able to implicitly cast to VARBINARY - rule.add(MinorType.INT); - rule.add(MinorType.BIGINT); - rule.add(MinorType.FLOAT4); - rule.add(MinorType.FLOAT8); - rule.add(MinorType.VARCHAR); - secondaryImplicitCastRules.put(MinorType.VARBINARY, rule); - - rule = new HashSet<>(); - - // Be able to implicitly cast to VARDECIMAL - rule.add(MinorType.FLOAT8); - secondaryImplicitCastRules.put(MinorType.VARDECIMAL, rule); - - rule = new HashSet<>(); - - // Be able to implicitly cast to DECIMAL9 - rule.add(MinorType.VARDECIMAL); - secondaryImplicitCastRules.put(MinorType.DECIMAL9, rule); - - rule = new HashSet<>(); - - // Be able to implicitly cast to DECIMAL18 - rule.add(MinorType.VARDECIMAL); - secondaryImplicitCastRules.put(MinorType.DECIMAL18, rule); - - rule = new HashSet<>(); - - // Be able to implicitly cast to DECIMAL28SPARSE - rule.add(MinorType.VARDECIMAL); - secondaryImplicitCastRules.put(MinorType.DECIMAL28SPARSE, rule); - - rule = new HashSet<>(); - - // Be able to implicitly cast to DECIMAL38SPARSE - rule.add(MinorType.VARDECIMAL); - secondaryImplicitCastRules.put(MinorType.DECIMAL38SPARSE, rule); + /** + * A struct to hold working data used by Dijkstra's algorithm and allowing + * comparison based on total distance from the source vertex to this vertex. + */ + static class VertexDatum implements Comparable<VertexDatum> { + final MinorType vertex; + float totalDistance; + VertexDatum predecessor; + + public VertexDatum(MinorType vertex, float totalDistance, VertexDatum predecessor) { + this.vertex = vertex; + this.totalDistance = totalDistance; + this.predecessor = predecessor; + } + + @Override + public int compareTo(VertexDatum other) { + int distComparison = Float.compare(this.totalDistance, other.totalDistance); + // TreeSet uses this method to determine member equality, not equals(), so we + // need to differentiate between vertices with the same totalDistance. + return distComparison != 0 ? distComparison : this.vertex.compareTo(other.vertex); + } + + @Override + public String toString() { + return String.format("vertex: %s, totalDistance: %f, predecessor: %s", vertex, totalDistance, predecessor); + } } } diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/resolver/TypeCastRules.java b/exec/java-exec/src/main/java/org/apache/drill/exec/resolver/TypeCastRules.java index b617572eea..1f82592fba 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/resolver/TypeCastRules.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/resolver/TypeCastRules.java @@ -21,6 +21,7 @@ import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import org.apache.drill.common.expression.LogicalExpression; @@ -700,7 +701,7 @@ public class TypeCastRules { (rules.get(to) != null && rules.get(to).contains(from)); } - public static DataMode getLeastRestrictiveDataMode(List<DataMode> dataModes) { + public static DataMode getLeastRestrictiveDataMode(DataMode... dataModes) { boolean hasOptional = false; for(DataMode dataMode : dataModes) { switch (dataMode) { @@ -708,6 +709,7 @@ public class TypeCastRules { return dataMode; case OPTIONAL: hasOptional = true; + default: } } @@ -718,20 +720,14 @@ public class TypeCastRules { } } - /* - * Function checks if casting is allowed from the 'from' -> 'to' minor type. If its allowed - * we also check if the precedence map allows such a cast and return true if both cases are satisfied - */ - public static MinorType getLeastRestrictiveType(List<MinorType> types) { - assert types.size() >= 2; - MinorType result = types.get(0); + public static MinorType getLeastRestrictiveType(MinorType... types) { + MinorType result = types[0]; if (result == MinorType.UNION) { return result; } - int resultPrec = ResolverTypePrecedence.precedenceMap.get(result); - for (int i = 1; i < types.size(); i++) { - MinorType next = types.get(i); + for (int i = 1; i < types.length; i++) { + MinorType next = types[i]; if (next == MinorType.UNION) { return next; } @@ -740,14 +736,13 @@ public class TypeCastRules { continue; } - int nextPrec = ResolverTypePrecedence.precedenceMap.get(next); + float resultCastCost = ResolverTypePrecedence.computeCost(next, result); + float nextCastCost = ResolverTypePrecedence.computeCost(result, next); - if (isCastable(next, result) && resultPrec >= nextPrec) { - // result is the least restrictive between the two args; nothing to do continue + if (isCastable(next, result) && resultCastCost <= nextCastCost && resultCastCost < Float.POSITIVE_INFINITY) { continue; - } else if(isCastable(result, next) && nextPrec >= resultPrec) { + } else if (isCastable(result, next) && nextCastCost <= resultCastCost && nextCastCost < Float.POSITIVE_INFINITY) { result = next; - resultPrec = nextPrec; } else { return null; } @@ -756,28 +751,51 @@ public class TypeCastRules { return result; } - private static final int DATAMODE_CAST_COST = 1; - private static final int VARARG_COST = Integer.MAX_VALUE / 2; + /** + * Finds the type in a given set that has the cheapest cast from a given + * starting type. + * @param fromType type to cast from. + * @param toTypes candidate types to cast to. + * @return the type in toTypes that has the cheapest cast or empty if no + * finite cost cast can be found. + */ + public static Optional<MinorType> getCheapestCast(MinorType fromType, MinorType... toTypes) { + MinorType cheapest = null; + float cheapestCost = Float.POSITIVE_INFINITY; + + for (MinorType toType: toTypes) { + float toTypeCost = ResolverTypePrecedence.computeCost(fromType, toType); + if (toTypeCost < cheapestCost) { + cheapest = toType; + cheapestCost = toTypeCost; + } + } + + return Optional.ofNullable(cheapest); + } + + // cost of changing mode from required to optional + private static final float DATAMODE_CHANGE_COST = 1f; + // cost of casting to a field reader, compare to edge weights in ResolverTypePrecedence + private static final float FIELD_READER_COST = 100f; + // cost of matching a vararg function, compare to edge weights in ResolverTypePrecedence + private static final float VARARG_COST = 100f; /** - * Decide whether it's legal to do implicit cast. -1 : not allowed for - * implicit cast > 0: cost associated with implicit cast. ==0: params are - * exactly same type of arg. No need of implicit. + * Decide whether it's legal to do implicit cast. + * @returns + * 0: param types equal arg types, no casting needed. + * (0,+∞): the cost of implicit casting. + * +∞: implicit casting is not allowed. */ - public static int getCost(List<MajorType> argumentTypes, DrillFuncHolder holder) { - int cost = 0; + public static float getCost(List<MajorType> argumentTypes, DrillFuncHolder holder) { + // Running sum of casting cost. + float totalCost = 0; if (argumentTypes.size() != holder.getParamCount() && !holder.isVarArg()) { - return -1; + return Float.POSITIVE_INFINITY; } - // Indicates whether we used secondary cast rules - boolean secondaryCast = false; - - // number of arguments that could implicitly casts using precedence map or - // didn't require casting at all - int nCasts = 0; - /* * If we are determining function holder for decimal data type, we need to * make sure the output type of the function can fit the precision that we @@ -792,7 +810,7 @@ public class TypeCastRules { if (DRILL_REL_DATATYPE_SYSTEM.getMaxNumericPrecision() < holder.getReturnType(logicalExpressions).getPrecision()) { - return -1; + return Float.POSITIVE_INFINITY; } } @@ -804,74 +822,32 @@ public class TypeCastRules { //@Param FieldReader will match any type if (holder.isFieldReader(i)) { // if (Types.isComplex(call.args.get(i).getMajorType()) ||Types.isRepeated(call.args.get(i).getMajorType()) ) - // add the max cost when encountered with a field reader considering + // add a weighted cost when we encounter a field reader considering // that it is the most expensive factor contributing to the cost. - cost += ResolverTypePrecedence.MAX_IMPLICIT_CAST_COST; + totalCost += FIELD_READER_COST; continue; } if (!TypeCastRules.isCastableWithNullHandling(argType, paramType, holder.getNullHandling())) { - return -1; - } - - Integer paramVal = ResolverTypePrecedence.precedenceMap.get(paramType - .getMinorType()); - Integer argVal = ResolverTypePrecedence.precedenceMap.get(argType - .getMinorType()); - - if (paramVal == null) { - throw new RuntimeException(String.format( - "Precedence for type %s is not defined", paramType.getMinorType().name())); + // one uncastable argument is enough to kill the party + return Float.POSITIVE_INFINITY; } - if (argVal == null) { - throw new RuntimeException(String.format( - "Precedence for type %s is not defined", argType.getMinorType().name())); - } + float castCost = ResolverTypePrecedence.computeCost( + argType.getMinorType(), + paramType.getMinorType() + ); - if (paramVal - argVal < 0) { - - /* Precedence rules do not allow to implicit cast, however check - * if the secondary rules allow us to cast - */ - Set<MinorType> rules; - if ((rules = (ResolverTypePrecedence.secondaryImplicitCastRules.get(paramType.getMinorType()))) != null - && rules.contains(argType.getMinorType())) { - secondaryCast = true; - } else { - return -1; - } - } - // Check null vs non-null, using same logic as that in Types.softEqual() - // Only when the function uses NULL_IF_NULL, nullable and non-nullable are interchangeable. - // Otherwise, the function implementation is not a match. - if (argType.getMode() != paramType.getMode()) { - // TODO - this does not seem to do what it is intended to -// if (!((holder.getNullHandling() == NullHandling.NULL_IF_NULL) && -// (argType.getMode() == DataMode.OPTIONAL || -// argType.getMode() == DataMode.REQUIRED || -// paramType.getMode() == DataMode.OPTIONAL || -// paramType.getMode() == DataMode.REQUIRED ))) -// return -1; - // if the function is designed to take optional with custom null handling, and a required - // is being passed, increase the cost to account for a null check - // this allows for a non-nullable implementation to be preferred - if (holder.getNullHandling() == NullHandling.INTERNAL) { - // a function that expects required output, but nullable was provided - if (paramType.getMode() == DataMode.REQUIRED && argType.getMode() == DataMode.OPTIONAL) { - return -1; - } else if (paramType.getMode() == DataMode.OPTIONAL && argType.getMode() == DataMode.REQUIRED) { - cost+= DATAMODE_CAST_COST; - } - } + if (castCost == Float.POSITIVE_INFINITY) { + // A single uncastable argument is enough to kill the party + return Float.POSITIVE_INFINITY; } - int castCost; + totalCost += castCost; + totalCost += holder.getNullHandling() == NullHandling.INTERNAL && paramType.getMode() != argType.getMode() + ? DATAMODE_CHANGE_COST + : 0; - if ((castCost = (paramVal - argVal)) >= 0) { - nCasts++; - cost += castCost; - } } if (holder.isVarArg()) { @@ -883,30 +859,19 @@ public class TypeCastRules { && holder.getParamMajorType(varArgIndex).getMode() != argumentTypes.get(i).getMode()) { // prohibit using vararg functions for types with different nullability // if function accepts required arguments, but provided optional - return -1; + return Float.POSITIVE_INFINITY; } } - // increase cost for var arg functions to prioritize regular ones - Integer additionalCost = ResolverTypePrecedence.precedenceMap.get(holder.getParamMajorType(varArgIndex).getMinorType()); - cost += additionalCost != null ? additionalCost : VARARG_COST; - cost += holder.getParamMajorType(varArgIndex).getMode() == DataMode.REQUIRED ? 0 : 1; + totalCost += VARARG_COST; + totalCost += ResolverTypePrecedence.computeCost( + MinorType.NULL, + holder.getParamMajorType(varArgIndex).getMinorType() + ); + totalCost += holder.getParamMajorType(varArgIndex).getMode() == DataMode.REQUIRED ? 0 : 1; } - if (secondaryCast) { - // We have a secondary cast for one or more of the arguments, determine the cost associated - int secondaryCastCost = Integer.MAX_VALUE - 1; - - // Subtract maximum possible implicit costs from the secondary cast cost - secondaryCastCost -= (nCasts * (ResolverTypePrecedence.MAX_IMPLICIT_CAST_COST + DATAMODE_CAST_COST)); - - // Add cost of implicitly casting the rest of the arguments that didn't use secondary casting - secondaryCastCost += cost; - - return secondaryCastCost; - } - - return cost; + return totalCost; } /* @@ -934,5 +899,4 @@ public class TypeCastRules { return false; } } - } diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetTableMetadataUtils.java b/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetTableMetadataUtils.java index dea7f58443..d1505d1120 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetTableMetadataUtils.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetTableMetadataUtils.java @@ -64,7 +64,6 @@ import java.math.BigDecimal; import java.math.BigInteger; import java.nio.charset.StandardCharsets; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.HashMap; @@ -680,7 +679,10 @@ public class ParquetTableMetadataUtils { if (majorType == null) { columns.put(columnPath, type); } else if (!majorType.equals(type)) { - TypeProtos.MinorType leastRestrictiveType = TypeCastRules.getLeastRestrictiveType(Arrays.asList(majorType.getMinorType(), type.getMinorType())); + TypeProtos.MinorType leastRestrictiveType = TypeCastRules.getLeastRestrictiveType( + majorType.getMinorType(), + type.getMinorType() + ); if (leastRestrictiveType != majorType.getMinorType()) { columns.put(columnPath, type); } diff --git a/exec/java-exec/src/test/java/org/apache/drill/TestImplicitCasting.java b/exec/java-exec/src/test/java/org/apache/drill/TestImplicitCasting.java index 4994bb20a8..a167ce614d 100644 --- a/exec/java-exec/src/test/java/org/apache/drill/TestImplicitCasting.java +++ b/exec/java-exec/src/test/java/org/apache/drill/TestImplicitCasting.java @@ -19,26 +19,223 @@ package org.apache.drill; import org.apache.drill.categories.SqlTest; import org.apache.drill.common.types.TypeProtos; +import org.apache.drill.exec.physical.rowSet.DirectRowSet; +import org.apache.drill.exec.physical.rowSet.RowSet; +import org.apache.drill.exec.record.metadata.SchemaBuilder; +import org.apache.drill.exec.record.metadata.TupleMetadata; +import org.apache.drill.exec.resolver.ResolverTypePrecedence; import org.apache.drill.exec.resolver.TypeCastRules; -import org.apache.drill.test.BaseTest; +import org.apache.drill.test.ClusterFixtureBuilder; +import org.apache.drill.test.ClusterTest; +import org.apache.drill.test.rowSet.RowSetUtilities; +import static org.hamcrest.MatcherAssert.assertThat; + +import org.joda.time.Period; +import org.junit.BeforeClass; import org.junit.Test; -import org.apache.drill.shaded.guava.com.google.common.collect.Lists; import org.junit.experimental.categories.Category; -import java.util.List; - +import static org.hamcrest.Matchers.lessThan; import static org.junit.Assert.assertEquals; @Category(SqlTest.class) -public class TestImplicitCasting extends BaseTest { +public class TestImplicitCasting extends ClusterTest { + + @BeforeClass + public static void setup() throws Exception { + ClusterTest.startCluster(new ClusterFixtureBuilder(dirTestWatcher)); + } + @Test public void testTimeStampAndTime() { - final List<TypeProtos.MinorType> inputTypes = Lists.newArrayList(); - inputTypes.add(TypeProtos.MinorType.TIME); - inputTypes.add(TypeProtos.MinorType.TIMESTAMP); - final TypeProtos.MinorType result = TypeCastRules.getLeastRestrictiveType(inputTypes); + final TypeProtos.MinorType result = TypeCastRules.getLeastRestrictiveType( + TypeProtos.MinorType.TIME, + TypeProtos.MinorType.TIMESTAMP + ); + + assertEquals(TypeProtos.MinorType.TIME, result); + } + + @Test + /** + * Tests path cost arithmetic computed over the graph in ResolveTypePrecedence. + */ + public void testCastingCosts() { + // INT -> BIGINT + assertEquals( + 10f, + ResolverTypePrecedence.computeCost(TypeProtos.MinorType.INT, TypeProtos.MinorType.BIGINT), + 0f + ); + // INT -> BIGINT -> VARDECIMAL + assertEquals( + 10f+100f, + ResolverTypePrecedence.computeCost(TypeProtos.MinorType.INT, TypeProtos.MinorType.VARDECIMAL), + 0f + ); + // DECIMAL9 -> DECIMAL18 -> DECIMAL28SPARSE -> DECIMAL28DENSE -> DECIMAL38SPARSE -> + // -> DECIMAL38DENSE -> VARDECIMAL + assertEquals( + 10f+10f+10f+10f+10f+10f, + ResolverTypePrecedence.computeCost(TypeProtos.MinorType.DECIMAL9, TypeProtos.MinorType.VARDECIMAL), + 0f + ); + // FLOAT4 -> FLOAT8 -> VARDECIMAL -> INT + assertEquals( + 10f+10_000f+1_002f, + ResolverTypePrecedence.computeCost(TypeProtos.MinorType.FLOAT4, TypeProtos.MinorType.INT), + 0f + ); + // TIMESTAMP -> DATE + assertEquals( + 100f, + ResolverTypePrecedence.computeCost(TypeProtos.MinorType.TIMESTAMP, TypeProtos.MinorType.DATE), + 0f + ); + // No path from MAP to FLOAT8 + assertEquals( + Float.POSITIVE_INFINITY, + ResolverTypePrecedence.computeCost(TypeProtos.MinorType.MAP, TypeProtos.MinorType.FLOAT8), + 0f + ); + // VARCHAR -> INT -> BIGINT + assertEquals( + 10f+10f, + ResolverTypePrecedence.computeCost(TypeProtos.MinorType.VARCHAR, TypeProtos.MinorType.BIGINT), + 0f + ); + } + + @Test + public void testCastingPreferences() { + // All of the constraints that follow should be satisfied in order for + // queries involving implicit casts to behave as expected. + assertThat( + ResolverTypePrecedence.computeCost(TypeProtos.MinorType.VARCHAR, TypeProtos.MinorType.BIT), + lessThan(ResolverTypePrecedence.computeCost(TypeProtos.MinorType.BIT, TypeProtos.MinorType.INT)) + ); + assertThat( + ResolverTypePrecedence.computeCost(TypeProtos.MinorType.UINT1, TypeProtos.MinorType.BIGINT), + lessThan(ResolverTypePrecedence.computeCost(TypeProtos.MinorType.UINT1, TypeProtos.MinorType.FLOAT4)) + ); + assertThat( + ResolverTypePrecedence.computeCost(TypeProtos.MinorType.UINT1, TypeProtos.MinorType.BIGINT), + lessThan(ResolverTypePrecedence.computeCost(TypeProtos.MinorType.UINT1, TypeProtos.MinorType.FLOAT4)) + ); + assertThat( + ResolverTypePrecedence.computeCost(TypeProtos.MinorType.INT, TypeProtos.MinorType.FLOAT4), + lessThan(ResolverTypePrecedence.computeCost(TypeProtos.MinorType.INT, TypeProtos.MinorType.VARDECIMAL)) + ); + assertThat( + ResolverTypePrecedence.computeCost(TypeProtos.MinorType.VARDECIMAL, TypeProtos.MinorType.FLOAT8), + lessThan(ResolverTypePrecedence.computeCost(TypeProtos.MinorType.FLOAT8, TypeProtos.MinorType.VARDECIMAL)) + ); + assertThat( + ResolverTypePrecedence.computeCost(TypeProtos.MinorType.INT, TypeProtos.MinorType.VARDECIMAL), + lessThan(ResolverTypePrecedence.computeCost(TypeProtos.MinorType.VARDECIMAL, TypeProtos.MinorType.FLOAT4)) + ); + assertThat( + ResolverTypePrecedence.computeCost(TypeProtos.MinorType.VARCHAR, TypeProtos.MinorType.INTERVAL), + lessThan(ResolverTypePrecedence.computeCost(TypeProtos.MinorType.INTERVAL, TypeProtos.MinorType.VARCHAR)) + ); + assertThat( + ResolverTypePrecedence.computeCost(TypeProtos.MinorType.VARCHAR, TypeProtos.MinorType.TIME), + lessThan(ResolverTypePrecedence.computeCost(TypeProtos.MinorType.TIME, TypeProtos.MinorType.VARCHAR)) + ); + assertThat( + ResolverTypePrecedence.computeCost(TypeProtos.MinorType.DATE, TypeProtos.MinorType.TIMESTAMP), + lessThan(ResolverTypePrecedence.computeCost(TypeProtos.MinorType.TIMESTAMP, TypeProtos.MinorType.DATE)) + ); + assertThat( + ResolverTypePrecedence.computeCost(TypeProtos.MinorType.VARDECIMAL, TypeProtos.MinorType.FLOAT8), + lessThan(ResolverTypePrecedence.computeCost(TypeProtos.MinorType.VARDECIMAL, TypeProtos.MinorType.FLOAT4)) + ); + assertThat( + ResolverTypePrecedence.computeCost(TypeProtos.MinorType.VARCHAR, TypeProtos.MinorType.BIGINT), + lessThan(ResolverTypePrecedence.computeCost(TypeProtos.MinorType.VARCHAR, TypeProtos.MinorType.TIMESTAMP)) + ); + assertThat( + ResolverTypePrecedence.computeCost(TypeProtos.MinorType.VARCHAR, TypeProtos.MinorType.FLOAT8), + lessThan(ResolverTypePrecedence.computeCost(TypeProtos.MinorType.VARCHAR, TypeProtos.MinorType.FLOAT4)) + ); + assertThat(ResolverTypePrecedence.computeCost(TypeProtos.MinorType.VARCHAR, TypeProtos.MinorType.FLOAT4) + + ResolverTypePrecedence.computeCost(TypeProtos.MinorType.FLOAT4, TypeProtos.MinorType.FLOAT4), + lessThan(ResolverTypePrecedence.computeCost(TypeProtos.MinorType.VARCHAR, TypeProtos.MinorType.FLOAT8) + + ResolverTypePrecedence.computeCost(TypeProtos.MinorType.FLOAT4, TypeProtos.MinorType.FLOAT8)) + ); + assertThat( + ResolverTypePrecedence.computeCost(TypeProtos.MinorType.NULL, TypeProtos.MinorType.FLOAT4), + lessThan(ResolverTypePrecedence.computeCost(TypeProtos.MinorType.FLOAT4, TypeProtos.MinorType.FLOAT8)) + ); + } + + @Test + public void testSqrtOfString() throws Exception { + String sql = "select sqrt('5')"; + + DirectRowSet results = queryBuilder().sql(sql).rowSet(); + + TupleMetadata expectedSchema = new SchemaBuilder() + .add("EXPR$0", TypeProtos.MinorType.FLOAT8) + .build(); + + RowSet expected = client.rowSetBuilder(expectedSchema) + .addRow(2.23606797749979) + .build(); + + RowSetUtilities.verify(expected, results); + } + + @Test + public void testDateDiffOnStringDate() throws Exception { + String sql = "select date_diff('2022-01-01', date '1970-01-01')"; + + DirectRowSet results = queryBuilder().sql(sql).rowSet(); + + TupleMetadata expectedSchema = new SchemaBuilder() + .add("EXPR$0", TypeProtos.MinorType.INTERVALDAY) + .build(); + + RowSet expected = client.rowSetBuilder(expectedSchema) + .addRow(new Period("P18993D")) + .build(); + + RowSetUtilities.verify(expected, results); + } + + @Test + public void testSubstringOfDate() throws Exception { + String sql = "select substring(date '2022-09-09', 1, 4)"; + + DirectRowSet results = queryBuilder().sql(sql).rowSet(); + + TupleMetadata expectedSchema = new SchemaBuilder() + .add("EXPR$0", TypeProtos.MinorType.VARCHAR, 65535) + .build(); + + RowSet expected = client.rowSetBuilder(expectedSchema) + .addRow("2022") + .build(); + + RowSetUtilities.verify(expected, results); + } + + @Test + public void testBooleanStringEquality() throws Exception { + String sql = "select true = 'true', true = 'FalsE'"; + + DirectRowSet results = queryBuilder().sql(sql).rowSet(); + + TupleMetadata expectedSchema = new SchemaBuilder() + .add("EXPR$0", TypeProtos.MinorType.BIT) + .add("EXPR$1", TypeProtos.MinorType.BIT) + .build(); + + RowSet expected = client.rowSetBuilder(expectedSchema) + .addRow(true, false) + .build(); - assertEquals(result, TypeProtos.MinorType.TIME); + RowSetUtilities.verify(expected, results); } } diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestReverseImplicitCast.java b/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestReverseImplicitCast.java index 740bab079a..6dd52da5ed 100644 --- a/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestReverseImplicitCast.java +++ b/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestReverseImplicitCast.java @@ -64,8 +64,8 @@ public class TestReverseImplicitCast extends PopUnitTestBase { ValueVector.Accessor varcharAccessor1 = itr.next().getValueVector().getAccessor(); for (int i = 0; i < intAccessor1.getValueCount(); i++) { - assertEquals(intAccessor1.getObject(i), 10); - assertEquals(varcharAccessor1.getObject(i).toString(), "101"); + assertEquals(10, intAccessor1.getObject(i)); + assertEquals("101", varcharAccessor1.getObject(i).toString()); } batchLoader.clear(); diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/store/json/TestJsonReaderFns.java b/exec/java-exec/src/test/java/org/apache/drill/exec/store/json/TestJsonReaderFns.java index 000dbaac39..38d9e4e4a9 100644 --- a/exec/java-exec/src/test/java/org/apache/drill/exec/store/json/TestJsonReaderFns.java +++ b/exec/java-exec/src/test/java/org/apache/drill/exec/store/json/TestJsonReaderFns.java @@ -201,8 +201,10 @@ public class TestJsonReaderFns extends BaseTestJsonReader { runBoth(this::doTestRepeatedContainsFloat4); } + // Since decimal literals are read as FLOAT8s by the JSON readers, I don't + // believe that this tests the version of repeated_contains that it aims to. private void doTestRepeatedContainsFloat4() throws Exception { - final RowSet results = runTest("select repeated_contains(FLOAT4_col, -1000000000000.0) from cp.`parquet/alltypes_repeated.json`"); + final RowSet results = runTest("select repeated_contains(FLOAT4_col, cast(5.0 as float)) from cp.`parquet/alltypes_repeated.json`"); final RowSet expected = client.rowSetBuilder(bitCountSchema()) .addSingleCol(1) .addSingleCol(0) @@ -212,6 +214,22 @@ public class TestJsonReaderFns extends BaseTestJsonReader { new RowSetComparison(expected).verifyAndClearAll(results); } + @Test + public void testRepeatedContainsFloat8() throws Exception { + runBoth(this::doTestRepeatedContainsFloat8); + } + + private void doTestRepeatedContainsFloat8() throws Exception { + final RowSet results = runTest("select repeated_contains(FLOAT8_col, -10000000000000.0) from cp.`parquet/alltypes_repeated.json`"); + final RowSet expected = client.rowSetBuilder(bitCountSchema()) + .addSingleCol(1) + .addSingleCol(0) + .addSingleCol(0) + .addSingleCol(0) + .build(); + new RowSetComparison(expected).verifyAndClearAll(results); + } + @Test public void testRepeatedContainsVarchar() throws Exception { runBoth(this::doTestRepeatedContainsVarchar); diff --git a/exec/java-exec/src/test/resources/functions/cast/two_way_implicit_cast.json b/exec/java-exec/src/test/resources/functions/cast/two_way_implicit_cast.json index 008a59d85d..d9cf833d0d 100644 --- a/exec/java-exec/src/test/resources/functions/cast/two_way_implicit_cast.json +++ b/exec/java-exec/src/test/resources/functions/cast/two_way_implicit_cast.json @@ -23,8 +23,8 @@ child: 1, pop:"project", exprs: [ - {ref: "str_to_int_cast", expr:"8 + '2'" }, - {ref: "int_to_str_cast", expr:"substr(10123, 1, 3)" } + {ref: "str_to_num_cast", expr:"8 + '2'" }, + {ref: "num_to_str_cast", expr:"substr(10123, 1, 3)" } ] }, { diff --git a/protocol/src/main/java/org/apache/drill/common/types/TypeProtos.java b/protocol/src/main/java/org/apache/drill/common/types/TypeProtos.java index fe08ab001f..ecf49fd0c7 100644 --- a/protocol/src/main/java/org/apache/drill/common/types/TypeProtos.java +++ b/protocol/src/main/java/org/apache/drill/common/types/TypeProtos.java @@ -118,7 +118,7 @@ public final class TypeProtos { DECIMAL38SPARSE(10), /** * <pre> - * signed decimal with two digit precision + * signed decimal with two digit scale * </pre> * * <code>MONEY = 11;</code> @@ -423,7 +423,7 @@ public final class TypeProtos { public static final int DECIMAL38SPARSE_VALUE = 10; /** * <pre> - * signed decimal with two digit precision + * signed decimal with two digit scale * </pre> * * <code>MONEY = 11;</code> diff --git a/protocol/src/main/protobuf/Types.proto b/protocol/src/main/protobuf/Types.proto index d5cf943201..4ccc192b6b 100644 --- a/protocol/src/main/protobuf/Types.proto +++ b/protocol/src/main/protobuf/Types.proto @@ -35,7 +35,7 @@ enum MinorType { DECIMAL18 = 8; // a decimal supporting precision between 10 and 18 DECIMAL28SPARSE = 9; // a decimal supporting precision between 19 and 28 DECIMAL38SPARSE = 10; // a decimal supporting precision between 29 and 38 - MONEY = 11; // signed decimal with two digit precision + MONEY = 11; // signed decimal with two digit scale DATE = 12; // days since 4713bc TIME = 13; // time in micros before or after 2000/1/1 TIMETZ = 14; // time in micros before or after 2000/1/1 with timezone