Repository: impala Updated Branches: refs/heads/2.x 0ba181e5b -> 6d3fdee3f
IMPALA-6405: Error when string to decimal cast overflows Before this patch, when there was an error when converting a string to a decimal, a NULL was returned. In this patch, we change this behavior so that an error is returned if decimal_v2 is enabled. The reasoning is that we want stricter behavior in decimal_v2. Testing: - Added some EE tests. - Ran an exhaustive build, which passed. Change-Id: Icffccac1c1c2361447ae4b0de9b6c2ec7de071db Reviewed-on: http://gerrit.cloudera.org:8080/9339 Reviewed-by: Dan Hecht <dhe...@cloudera.com> Tested-by: Impala Public Jenkins Reviewed-on: http://gerrit.cloudera.org:8080/9530 Reviewed-by: Taras Bobrovytsky <tbobrovyt...@cloudera.com> Project: http://git-wip-us.apache.org/repos/asf/impala/repo Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/95d509fa Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/95d509fa Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/95d509fa Branch: refs/heads/2.x Commit: 95d509faeddaf2552d90530b9f111ed3adfebfda Parents: 0ba181e Author: Taras Bobrovytsky <tbobrovyt...@cloudera.com> Authored: Wed Jan 17 20:32:11 2018 -0800 Committer: Lars Volker <l...@cloudera.com> Committed: Mon Mar 12 23:28:52 2018 +0000 ---------------------------------------------------------------------- be/src/exprs/decimal-operators-ir.cc | 26 +++++++++++---- .../rewrite/RemoveRedundantStringCast.java | 3 +- .../queries/QueryTest/decimal-exprs.test | 33 ++++++++++++++++++++ 3 files changed, 55 insertions(+), 7 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/impala/blob/95d509fa/be/src/exprs/decimal-operators-ir.cc ---------------------------------------------------------------------- diff --git a/be/src/exprs/decimal-operators-ir.cc b/be/src/exprs/decimal-operators-ir.cc index fd0c404..0bde566 100644 --- a/be/src/exprs/decimal-operators-ir.cc +++ b/be/src/exprs/decimal-operators-ir.cc @@ -561,13 +561,27 @@ IR_ALWAYS_INLINE DecimalVal DecimalOperators::CastToDecimalVal( DCHECK(false); return DecimalVal::null(); } - // Like all the cast functions, we return the truncated value on underflow and NULL - // on overflow. - // TODO: log warning on underflow. - if (result == StringParser::PARSE_SUCCESS || result == StringParser::PARSE_UNDERFLOW) { - return dv; + + if (UNLIKELY(result == StringParser::PARSE_FAILURE)) { + if (is_decimal_v2) { + ctx->SetError("String to Decimal parse failed"); + } else { + ctx->AddWarning("String to Decimal parse failed"); + } + return DecimalVal::null(); + } + + if (UNLIKELY(result == StringParser::PARSE_OVERFLOW)) { + if (is_decimal_v2) { + ctx->SetError("String to Decimal cast overflowed"); + } else { + ctx->AddWarning("String to Decimal cast overflowed"); + } + return DecimalVal::null(); } - return DecimalVal::null(); + + DCHECK(result == StringParser::PARSE_SUCCESS || StringParser::PARSE_UNDERFLOW); + return dv; } StringVal DecimalOperators::CastToStringVal( http://git-wip-us.apache.org/repos/asf/impala/blob/95d509fa/fe/src/main/java/org/apache/impala/rewrite/RemoveRedundantStringCast.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/rewrite/RemoveRedundantStringCast.java b/fe/src/main/java/org/apache/impala/rewrite/RemoveRedundantStringCast.java index 40ab0fa..d305f27 100644 --- a/fe/src/main/java/org/apache/impala/rewrite/RemoveRedundantStringCast.java +++ b/fe/src/main/java/org/apache/impala/rewrite/RemoveRedundantStringCast.java @@ -82,7 +82,8 @@ public class RemoveRedundantStringCast implements ExprRewriteRule { analyzer.getQueryCtx()); // Need to trim() while comparing char(n) types as conversion might add trailing // spaces to the 'resultOfReverseCast'. - if (!resultOfReverseCast.isNullLiteral() && + if (resultOfReverseCast != null && + !resultOfReverseCast.isNullLiteral() && resultOfReverseCast.getStringValue().trim() .equals(literalExpr.getStringValue().trim())) { return new BinaryPredicate(op, castExprChild, http://git-wip-us.apache.org/repos/asf/impala/blob/95d509fa/testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test ---------------------------------------------------------------------- diff --git a/testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test b/testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test index be75c23..41441e5 100644 --- a/testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test +++ b/testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test @@ -454,3 +454,36 @@ cast(cast(12333333333 as decimal(38, 0)) as timestamp); ---- TYPES TIMESTAMP, TIMESTAMP, TIMESTAMP, TIMESTAMP, TIMESTAMP, TIMESTAMP, TIMESTAMP, TIMESTAMP, TIMESTAMP, TIMESTAMP, TIMESTAMP, TIMESTAMP ==== +---- QUERY +# IMPALA-6405: String to Decimal conversion errors +set decimal_v2=false; +select cast("abc" as decimal(5, 2)); +---- RESULTS +NULL +---- TYPES +DECIMAL +---- ERRORS +UDF WARNING: String to Decimal parse failed +==== +---- QUERY +set decimal_v2=true; +select cast("abc" as decimal(5, 2)); +---- CATCH +UDF ERROR: String to Decimal parse failed +==== +---- QUERY +set decimal_v2=false; +select cast("1234.5" as decimal(5, 2)); +---- RESULTS +NULL +---- TYPES +DECIMAL +---- ERRORS +UDF WARNING: String to Decimal cast overflowed +==== +---- QUERY +set decimal_v2=true; +select cast("1234.5" as decimal(5, 2)); +---- CATCH +UDF ERROR: String to Decimal cast overflowed +====
