You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by ta...@apache.org on 2018/07/11 05:54:02 UTC
[4/4] impala git commit: IMPALA-7260: Fix decimal binary predicates
IMPALA-7260: Fix decimal binary predicates
When casting the inputs to a function call, we would try to cast non
decimal numbers to a specific decimal type even though this is not
necessary. The specific decimal type would be calculated by looking at
all the inputs. It was possible for this calculation to fail due to some
decimal types being incompatible with each other.
We solve the problem by casting the non-decimal numerical inputs to a
generic getMinResolutionDecimal().
Testing:
- Added some analysis tests.
Change-Id: Icc442e84cdff74a376ba25bd9897b0b4df5cf0b4
Reviewed-on: http://gerrit.cloudera.org:8080/10888
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/36ce54bd
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/36ce54bd
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/36ce54bd
Branch: refs/heads/master
Commit: 36ce54bd5c5aca2a09294460dcc1c2e5a80ba497
Parents: c465150
Author: Taras Bobrovytsky <ta...@apache.org>
Authored: Fri Jul 6 18:16:02 2018 -0700
Committer: Impala Public Jenkins <im...@cloudera.com>
Committed: Wed Jul 11 05:09:38 2018 +0000
----------------------------------------------------------------------
.../java/org/apache/impala/analysis/Expr.java | 19 ++++++++++++++-----
.../apache/impala/analysis/AnalyzeExprsTest.java | 9 +++++++++
2 files changed, 23 insertions(+), 5 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/impala/blob/36ce54bd/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 e85a4a4..e34a58c 100644
--- a/fe/src/main/java/org/apache/impala/analysis/Expr.java
+++ b/fe/src/main/java/org/apache/impala/analysis/Expr.java
@@ -26,7 +26,6 @@ import java.util.ListIterator;
import java.util.Set;
import org.apache.impala.analysis.BinaryPredicate.Operator;
-import org.apache.impala.catalog.Catalog;
import org.apache.impala.catalog.Function;
import org.apache.impala.catalog.Function.CompareMode;
import org.apache.impala.catalog.ImpaladCatalog;
@@ -470,10 +469,20 @@ abstract public class Expr extends TreeNode<Expr> implements ParseNode, Cloneabl
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);
+ if (children_.get(i).type_.isDecimal() || !ignoreWildcardDecimals) {
+ Preconditions.checkState(resolvedWildcardType != null);
+ Preconditions.checkState(!resolvedWildcardType.isInvalid());
+ if (!children_.get(i).type_.equals(resolvedWildcardType)) {
+ castChild(resolvedWildcardType, i);
+ }
+ } else if (children_.get(i).type_.isNull()) {
+ castChild(ScalarType.createDecimalType(), i);
+ } else {
+ Preconditions.checkState(children_.get(i).type_.isScalarType());
+ // It is safe to assign an arbitrary decimal here only if the backend function
+ // can handle it (in which case ignoreWildcardDecimals is true).
+ Preconditions.checkState(ignoreWildcardDecimals);
+ castChild(((ScalarType) children_.get(i).type_).getMinResolutionDecimal(), i);
}
} else if (!children_.get(i).type_.matchesType(fnArgs[ix])) {
castChild(fnArgs[ix], i);
http://git-wip-us.apache.org/repos/asf/impala/blob/36ce54bd/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 76f979c..df0c4e7 100644
--- a/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
@@ -262,6 +262,15 @@ public class AnalyzeExprsTest extends AnalyzerTest {
}
}
+ // IMPALA-7260: Check that when a numerical type gets implicitly cast to decimal,
+ // we do not get an error when the type of the decimal operand is not able to fit
+ // all of the digits of the non-deicmal operand.
+ AnalyzesOk("select cast(1 as decimal(38,37)) " + operator + " cast(2 as bigint)");
+ AnalyzesOk("select cast(1 as bigint) " + operator + " cast(2 as decimal(38,37))");
+ AnalyzesOk("select cast(1 as decimal(38,1)) " + operator + " cast(2 as bigint)");
+ AnalyzesOk("select cast(1 as decimal(38,37)) " + operator + " cast(2 as tinyint)");
+ AnalyzesOk("select cast(1 as decimal(38,37)) " + operator + " cast(2 as double)");
+
// Chars of different length are comparable
for (int i = 1; i < 16; ++i) {
AnalyzesOk("select cast('hi' as char(" + i + ")) " + operator +