[GitHub] carbondata pull request #2730: [WIP] Performance improvements for Fileformat...

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user QiangCai commented on the issue:

    https://github.com/apache/carbondata/pull/2730
 
    @ravipesala
    can you describe the modification?


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2730#discussion_r224776123
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/metadata/datatype/DecimalConverterFactory.java ---
    @@ -95,6 +100,51 @@ public int getSizeInBytes() {
           return BigDecimal.valueOf((Long) valueToBeConverted, scale);
         }
     
    +    @Override public void fillVector(Object valuesToBeConverted, int size, ColumnVectorInfo info,
    +        BitSet nullBitset) {
    +      // TODO we need to find way to directly set to vector with out conversion. This way is very
    +      // inefficient.
    +      CarbonColumnVector vector = info.vector;
    +      int precision = info.measure.getMeasure().getPrecision();
    +      if (valuesToBeConverted instanceof byte[]) {
    +        byte[] data = (byte[]) valuesToBeConverted;
    +        for (int i = 0; i < size; i++) {
    +          if (nullBitset.get(i)) {
    +            vector.putNull(i);
    +          } else {
    +            vector.putDecimal(i, BigDecimal.valueOf(data[i], scale), precision);
    +          }
    +        }
    +      } else if (valuesToBeConverted instanceof short[]) {
    +        short[] data = (short[]) valuesToBeConverted;
    +        for (int i = 0; i < size; i++) {
    +          if (nullBitset.get(i)) {
    +            vector.putNull(i);
    +          } else {
    +            vector.putDecimal(i, BigDecimal.valueOf(data[i], scale), precision);
    +          }
    +        }
    +      } else if (valuesToBeConverted instanceof int[]) {
    +        int[] data = (int[]) valuesToBeConverted;
    +        for (int i = 0; i < size; i++) {
    +          if (nullBitset.get(i)) {
    +            vector.putNull(i);
    +          } else {
    +            vector.putDecimal(i, BigDecimal.valueOf(data[i], scale), precision);
    +          }
    +        }
    +      } else if (valuesToBeConverted instanceof long[]) {
    +        long[] data = (long[]) valuesToBeConverted;
    +        for (int i = 0; i < size; i++) {
    +          if (nullBitset.get(i)) {
    +            vector.putNull(i);
    +          } else {
    +            vector.putDecimal(i, BigDecimal.valueOf(data[i], scale), precision);
    +          }
    +        }
    --- End diff --
   
    Alter scenario where precision and scale can be changed is not handled here...please check for that case and handle...you can take reference from the `DecimalMeasureVectorFiller`


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2730#discussion_r224746003
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/scan/result/vector/impl/directread/ColumnarVectorWrapperDirectWithDeleteDelta.java ---
    @@ -0,0 +1,195 @@
    +/*
    + * 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.scan.result.vector.impl.directread;
    +
    +import java.math.BigDecimal;
    +import java.util.BitSet;
    +
    +import org.apache.carbondata.core.scan.result.vector.CarbonColumnVector;
    +
    +class ColumnarVectorWrapperDirectWithDeleteDelta extends AbstractCarbonColumnarVector {
    +
    +  private BitSet deletedRows;
    +
    +  private BitSet nullBits;
    +
    +  private int counter;
    +
    +  private CarbonColumnVector columnVector;
    --- End diff --
   
    `columnVector` can be made protected and moved to abstract class


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2730#discussion_r224738600
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/scan/result/vector/impl/directread/ColumnarVectorWrapperDirectWithDeleteDeltaAndInvertedIndex.java ---
    @@ -0,0 +1,228 @@
    +/*
    + * 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.scan.result.vector.impl.directread;
    +
    +import java.math.BigDecimal;
    +import java.util.BitSet;
    +
    +import org.apache.carbondata.core.metadata.datatype.DataType;
    +import org.apache.carbondata.core.metadata.datatype.DataTypes;
    +import org.apache.carbondata.core.metadata.datatype.DecimalType;
    +import org.apache.carbondata.core.scan.result.vector.CarbonColumnVector;
    +import org.apache.carbondata.core.scan.result.vector.impl.CarbonColumnVectorImpl;
    +
    +class ColumnarVectorWrapperDirectWithDeleteDeltaAndInvertedIndex
    +    extends AbstractCarbonColumnarVector implements ConvertableVector {
    --- End diff --
   
    AbstractCarbonColumnarVector  is already implementing ConvertableVector ...so it can be removed from here


---

[qiuchenjian-2]

Github user manishgupta88 commented on the issue:

    https://github.com/apache/carbondata/pull/2730
 
    @ravipesala ...Please add code comments wherever applicable...especially for new classes and to explain major flow or logic...it will make the code more readable and easy to understand


---

[qiuchenjian-2]

Github user ravipesala closed the pull request at:

    https://github.com/apache/carbondata/pull/2730


---

[qiuchenjian-2]

Github user zzcclp commented on the issue:

    https://github.com/apache/carbondata/pull/2730
 
    @ravipesala why close this pr?


---