carbondata-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ravipes...@apache.org
Subject [carbondata] 20/41: [CARBONDATA-3315] Fix for Range Filter failing with two between clauses as children of OR expression
Date Tue, 02 Apr 2019 02:41:40 GMT
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 <manish.nalla1994@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")


Mime
View raw message