IMPALA-4550: Fix CastExpr analysis for substituted slots During slot substitution, the type of the child of a CastExpr can change. If the previous child type matched the CastExpr, then the cast was flagged as noOp_. During substitution and subsequent re-analysis the noOp_ flag was not revisited so that no cast was performed, even after it had become necessary.
The fix is to always set noOp_ to the correct value in CastExpr.analyze(). Change-Id: I7f29cdc359558fad6df455b8eec0e0eaed00e996 Reviewed-on: http://gerrit.cloudera.org:8080/5267 Reviewed-by: Alex Behm <[email protected]> Tested-by: Internal 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/0f62bf35 Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/0f62bf35 Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/0f62bf35 Branch: refs/heads/master Commit: 0f62bf35fd2266d130cf06de0710b7d7797d84ac Parents: 9f497ba Author: Lars Volker <[email protected]> Authored: Tue Nov 29 23:56:21 2016 +0100 Committer: Internal Jenkins <[email protected]> Committed: Wed Nov 30 11:19:36 2016 +0000 ---------------------------------------------------------------------- .../main/java/org/apache/impala/analysis/CastExpr.java | 9 +++++---- .../functional-query/queries/QueryTest/exprs.test | 11 +++++++++++ 2 files changed, 16 insertions(+), 4 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/0f62bf35/fe/src/main/java/org/apache/impala/analysis/CastExpr.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/analysis/CastExpr.java b/fe/src/main/java/org/apache/impala/analysis/CastExpr.java index 33ae0e8..66ddd20 100644 --- a/fe/src/main/java/org/apache/impala/analysis/CastExpr.java +++ b/fe/src/main/java/org/apache/impala/analysis/CastExpr.java @@ -250,10 +250,11 @@ public class CastExpr extends Expr { Type childType = children_.get(0).type_; Preconditions.checkState(!childType.isNull()); - if (childType.equals(type_)) { - noOp_ = true; - return; - } + // IMPALA-4550: We always need to set noOp_ to the correct value, since we could + // be performing a subsequent analysis run and its correct value might have changed. + // This can happen if the child node gets substituted and its type changes. + noOp_ = childType.equals(type_); + if (noOp_) return; FunctionName fnName = new FunctionName(Catalog.BUILTINS_DB, getFnName(type_)); Type[] args = { childType }; http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/0f62bf35/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 2f04db9..88f6599 100644 --- a/testdata/workloads/functional-query/queries/QueryTest/exprs.test +++ b/testdata/workloads/functional-query/queries/QueryTest/exprs.test @@ -2563,3 +2563,14 @@ where timestamp_col < cast('2013-02-18 20:46:00.01' as timestamp) ---- TYPES BIGINT ==== +---- QUERY +# IMPALA-4550: Regression test for proper cast analysis after slot substitution within a +# no-op explicit cast. +select /* +straight_join */ a.id +from functional.alltypestiny a +join functional.alltypestiny b on a.string_col = b.timestamp_col +where (cast(a.string_col as string) > 'a'); +---- RESULTS +---- TYPES +INT +====
