[CARBONDATA-3315] Fix for Range Filter failing with two between claus…

…es 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

core/src/main/java/org/apache/carbondata/core/scan/expression/RangeExpressionEvaluator.java

@@ -208,6 +208,13 @@ private Map<String, List<FilterModificationNode>> convertFilterTreeToMap() {
     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 @@ private void fillExpressionMap(Map<String, List<FilterModificationNode>> filterE
         && 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);
         }
       }
     }

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")