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 2017/11/14 17:50:24 UTC

[02/18] carbondata git commit: [CARBONDATA-1699][FILTER] Filter is not working properly

[CARBONDATA-1699][FILTER] Filter is not working properly

Issue:
When having duplicate values in the data, filter results are wrong

Scenario:
Load data like below
a,11234567489.7976
b,11234567489.7976000000

Filter query on double_column = 11234567489.7976

Result - only either one of the row is selected
Actual Result - all the two rows should be selected(both the values will be same while parsing)

Logic of binary search while applying filter is changed (in case of duplicates in DOUBLE column)

This closes #1483


Project: http://git-wip-us.apache.org/repos/asf/carbondata/repo
Commit: http://git-wip-us.apache.org/repos/asf/carbondata/commit/1e408c5d
Tree: http://git-wip-us.apache.org/repos/asf/carbondata/tree/1e408c5d
Diff: http://git-wip-us.apache.org/repos/asf/carbondata/diff/1e408c5d

Branch: refs/heads/fgdatamap
Commit: 1e408c5d67aac8fc14fc0abeb55f119aff026611
Parents: fab102b
Author: dhatchayani <dh...@gmail.com>
Authored: Fri Nov 10 19:51:24 2017 +0530
Committer: ravipesala <ra...@gmail.com>
Committed: Tue Nov 14 12:21:12 2017 +0530

----------------------------------------------------------------------
 .../cache/dictionary/ColumnDictionaryInfo.java  | 48 ++++++++++++++------
 .../src/test/resources/uniq.csv                 |  3 ++
 .../src/test/resources/uniqwithoutheader.csv    |  2 +
 .../primitiveTypes/DoubleDataTypeTestCase.scala | 15 +++++-
 4 files changed, 52 insertions(+), 16 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/carbondata/blob/1e408c5d/core/src/main/java/org/apache/carbondata/core/cache/dictionary/ColumnDictionaryInfo.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/carbondata/core/cache/dictionary/ColumnDictionaryInfo.java b/core/src/main/java/org/apache/carbondata/core/cache/dictionary/ColumnDictionaryInfo.java
