You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by tm...@apache.org on 2018/08/14 01:31:43 UTC
[5/6] impala git commit: IMPALA-7419: Fix NullPointerException in
SimplifyConditionalsRule
IMPALA-7419: Fix NullPointerException in SimplifyConditionalsRule
In SimplifyConditionalsRule, when simplifying a 'coalesce' where the
result is another coalesce with some children removed, we create a new
Expr for the new coalesce but previously were not analyzing the new
Expr.
This causes problems if the simplified Expr contains an aggregate
function, as we check to make sure the simplification didn't eliminate
any aggregates and the check expects the Expr to be analyzed.
The solution is to analyze the new Expr before performing the check
for aggregates.
Testing:
- Added a FE test to ExprRewriteRulesTest
Change-Id: I5d066b48c2824cc09ce21260ad06b3d50b14c54a
Reviewed-on: http://gerrit.cloudera.org:8080/11179
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/b07a65a6
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/b07a65a6
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/b07a65a6
Branch: refs/heads/master
Commit: b07a65a67afe1ee69f0f8983293671a98aaa04bd
Parents: 2e5df13
Author: Thomas Tauber-Marshall <tm...@cloudera.com>
Authored: Thu Aug 9 14:42:20 2018 -0700
Committer: Impala Public Jenkins <im...@cloudera.com>
Committed: Tue Aug 14 00:01:46 2018 +0000
----------------------------------------------------------------------
.../org/apache/impala/rewrite/SimplifyConditionalsRule.java | 9 ++++++---
.../org/apache/impala/analysis/ExprRewriteRulesTest.java | 3 +++
2 files changed, 9 insertions(+), 3 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/impala/blob/b07a65a6/fe/src/main/java/org/apache/impala/rewrite/SimplifyConditionalsRule.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/rewrite/SimplifyConditionalsRule.java b/fe/src/main/java/org/apache/impala/rewrite/SimplifyConditionalsRule.java
index 0ad5748..1a887b1 100644
--- a/fe/src/main/java/org/apache/impala/rewrite/SimplifyConditionalsRule.java
+++ b/fe/src/main/java/org/apache/impala/rewrite/SimplifyConditionalsRule.java
@@ -76,9 +76,12 @@ public class SimplifyConditionalsRule implements ExprRewriteRule {
// IMPALA-5125: We can't eliminate aggregates as this may change the meaning of the
// query, for example:
// 'select if (true, 0, sum(id)) from alltypes' != 'select 0 from alltypes'
- if (expr != simplified && expr.contains(Expr.isAggregatePredicate())
- && !simplified.contains(Expr.isAggregatePredicate())) {
- return expr;
+ if (expr != simplified) {
+ simplified.analyze(analyzer);
+ if (expr.contains(Expr.isAggregatePredicate())
+ && !simplified.contains(Expr.isAggregatePredicate())) {
+ return expr;
+ }
}
return simplified;
}
http://git-wip-us.apache.org/repos/asf/impala/blob/b07a65a6/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 453b0aa..547e7a2 100644
--- a/fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java
@@ -447,6 +447,9 @@ public class ExprRewriteRulesTest extends FrontendTestBase {
// Don't rewrite based on nullability of slots. TODO (IMPALA-5753).
RewritesOk("coalesce(year, id)", rule, null);
RewritesOk("functional_kudu.alltypessmall", "coalesce(id, year)", rule, null);
+ // IMPALA-7419: coalesce that gets simplified and contains an aggregate
+ RewritesOk("coalesce(null, min(distinct tinyint_col), 42)", rule,
+ "coalesce(min(tinyint_col), 42)");
}
@Test