IMPALA-4716: Expr rewrite causes IllegalStateException The DECODE constructor in CaseExpr uses the same decodeExpr object when building the BinaryPredicates that compare the decodeExpr to each 'when' of the DECODE. This causes problems when different BinaryPredicates try to cast the same decodeExpr object to different types during analysis, in this case leading to a Precondition check failure.
The solution is to clone the decodeExpr in the DECODE constructor in CaseExpr for each generated BinaryPredicate. Testing: - Added a regression test to exprs.test Change-Id: I4de9ed7118c8d18ec3f02ff74c9cca211c716e51 Reviewed-on: http://gerrit.cloudera.org:8080/5631 Reviewed-by: Marcel Kornacker <[email protected]> Tested-by: Impala Public Jenkins Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/89a3d3c1 Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/89a3d3c1 Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/89a3d3c1 Branch: refs/heads/master Commit: 89a3d3c1ebf85629bb20457a9672d71fbf2504ca Parents: c4f2458 Author: Thomas Tauber-Marshall <[email protected]> Authored: Thu Jan 5 16:15:32 2017 -0800 Committer: Impala Public Jenkins <[email protected]> Committed: Fri Jan 13 04:44:07 2017 +0000 ---------------------------------------------------------------------- .../java/org/apache/impala/analysis/CaseExpr.java | 14 ++++++++++---- .../java/org/apache/impala/common/TreeNode.java | 9 +++++++++ .../functional-query/queries/QueryTest/exprs.test | 16 ++++++++++++++++ 3 files changed, 35 insertions(+), 4 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/89a3d3c1/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 4440690..3d96662 100644 --- a/fe/src/main/java/org/apache/impala/analysis/CaseExpr.java +++ b/fe/src/main/java/org/apache/impala/analysis/CaseExpr.java @@ -127,16 +127,17 @@ public class CaseExpr extends Expr { if (candidate.isNullLiteral()) { // An example case is DECODE(foo, NULL, bar), since NULLs are considered // equal, this becomes CASE WHEN foo IS NULL THEN bar END. - children_.add(encodedIsNull); + children_.add(encodedIsNull.clone()); } else { children_.add(new BinaryPredicate( - BinaryPredicate.Operator.EQ, encoded, candidate)); + BinaryPredicate.Operator.EQ, encoded.clone(), candidate)); } } else { children_.add(new CompoundPredicate(CompoundPredicate.Operator.OR, new CompoundPredicate(CompoundPredicate.Operator.AND, - encodedIsNull, new IsNullPredicate(candidate, false)), - new BinaryPredicate(BinaryPredicate.Operator.EQ, encoded, candidate))); + encodedIsNull.clone(), new IsNullPredicate(candidate, false)), + new BinaryPredicate(BinaryPredicate.Operator.EQ, encoded.clone(), + candidate))); } // Add the value @@ -148,6 +149,11 @@ public class CaseExpr extends Expr { hasElseExpr_ = true; children_.add(decodeExpr.getChild(childIdx)); } + + // Check that these exprs were cloned above, as reusing the same Expr object in + // different places can lead to bugs, eg. if the Expr has multiple parents, they may + // try to cast it to different types. + Preconditions.checkState(!contains(encoded) && !contains(encodedIsNull)); } /** http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/89a3d3c1/fe/src/main/java/org/apache/impala/common/TreeNode.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/common/TreeNode.java b/fe/src/main/java/org/apache/impala/common/TreeNode.java index 3231e33..730fd44 100644 --- a/fe/src/main/java/org/apache/impala/common/TreeNode.java +++ b/fe/src/main/java/org/apache/impala/common/TreeNode.java @@ -158,6 +158,15 @@ public abstract class TreeNode<NodeType extends TreeNode<NodeType>> { } /** + * Returns true if this node or any of its children is 'node'. + */ + public boolean contains(NodeType node) { + if (this == node) return true; + for (NodeType child: children_) if (child.contains(node)) return true; + return false; + } + + /** * For each node in nodeList, return true if any subexpression satisfies * contains('predicate'). */ http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/89a3d3c1/testdata/workloads/functional-query/queries/QueryTest/exprs.test ---------------------------------------------------------------------- diff --git a/testdata/workloads/functional-query/queries/QueryTest/exprs.test b/testdata/workloads/functional-query/queries/QueryTest/exprs.test index 4b1ba76..ed3c9d9 100644 --- a/testdata/workloads/functional-query/queries/QueryTest/exprs.test +++ b/testdata/workloads/functional-query/queries/QueryTest/exprs.test @@ -2649,3 +2649,19 @@ select count(*) from functional.alltypestiny group by concat(uuid(), "_test") ---- TYPES BIGINT ==== +---- QUERY +# IMPALA-4716: Tests that decode with 'when' values of different types is analyzed +# correctly when expr rewrites are enabled. +select decode(0, 1, 0, id, 1, 2) a from functional.alltypestiny order by a +---- RESULTS +1 +2 +2 +2 +2 +2 +2 +2 +---- TYPES +TINYINT +====
