IMPALA-6340,IMPALA-6518: Check that decimal types are compatible in FE In this patch we implement strict decimal type checking in the FE in various situations when DECIMAL_V2 is enabled. What is affected: - Union. If we union two decimals and it is not possible to come up with a decimal that will be able to contain all the digits, an error is thrown. For example, the union(decimal(20, 10), decimal(20, 20)) returns decimal(30, 20). However, for union(decimal(38, 0), decimal(38, 38)) the ideal return type would be decimal(76,38), but this is too large, so an error is thrown. - Insert. If we are inserting a decimal value into a column where we are not guaranteed that all digits will fit, an error is thrown. For example, inserting a decimal(38,0) value into a decimal(38,38) column. - Functions such as coalesce(). If we are unable to determine the output type that guarantees that all digits will fit from all the arguments, an error is thrown. For example, coalesce(decimal(38,38), decimal(38,0)) will throw an error. - Hash Join. When joining on two decimals, if a type cannot be determined that both columns can be cast to, we throw an error. For example, join on decimal(38,0) and decimal(38,38) will result in an error.
To avoid these errors, you need to use CAST() on some of the decimals. In this patch we also change the output decimal calculation of decimal round, truncate and related functions. If these functions are a no-op, the resulting decimal type is the same as the input type. Testing: - Ran a core build which passed. Change-Id: Id406f4189e01a909152985fabd5cca7a1527a568 Reviewed-on: http://gerrit.cloudera.org:8080/9930 Reviewed-by: Alex Behm <alex.b...@cloudera.com> Tested-by: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Project: http://git-wip-us.apache.org/repos/asf/impala/repo Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/d0f838b6 Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/d0f838b6 Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/d0f838b6 Branch: refs/heads/master Commit: d0f838b66a935310de9aeae7b9ba6513a5197ba9 Parents: 22c7ded Author: Taras Bobrovytsky <tbobrovyt...@cloudera.com> Authored: Wed Mar 28 18:26:59 2018 -0700 Committer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Committed: Sat Apr 28 03:33:02 2018 +0000 ---------------------------------------------------------------------- be/src/exprs/expr-test.cc | 20 +- .../apache/impala/analysis/AnalyticExpr.java | 6 +- .../org/apache/impala/analysis/Analyzer.java | 3 +- .../apache/impala/analysis/ArithmeticExpr.java | 4 +- .../apache/impala/analysis/BinaryPredicate.java | 8 +- .../org/apache/impala/analysis/CaseExpr.java | 3 +- .../org/apache/impala/analysis/ColumnDef.java | 3 +- .../impala/analysis/CompoundPredicate.java | 2 +- .../java/org/apache/impala/analysis/Expr.java | 54 +++-- .../impala/analysis/FunctionCallExpr.java | 17 +- .../org/apache/impala/analysis/InPredicate.java | 2 +- .../org/apache/impala/analysis/InsertStmt.java | 6 +- .../apache/impala/analysis/LikePredicate.java | 2 +- .../org/apache/impala/analysis/ModifyStmt.java | 4 +- .../apache/impala/analysis/PartitionSpec.java | 4 +- .../apache/impala/analysis/RangePartition.java | 2 +- .../apache/impala/analysis/StatementBase.java | 12 +- .../analysis/TimestampArithmeticExpr.java | 2 +- .../org/apache/impala/analysis/TypesUtil.java | 40 ++-- .../org/apache/impala/catalog/Function.java | 5 +- .../org/apache/impala/catalog/ScalarType.java | 27 ++- .../java/org/apache/impala/catalog/Type.java | 32 ++- .../org/apache/impala/planner/HashJoinNode.java | 9 +- .../impala/analysis/AnalyzeExprsTest.java | 73 +++++-- .../impala/analysis/AnalyzeStmtsTest.java | 95 +++++++- .../impala/analysis/ExprRewriteRulesTest.java | 5 +- .../apache/impala/analysis/TypesUtilTest.java | 216 ++++++++++--------- .../apache/impala/planner/PlannerTestBase.java | 42 ++-- .../PlannerTest/complex-types-file-formats.test | 30 +-- .../queries/PlannerTest/joins.test | 46 ++++ .../queries/PlannerTest/mt-dop-validation.test | 16 +- .../queries/PlannerTest/nested-loop-join.test | 10 +- .../queries/QueryTest/decimal-exprs.test | 37 +++- .../queries/QueryTest/decimal.test | 11 - 34 files changed, 553 insertions(+), 295 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/impala/blob/d0f838b6/be/src/exprs/expr-test.cc ---------------------------------------------------------------------- diff --git a/be/src/exprs/expr-test.cc b/be/src/exprs/expr-test.cc index 0f3f3d4..af8e753 100644 --- a/be/src/exprs/expr-test.cc +++ b/be/src/exprs/expr-test.cc @@ -7528,8 +7528,8 @@ TEST_F(ExprTest, DecimalFunctions) { TestValue("scale(round(cast(\"123.456\" as decimal(6, 3)), -2))", TYPE_INT, 0); TestValue("precision(round(123.456, -2))", TYPE_INT, 4); - TestValue("scale(truncate(123.456, 10))", TYPE_INT, 10); - TestValue("precision(truncate(cast(\"123.456\" as decimal(6, 3)), 10))", TYPE_INT, 13); + TestValue("scale(truncate(123.456, 10))", TYPE_INT, 3); + TestValue("precision(truncate(cast(\"123.456\" as decimal(6, 3)), 10))", TYPE_INT, 6); TestValue("scale(round(123.456, -10))", TYPE_INT, 0); TestValue("precision(round(cast(\"123.456\" as decimal(6, 3)), -10))", TYPE_INT, 4); @@ -7860,8 +7860,8 @@ TEST_F(ExprTest, DecimalFunctions) { ColumnType::CreateDecimalType(6, 3)); TestDecimalValue("round(cast('3.1615' as decimal(6,4)), 4)", Decimal4Value(31615), ColumnType::CreateDecimalType(6, 4)); - TestDecimalValue("round(cast('-3.1615' as decimal(6,4)), 5)", Decimal4Value(-316150), - ColumnType::CreateDecimalType(7, 5)); + TestDecimalValue("round(cast('-3.1615' as decimal(6,4)), 5)", Decimal4Value(-31615), + ColumnType::CreateDecimalType(6, 4)); TestDecimalValue("round(cast('175.0' as decimal(6,1)), 0)", Decimal4Value(175), ColumnType::CreateDecimalType(6, 0)); TestDecimalValue("round(cast('-175.0' as decimal(6,1)), -1)", Decimal4Value(-180), @@ -7887,8 +7887,8 @@ TEST_F(ExprTest, DecimalFunctions) { ColumnType::CreateDecimalType(16, 3)); TestDecimalValue("round(cast('-3.1615' as decimal(16,4)), 4)", Decimal8Value(-31615), ColumnType::CreateDecimalType(16, 4)); - TestDecimalValue("round(cast('3.1615' as decimal(16,4)), 5)", Decimal8Value(316150), - ColumnType::CreateDecimalType(17, 5)); + TestDecimalValue("round(cast('3.1615' as decimal(16,4)), 5)", Decimal8Value(31615), + ColumnType::CreateDecimalType(16, 4)); TestDecimalValue("round(cast('-999.951' as decimal(16,3)), 1)", Decimal8Value(-10000), ColumnType::CreateDecimalType(15, 1)); @@ -7950,8 +7950,8 @@ TEST_F(ExprTest, DecimalFunctions) { ColumnType::CreateDecimalType(5, 3)); TestDecimalValue("truncate(cast('-3.1615' as decimal(6,4)), 4)", Decimal4Value(-31615), ColumnType::CreateDecimalType(6, 4)); - TestDecimalValue("truncate(cast('3.1615' as decimal(6,4)), 5)", Decimal4Value(316150), - ColumnType::CreateDecimalType(7, 5)); + TestDecimalValue("truncate(cast('3.1615' as decimal(6,4)), 5)", Decimal4Value(31615), + ColumnType::CreateDecimalType(6, 4)); TestDecimalValue("truncate(cast('175.0' as decimal(6,1)), 0)", Decimal4Value(175), ColumnType::CreateDecimalType(5, 0)); TestDecimalValue("truncate(cast('-175.0' as decimal(6,1)), -1)", Decimal4Value(-170), @@ -7974,7 +7974,7 @@ TEST_F(ExprTest, DecimalFunctions) { TestDecimalValue("truncate(cast('3.1615' as decimal(16,4)), 4)", Decimal8Value(31615), ColumnType::CreateDecimalType(16, 4)); TestDecimalValue("truncate(cast('-3.1615' as decimal(16,4)), 5)", - Decimal8Value(-316150), ColumnType::CreateDecimalType(17, 5)); + Decimal8Value(-31615), ColumnType::CreateDecimalType(16, 4)); TestDecimalValue("truncate(cast('-175.0' as decimal(15,1)), 0)", Decimal8Value(-175), ColumnType::CreateDecimalType(14, 0)); TestDecimalValue("truncate(cast('175.0' as decimal(15,1)), -1)", Decimal8Value(170), @@ -7997,7 +7997,7 @@ TEST_F(ExprTest, DecimalFunctions) { TestDecimalValue("truncate(cast('-3.1615' as decimal(32,4)), 4)", Decimal16Value(-31615), ColumnType::CreateDecimalType(32, 4)); TestDecimalValue("truncate(cast('3.1615' as decimal(32,4)), 5)", - Decimal16Value(316150), ColumnType::CreateDecimalType(33, 5)); + Decimal16Value(31615), ColumnType::CreateDecimalType(32, 4)); TestDecimalValue("truncate(cast('-175.0' as decimal(35,1)), 0)", Decimal16Value(-175), ColumnType::CreateDecimalType(34, 0)); TestDecimalValue("truncate(cast('175.0' as decimal(35,1)), -1)", http://git-wip-us.apache.org/repos/asf/impala/blob/d0f838b6/fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java b/fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java index e9a4f1b..d033ee1 100644 --- a/fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java +++ b/fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java @@ -373,8 +373,8 @@ public class AnalyticExpr extends Expr { * Checks that the value expr of an offset boundary of a RANGE window is compatible * with orderingExprs (and that there's only a single ordering expr). */ - private void checkRangeOffsetBoundaryExpr(AnalyticWindow.Boundary boundary) - throws AnalysisException { + private void checkRangeOffsetBoundaryExpr( + AnalyticWindow.Boundary boundary) throws AnalysisException { Preconditions.checkState(boundary.getType().isOffset()); if (orderByElements_.size() > 1) { throw new AnalysisException("Only one ORDER BY expression allowed if used with " @@ -382,7 +382,7 @@ public class AnalyticExpr extends Expr { } Expr rangeExpr = boundary.getExpr(); if (!Type.isImplicitlyCastable( - rangeExpr.getType(), orderByElements_.get(0).getExpr().getType(), false)) { + rangeExpr.getType(), orderByElements_.get(0).getExpr().getType(), false, true)) { throw new AnalysisException( "The value expression of a PRECEDING/FOLLOWING clause of a RANGE window must " + "be implicitly convertable to the ORDER BY expression's type: " http://git-wip-us.apache.org/repos/asf/impala/blob/d0f838b6/fe/src/main/java/org/apache/impala/analysis/Analyzer.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/analysis/Analyzer.java b/fe/src/main/java/org/apache/impala/analysis/Analyzer.java index 02bcf31..f0a4b84 100644 --- a/fe/src/main/java/org/apache/impala/analysis/Analyzer.java +++ b/fe/src/main/java/org/apache/impala/analysis/Analyzer.java @@ -2245,7 +2245,7 @@ public class Analyzer { newCompatibleType = expr.getType(); } else { newCompatibleType = Type.getAssignmentCompatibleType( - lastCompatibleType, expr.getType(), false); + lastCompatibleType, expr.getType(), false, isDecimalV2()); } if (!newCompatibleType.isValid()) { throw new AnalysisException(String.format( @@ -2320,6 +2320,7 @@ public class Analyzer { public TQueryOptions getQueryOptions() { return globalState_.queryCtx.client_request.getQuery_options(); } + public boolean isDecimalV2() { return getQueryOptions().isDecimal_v2(); } public AuthorizationConfig getAuthzConfig() { return globalState_.authzConfig; } public ListMap<TNetworkAddress> getHostIndex() { return globalState_.hostIndex; } public ColumnLineageGraph getColumnLineageGraph() { return globalState_.lineageGraph; } http://git-wip-us.apache.org/repos/asf/impala/blob/d0f838b6/fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java b/fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java index 48ac2f2..e58f787 100644 --- a/fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java +++ b/fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java @@ -214,7 +214,7 @@ public class ArithmeticExpr extends Expr { throw new AnalysisException("Invalid non-integer argument to operation '" + op_.toString() + "': " + this.toSql()); } - type_ = Type.getAssignmentCompatibleType(t0, t1, false); + type_ = Type.getAssignmentCompatibleType(t0, t1, false, false); // If both of the children are null, we'll default to the INT version of the // operator. This prevents the BE from seeing NULL_TYPE. if (type_.isNull()) type_ = Type.INT; @@ -236,7 +236,7 @@ public class ArithmeticExpr extends Expr { fn_ = getBuiltinFunction(analyzer, op_.getName(), collectChildReturnTypes(), CompareMode.IS_SUPERTYPE_OF); Preconditions.checkNotNull(fn_); - castForFunctionCall(false); + castForFunctionCall(false, analyzer.isDecimalV2()); type_ = fn_.getReturnType(); return; default: http://git-wip-us.apache.org/repos/asf/impala/blob/d0f838b6/fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java b/fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java index e15b91f..cc82624 100644 --- a/fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java +++ b/fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java @@ -200,9 +200,11 @@ public class BinaryPredicate extends Predicate { } } - // Don't perform any casting for predicates with subqueries here. Any casting - // required will be performed when the subquery is unnested. - if (!contains(Subquery.class)) castForFunctionCall(true); + if (!contains(Subquery.class)) { + // Don't perform any casting for predicates with subqueries here. Any casting + // required will be performed when the subquery is unnested. + castForFunctionCall(true, analyzer.isDecimalV2()); + } // Determine selectivity // TODO: Compute selectivity for nested predicates. http://git-wip-us.apache.org/repos/asf/impala/blob/d0f838b6/fe/src/main/java/org/apache/impala/analysis/CaseExpr.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/analysis/CaseExpr.java b/fe/src/main/java/org/apache/impala/analysis/CaseExpr.java index a51ca60..196c5cd 100644 --- a/fe/src/main/java/org/apache/impala/analysis/CaseExpr.java +++ b/fe/src/main/java/org/apache/impala/analysis/CaseExpr.java @@ -287,7 +287,8 @@ public class CaseExpr extends Expr { } else { // If no case expr was given, then the when exprs should always return // boolean or be castable to boolean. - if (!Type.isImplicitlyCastable(whenExpr.getType(), Type.BOOLEAN, false)) { + if (!Type.isImplicitlyCastable(whenExpr.getType(), Type.BOOLEAN, + false, analyzer.isDecimalV2())) { Preconditions.checkState(isCase()); throw new AnalysisException("When expr '" + whenExpr.toSql() + "'" + " is not of type boolean and not castable to type boolean."); http://git-wip-us.apache.org/repos/asf/impala/blob/d0f838b6/fe/src/main/java/org/apache/impala/analysis/ColumnDef.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/analysis/ColumnDef.java b/fe/src/main/java/org/apache/impala/analysis/ColumnDef.java index ca17b1c..24f6029 100644 --- a/fe/src/main/java/org/apache/impala/analysis/ColumnDef.java +++ b/fe/src/main/java/org/apache/impala/analysis/ColumnDef.java @@ -275,7 +275,8 @@ public class ColumnDef { } } - if (!Type.isImplicitlyCastable(defaultValLiteral.getType(), type_, true)) { + if (!Type.isImplicitlyCastable(defaultValLiteral.getType(), type_, + true, analyzer.isDecimalV2())) { throw new AnalysisException(String.format("Default value %s (type: %s) " + "is not compatible with column '%s' (type: %s).", defaultValue_.toSql(), defaultValue_.getType().toSql(), colName_, type_.toSql())); http://git-wip-us.apache.org/repos/asf/impala/blob/d0f838b6/fe/src/main/java/org/apache/impala/analysis/CompoundPredicate.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/analysis/CompoundPredicate.java b/fe/src/main/java/org/apache/impala/analysis/CompoundPredicate.java index f0ff00f..579fc79 100644 --- a/fe/src/main/java/org/apache/impala/analysis/CompoundPredicate.java +++ b/fe/src/main/java/org/apache/impala/analysis/CompoundPredicate.java @@ -132,7 +132,7 @@ public class CompoundPredicate extends Predicate { CompareMode.IS_NONSTRICT_SUPERTYPE_OF); Preconditions.checkState(fn_ != null); Preconditions.checkState(fn_.getReturnType().isBoolean()); - castForFunctionCall(false); + castForFunctionCall(false, analyzer.isDecimalV2()); if (!getChild(0).hasSelectivity() || (children_.size() == 2 && !getChild(1).hasSelectivity())) { http://git-wip-us.apache.org/repos/asf/impala/blob/d0f838b6/fe/src/main/java/org/apache/impala/analysis/Expr.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/analysis/Expr.java b/fe/src/main/java/org/apache/impala/analysis/Expr.java index c519ea4..5cdcb16 100644 --- a/fe/src/main/java/org/apache/impala/analysis/Expr.java +++ b/fe/src/main/java/org/apache/impala/analysis/Expr.java @@ -426,7 +426,6 @@ abstract public class Expr extends TreeNode<Expr> implements ParseNode, Cloneabl /** * Generates the necessary casts for the children of this expr to call fn_. * child(0) is cast to the function's first argument, child(1) to the second etc. - * This does not do any validation and the casts are assumed to be safe. * * If ignoreWildcardDecimals is true, the function will not cast arguments that * are wildcard decimals. This is used for builtins where the cast is done within @@ -437,18 +436,42 @@ abstract public class Expr extends TreeNode<Expr> implements ParseNode, Cloneabl * e.g. fn(decimal(*), decimal(*)) * called with fn(decimal(10,2), decimal(5,3)) * both children will be cast to (11, 3). + * + * If strictDecimal is true, we will only consider casts between decimal types that + * result in no loss of information. If it is not possible to come with such casts, + * we will throw an exception. */ - protected void castForFunctionCall(boolean ignoreWildcardDecimals) - throws AnalysisException { + protected void castForFunctionCall( + boolean ignoreWildcardDecimals, boolean strictDecimal) throws AnalysisException { Preconditions.checkState(fn_ != null); Type[] fnArgs = fn_.getArgs(); - Type resolvedWildcardType = getResolvedWildCardType(); + Type resolvedWildcardType = getResolvedWildCardType(strictDecimal); + if (resolvedWildcardType != null) { + if (resolvedWildcardType.isNull()) { + throw new AnalysisException(String.format( + "Cannot resolve DECIMAL precision and scale from NULL type in %s function.", + fn_.getFunctionName().getFunction())); + } + if (resolvedWildcardType.isInvalid() && !ignoreWildcardDecimals) { + StringBuilder argTypes = new StringBuilder(); + for (int j = 0; j < children_.size(); ++j) { + if (argTypes.length() > 0) argTypes.append(", "); + Type childType = children_.get(j).type_; + argTypes.append(childType.toSql()); + } + throw new AnalysisException(String.format( + "Cannot resolve DECIMAL types of the %s(%s) function arguments. You need " + + "to wrap the arguments in a CAST.", fn_.getFunctionName().getFunction(), + argTypes.toString())); + } + } for (int i = 0; i < children_.size(); ++i) { // For varargs, we must compare with the last type in fnArgs.argTypes. int ix = Math.min(fnArgs.length - 1, i); if (fnArgs[ix].isWildcardDecimal()) { if (children_.get(i).type_.isDecimal() && ignoreWildcardDecimals) continue; Preconditions.checkState(resolvedWildcardType != null); + Preconditions.checkState(!resolvedWildcardType.isInvalid()); if (!children_.get(i).type_.equals(resolvedWildcardType)) { castChild(resolvedWildcardType, i); } @@ -460,9 +483,11 @@ abstract public class Expr extends TreeNode<Expr> implements ParseNode, Cloneabl /** * Returns the max resolution type of all the wild card decimal types. - * Returns null if there are no wild card types. + * Returns null if there are no wild card types. If strictDecimal is enabled, will + * return an invalid type if it is not possible to come up with a decimal type that + * is guaranteed to not lose information. */ - Type getResolvedWildCardType() throws AnalysisException { + Type getResolvedWildCardType(boolean strictDecimal) { Type result = null; Type[] fnArgs = fn_.getArgs(); for (int i = 0; i < children_.size(); ++i) { @@ -475,19 +500,18 @@ abstract public class Expr extends TreeNode<Expr> implements ParseNode, Cloneabl "Child expr should have been resolved."); Preconditions.checkState(childType.isScalarType(), "Function should not have resolved with a non-scalar child type."); - ScalarType decimalType = (ScalarType) childType; if (result == null) { + ScalarType decimalType = (ScalarType) childType; result = decimalType.getMinResolutionDecimal(); } else { - result = Type.getAssignmentCompatibleType(result, childType, false); + Preconditions.checkState(childType.isDecimal() || result.isDecimal()); + result = Type.getAssignmentCompatibleType( + result, childType, false, strictDecimal); } } - if (result != null) { - if (result.isNull()) { - throw new AnalysisException( - "Cannot resolve DECIMAL precision and scale from NULL type."); - } - Preconditions.checkState(result.isDecimal() && !result.isWildcardDecimal()); + if (result != null && !result.isNull()) { + Preconditions.checkState(result.isDecimal() || result.isInvalid()); + Preconditions.checkState(!result.isWildcardDecimal()); } return result; } @@ -1232,7 +1256,7 @@ abstract public class Expr extends TreeNode<Expr> implements ParseNode, Cloneabl * failure to convert a string literal to a date literal */ public final Expr castTo(Type targetType) throws AnalysisException { - Type type = Type.getAssignmentCompatibleType(this.type_, targetType, false); + Type type = Type.getAssignmentCompatibleType(this.type_, targetType, false, false); Preconditions.checkState(type.isValid(), "cast %s to %s", this.type_, targetType); // If the targetType is NULL_TYPE then ignore the cast because NULL_TYPE // is compatible with all types and no cast is necessary. http://git-wip-us.apache.org/repos/asf/impala/blob/d0f838b6/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java b/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java index 46000b3..321cfbd 100644 --- a/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java +++ b/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java @@ -428,11 +428,10 @@ public class FunctionCallExpr extends Expr { } NumericLiteral scaleLiteral = (NumericLiteral) LiteralExpr.create( children_.get(1), analyzer.getQueryCtx()); - digitsAfter = (int)scaleLiteral.getLongValue(); - if (Math.abs(digitsAfter) > ScalarType.MAX_SCALE) { - throw new AnalysisException("Cannot round/truncate to scales greater than " + - ScalarType.MAX_SCALE + "."); - } + // If scale is greater than the scale of the decimal, this should be a no-op, + // so we do not need change the scale of the output decimal. + digitsAfter = Math.min(digitsAfter, (int)scaleLiteral.getLongValue()); + Preconditions.checkState(digitsAfter <= ScalarType.MAX_SCALE); // Round/Truncate to a negative scale means to round to the digit before // the decimal e.g. round(1234.56, -2) would be 1200. // The resulting scale is always 0. @@ -446,11 +445,15 @@ public class FunctionCallExpr extends Expr { fnName_.getFunction().equalsIgnoreCase("dround")) && digitsAfter < childType.decimalScale()) { // If we are rounding to fewer decimal places, it's possible we need another - // digit before the decimal. + // digit before the decimal if the value gets rounded up. ++digitsBefore; } } Preconditions.checkState(returnType.isDecimal() && !returnType.isWildcardDecimal()); + if (analyzer.isDecimalV2()) { + if (digitsBefore + digitsAfter > 38) return Type.INVALID; + return ScalarType.createDecimalType(digitsBefore + digitsAfter, digitsAfter); + } return ScalarType.createClippedDecimalType(digitsBefore + digitsAfter, digitsAfter); } @@ -587,7 +590,7 @@ public class FunctionCallExpr extends Expr { "Analytic function requires an OVER clause: " + toSql()); } - castForFunctionCall(false); + castForFunctionCall(false, analyzer.isDecimalV2()); type_ = fn_.getReturnType(); if (type_.isDecimal() && type_.isWildcardDecimal()) { type_ = resolveDecimalReturnType(analyzer); http://git-wip-us.apache.org/repos/asf/impala/blob/d0f838b6/fe/src/main/java/org/apache/impala/analysis/InPredicate.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/analysis/InPredicate.java b/fe/src/main/java/org/apache/impala/analysis/InPredicate.java index 89479ad..a1f545f 100644 --- a/fe/src/main/java/org/apache/impala/analysis/InPredicate.java +++ b/fe/src/main/java/org/apache/impala/analysis/InPredicate.java @@ -168,7 +168,7 @@ public class InPredicate extends Predicate { } Preconditions.checkNotNull(fn_); Preconditions.checkState(fn_.getReturnType().isBoolean()); - castForFunctionCall(false); + castForFunctionCall(false, analyzer.isDecimalV2()); } // TODO: Fix selectivity_ for nested predicate http://git-wip-us.apache.org/repos/asf/impala/blob/d0f838b6/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java b/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java index 43d582b..1d31b3e 100644 --- a/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java +++ b/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java @@ -669,7 +669,8 @@ public class InsertStmt extends StatementBase { for (int i = 0; i < selectListExprs.size(); ++i) { Column targetColumn = selectExprTargetColumns.get(i); Expr compatibleExpr = checkTypeCompatibility( - targetTableName_.toString(), targetColumn, selectListExprs.get(i)); + targetTableName_.toString(), targetColumn, selectListExprs.get(i), + analyzer.getQueryOptions().isDecimal_v2()); if (targetColumn.getPosition() < numClusteringCols) { // This is a dynamic clustering column tmpPartitionKeyExprs.add(compatibleExpr); @@ -691,7 +692,8 @@ public class InsertStmt extends StatementBase { // tableColumns is guaranteed to exist after the earlier analysis checks Column tableColumn = table_.getColumn(pkv.getColName()); Expr compatibleExpr = checkTypeCompatibility( - targetTableName_.toString(), tableColumn, pkv.getLiteralValue()); + targetTableName_.toString(), tableColumn, + pkv.getLiteralValue(), analyzer.isDecimalV2()); tmpPartitionKeyExprs.add(compatibleExpr); tmpPartitionKeyNames.add(pkv.getColName()); } http://git-wip-us.apache.org/repos/asf/impala/blob/d0f838b6/fe/src/main/java/org/apache/impala/analysis/LikePredicate.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/analysis/LikePredicate.java b/fe/src/main/java/org/apache/impala/analysis/LikePredicate.java index 74d5fc6..bc10b2a 100644 --- a/fe/src/main/java/org/apache/impala/analysis/LikePredicate.java +++ b/fe/src/main/java/org/apache/impala/analysis/LikePredicate.java @@ -141,7 +141,7 @@ public class LikePredicate extends Predicate { "invalid regular expression in '" + this.toSql() + "'"); } } - castForFunctionCall(false); + castForFunctionCall(false, analyzer.isDecimalV2()); } @Override http://git-wip-us.apache.org/repos/asf/impala/blob/d0f838b6/fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java b/fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java index 32b4a1d..c73109e 100644 --- a/fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java +++ b/fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java @@ -283,8 +283,8 @@ public abstract class ModifyStmt extends StatementBase { format("Duplicate value assignment to column: '%s'", lhsSlotRef.toSql())); } - rhsExpr = checkTypeCompatibility( - targetTableRef_.getDesc().getTable().getFullName(), c, rhsExpr); + rhsExpr = checkTypeCompatibility(targetTableRef_.getDesc().getTable().getFullName(), + c, rhsExpr, analyzer.isDecimalV2()); uniqueSlots.add(lhsSlotRef.getSlotId()); selectList.add(new SelectListItem(rhsExpr, null)); referencedColumns.add(colIndexMap.get(c.getName())); http://git-wip-us.apache.org/repos/asf/impala/blob/d0f838b6/fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java b/fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java index c9b289f..1d76df6 100644 --- a/fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java +++ b/fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java @@ -95,8 +95,8 @@ public class PartitionSpec extends PartitionSpecBase { Type colType = c.getType(); Type literalType = pk.getValue().getType(); - Type compatibleType = - Type.getAssignmentCompatibleType(colType, literalType, false); + Type compatibleType = Type.getAssignmentCompatibleType(colType, literalType, + false, analyzer.isDecimalV2()); if (!compatibleType.isValid()) { throw new AnalysisException(String.format("Value of partition spec (column=%s) " + "has incompatible type: '%s'. Expected type: '%s'.", http://git-wip-us.apache.org/repos/asf/impala/blob/d0f838b6/fe/src/main/java/org/apache/impala/analysis/RangePartition.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/analysis/RangePartition.java b/fe/src/main/java/org/apache/impala/analysis/RangePartition.java index b45a785..caac462 100644 --- a/fe/src/main/java/org/apache/impala/analysis/RangePartition.java +++ b/fe/src/main/java/org/apache/impala/analysis/RangePartition.java @@ -186,7 +186,7 @@ public class RangePartition implements ParseNode { org.apache.impala.catalog.Type literalType = literal.getType(); if (!org.apache.impala.catalog.Type.isImplicitlyCastable(literalType, colType, - true)) { + true, analyzer.isDecimalV2())) { throw new AnalysisException(String.format("Range partition value %s " + "(type: %s) is not type compatible with partitioning column '%s' (type: %s).", literal.toSql(), literalType, pkColumn.getColName(), colType.toSql())); http://git-wip-us.apache.org/repos/asf/impala/blob/d0f838b6/fe/src/main/java/org/apache/impala/analysis/StatementBase.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/analysis/StatementBase.java b/fe/src/main/java/org/apache/impala/analysis/StatementBase.java index 30dc9b5..c770742 100644 --- a/fe/src/main/java/org/apache/impala/analysis/StatementBase.java +++ b/fe/src/main/java/org/apache/impala/analysis/StatementBase.java @@ -173,19 +173,23 @@ public abstract class StatementBase implements ParseNode { /** * Checks that 'srcExpr' is type compatible with 'dstCol' and returns a type compatible - * expression by applying a CAST() if needed. Throws an AnalysisException the types + * expression by applying a CAST() if needed. Throws an AnalysisException if the types * are incompatible. 'dstTableName' is only used when constructing an AnalysisException * message. + * + * If strictDecimal is true, only consider casts that result in no loss of information + * when casting between decimal types. */ - protected Expr checkTypeCompatibility(String dstTableName, Column dstCol, Expr srcExpr) - throws AnalysisException { + protected Expr checkTypeCompatibility(String dstTableName, Column dstCol, + Expr srcExpr, boolean strictDecimal) throws AnalysisException { Type dstColType = dstCol.getType(); Type srcExprType = srcExpr.getType(); // Trivially compatible, unless the type is complex. if (dstColType.equals(srcExprType) && !dstColType.isComplexType()) return srcExpr; - Type compatType = Type.getAssignmentCompatibleType(dstColType, srcExprType, false); + Type compatType = Type.getAssignmentCompatibleType( + dstColType, srcExprType, false, strictDecimal); if (!compatType.isValid()) { throw new AnalysisException(String.format( "Target table '%s' is incompatible with source expressions.\nExpression '%s' " + http://git-wip-us.apache.org/repos/asf/impala/blob/d0f838b6/fe/src/main/java/org/apache/impala/analysis/TimestampArithmeticExpr.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/analysis/TimestampArithmeticExpr.java b/fe/src/main/java/org/apache/impala/analysis/TimestampArithmeticExpr.java index 5a5468d..982f5e4 100644 --- a/fe/src/main/java/org/apache/impala/analysis/TimestampArithmeticExpr.java +++ b/fe/src/main/java/org/apache/impala/analysis/TimestampArithmeticExpr.java @@ -159,7 +159,7 @@ public class TimestampArithmeticExpr extends Expr { fn_ = getBuiltinFunction(analyzer, funcOpName.toLowerCase(), collectChildReturnTypes(), CompareMode.IS_NONSTRICT_SUPERTYPE_OF); - castForFunctionCall(false); + castForFunctionCall(false, analyzer.isDecimalV2()); Preconditions.checkNotNull(fn_); Preconditions.checkState(fn_.getReturnType().isTimestamp()); http://git-wip-us.apache.org/repos/asf/impala/blob/d0f838b6/fe/src/main/java/org/apache/impala/analysis/TypesUtil.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/analysis/TypesUtil.java b/fe/src/main/java/org/apache/impala/analysis/TypesUtil.java index 9effc33..211a296 100644 --- a/fe/src/main/java/org/apache/impala/analysis/TypesUtil.java +++ b/fe/src/main/java/org/apache/impala/analysis/TypesUtil.java @@ -47,31 +47,17 @@ public class TypesUtil { } /** - * Returns the smallest integer type that can store decType without loss - * of precision. decType must have scale == 0. - * In the case where the decimal can be bigger than BIGINT, we return - * BIGINT (and the execution will report it as overflows). - */ - public static ScalarType getContainingIntType(ScalarType decType) { - Preconditions.checkState(decType.isFullySpecifiedDecimal()); - Preconditions.checkState(decType.decimalScale() == 0); - // TINYINT_MAX = 128 - if (decType.decimalPrecision() <= 2) return Type.TINYINT; - // SMALLINT_MAX = 32768 - if (decType.decimalPrecision() <= 4) return Type.SMALLINT; - // INT_MAX = 2147483648 - if (decType.decimalPrecision() <= 9) return Type.INT; - return Type.BIGINT; - } - - /** * Returns the decimal type that can hold t1 and t2 without loss of precision. * decimal(10, 2) && decimal(12, 2) -> decimal(12, 2) * decimal (10, 5) && decimal(12, 3) -> decimal(14, 5) * Either t1 or t2 can be a wildcard decimal (but not both). + * + * If strictDecimal is true, returns a type that results in no loss of information. If + * this is not possible, returns INVLAID. For example, + * decimal(38,0) && decimal(38,38) -> INVALID. */ public static ScalarType getDecimalAssignmentCompatibleType( - ScalarType t1, ScalarType t2) { + ScalarType t1, ScalarType t2, boolean strictDecimal) { Preconditions.checkState(t1.isDecimal()); Preconditions.checkState(t2.isDecimal()); Preconditions.checkState(!(t1.isWildcardDecimal() && t2.isWildcardDecimal())); @@ -87,6 +73,10 @@ public class TypesUtil { int p2 = t2.decimalPrecision(); int digitsBefore = Math.max(p1 - s1, p2 - s2); int digitsAfter = Math.max(s1, s2); + if (strictDecimal) { + if (digitsBefore + digitsAfter > 38) return Type.INVALID; + return ScalarType.createDecimalType(digitsBefore + digitsAfter, digitsAfter); + } return ScalarType.createClippedDecimalType(digitsBefore + digitsAfter, digitsAfter); } @@ -95,7 +85,7 @@ public class TypesUtil { * if the operation does not make sense for the types. */ public static Type getArithmeticResultType(Type t1, Type t2, - ArithmeticExpr.Operator op, boolean decimal_v2) throws AnalysisException { + ArithmeticExpr.Operator op, boolean decimalV2) throws AnalysisException { Preconditions.checkState(t1.isNumericType() || t1.isNull()); Preconditions.checkState(t2.isNumericType() || t2.isNull()); @@ -116,7 +106,7 @@ public class TypesUtil { t2 = ((ScalarType) t2).getMinResolutionDecimal(); Preconditions.checkState(t1.isDecimal()); Preconditions.checkState(t2.isDecimal()); - return getDecimalArithmeticResultType(t1, t2, op, decimal_v2); + return getDecimalArithmeticResultType(t1, t2, op, decimalV2); } Type type = null; @@ -128,12 +118,12 @@ public class TypesUtil { // Otherwise, promote the compatible type to the next higher resolution type, // to ensure that that a <op> b won't overflow/underflow. Type compatibleType = - ScalarType.getAssignmentCompatibleType(t1, t2, false); + ScalarType.getAssignmentCompatibleType(t1, t2, false, false); Preconditions.checkState(compatibleType.isScalarType()); type = ((ScalarType) compatibleType).getNextResolutionType(); break; case MOD: - type = ScalarType.getAssignmentCompatibleType(t1, t2, false); + type = ScalarType.getAssignmentCompatibleType(t1, t2, false, false); break; case DIVIDE: type = Type.DOUBLE; @@ -152,8 +142,8 @@ public class TypesUtil { * TODO: IMPALA-4924: remove DECIMAL V1 code. */ private static ScalarType getDecimalArithmeticResultType(Type t1, Type t2, - ArithmeticExpr.Operator op, boolean decimal_v2) throws AnalysisException { - if (decimal_v2) return getDecimalArithmeticResultTypeV2(t1, t2, op); + ArithmeticExpr.Operator op, boolean decimalV2) throws AnalysisException { + if (decimalV2) return getDecimalArithmeticResultTypeV2(t1, t2, op); return getDecimalArithmeticResultTypeV1(t1, t2, op); } http://git-wip-us.apache.org/repos/asf/impala/blob/d0f838b6/fe/src/main/java/org/apache/impala/catalog/Function.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/catalog/Function.java b/fe/src/main/java/org/apache/impala/catalog/Function.java index 59ee5ea..60893a4 100644 --- a/fe/src/main/java/org/apache/impala/catalog/Function.java +++ b/fe/src/main/java/org/apache/impala/catalog/Function.java @@ -213,7 +213,8 @@ public class Function extends CatalogObjectImpl { } if (this.hasVarArgs_ && other.argTypes_.length < this.argTypes_.length) return false; for (int i = 0; i < this.argTypes_.length; ++i) { - if (!Type.isImplicitlyCastable(other.argTypes_[i], this.argTypes_[i], strict)) { + if (!Type.isImplicitlyCastable( + other.argTypes_[i], this.argTypes_[i], strict, strict)) { return false; } } @@ -222,7 +223,7 @@ public class Function extends CatalogObjectImpl { for (int i = this.argTypes_.length; i < other.argTypes_.length; ++i) { if (other.argTypes_[i].matchesType(this.getVarArgsType())) continue; if (!Type.isImplicitlyCastable(other.argTypes_[i], this.getVarArgsType(), - strict)) { + strict, strict)) { return false; } } http://git-wip-us.apache.org/repos/asf/impala/blob/d0f838b6/fe/src/main/java/org/apache/impala/catalog/ScalarType.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/catalog/ScalarType.java b/fe/src/main/java/org/apache/impala/catalog/ScalarType.java index 068d6fa..43df496 100644 --- a/fe/src/main/java/org/apache/impala/catalog/ScalarType.java +++ b/fe/src/main/java/org/apache/impala/catalog/ScalarType.java @@ -376,7 +376,8 @@ public class ScalarType extends Type { } else if (isNull()) { return ScalarType.NULL; } else if (isDecimal()) { - return createClippedDecimalType(MAX_PRECISION, scale_); + Preconditions.checkState(scale_ <= MAX_PRECISION); + return createDecimalType(MAX_PRECISION, scale_); } else { return ScalarType.INVALID; } @@ -387,7 +388,8 @@ public class ScalarType extends Type { if (type_ == PrimitiveType.DOUBLE || type_ == PrimitiveType.BIGINT || isNull()) { return this; } else if (type_ == PrimitiveType.DECIMAL) { - return createClippedDecimalType(MAX_PRECISION, scale_); + Preconditions.checkState(scale_ <= MAX_PRECISION); + return createDecimalType(MAX_PRECISION, scale_); } return createType(PrimitiveType.values()[type_.ordinal() + 1]); } @@ -426,12 +428,16 @@ public class ScalarType extends Type { /** * Return type t such that values from both t1 and t2 can be assigned to t. - * If strict, only return types when there will be no loss of precision. * Returns INVALID_TYPE if there is no such type or if any of t1 and t2 * is INVALID_TYPE. + * + * If strictDecimal is true, only return types that result in no loss of information + * when both inputs are decimal. + * If strict is true, only return types that result in no loss of information + * when at least one of the inputs is not decimal. */ public static ScalarType getAssignmentCompatibleType(ScalarType t1, - ScalarType t2, boolean strict) { + ScalarType t2, boolean strict, boolean strictDecimal) { if (!t1.isValid() || !t2.isValid()) return INVALID; if (t1.equals(t2)) return t1; if (t1.isNull()) return t2; @@ -489,7 +495,8 @@ public class ScalarType extends Type { } if (t1Decimal.isSupertypeOf(t2Decimal)) return t1; if (t2Decimal.isSupertypeOf(t1Decimal)) return t2; - return TypesUtil.getDecimalAssignmentCompatibleType(t1Decimal, t2Decimal); + return TypesUtil.getDecimalAssignmentCompatibleType( + t1Decimal, t2Decimal, strictDecimal); } PrimitiveType smallerType = @@ -509,10 +516,14 @@ public class ScalarType extends Type { /** * Returns true t1 can be implicitly cast to t2, false otherwise. - * If strict is true, only consider casts that result in no loss of precision. + * + * If strictDecimal is true, only consider casts that result in no loss of information + * when casting between decimal types. + * If strict is true, only consider casts that result in no loss of information when + * casting between any two types other than both decimals. */ public static boolean isImplicitlyCastable(ScalarType t1, ScalarType t2, - boolean strict) { - return getAssignmentCompatibleType(t1, t2, strict).matchesType(t2); + boolean strict, boolean strictDecimal) { + return getAssignmentCompatibleType(t1, t2, strict, strictDecimal).matchesType(t2); } } http://git-wip-us.apache.org/repos/asf/impala/blob/d0f838b6/fe/src/main/java/org/apache/impala/catalog/Type.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/catalog/Type.java b/fe/src/main/java/org/apache/impala/catalog/Type.java index edf44f0..a85cd40 100644 --- a/fe/src/main/java/org/apache/impala/catalog/Type.java +++ b/fe/src/main/java/org/apache/impala/catalog/Type.java @@ -285,17 +285,23 @@ public abstract class Type { /** * Returns true if t1 can be implicitly cast to t2 according to Impala's casting rules. - * Implicit casts are always allowed when no loss of precision would result (i.e. every - * value of t1 can be represented exactly by a value of t2). Implicit casts are allowed - * in certain other cases such as casting numeric types to floating point types and - * converting strings to timestamps. - * If strict is true, only consider casts that result in no loss of precision. + * Implicit casts are always allowed when no loss of information would result (i.e. + * every value of t1 can be represented exactly by a value of t2). Implicit casts are + * allowed in certain other cases such as casting numeric types to floating point types + * and converting strings to timestamps. + * + * If strictDecimal is true, only consider casts that result in no loss of information + * when casting between decimal types. + * If strict is true, only consider casts that result in no loss of information when + * casting between any two types other than both decimals. + * * TODO: Support casting of non-scalar types. */ - public static boolean isImplicitlyCastable(Type t1, Type t2, boolean strict) { + public static boolean isImplicitlyCastable( + Type t1, Type t2, boolean strict, boolean strictDecimal) { if (t1.isScalarType() && t2.isScalarType()) { return ScalarType.isImplicitlyCastable( - (ScalarType) t1, (ScalarType) t2, strict); + (ScalarType) t1, (ScalarType) t2, strict, strictDecimal); } return false; } @@ -305,12 +311,20 @@ public abstract class Type { * explicit cast. If strict, does not consider conversions that would result in loss * of precision (e.g. converting decimal to float). Returns INVALID_TYPE if there is * no such type or if any of t1 and t2 is INVALID_TYPE. + * + * If strictDecimal is true, only consider casts that result in no loss of information + * when casting between decimal types. + * If strict is true, only consider casts that result in no loss of information when + * casting between any two types other than both decimals. + * + * * TODO: Support non-scalar types. */ - public static Type getAssignmentCompatibleType(Type t1, Type t2, boolean strict) { + public static Type getAssignmentCompatibleType( + Type t1, Type t2, boolean strict, boolean strictDecimal) { if (t1.isScalarType() && t2.isScalarType()) { return ScalarType.getAssignmentCompatibleType( - (ScalarType) t1, (ScalarType) t2, strict); + (ScalarType) t1, (ScalarType) t2, strict, strictDecimal); } return ScalarType.INVALID; } http://git-wip-us.apache.org/repos/asf/impala/blob/d0f838b6/fe/src/main/java/org/apache/impala/planner/HashJoinNode.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/planner/HashJoinNode.java b/fe/src/main/java/org/apache/impala/planner/HashJoinNode.java index d04a15e..2730cfc 100644 --- a/fe/src/main/java/org/apache/impala/planner/HashJoinNode.java +++ b/fe/src/main/java/org/apache/impala/planner/HashJoinNode.java @@ -82,7 +82,14 @@ public class HashJoinNode extends JoinNode { throw new InternalException("Cannot compare " + t0.toSql() + " to " + t1.toSql() + " in join predicate."); } - Type compatibleType = Type.getAssignmentCompatibleType(t0, t1, false); + Type compatibleType = Type.getAssignmentCompatibleType( + t0, t1, false, analyzer.isDecimalV2()); + if (compatibleType.isInvalid()) { + throw new InternalException(String.format( + "Unable create a hash join with equi-join predicate %s " + + "because the operands cannot be cast without loss of precision. " + + "Operand types: %s = %s.", eqPred.toSql(), t0.toSql(), t1.toSql())); + } Preconditions.checkState(compatibleType.isDecimal() || compatibleType.isStringType()); try { http://git-wip-us.apache.org/repos/asf/impala/blob/d0f838b6/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java ---------------------------------------------------------------------- diff --git a/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java b/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java index df99a62..412c254 100644 --- a/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java +++ b/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java @@ -1213,7 +1213,7 @@ public class AnalyzeExprsTest extends AnalyzerTest { Type.BIGINT, Type.FLOAT, Type.DOUBLE , Type.NULL }; for (Type type1: numericTypes) { for (Type type2: numericTypes) { - Type t = Type.getAssignmentCompatibleType(type1, type2, false); + Type t = Type.getAssignmentCompatibleType(type1, type2, false, false); assertTrue(t.isScalarType()); ScalarType compatibleType = (ScalarType) t; Type promotedType = compatibleType.getNextResolutionType(); @@ -1295,9 +1295,11 @@ public class AnalyzeExprsTest extends AnalyzerTest { for (Type type1: types) { for (Type type2: types) { // Prefer strict matching. - Type compatibleType = Type.getAssignmentCompatibleType(type1, type2, true); + Type compatibleType = Type.getAssignmentCompatibleType( + type1, type2, true, true); if (compatibleType.isInvalid()) { - compatibleType = Type.getAssignmentCompatibleType(type1, type2, false); + compatibleType = Type.getAssignmentCompatibleType( + type1, type2, false, false); } typeCastTest(type1, type2, false, null, cmpOp, compatibleType); typeCastTest(type1, type2, true, null, cmpOp, compatibleType); @@ -2511,14 +2513,13 @@ public class AnalyzeExprsTest extends AnalyzerTest { AnalysisError("select round(cast(1.123 as decimal(10,3)), 5.1)", "No matching function with signature: round(DECIMAL(10,3), DECIMAL(2,1))"); - AnalysisError("select round(cast(1.123 as decimal(30,20)), 40)", - "Cannot round/truncate to scales greater than 38."); + AnalyzesOk("select round(cast(1.123 as decimal(30,20)), 40)"); for (final String alias : aliasesOfTruncate) { - AnalysisError(String.format("select truncate(cast(1.123 as decimal(10,3)), 40)", - alias), "Cannot round/truncate to scales greater than 38."); + AnalyzesOk(String.format("select %s(cast(1.123 as decimal(10,3)), 40)", alias)); AnalyzesOk(String.format("select %s(NULL)", alias)); AnalysisError(String.format("select %s(NULL, 1)", alias), - "Cannot resolve DECIMAL precision and scale from NULL type."); + String.format("Cannot resolve DECIMAL precision and scale from NULL type " + + "in %s function.", alias)); } AnalysisError("select round(cast(1.123 as decimal(10,3)), NULL)", "round() cannot be called with a NULL second argument."); @@ -2531,18 +2532,22 @@ public class AnalyzeExprsTest extends AnalyzerTest { "No matching function with signature: precision(FLOAT)"); AnalysisError("select precision(NULL)", - "Cannot resolve DECIMAL precision and scale from NULL type."); + "Cannot resolve DECIMAL precision and scale from NULL type in " + + "precision function"); AnalysisError("select scale(NULL)", - "Cannot resolve DECIMAL precision and scale from NULL type."); + "Cannot resolve DECIMAL precision and scale from NULL type in " + + "scale function."); testDecimalExpr("round(1.23)", ScalarType.createDecimalType(2, 0)); testDecimalExpr("round(1.23, 1)", ScalarType.createDecimalType(3, 1)); testDecimalExpr("round(1.23, 0)", ScalarType.createDecimalType(2, 0)); - testDecimalExpr("round(1.23, 3)", ScalarType.createDecimalType(4, 3)); + testDecimalExpr("round(1.23, 3)", ScalarType.createDecimalType(3, 2)); testDecimalExpr("round(1.23, -1)", ScalarType.createDecimalType(2, 0)); testDecimalExpr("round(1.23, -2)", ScalarType.createDecimalType(2, 0)); testDecimalExpr("round(cast(1.23 as decimal(3,2)), -2)", ScalarType.createDecimalType(2, 0)); + testDecimalExpr("round(cast(1.23 as decimal(30, 20)), 40)", + ScalarType.createDecimalType(30, 20)); testDecimalExpr("ceil(123.45)", ScalarType.createDecimalType(4, 0)); testDecimalExpr("floor(12.345)", ScalarType.createDecimalType(3, 0)); @@ -2555,12 +2560,56 @@ public class AnalyzeExprsTest extends AnalyzerTest { testDecimalExpr(String.format("%s(1.23, 0)", alias), ScalarType.createDecimalType(1, 0)); testDecimalExpr(String.format("%s(1.23, 3)", alias), - ScalarType.createDecimalType(4, 3)); + ScalarType.createDecimalType(3, 2)); testDecimalExpr(String.format("%s(1.23, -1)", alias), ScalarType.createDecimalType(1, 0)); testDecimalExpr(String.format("%s(1.23, -2)", alias), ScalarType.createDecimalType(1, 0)); + testDecimalExpr(String.format("%s(cast(1.23 as decimal(30, 20)), 40)", alias), + ScalarType.createDecimalType(30, 20)); } + + AnalysisContext decimalV1Ctx = createAnalysisCtx(); + decimalV1Ctx.getQueryOptions().setDecimal_v2(false); + AnalysisContext decimalV2Ctx = createAnalysisCtx(); + decimalV2Ctx.getQueryOptions().setDecimal_v2(true); + + testDecimalExpr("coalesce(cast(0.789 as decimal(19, 19)), " + + "cast(123 as decimal(19, 0)))", ScalarType.createDecimalType(38, 19)); + AnalyzesOk("select coalesce(cast(0.789 as decimal(20, 20)), " + + "cast(123 as decimal(19, 0)))", decimalV1Ctx); + AnalysisError("select coalesce(cast(0.789 as decimal(20, 20)), " + + "cast(123 as decimal(19, 0)))", decimalV2Ctx, + "Cannot resolve DECIMAL types of the coalesce(DECIMAL(20,20), " + + "DECIMAL(19,0)) function arguments. You need to wrap the arguments in a CAST."); + + testDecimalExpr("if(true, cast(0.789 as decimal(19, 19)), " + + "cast(123 as decimal(19, 0)))", ScalarType.createDecimalType(38, 19)); + AnalyzesOk("select if(true, cast(0.789 as decimal(20, 20)), " + + "cast(123 as decimal(19, 0)))", decimalV1Ctx); + AnalysisError("select if(true, cast(0.789 as decimal(20, 20)), " + + "cast(123 as decimal(19, 0)))", decimalV2Ctx, + "Cannot resolve DECIMAL types of the if(BOOLEAN, " + + "DECIMAL(20,20), DECIMAL(19,0)) function arguments. " + + "You need to wrap the arguments in a CAST."); + + testDecimalExpr("isnull(cast(0.789 as decimal(19, 19)), " + + "cast(123 as decimal(19, 0)))", ScalarType.createDecimalType(38, 19)); + AnalyzesOk("select isnull(cast(0.789 as decimal(20, 20)), " + + "cast(123 as decimal(19, 0)))", decimalV1Ctx); + AnalysisError("select isnull(cast(0.789 as decimal(20, 20)), " + + "cast(123 as decimal(19, 0)))", decimalV2Ctx, + "Cannot resolve DECIMAL types of the isnull(DECIMAL(20,20), " + + "DECIMAL(19,0)) function arguments. You need to wrap the arguments in a CAST."); + + testDecimalExpr("case 1 when 0 then cast(0.789 as decimal(19, 19)) " + + "else cast(123 as decimal(19, 0)) end", ScalarType.createDecimalType(38, 19)); + AnalyzesOk("select case 1 when 0 then cast(0.789 as decimal(19, 19)) " + + "else cast(123 as decimal(20, 0)) end", decimalV1Ctx); + AnalysisError("select case 1 when 0 then cast(0.789 as decimal(19, 19)) " + + "else cast(123 as decimal(20, 0)) end", decimalV2Ctx, + "Incompatible return types 'DECIMAL(19,19)' and 'DECIMAL(20,0)' " + + "of exprs 'CAST(0.789 AS DECIMAL(19,19))' and 'CAST(123 AS DECIMAL(20,0))'."); } /** http://git-wip-us.apache.org/repos/asf/impala/blob/d0f838b6/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java ---------------------------------------------------------------------- diff --git a/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java b/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java index 18b1483..6d2b100 100644 --- a/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java +++ b/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java @@ -2650,6 +2650,26 @@ public class AnalyzeStmtsTest extends AnalyzerTest { // Regression test for IMPALA-1128, union of decimal and an int type that converts // to the identical decimal. AnalyzesOk("select cast(1 as bigint) union select cast(1 as decimal(19, 0))"); + + AnalysisContext decimalV1Ctx = createAnalysisCtx(); + decimalV1Ctx.getQueryOptions().setDecimal_v2(false); + AnalysisContext decimalV2Ctx = createAnalysisCtx(); + decimalV2Ctx.getQueryOptions().setDecimal_v2(true); + + // IMPALA-6518: union of two incompatible decimal columns. There is no implicit cast + // if decimal_v2 is enabled. + String query = "select cast(123 as decimal(38, 0)) " + + "union all select cast(0.789 as decimal(38, 38))"; + AnalyzesOk(query, decimalV1Ctx); + AnalysisError(query, decimalV2Ctx, "Incompatible return types 'DECIMAL(38,0)' and " + + "'DECIMAL(38,38)' of exprs 'CAST(123 AS DECIMAL(38,0))' and " + + "'CAST(0.789 AS DECIMAL(38,38))'."); + + query = "select cast(123 as double) " + + "union all select cast(0.456 as float)" + + "union all select cast(0.789 as decimal(38, 38))"; + AnalyzesOk(query, decimalV1Ctx); + AnalyzesOk(query, decimalV2Ctx); } @Test @@ -2660,11 +2680,12 @@ public class AnalyzeStmtsTest extends AnalyzerTest { AnalyzesOk("values(1.0, 2, NULL) union all values(1, 2.0, 3)"); AnalyzesOk("insert overwrite table functional.alltypes " + "partition (year=2009, month=10)" + - "values(1, true, 1, 1, 1, 1, 1.0, 1.0, 'a', 'a', cast(0 as timestamp))"); + "values(1, true, 1, 1, 1, 1, cast(1.0 as float), cast(1.0 as double), " + + "'a', 'a', cast(0 as timestamp))"); AnalyzesOk("insert overwrite table functional.alltypes " + "partition (year, month) " + - "values(1, true, 1, 1, 1, 1, 1.0, 1.0, 'a', 'a', cast(0 as timestamp)," + - "2009, 10)"); + "values(1, true, 1, 1, 1, 1, cast(1.0 as float), cast(1.0 as double), " + + "'a', 'a', cast(0 as timestamp), 2009, 10)"); // Values stmt with multiple rows. AnalyzesOk("values((1, 2, 3), (4, 5, 6))"); AnalyzesOk("select * from (values('a', 'b', 'c')) as t"); @@ -2673,15 +2694,21 @@ public class AnalyzeStmtsTest extends AnalyzerTest { AnalyzesOk("insert overwrite table functional.alltypes " + "partition (year=2009, month=10) " + "values(" + - "(1, true, 1, 1, 1, 1, 1.0, 1.0, 'a', 'a', cast(0 as timestamp))," + - "(2, false, 2, 2, NULL, 2, 2.0, 2.0, 'b', 'b', cast(0 as timestamp))," + - "(3, true, 3, 3, 3, 3, 3.0, 3.0, 'c', 'c', cast(0 as timestamp)))"); + "(1, true, 1, 1, 1, 1, cast(1.0 as float), cast(1.0 as double), " + + "'a', 'a', cast(0 as timestamp))," + + "(2, false, 2, 2, NULL, 2, cast(2.0 as float), cast(2.0 as double), " + + "'b', 'b', cast(0 as timestamp))," + + "(3, true, 3, 3, 3, 3, cast(3.0 as float), cast(3.0 as double), " + + "'c', 'c', cast(0 as timestamp)))"); AnalyzesOk("insert overwrite table functional.alltypes " + "partition (year, month) " + "values(" + - "(1, true, 1, 1, 1, 1, 1.0, 1.0, 'a', 'a', cast(0 as timestamp), 2009, 10)," + - "(2, false, 2, 2, NULL, 2, 2.0, 2.0, 'b', 'b', cast(0 as timestamp), 2009, 2)," + - "(3, true, 3, 3, 3, 3, 3.0, 3.0, 'c', 'c', cast(0 as timestamp), 2009, 3))"); + "(1, true, 1, 1, 1, 1, cast(1.0 as float), cast(1.0 as double), " + + "'a', 'a', cast(0 as timestamp), 2009, 10)," + + "(2, false, 2, 2, NULL, 2, cast(2.0 as float), cast(2.0 as double), " + + "'b', 'b', cast(0 as timestamp), 2009, 2)," + + "(3, true, 3, 3, 3, 3, cast(3.0 as float), cast(3.0 as double), " + + "'c', 'c', cast(0 as timestamp), 2009, 3))"); // Test multiple aliases. Values() is like union, the column labels are 'x' and 'y'. AnalyzesOk("values((1 as x, 'a' as y), (2 as k, 'b' as j))"); @@ -3204,6 +3231,56 @@ public class AnalyzeStmtsTest extends AnalyzerTest { // Mixed column name case, on both primary key and non-primary key cols. AnalyzesOk("insert into functional_kudu.alltypes (ID, BOOL_COL) values (0, true)"); } + + addTestDb("d", null); + addTestTable("create table d.dec1 (c decimal(38,37)) location '/'"); + addTestTable("create table d.dec2 (c decimal(38,1)) location '/'"); + addTestTable("create table d.dbl (c double) location '/'"); + addTestTable("create table d.flt (c float) location '/'"); + + AnalysisContext decimalV1Ctx = createAnalysisCtx("d"); + decimalV1Ctx.getQueryOptions().setDecimal_v2(false); + AnalysisContext decimalV2Ctx = createAnalysisCtx("d"); + decimalV2Ctx.getQueryOptions().setDecimal_v2(true); + + AnalyzesOk("insert into d.dec1 select cast(1 as decimal(38, 0))", decimalV1Ctx); + AnalysisError("insert into d.dec1 select cast(1 as decimal(38, 0))", decimalV2Ctx, + "Target table 'd.dec1' is incompatible with source expressions.\n" + + "Expression 'CAST(1 AS DECIMAL(38,0))' (type: DECIMAL(38,0)) is not " + + "compatible with column 'c' (type: DECIMAL(38,37))"); + + AnalysisError("insert into d.dec2 select cast(11.1 as decimal(38, 20));", + decimalV1Ctx, "Possible loss of precision for target table 'd.dec2'.\n" + + "Expression 'CAST(11.1 AS DECIMAL(38,20))' (type: DECIMAL(38,20)) would need " + + "to be cast to DECIMAL(38,1) for column 'c'"); + AnalysisError("insert into d.dec2 select cast(11.1 as decimal(38, 20));", + decimalV2Ctx, "Target table 'd.dec2' is incompatible with source expressions.\n" + + "Expression 'CAST(11.1 AS DECIMAL(38,20))' (type: DECIMAL(38,20)) is not " + + "compatible with column 'c' (type: DECIMAL(38,1))"); + + AnalysisError("insert into d.dec1 select cast(1 as double)", decimalV1Ctx, + "Possible loss of precision for target table 'd.dec1'.\n" + + "Expression 'CAST(1 AS DOUBLE)' (type: DOUBLE) would need to be cast to " + + "DECIMAL(38,37) for column 'c'"); + AnalysisError("insert into d.dec1 select cast(1 as double)", decimalV2Ctx, + "Possible loss of precision for target table 'd.dec1'.\n" + + "Expression 'CAST(1 AS DOUBLE)' (type: DOUBLE) would need to be cast to " + + "DECIMAL(38,37) for column 'c'"); + + AnalysisError("insert into d.dec1 select cast(1 as float)", decimalV1Ctx, + "Possible loss of precision for target table 'd.dec1'.\n" + + "Expression 'CAST(1 AS FLOAT)' (type: FLOAT) would need to be cast to " + + "DECIMAL(38,37) for column 'c'"); + AnalysisError("insert into d.dec1 select cast(1 as float)", decimalV2Ctx, + "Possible loss of precision for target table 'd.dec1'.\n" + + "Expression 'CAST(1 AS FLOAT)' (type: FLOAT) would need to be cast to " + + "DECIMAL(38,37) for column 'c'"); + + AnalyzesOk("insert into d.dbl select cast(1 as decimal(20, 10))", decimalV1Ctx); + AnalyzesOk("insert into d.dbl select cast(1 as decimal(20, 10))", decimalV2Ctx); + + AnalyzesOk("insert into d.flt select cast(1 as decimal(20, 10))", decimalV1Ctx); + AnalyzesOk("insert into d.flt select cast(1 as decimal(20, 10))", decimalV2Ctx); } /** http://git-wip-us.apache.org/repos/asf/impala/blob/d0f838b6/fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java ---------------------------------------------------------------------- diff --git a/fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java b/fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java index 209e69d..453b0aa 100644 --- a/fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java +++ b/fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java @@ -296,6 +296,9 @@ public class ExprRewriteRulesTest extends FrontendTestBase { "TIMESTAMP '2016-11-09 00:00:00'"); RewritesOk("cast('2016-11-09' as timestamp) + interval 1 year", rule, "TIMESTAMP '2017-11-09 00:00:00'"); + // Tests that exprs that warn during their evaluation are not folded. + RewritesOk("CAST('9999-12-31 21:00:00' AS TIMESTAMP) + INTERVAL 1 DAYS", rule, + "TIMESTAMP '9999-12-31 21:00:00' + INTERVAL 1 DAYS"); // Tests correct handling of strings with escape sequences. RewritesOk("'_' LIKE '\\\\_'", rule, "TRUE"); @@ -308,8 +311,6 @@ public class ExprRewriteRulesTest extends FrontendTestBase { RewritesOk("rand()", rule, null); RewritesOk("random()", rule, null); RewritesOk("uuid()", rule, null); - // Tests that exprs that warn during their evaluation are not folded. - RewritesOk("coalesce(1.8, cast(int_col as decimal(38,38)))", rule, null); } @Test http://git-wip-us.apache.org/repos/asf/impala/blob/d0f838b6/fe/src/test/java/org/apache/impala/analysis/TypesUtilTest.java ---------------------------------------------------------------------- diff --git a/fe/src/test/java/org/apache/impala/analysis/TypesUtilTest.java b/fe/src/test/java/org/apache/impala/analysis/TypesUtilTest.java index 9b36608..872323e 100644 --- a/fe/src/test/java/org/apache/impala/analysis/TypesUtilTest.java +++ b/fe/src/test/java/org/apache/impala/analysis/TypesUtilTest.java @@ -44,64 +44,84 @@ public class TypesUtilTest extends AnalyzerTest { assertTrue(t1.equals(t2)); } + private void verifyInvalid(Type t) { + assertTrue(t.equals(Type.INVALID)); + } + @Test // Tests to verify that we can compute the correct type for assignment. public void TestDecimalAssignementType() { - verifyDecimalType( - TypesUtil.getDecimalAssignmentCompatibleType( - Type.DEFAULT_DECIMAL, Type.DEFAULT_DECIMAL), - Type.DEFAULT_DECIMAL); - verifyDecimalType( - TypesUtil.getDecimalAssignmentCompatibleType( - ScalarType.createDecimalType(10, 2), ScalarType.createDecimalType(12, 2)), - ScalarType.createDecimalType(12, 2)); - verifyDecimalType( - TypesUtil.getDecimalAssignmentCompatibleType( - ScalarType.createDecimalType(10, 5), ScalarType.createDecimalType(12, 3)), - ScalarType.createDecimalType(14, 5)); - verifyDecimalType( - TypesUtil.getDecimalAssignmentCompatibleType( - ScalarType.createDecimalType(12, 2), ScalarType.createDecimalType(10, 2)), - ScalarType.createDecimalType(12, 2)); - verifyDecimalType( - TypesUtil.getDecimalAssignmentCompatibleType( - ScalarType.createDecimalType(12, 3), ScalarType.createDecimalType(10, 5)), - ScalarType.createDecimalType(14, 5)); - verifyDecimalType( - TypesUtil.getDecimalAssignmentCompatibleType(ScalarType.createDecimalType(10, 0), - ScalarType.createDecimalType(16, 5)), - ScalarType.createDecimalType(16, 5)); - - // Decimal(10, 0) && Decimal(10, 0) --> Decimal(10, 0) - verifyDecimalType( - TypesUtil.getDecimalAssignmentCompatibleType( - ScalarType.createDecimalType(), ScalarType.createDecimalType()), - ScalarType.createDecimalType()); - - // decimal(10, 2) && decimal(12, 2) -> decimal(12, 2) - verifyDecimalType( - TypesUtil.getDecimalAssignmentCompatibleType( - ScalarType.createDecimalType(10, 2), ScalarType.createDecimalType(12, 2)), - ScalarType.createDecimalType(12, 2)); - + for (final boolean decimalV2 : new boolean[] { false, true} ) { + verifyDecimalType( + TypesUtil.getDecimalAssignmentCompatibleType( + Type.DEFAULT_DECIMAL, + Type.DEFAULT_DECIMAL, decimalV2), + Type.DEFAULT_DECIMAL); + verifyDecimalType( + TypesUtil.getDecimalAssignmentCompatibleType( + ScalarType.createDecimalType(10, 2), + ScalarType.createDecimalType(12, 2), decimalV2), + ScalarType.createDecimalType(12, 2)); + verifyDecimalType( + TypesUtil.getDecimalAssignmentCompatibleType( + ScalarType.createDecimalType(10, 5), + ScalarType.createDecimalType(12, 3), decimalV2), + ScalarType.createDecimalType(14, 5)); + verifyDecimalType( + TypesUtil.getDecimalAssignmentCompatibleType( + ScalarType.createDecimalType(12, 2), + ScalarType.createDecimalType(10, 2), decimalV2), + ScalarType.createDecimalType(12, 2)); + verifyDecimalType( + TypesUtil.getDecimalAssignmentCompatibleType( + ScalarType.createDecimalType(12, 3), + ScalarType.createDecimalType(10, 5), decimalV2), + ScalarType.createDecimalType(14, 5)); + verifyDecimalType( + TypesUtil.getDecimalAssignmentCompatibleType( + ScalarType.createDecimalType(10, 0), + ScalarType.createDecimalType(16, 5), decimalV2), + ScalarType.createDecimalType(16, 5)); + + // Decimal(10, 0) && Decimal(10, 0) --> Decimal(10, 0) + verifyDecimalType( + TypesUtil.getDecimalAssignmentCompatibleType( + ScalarType.createDecimalType(), + ScalarType.createDecimalType(), decimalV2), + ScalarType.createDecimalType()); + + // decimal(10, 2) && decimal(12, 2) -> decimal(12, 2) + verifyDecimalType( + TypesUtil.getDecimalAssignmentCompatibleType( + ScalarType.createDecimalType(10, 2), + ScalarType.createDecimalType(12, 2), decimalV2), + ScalarType.createDecimalType(12, 2)); + + // Decimal(5,0) with Decimal(*,*) should be Decimal(5,0) + verifyDecimalType( + TypesUtil.getDecimalAssignmentCompatibleType( + ScalarType.createDecimalType(5, 0), + Type.DECIMAL, decimalV2), + ScalarType.createDecimalType(5, 0)); + verifyDecimalType( + TypesUtil.getDecimalAssignmentCompatibleType( + Type.DECIMAL, + ScalarType.createDecimalType(5, 0), decimalV2), + ScalarType.createDecimalType(5, 0)); + } // decimal (38, 38) && decimal(3, 0) -> decimal(38 , 38) // In this case, since we only support 38 digits, there is no type (we'd - // need 41 digits). Return the best we can do. + // need 41 digits). Return a clipped decimal if decimal_v2 is disabled. verifyDecimalType( TypesUtil.getDecimalAssignmentCompatibleType( - ScalarType.createDecimalType(38, 38), ScalarType.createDecimalType(3)), + ScalarType.createDecimalType(38, 38), + ScalarType.createDecimalType(3), false), ScalarType.createDecimalType(38, 38)); - - // Decimal(5,0) with Decimal(*,*) should be Decimal(5,0) - verifyDecimalType( - TypesUtil.getDecimalAssignmentCompatibleType( - ScalarType.createDecimalType(5, 0), Type.DECIMAL), - ScalarType.createDecimalType(5, 0)); - verifyDecimalType( + verifyInvalid( TypesUtil.getDecimalAssignmentCompatibleType( - Type.DECIMAL, ScalarType.createDecimalType(5, 0)), - ScalarType.createDecimalType(5, 0)); + ScalarType.createDecimalType(38, 38), + ScalarType.createDecimalType(3), true)); } @Test @@ -109,56 +129,56 @@ public class TypesUtilTest extends AnalyzerTest { public void TestNumericImplicitCast() { // Decimals can be cast to integers if there is no loss of precision. Assert.assertTrue(Type.isImplicitlyCastable( - ScalarType.createDecimalType(2, 0), Type.TINYINT, false)); + ScalarType.createDecimalType(2, 0), Type.TINYINT, false, false)); Assert.assertTrue(Type.isImplicitlyCastable( - ScalarType.createDecimalType(4, 0), Type.SMALLINT, false)); + ScalarType.createDecimalType(4, 0), Type.SMALLINT, false, false)); Assert.assertTrue(Type.isImplicitlyCastable( - ScalarType.createDecimalType(9, 0), Type.INT, false)); + ScalarType.createDecimalType(9, 0), Type.INT, false, false)); Assert.assertTrue(Type.isImplicitlyCastable( - ScalarType.createDecimalType(18, 0), Type.BIGINT, false)); + ScalarType.createDecimalType(18, 0), Type.BIGINT, false, false)); Assert.assertFalse(Type.isImplicitlyCastable( - ScalarType.createDecimalType(3, 0), Type.TINYINT, false)); + ScalarType.createDecimalType(3, 0), Type.TINYINT, false, false)); Assert.assertFalse(Type.isImplicitlyCastable( - ScalarType.createDecimalType(5, 0), Type.SMALLINT, false)); + ScalarType.createDecimalType(5, 0), Type.SMALLINT, false, false)); Assert.assertFalse(Type.isImplicitlyCastable( - ScalarType.createDecimalType(10, 0), Type.INT, false)); + ScalarType.createDecimalType(10, 0), Type.INT, false, false)); Assert.assertFalse(Type.isImplicitlyCastable( - ScalarType.createDecimalType(19, 0), Type.BIGINT, false)); + ScalarType.createDecimalType(19, 0), Type.BIGINT, false, false)); Assert.assertFalse(Type.isImplicitlyCastable( - ScalarType.createDecimalType(2, 1), Type.TINYINT, false)); + ScalarType.createDecimalType(2, 1), Type.TINYINT, false, false)); Assert.assertFalse(Type.isImplicitlyCastable( - ScalarType.createDecimalType(4, 1), Type.SMALLINT, false)); + ScalarType.createDecimalType(4, 1), Type.SMALLINT, false, false)); Assert.assertFalse(Type.isImplicitlyCastable( - ScalarType.createDecimalType(2, 1), Type.INT, false)); + ScalarType.createDecimalType(2, 1), Type.INT, false, false)); Assert.assertFalse(Type.isImplicitlyCastable( - ScalarType.createDecimalType(18, 5), Type.BIGINT, false)); + ScalarType.createDecimalType(18, 5), Type.BIGINT, false, false)); // Integers are only converted to decimal when all values of the source type can be // represented in the destination type. Assert.assertFalse(Type.isImplicitlyCastable( - Type.TINYINT, ScalarType.createDecimalType(2, 0), false)); + Type.TINYINT, ScalarType.createDecimalType(2, 0), false, false)); Assert.assertFalse(Type.isImplicitlyCastable( - Type.SMALLINT, ScalarType.createDecimalType(4, 0), false)); + Type.SMALLINT, ScalarType.createDecimalType(4, 0), false, false)); Assert.assertFalse(Type.isImplicitlyCastable( - Type.INT, ScalarType.createDecimalType(9, 0), false)); + Type.INT, ScalarType.createDecimalType(9, 0), false, false)); Assert.assertFalse(Type.isImplicitlyCastable( - Type.BIGINT, ScalarType.createDecimalType(18, 0), false)); + Type.BIGINT, ScalarType.createDecimalType(18, 0), false, false)); Assert.assertTrue(Type.isImplicitlyCastable( - Type.TINYINT, ScalarType.createDecimalType(3, 0), false)); + Type.TINYINT, ScalarType.createDecimalType(3, 0), false, false)); Assert.assertTrue(Type.isImplicitlyCastable( - Type.SMALLINT, ScalarType.createDecimalType(5, 0), false)); + Type.SMALLINT, ScalarType.createDecimalType(5, 0), false, false)); Assert.assertTrue(Type.isImplicitlyCastable( - Type.INT, ScalarType.createDecimalType(10, 0), false)); + Type.INT, ScalarType.createDecimalType(10, 0), false, false)); Assert.assertTrue(Type.isImplicitlyCastable( - Type.BIGINT, ScalarType.createDecimalType(19, 0), false)); + Type.BIGINT, ScalarType.createDecimalType(19, 0), false, false)); Assert.assertTrue(Type.isImplicitlyCastable( - Type.TINYINT, ScalarType.createDecimalType(4, 1), false)); + Type.TINYINT, ScalarType.createDecimalType(4, 1), false, false)); Assert.assertTrue(Type.isImplicitlyCastable( - Type.SMALLINT, ScalarType.createDecimalType(6, 1), false)); + Type.SMALLINT, ScalarType.createDecimalType(6, 1), false, false)); Assert.assertTrue(Type.isImplicitlyCastable( - Type.INT, ScalarType.createDecimalType(11, 1), false)); + Type.INT, ScalarType.createDecimalType(11, 1), false, false)); Assert.assertTrue(Type.isImplicitlyCastable( - Type.BIGINT, ScalarType.createDecimalType(20, 1), false)); + Type.BIGINT, ScalarType.createDecimalType(20, 1), false, false)); // Only promotions are allowed for integer types. List<Type> intTypes = Arrays.<Type>asList(Type.TINYINT, Type.SMALLINT, Type.INT, @@ -166,35 +186,35 @@ public class TypesUtilTest extends AnalyzerTest { for (Type t1: intTypes) { for (Type t2: intTypes) { if (t1.getSlotSize() == t2.getSlotSize()) { - Assert.assertTrue(Type.isImplicitlyCastable(t1, t2, true)); - Assert.assertTrue(Type.isImplicitlyCastable(t1, t2, false)); + Assert.assertTrue(Type.isImplicitlyCastable(t1, t2, true, false)); + Assert.assertTrue(Type.isImplicitlyCastable(t1, t2, false, false)); } else if (t1.getSlotSize() < t2.getSlotSize()) { - Assert.assertTrue(Type.isImplicitlyCastable(t1, t2, true)); - Assert.assertTrue(Type.isImplicitlyCastable(t1, t2, false)); - Assert.assertFalse(Type.isImplicitlyCastable(t2, t1, true)); - Assert.assertFalse(Type.isImplicitlyCastable(t2, t1, false)); + Assert.assertTrue(Type.isImplicitlyCastable(t1, t2, true, false)); + Assert.assertTrue(Type.isImplicitlyCastable(t1, t2, false, false)); + Assert.assertFalse(Type.isImplicitlyCastable(t2, t1, true, false)); + Assert.assertFalse(Type.isImplicitlyCastable(t2, t1, false, false)); } else { - Assert.assertFalse(Type.isImplicitlyCastable(t1, t2, true)); - Assert.assertFalse(Type.isImplicitlyCastable(t1, t2, false)); - Assert.assertTrue(Type.isImplicitlyCastable(t2, t1, true)); - Assert.assertTrue(Type.isImplicitlyCastable(t2, t1, false)); + Assert.assertFalse(Type.isImplicitlyCastable(t1, t2, true, false)); + Assert.assertFalse(Type.isImplicitlyCastable(t1, t2, false, false)); + Assert.assertTrue(Type.isImplicitlyCastable(t2, t1, true, false)); + Assert.assertTrue(Type.isImplicitlyCastable(t2, t1, false, false)); } } } // Only promotions are allowed for floating point types. - Assert.assertTrue(Type.isImplicitlyCastable(Type.FLOAT, Type.FLOAT, true)); - Assert.assertFalse(Type.isImplicitlyCastable(Type.DOUBLE, Type.FLOAT, false)); - Assert.assertTrue(Type.isImplicitlyCastable(Type.FLOAT, Type.DOUBLE, false)); - Assert.assertTrue(Type.isImplicitlyCastable(Type.FLOAT, Type.DOUBLE, true)); + Assert.assertTrue(Type.isImplicitlyCastable(Type.FLOAT, Type.FLOAT, true, false)); + Assert.assertFalse(Type.isImplicitlyCastable(Type.DOUBLE, Type.FLOAT, false, false)); + Assert.assertTrue(Type.isImplicitlyCastable(Type.FLOAT, Type.DOUBLE, false, false)); + Assert.assertTrue(Type.isImplicitlyCastable(Type.FLOAT, Type.DOUBLE, true, false)); // Decimal is convertible to a floating point types only in non-strict mode. List<ScalarType> dts = Arrays.asList(ScalarType.createDecimalType(30, 10), ScalarType.createDecimalType(2, 0)); for (Type dt: dts) { - Assert.assertFalse(Type.isImplicitlyCastable(dt, Type.FLOAT, true)); - Assert.assertTrue(Type.isImplicitlyCastable(dt, Type.FLOAT, false)); - Assert.assertFalse(Type.isImplicitlyCastable(dt, Type.DOUBLE, true)); - Assert.assertTrue(Type.isImplicitlyCastable(dt, Type.DOUBLE, false)); + Assert.assertFalse(Type.isImplicitlyCastable(dt, Type.FLOAT, true, false)); + Assert.assertTrue(Type.isImplicitlyCastable(dt, Type.FLOAT, false, false)); + Assert.assertFalse(Type.isImplicitlyCastable(dt, Type.DOUBLE, true, false)); + Assert.assertTrue(Type.isImplicitlyCastable(dt, Type.DOUBLE, false, false)); } } @@ -202,16 +222,16 @@ public class TypesUtilTest extends AnalyzerTest { // Test that we don't allow casting to/from complex types. public void TestComplexImplicitCast() { ArrayType arrayType = new ArrayType(Type.INT); - Assert.assertFalse(Type.isImplicitlyCastable(Type.INT, arrayType, false)); - Assert.assertFalse(Type.isImplicitlyCastable(arrayType, Type.INT, false)); + Assert.assertFalse(Type.isImplicitlyCastable(Type.INT, arrayType, false, false)); + Assert.assertFalse(Type.isImplicitlyCastable(arrayType, Type.INT, false, false)); MapType mapType = new MapType(Type.STRING, Type.INT); - Assert.assertFalse(Type.isImplicitlyCastable(Type.INT, mapType, false)); - Assert.assertFalse(Type.isImplicitlyCastable(mapType, Type.INT, false)); - Assert.assertFalse(Type.isImplicitlyCastable(mapType, arrayType, false)); + Assert.assertFalse(Type.isImplicitlyCastable(Type.INT, mapType, false, false)); + Assert.assertFalse(Type.isImplicitlyCastable(mapType, Type.INT, false, false)); + Assert.assertFalse(Type.isImplicitlyCastable(mapType, arrayType, false, false)); StructType structType = new StructType(Lists.newArrayList( new StructField("foo", Type.FLOAT, ""), new StructField("bar", Type.FLOAT, ""))); - Assert.assertFalse(Type.isImplicitlyCastable(structType, Type.INT, false)); - Assert.assertFalse(Type.isImplicitlyCastable(Type.INT, structType, false)); - Assert.assertFalse(Type.isImplicitlyCastable(arrayType, structType, false)); + Assert.assertFalse(Type.isImplicitlyCastable(structType, Type.INT, false, false)); + Assert.assertFalse(Type.isImplicitlyCastable(Type.INT, structType, false, false)); + Assert.assertFalse(Type.isImplicitlyCastable(arrayType, structType, false, false)); } } http://git-wip-us.apache.org/repos/asf/impala/blob/d0f838b6/fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java ---------------------------------------------------------------------- diff --git a/fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java b/fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java index 7c47d74..3efbc63 100644 --- a/fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java +++ b/fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java @@ -40,7 +40,6 @@ import org.apache.impala.catalog.Catalog; import org.apache.impala.catalog.CatalogException; import org.apache.impala.common.FrontendTestBase; import org.apache.impala.common.ImpalaException; -import org.apache.impala.common.NotImplementedException; import org.apache.impala.common.RuntimeEnv; import org.apache.impala.testutil.TestFileParser; import org.apache.impala.testutil.TestFileParser.Section; @@ -339,36 +338,30 @@ public class PlannerTestBase extends FrontendTestBase { /** * Extracts and returns the expected error message from expectedPlan. * Returns null if expectedPlan is empty or its first element is not an error message. - * The accepted format for error messages is 'not implemented: expected error message' - * Returns the empty string if expectedPlan starts with 'not implemented' but no - * expected error message was given. + * The accepted format for error messages is the exception string. We currently + * support only NotImplementedException and InternalException. */ private String getExpectedErrorMessage(ArrayList<String> expectedPlan) { if (expectedPlan == null || expectedPlan.isEmpty()) return null; - if (!expectedPlan.get(0).toLowerCase().startsWith("not implemented")) return null; - // Find first ':' and extract string on right hand side as error message. - int ix = expectedPlan.get(0).indexOf(":"); - if (ix + 1 > 0) { - return expectedPlan.get(0).substring(ix + 1).trim(); - } else { - return ""; - } + if (!expectedPlan.get(0).contains("NotImplementedException") && + !expectedPlan.get(0).contains("InternalException")) return null; + return expectedPlan.get(0).trim(); } - private void handleNotImplException(String query, String expectedErrorMsg, + private void handleException(String query, String expectedErrorMsg, StringBuilder errorLog, StringBuilder actualOutput, Throwable e) { - boolean isImplemented = expectedErrorMsg == null; - actualOutput.append("not implemented: " + e.getMessage() + "\n"); - if (isImplemented) { - errorLog.append("query:\n" + query + "\nPLAN not implemented: " - + e.getMessage() + "\n"); + actualOutput.append(e.toString() + "\n"); + if (expectedErrorMsg == null) { + // Exception is unexpected + errorLog.append(String.format("Query:\n%s\nError Stack:\n%s\n", query, + ExceptionUtils.getStackTrace(e))); } else { // Compare actual and expected error messages. if (expectedErrorMsg != null && !expectedErrorMsg.isEmpty()) { - if (!e.getMessage().toLowerCase().startsWith(expectedErrorMsg.toLowerCase())) { + String actualErrorMsg = e.getClass().getSimpleName() + ": " + e.getMessage(); + if (!actualErrorMsg.toLowerCase().startsWith(expectedErrorMsg.toLowerCase())) { errorLog.append("query:\n" + query + "\nExpected error message: '" - + expectedErrorMsg + "'\nActual error message: '" - + e.getMessage() + "'\n"); + + expectedErrorMsg + "'\nActual error message: '" + actualErrorMsg + "'\n"); } } } @@ -513,12 +506,9 @@ public class PlannerTestBase extends FrontendTestBase { if (sectionExists) actualOutput.append(section.getHeader() + "\n"); try { execRequest = frontend_.createExecRequest(queryCtx, explainBuilder); - } catch (NotImplementedException e) { - if (!sectionExists) return null; - handleNotImplException(query, expectedErrorMsg, errorLog, actualOutput, e); } catch (Exception e) { - errorLog.append(String.format("Query:\n%s\nError Stack:\n%s\n", query, - ExceptionUtils.getStackTrace(e))); + if (!sectionExists) return null; + handleException(query, expectedErrorMsg, errorLog, actualOutput, e); } // No expected plan was specified for section. Skip expected/actual comparison. if (!sectionExists) return execRequest;
