[GitHub] carbondata pull request #1751: [WIP]Measure null

[qiuchenjian-2]

Github user ravipesala commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/1751#discussion_r159829745
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/datastore/page/statistics/SimpleStatsResult.java ---
    @@ -25,6 +25,8 @@
     
       Object getMax();
     
    +  byte getNull();
    --- End diff --
   
    Better use boolean and change method to isNull


---

[qiuchenjian-2]

Github user ravipesala commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/1751#discussion_r159831719
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/datastore/page/statistics/TablePageStatistics.java ---
    @@ -71,6 +82,10 @@ private void updateMeasureMinMax(EncodedColumnPage[] measures) {
         return dimensionMaxValue;
       }
     
    +  public byte[] getDimentionNullValue() {
    --- End diff --
   
    typo getDimensionNullValue


---

[qiuchenjian-2]

Github user ravipesala commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/1751#discussion_r159832035
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/indexstore/blockletindex/BlockletDataMap.java ---
    @@ -728,9 +746,9 @@ private DataMapRow convertToRow(IndexKey key) {
     
       private List<String> getPartitions() {
         DataMapRow unsafeRow = unsafeMemorySummaryDMStore.getUnsafeRow(0);
    -    if (unsafeRow.getColumnCount() > 2) {
    +    if (unsafeRow.getColumnCount() > 3) {
           List<String> partitions = new ArrayList<>();
    -      DataMapRow row = unsafeRow.getRow(2);
    +      DataMapRow row = unsafeRow.getRow(3);
    --- End diff --
   
    Please add constant to partition index


---

[qiuchenjian-2]

Github user ravipesala commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/1751#discussion_r159832127
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/indexstore/blockletindex/BlockletDataRefNodeWrapper.java ---
    @@ -99,6 +99,10 @@ public BlockletDataRefNodeWrapper(List<TableBlockInfo> blockInfos, int index,
         return null;
       }
     
    +  @Override public byte[] getColumnsNullValue() {
    --- End diff --
   
    Change byte[] to BitSet


---

[qiuchenjian-2]

Github user ravipesala commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/1751#discussion_r159832676
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/scan/filter/executer/FilterExecuter.java ---
    @@ -46,7 +46,7 @@ boolean applyFilter(RowIntf value, int dimOrdinalMax)
        * @param blockMinValue
        * @return BitSet
        */
    -  BitSet isScanRequired(byte[][] blockMaxValue, byte[][] blockMinValue);
    +  BitSet isScanRequired(byte[][] blockMaxValue, byte[][] blockMinValue, byte[] blockNullValue);
    --- End diff --
   
    Better wrap these parameters to new BlockletStats object


---

[qiuchenjian-2]

Github user ravipesala commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/1751#discussion_r159832781
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/scan/filter/executer/ImplicitColumnFilterExecutor.java ---
    @@ -43,5 +43,6 @@ BitSet isFilterValuesPresentInBlockOrBlocklet(byte[][] maxValue, byte[][] minVal
        *
        * @return
        */
    -  Boolean isFilterValuesPresentInAbstractIndex(byte[][] maxValue, byte[][] minValue);
    +  Boolean isFilterValuesPresentInAbstractIndex(byte[][] maxValue, byte[][] minValue,
    --- End diff --
   
    Here also use BlockletStats object


---

[qiuchenjian-2]

Github user ravipesala commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/1751#discussion_r159832789
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/scan/filter/executer/ImplicitColumnFilterExecutor.java ---
    @@ -34,7 +34,7 @@
        * @return
        */
       BitSet isFilterValuesPresentInBlockOrBlocklet(byte[][] maxValue, byte[][] minValue,
    -      String uniqueBlockPath);
    +      byte[] nullValue, String uniqueBlockPath);
    --- End diff --
   
    Here also use BlockletStats object


---

[qiuchenjian-2]

Github user kumarvishal09 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/1751#discussion_r160157245
 
    --- Diff: format/src/main/thrift/carbondata.thrift ---
    @@ -45,6 +45,7 @@ struct BlockletBTreeIndex{
     struct BlockletMinMaxIndex{
         1: required list<binary> min_values; //Min value of all columns of one blocklet Bit-Packed
         2: required list<binary> max_values; //Max value of all columns of one blocklet Bit-Packed
    +    3: optional list<byte> null_value;   // Null Bool turn on if null value present.
    --- End diff --
   
    we can use PresenceMeta to store null value


---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/1751
 
    retest this please


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1751
 
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/2653/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1751
 
    Build Success with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/1421/



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/1751
 
    retest this please


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1751
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/2660/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1751
 
    Build Success with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/1426/



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/1751
 
    retest this please


---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/1751
 
    retest sdv please


---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/1751
 
    SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/2810/



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/1751
 
    retest sdv please


---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/1751
 
    SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/2811/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1751
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/2718/



---