index db49fa1..3b915e0 100644
--- a/core/src/main/java/org/apache/carbondata/core/cache/dictionary/ColumnDictionaryInfo.java
+++ b/core/src/main/java/org/apache/carbondata/core/cache/dictionary/ColumnDictionaryInfo.java
@@ -236,27 +236,27 @@ public class ColumnDictionaryInfo extends AbstractColumnDictionaryInfo {
       while (low <= high) {
         int mid = (low + high) >>> 1;
         int surrogateKey = sortedSurrogates.get(mid);
-        byte[] dictionaryValue = getDictionaryBytesFromSurrogate(surrogateKey);
-        int cmp = -1;
-        //fortify fix
-        if (null == dictionaryValue) {
-          cmp = -1;
-        } else if (this.getDataType() != DataTypes.STRING) {
-          cmp = compareFilterKeyWithDictionaryKey(
-              new String(dictionaryValue, Charset.forName(CarbonCommonConstants.DEFAULT_CHARSET)),
-              filterKey, this.getDataType());
-
-        } else {
-          cmp =
-              ByteUtil.UnsafeComparer.INSTANCE.compareTo(dictionaryValue, byteValueOfFilterMember);
-        }
+        int cmp =
+            compareFilterValue(surrogateKey, sortedSurrogates, byteValueOfFilterMember, filterKey);
         if (cmp < 0) {
           low = mid + 1;
         } else if (cmp > 0) {
           high = mid - 1;
         } else {
-
           surrogates.add(surrogateKey);
+          if (this.getDataType() == DataTypes.DOUBLE) {
+            int tmp_mid = mid - 1;
+            int tmp_low = low > 0 ? low + 1 : 0;
+            while (tmp_mid >= tmp_low) {
+              surrogateKey = sortedSurrogates.get(tmp_mid);
+              cmp = compareFilterValue(surrogateKey, sortedSurrogates, byteValueOfFilterMember,
+                  filterKey);
+              if (cmp == 0) {
+                surrogates.add(surrogateKey);
+              }
+              tmp_mid--;
+            }
+          }
           low = mid;
           break;
         }
@@ -268,6 +268,24 @@ public class ColumnDictionaryInfo extends AbstractColumnDictionaryInfo {
     }
   }
 
+  private int compareFilterValue(int surrogateKey, List<Integer> sortedSurrogates,
+      byte[] byteValueOfFilterMember, String filterKey) {
+    byte[] dictionaryValue = getDictionaryBytesFromSurrogate(surrogateKey);
+    int cmp = -1;
+    //fortify fix
+    if (null == dictionaryValue) {
+      cmp = -1;
+    } else if (this.getDataType() != DataTypes.STRING) {
+      cmp = compareFilterKeyWithDictionaryKey(
+          new String(dictionaryValue, Charset.forName(CarbonCommonConstants.DEFAULT_CHARSET)),
+          filterKey, this.getDataType());
+
+    } else {
+      cmp = ByteUtil.UnsafeComparer.INSTANCE.compareTo(dictionaryValue, byteValueOfFilterMember);
+    }
+    return cmp;
+  }
+
   private int compareFilterKeyWithDictionaryKey(String dictionaryVal, String memberVal,
       DataType dataType) {
     try {

http://git-wip-us.apache.org/repos/asf/carbondata/blob/1e408c5d/integration/spark-common-test/src/test/resources/uniq.csv
----------------------------------------------------------------------
diff --git a/integration/spark-common-test/src/test/resources/uniq.csv b/integration/spark-common-test/src/test/resources/uniq.csv
new file mode 100644
index 0000000..bcb2ee3
--- /dev/null
+++ b/integration/spark-common-test/src/test/resources/uniq.csv
@@ -0,0 +1,3 @@
+name,double_column
+a,11234567489.7976
+b,11234567489.7976000000
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/carbondata/blob/1e408c5d/integration/spark-common-test/src/test/resources/uniqwithoutheader.csv
----------------------------------------------------------------------
diff --git a/integration/spark-common-test/src/test/resources/uniqwithoutheader.csv b/integration/spark-common-test/src/test/resources/uniqwithoutheader.csv
new file mode 100644
index 0000000..5af1e6b
--- /dev/null
+++ b/integration/spark-common-test/src/test/resources/uniqwithoutheader.csv
@@ -0,0 +1,2 @@
+a,11234567489.7976
+b,11234567489.7976000000
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/carbondata/blob/1e408c5d/integration/spark-common-test/src/test/scala/org/apache/carbondata/integration/spark/testsuite/primitiveTypes/DoubleDataTypeTestCase.scala
----------------------------------------------------------------------
diff --git a/integration/spark-common-test/src/test/scala/org/apache/carbondata/integration/spark/testsuite/primitiveTypes/DoubleDataTypeTestCase.scala b/integration/spark-common-test/src/test/scala/org/apache/carbondata/integration/spark/testsuite/primitiveTypes/DoubleDataTypeTestCase.scala
index 67bbd10..1d63eda 100644
--- a/integration/spark-common-test/src/test/scala/org/apache/carbondata/integration/spark/testsuite/primitiveTypes/DoubleDataTypeTestCase.scala
+++ b/integration/spark-common-test/src/test/scala/org/apache/carbondata/integration/spark/testsuite/primitiveTypes/DoubleDataTypeTestCase.scala
@@ -19,8 +19,8 @@ package org.apache.carbondata.integration.spark.testsuite.primitiveTypes
 import java.util.Random
 
 import org.apache.spark.sql.test.util.QueryTest
-import org.apache.spark.sql.{DataFrame, Row, SaveMode}
 import org.apache.spark.sql.types._
+import org.apache.spark.sql.{DataFrame, Row, SaveMode}
 import org.scalatest.BeforeAndAfterAll
 
 /**
@@ -51,6 +51,8 @@ class DoubleDataTypeTestCase extends QueryTest with BeforeAndAfterAll {
   }
 
   override def beforeAll {
+    sql("drop table if exists uniq_carbon")
+    sql("drop table if exists uniq_hive")
     sql("drop table if exists doubleTypeCarbonTable")
     sql("drop table if exists doubleTypeHiveTable")
 
@@ -76,6 +78,15 @@ class DoubleDataTypeTestCase extends QueryTest with BeforeAndAfterAll {
 
   }
 
+  test("duplicate values") {
+    sql("create table uniq_carbon(name string, double_column double) stored by 'carbondata' TBLPROPERTIES ('DICTIONARY_INCLUDE'='double_column')")
+    sql(s"load data inpath '$resourcesPath/uniq.csv' into table uniq_carbon")
+    sql("create table uniq_hive(name string, double_column double) ROW FORMAT DELIMITED FIELDS TERMINATED BY ','")
+    sql(s"load data inpath '$resourcesPath/uniqwithoutheader.csv' into table uniq_hive")
+    checkAnswer(sql("select * from uniq_carbon where double_column>=11"),
+      sql("select * from uniq_hive where double_column>=11"))
+  }
+
 //  test("agg query") {
 //    checkAnswer(sql("select city, sum(m1), avg(m1), count(m1), max(m1), min(m1) from doubleTypeCarbonTable group by city"),
 //      sql("select city, sum(m1), avg(m1), count(m1), max(m1), min(m1) from doubleTypeHiveTable group by city"))
@@ -85,6 +96,8 @@ class DoubleDataTypeTestCase extends QueryTest with BeforeAndAfterAll {
 //  }
 
   override def afterAll {
+    sql("drop table if exists uniq_carbon")
+    sql("drop table if exists uniq_hive")
     sql("drop table if exists doubleTypeCarbonTable")
     sql("drop table if exists doubleTypeHiveTable")
   }