[GitHub] [carbondata] KanakaKumar commented on a change in pull request #3209: [CARBONDATA-3373] Optimize scenes with in numbers in SQL

[GitBox]

KanakaKumar commented on a change in pull request #3209: [CARBONDATA-3373] Optimize scenes with in numbers in SQL
URL: https://github.com/apache/carbondata/pull/3209#discussion_r283874906
 
 

 ##########
 File path: core/src/main/java/org/apache/carbondata/core/scan/filter/executer/IncludeFilterExecuterImpl.java
 ##########
 @@ -272,29 +273,21 @@ private BitSet getFilteredIndexesForMeasures(ColumnPage columnPage,
     // Get the measure values from the chunk. compare sequentially with the
     // the filter values. The one that matches sets it Bitset.
     BitSet bitSet = new BitSet(rowsInPage);
-    Object[] filterValues = msrColumnExecutorInfo.getFilterKeys();
-
-    SerializableComparator comparator = Comparator.getComparatorByDataTypeForMeasure(msrType);
+    Set filterValuesSet = msrColumnExecutorInfo.getFilterKeysSet();
     BitSet nullBitSet = columnPage.getNullBits();
-    for (int i = 0; i < filterValues.length; i++) {
-      if (filterValues[i] == null) {
-        for (int j = nullBitSet.nextSetBit(0); j >= 0; j = nullBitSet.nextSetBit(j + 1)) {
-          bitSet.set(j);
-        }
-        continue;
-      }
-      for (int startIndex = 0; startIndex < rowsInPage; startIndex++) {
-        if (!nullBitSet.get(startIndex)) {
-          // Check if filterValue[i] matches with measure Values.
-          Object msrValue = DataTypeUtil
-              .getMeasureObjectBasedOnDataType(columnPage, startIndex,
-                  msrType, msrColumnEvaluatorInfo.getMeasure());
-
-          if (comparator.compare(msrValue, filterValues[i]) == 0) {
-            // This is a match.
-            bitSet.set(startIndex);
-          }
+    for (int startIndex = 0; startIndex < rowsInPage; startIndex++) {
+      if (!nullBitSet.get(startIndex)) {
+        // Check if filterValue[i] matches with measure Values.
+        Object msrValue = DataTypeUtil
+            .getMeasureObjectBasedOnDataType(columnPage, startIndex,
+                msrType, msrColumnEvaluatorInfo.getMeasure());
+
+        if (filterValuesSet.contains(msrValue)) {
 
 Review comment:
   Ok.. Float and Double hashcode implementation is taken care. But Decimal data type seems creates problem. Please refer below sample code.
   
   BigDecimal bigDecimal = new BigDecimal("4743.00");
       BigDecimal  bigDecimal2 = new BigDecimal("4743.0");
   
       System.out.println(bigDecimal.compareTo(bigDecimal2)==0); // compareTo returns true
   
       HashSet<BigDecimal> decimals = new HashSet<>();
       decimals.add(bigDecimal);
       decimals.add(bigDecimal2);
   
       System.out.println(decimals.toString()); // Output: [4743.00, 4743.0]
       System.out.println(decimals.contains(new BigDecimal("4743"))); //Returns false
       System.out.println(decimals.contains(new BigDecimal("4743.0000"))); // Returns false
       System.out.println(decimals.contains(new BigDecimal("4743.00"))); // Returns true
       System.out.println(decimals.contains(new BigDecimal("4743.0"))); // Returns true
   
       TreeSet<BigDecimal> treeSet = new TreeSet();
       treeSet.add(bigDecimal);
       treeSet.add(bigDecimal2);
   
       System.out.println(treeSet.toString()); // Output:  [4743.00]
       System.out.println(treeSet.contains(new BigDecimal("4743"))); //Returns true
       System.out.println(treeSet.contains(new BigDecimal("4743.0000"))); // Returns true
       System.out.println(treeSet.contains(new BigDecimal("4743.00"))); // Returns true
       System.out.println(treeSet.contains(new BigDecimal("4743.0"))); // Returns true
   
   So, please add a test case to validate this scenario also.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services