You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by st...@apache.org on 2023/03/22 03:10:37 UTC
[impala] 02/03: IMPALA-8054: Fix BetweenToCompound rewrite rule binary predicate creation
This is an automated email from the ASF dual-hosted git repository.
stigahuang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit 04f7e8c0f9158fdfc1ab9cd85d04f3a72e9a4aea
Author: Peter Rozsa <pr...@cloudera.com>
AuthorDate: Tue Mar 14 10:20:54 2023 +0100
IMPALA-8054: Fix BetweenToCompound rewrite rule binary predicate creation
This patch fixes the BetweenToCompound rewrite rule's binary predicate
creation. When the BETWEEN expression gets separated, the first
operand's reference is assigned to both upper and lower binary
predicates, but in the case of different typed second and third
operands, the first operand must be cloned to make type casting unique
for both binary predicates.
Testing:
- test cases added to exprs.test
Change-Id: Iaff4199f6d0875c38fa7e91033385c9290c57bf5
Reviewed-on: http://gerrit.cloudera.org:8080/19618
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
.../impala/rewrite/BetweenToCompoundRule.java | 34 +++++++----
.../functional-query/queries/QueryTest/exprs.test | 71 ++++++++++++++++++++++
2 files changed, 92 insertions(+), 13 deletions(-)
diff --git a/fe/src/main/java/org/apache/impala/rewrite/BetweenToCompoundRule.java b/fe/src/main/java/org/apache/impala/rewrite/BetweenToCompoundRule.java
index bea4525ca..bf2a39ce7 100644
--- a/fe/src/main/java/org/apache/impala/rewrite/BetweenToCompoundRule.java
+++ b/fe/src/main/java/org/apache/impala/rewrite/BetweenToCompoundRule.java
@@ -34,29 +34,37 @@ import org.apache.impala.analysis.Predicate;
* A NOT BETWEEN X AND Y ==> A < X OR A > Y
*/
public class BetweenToCompoundRule implements ExprRewriteRule {
- public static ExprRewriteRule INSTANCE = new BetweenToCompoundRule();
+ public static final ExprRewriteRule INSTANCE = new BetweenToCompoundRule();
@Override
public Expr apply(Expr expr, Analyzer analyzer) {
if (!(expr instanceof BetweenPredicate)) return expr;
BetweenPredicate bp = (BetweenPredicate) expr;
- Expr result = null;
+ Expr value = bp.getChild(0);
+ // Using a cloned value for the 'upper' binary predicate to avoid being affected by
+ // changes in the 'lower' binary predicate.
+ Expr clonedValue = value.clone();
+ Expr lowerBound = bp.getChild(1);
+ Expr upperBound = bp.getChild(2);
+
+ BinaryPredicate.Operator lowerOperator;
+ BinaryPredicate.Operator upperOperator;
+ CompoundPredicate.Operator compoundOperator;
+
if (bp.isNotBetween()) {
// Rewrite into disjunction.
- Predicate lower = new BinaryPredicate(BinaryPredicate.Operator.LT,
- bp.getChild(0), bp.getChild(1));
- Predicate upper = new BinaryPredicate(BinaryPredicate.Operator.GT,
- bp.getChild(0), bp.getChild(2));
- result = new CompoundPredicate(CompoundPredicate.Operator.OR, lower, upper);
+ lowerOperator = BinaryPredicate.Operator.LT;
+ upperOperator = BinaryPredicate.Operator.GT;
+ compoundOperator = CompoundPredicate.Operator.OR;
} else {
// Rewrite into conjunction.
- Predicate lower = new BinaryPredicate(BinaryPredicate.Operator.GE,
- bp.getChild(0), bp.getChild(1));
- Predicate upper = new BinaryPredicate(BinaryPredicate.Operator.LE,
- bp.getChild(0), bp.getChild(2));
- result = new CompoundPredicate(CompoundPredicate.Operator.AND, lower, upper);
+ lowerOperator = BinaryPredicate.Operator.GE;
+ upperOperator = BinaryPredicate.Operator.LE;
+ compoundOperator = CompoundPredicate.Operator.AND;
}
- return result;
+ Predicate lower = new BinaryPredicate(lowerOperator, value, lowerBound);
+ Predicate upper = new BinaryPredicate(upperOperator, clonedValue, upperBound);
+ return new CompoundPredicate(compoundOperator, lower, upper);
}
private BetweenToCompoundRule() {}
diff --git a/testdata/workloads/functional-query/queries/QueryTest/exprs.test b/testdata/workloads/functional-query/queries/QueryTest/exprs.test
index cff4f15cf..30e28b39d 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/exprs.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/exprs.test
@@ -1399,6 +1399,77 @@ and cast('2010-01-01 01:40:00' as timestamp)
bigint
====
---- QUERY
+# Regression tests for IMPALA-8054
+select count(*) from functional.alltypes where 10 between float_col and bigint_col;
+---- RESULTS
+6570
+---- TYPES
+bigint
+====
+---- QUERY
+select count(*) from functional.alltypes where !(10 between null and bigint_col);
+---- RESULTS
+730
+---- TYPES
+bigint
+====
+---- QUERY
+select count(*) from functional.alltypes where !(5 between float_col and null);
+---- RESULTS
+3650
+---- TYPES
+bigint
+====
+---- QUERY
+select count(*) from functional.alltypes where 10 between null and null;
+---- RESULTS
+0
+---- TYPES
+bigint
+====
+---- QUERY
+select count(*) from functional.alltypes where null between null and null;
+---- RESULTS
+0
+---- TYPES
+bigint
+====
+---- QUERY
+select count(*) from functional.alltypes where 10 not between float_col and bigint_col;
+---- RESULTS
+730
+---- TYPES
+bigint
+====
+---- QUERY
+select count(*) from functional.alltypes where 10 not between null and bigint_col;
+---- RESULTS
+730
+---- TYPES
+bigint
+====
+---- QUERY
+select count(*) from functional.alltypes where 5 not between float_col and null;
+---- RESULTS
+3650
+---- TYPES
+bigint
+====
+---- QUERY
+select count(*) from functional.alltypes where 10 not between null and null;
+---- RESULTS
+0
+---- TYPES
+bigint
+====
+---- QUERY
+select count(*) from functional.alltypes where null not between null and null;
+---- RESULTS
+0
+---- TYPES
+bigint
+====
+---- QUERY
# Test pid() function, this should only return one pid. If pid() were not implemented
# correctly via the global state variable, this could return multiple pids.
select pid() p from functional.alltypes