[GitHub] carbondata pull request #2863: [WIP] Optimise decompressing while filling th...

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2863#discussion_r230257378
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/datastore/compression/SnappyCompressor.java ---
    @@ -90,7 +90,7 @@ public String getName() {
         try {
           uncompressedLength = Snappy.uncompressedLength(compInput, offset, length);
           data = new byte[uncompressedLength];
    -      Snappy.uncompress(compInput, offset, length, data, 0);
    +      snappyNative.rawUncompress(compInput, offset, length, data, 0);
    --- End diff --
   
    is it safe to use `SnappyNative` class directly? It's documentation says we should not use this class directly


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2863#discussion_r230260860
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/datastore/page/encoding/compress/DirectCompressCodec.java ---
    @@ -224,135 +239,134 @@ public void decodeAndFillVector(ColumnPage columnPage, ColumnVectorInfo vectorIn
           }
         }
     
    -    private void fillVector(ColumnPage columnPage, CarbonColumnVector vector,
    -        DataType vectorDataType, DataType pageDataType, int pageSize, ColumnVectorInfo vectorInfo) {
    +    private void fillVector(byte[] pageData, CarbonColumnVector vector, DataType vectorDataType,
    +        DataType pageDataType, int pageSize, ColumnVectorInfo vectorInfo, BitSet nullBits) {
    +      int k = 0;
    --- End diff --
   
    Rename `k` to a meaningful name


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2863#discussion_r230117118
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/datastore/page/encoding/adaptive/AdaptiveDeltaFloatingCodec.java ---
    @@ -244,59 +243,56 @@ public double decodeDouble(long value) {
         }
     
         @Override
    -    public void decodeAndFillVector(ColumnPage columnPage, ColumnVectorInfo vectorInfo) {
    +    public void decodeAndFillVector(byte[] pageData, ColumnVectorInfo vectorInfo, BitSet nullBits,
    +        DataType pageDataType, int pageSize) {
           CarbonColumnVector vector = vectorInfo.vector;
    -      BitSet nullBits = columnPage.getNullBits();
    -      DataType pageDataType = columnPage.getDataType();
    -      int pageSize = columnPage.getPageSize();
           BitSet deletedRows = vectorInfo.deletedRows;
           DataType vectorDataType = vector.getType();
           vector = ColumnarVectorWrapperDirectFactory
               .getDirectVectorWrapperFactory(vector, null, nullBits, deletedRows, true, false);
    +      int k = 0;
    --- End diff --
   
    Rename `k` to a meaningful name


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2863#discussion_r230625480
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/datastore/page/ColumnPageValueConverter.java ---
    @@ -37,5 +40,6 @@
       double decodeDouble(long value);
       double decodeDouble(float value);
       double decodeDouble(double value);
    -  void decodeAndFillVector(ColumnPage columnPage, ColumnVectorInfo vectorInfo);
    +  void decodeAndFillVector(byte[] pageData, ColumnVectorInfo vectorInfo, BitSet nullBits,
    +      DataType pageDataType, int pageSize);
    --- End diff --
   
    It's not feasible now as the codec metadata  like max, factor is required to decode the data and that is present in codecs only.


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2863#discussion_r230625574
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/datastore/page/statistics/PrimitivePageStatsCollector.java ---
    @@ -243,6 +244,11 @@ private int getDecimalCount(double value) {
           int integerPlaces = strValue.indexOf('.');
           if (-1 != integerPlaces) {
             decimalPlaces = strValue.length() - integerPlaces - 1;
    +        if (decimalPlaces == 1) {
    --- End diff --
   
    ok


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2863#discussion_r230625583
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/datastore/page/encoding/compress/DirectCompressCodec.java ---
    @@ -224,135 +239,134 @@ public void decodeAndFillVector(ColumnPage columnPage, ColumnVectorInfo vectorIn
           }
         }
     
    -    private void fillVector(ColumnPage columnPage, CarbonColumnVector vector,
    -        DataType vectorDataType, DataType pageDataType, int pageSize, ColumnVectorInfo vectorInfo) {
    +    private void fillVector(byte[] pageData, CarbonColumnVector vector, DataType vectorDataType,
    +        DataType pageDataType, int pageSize, ColumnVectorInfo vectorInfo, BitSet nullBits) {
    +      int k = 0;
    --- End diff --
   
    ok


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2863#discussion_r230625603
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/datastore/page/encoding/adaptive/AdaptiveDeltaFloatingCodec.java ---
    @@ -244,59 +243,56 @@ public double decodeDouble(long value) {
         }
     
         @Override
    -    public void decodeAndFillVector(ColumnPage columnPage, ColumnVectorInfo vectorInfo) {
    +    public void decodeAndFillVector(byte[] pageData, ColumnVectorInfo vectorInfo, BitSet nullBits,
    +        DataType pageDataType, int pageSize) {
           CarbonColumnVector vector = vectorInfo.vector;
    -      BitSet nullBits = columnPage.getNullBits();
    -      DataType pageDataType = columnPage.getDataType();
    -      int pageSize = columnPage.getPageSize();
           BitSet deletedRows = vectorInfo.deletedRows;
           DataType vectorDataType = vector.getType();
           vector = ColumnarVectorWrapperDirectFactory
               .getDirectVectorWrapperFactory(vector, null, nullBits, deletedRows, true, false);
    +      int k = 0;
    --- End diff --
   
    ok


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2863#discussion_r230625720
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/datastore/page/ColumnPageByteUtil.java ---
    @@ -0,0 +1,49 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.carbondata.core.datastore.page;
    +
    +/**
    + * Utility methods to converts to primitive types only column page data decode.
    + */
    +public class ColumnPageByteUtil {
    --- End diff --
   
    its not duplicating, this conversion is in Little_endian so kept here, anyway this also moved to ByteUtil


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2863
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1258/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2863
 
    Build Failed with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/1473/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2863
 
    Build Failed  with Spark 2.3.1, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/9522/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2863
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1266/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2863
 
    Build Success with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/1481/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2863
 
    Build Success with Spark 2.3.1, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/9530/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2863
 
    Build Failed with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/1578/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2863
 
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1368/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2863
 
    Build Failed  with Spark 2.3.1, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/9626/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2863
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1382/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2863
 
    Build Success with Spark 2.3.1, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/9640/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2863
 
    Build Success with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/1592/



---