You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@carbondata.apache.org by ra...@apache.org on 2019/04/02 02:41:40 UTC
[carbondata] 20/41: [CARBONDATA-3315] Fix for Range Filter failing
with two between clauses as children of OR expression
This is an automated email from the ASF dual-hosted git repository.
ravipesala pushed a commit to branch branch-1.5
in repository https://gitbox.apache.org/repos/asf/carbondata.git
commit d78eed4befc06b9793a2164eb7276032b9ff0a6c
Author: manishnalla1994 <ma...@gmail.com>
AuthorDate: Tue Mar 12 18:58:43 2019 +0530
[CARBONDATA-3315] Fix for Range Filter failing with two between clauses as children of OR expression
Problem : Range filter with Or and two between clauses as children fails because while evaluating the Or we combine both the sub-trees into one and then evaluate, which is wrong.
Solution : Instead of combining them we have to treat both the children of 'OR' as different expressions and evaluate separately.
This closes #3145
---
.../scan/expression/RangeExpressionEvaluator.java | 30 +++++++++++++----
.../detailquery/RangeFilterTestCase.scala | 38 ++++++++++++++++++++++
2 files changed, 61 insertions(+), 7 deletions(-)
diff --git a/core/src/main/java/org/apache/carbondata/core/scan/expression/RangeExpressionEvaluator.java b/core/src/main/java/org/apache/carbondata/core/scan/expression/RangeExpressionEvaluator.java
index c47d5ff..f131c92 100644
--- a/core/src/main/java/org/apache/carbondata/core/scan/expression/RangeExpressionEvaluator.java
+++ b/core/src/main/java/org/apache/carbondata/core/scan/expression/RangeExpressionEvaluator.java
@@ -208,6 +208,13 @@ public class RangeExpressionEvaluator {
return filterExpressionMap;
}
+ private void evaluateOrExpression(Expression currentNode, Expression orExpChild) {
+ Map<String, List<FilterModificationNode>> filterExpressionMapNew =
+ new HashMap<>(CarbonCommonConstants.DEFAULT_COLLECTION_SIZE);
+ fillExpressionMap(filterExpressionMapNew, orExpChild, currentNode);
+ replaceWithRangeExpression(filterExpressionMapNew);
+ filterExpressionMapNew.clear();
+ }
private void fillExpressionMap(Map<String, List<FilterModificationNode>> filterExpressionMap,
Expression currentNode, Expression parentNode) {
@@ -222,13 +229,22 @@ public class RangeExpressionEvaluator {
&& eligibleForRangeExpConv(currentNode))) {
addFilterExpressionMap(filterExpressionMap, currentNode, parentNode);
}
-
- for (Expression exp : currentNode.getChildren()) {
- if (null != exp) {
- fillExpressionMap(filterExpressionMap, exp, currentNode);
- if (exp instanceof OrExpression) {
- replaceWithRangeExpression(filterExpressionMap);
- filterExpressionMap.clear();
+ // In case of Or Exp we have to evaluate both the subtrees of expression separately
+ // else it will combine the results of both the subtrees into one expression
+ // which wont give us correct result
+ if (currentNode instanceof OrExpression) {
+ Expression leftChild = ((OrExpression) currentNode).left;
+ Expression rightChild = ((OrExpression) currentNode).right;
+ if (null != leftChild) {
+ evaluateOrExpression(currentNode, leftChild);
+ }
+ if (null != rightChild) {
+ evaluateOrExpression(currentNode, rightChild);
+ }
+ } else {
+ for (Expression exp : currentNode.getChildren()) {
+ if (null != exp) {
+ fillExpressionMap(filterExpressionMap, exp, currentNode);
}
}
}
diff --git a/integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/detailquery/RangeFilterTestCase.scala b/integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/detailquery/RangeFilterTestCase.scala
index cd7f7fc..e9a83ec 100644
--- a/integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/detailquery/RangeFilterTestCase.scala
+++ b/integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/detailquery/RangeFilterTestCase.scala
@@ -39,6 +39,20 @@ class RangeFilterTestCase extends QueryTest with BeforeAndAfterAll {
sql("drop table if exists DICTIONARY_CARBON_6")
sql("drop table if exists NO_DICTIONARY_CARBON_7")
sql("drop table if exists NO_DICTIONARY_HIVE_8")
+ sql("drop table if exists carbontest")
+ sql("drop table if exists carbontest_hive")
+ sql(
+ "create table carbontest(c1 string, c2 string, c3 int) stored by 'carbondata' tblproperties" +
+ "('sort_columns'='c3')")
+ (0 to 10).foreach { index =>
+ sql(s"insert into carbontest select '$index','$index',$index")
+ }
+ sql(
+ "create table carbontest_hive(c1 string, c2 string, c3 int) row format delimited fields " +
+ "terminated by ',' tblproperties('sort_columns'='c3')")
+ (0 to 10).foreach { index =>
+ sql(s"insert into carbontest_hive select '$index','$index',$index")
+ }
sql("CREATE TABLE NO_DICTIONARY_HIVE_1 (CUST_ID int,CUST_NAME String,ACTIVE_EMUI_VERSION " +
"string, DOB timestamp, DOJ timestamp, BIGINT_COLUMN1 bigint,BIGINT_COLUMN2 bigint," +
@@ -587,7 +601,31 @@ class RangeFilterTestCase extends QueryTest with BeforeAndAfterAll {
sql("select empname from NO_DICTIONARY_HIVE_8 where empname <= '107'"))
}
+ test("Range filter with two between clauses") {
+
+ checkAnswer(sql("select * from carbontest where c3 between 2 and 3 or c3 between 3 and 4"),
+ sql("select * from carbontest_hive where c3 between 2 and 3 or c3 between 3 and 4"))
+
+ checkAnswer(sql("select * from carbontest where c3 >= 2 and c3 <= 3 or c3 >= 3 and c3 <= 4"),
+ sql("select * from carbontest_hive where c3 >= 2 and c3 <= 3 or c3 >= 3 and c3 <= 4"))
+
+ checkAnswer(sql(
+ "select * from carbontest where (c3 between 2 and 3 or c3 between 3 and 4) and (c3 between " +
+ "2 and 4 or c3 between 4 and 5)"),
+ sql(
+ "select * from carbontest_hive where (c3 between 2 and 3 or c3 between 3 and 4) and (c3 " +
+ "between 2 and 4 or c3 between 4 and 5)"))
+
+ checkAnswer(sql(
+ "select * from carbontest where c3 >= 2 and c3 <= 5 and (c3 between 1 and 3 or c3 between 3" +
+ " and 6)"),
+ sql("select * from carbontest_hive where c3 >= 2 and c3 <= 5 and (c3 between 1 and 3 or c3 " +
+ "between 3 and 6)"))
+ }
+
override def afterAll {
+ sql("drop table if exists carbontest")
+ sql("drop table if exists carbontest_hive")
sql("drop table if exists filtertestTable")
sql("drop table if exists NO_DICTIONARY_HIVE_1")
sql("drop table if exists NO_DICTIONARY_CARBON_1")