[GitHub] carbondata pull request #2736: [CARBONDATA-2947] Adaptive encoding support f...

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2736#discussion_r219703939
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/datastore/chunk/DimensionColumnPage.java ---
    @@ -16,6 +16,8 @@
      */
     package org.apache.carbondata.core.datastore.chunk;
     
    +import java.util.BitSet;
    +
    --- End diff --
   
    Something wrong with format?


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2736#discussion_r219704067
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/datastore/chunk/DimensionColumnPage.java ---
    @@ -16,6 +16,8 @@
      */
     package org.apache.carbondata.core.datastore.chunk;
     
    +import java.util.BitSet;
    +
    --- End diff --
   
    it seems so.. locally it passed


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2736#discussion_r219704306
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/scan/filter/executer/RowLevelRangeGrtThanFiterExecuterImpl.java ---
    @@ -387,9 +387,11 @@ private BitSet getFilteredIndexes(DimensionColumnPage dimensionColumnPage,
         } else {
           bitSet = setFilterdIndexToBitSet(dimensionColumnPage, numerOfRows);
         }
    -    if (dimensionColumnPage.isNoDicitionaryColumn()) {
    +    if (dimensionColumnPage.isNoDicitionaryColumn() && !dimensionColumnPage.isAdaptiveEncoded()) {
    --- End diff --
   
    no need add condition here. Please handle inside 'removeNullValues' method. Not required to do changes in these classes


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2736#discussion_r219704431
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/util/DataTypeUtil.java ---
    @@ -78,24 +78,29 @@
        *
        * @param msrValue
        * @param dataType
    -   * @param carbonMeasure
    +   * @param scale
    +   * @param precision
    +   * @param dateFormat
        * @return
        */
    -  public static Object getMeasureValueBasedOnDataType(String msrValue, DataType dataType,
    -      int scale, int precision) {
    -    return getMeasureValueBasedOnDataType(msrValue, dataType, scale, precision, false);
    +  public static Object getMeasureValueBasedOnDataType(String msrValue, DataType dataType, int scale,
    +      int precision, String dateFormat) {
    --- End diff --
   
    format not related measure. Better add another method to convert timestamp to long and call directly


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2736#discussion_r219704518
 
    --- Diff: processing/src/main/java/org/apache/carbondata/processing/loading/converter/impl/RowConverterImpl.java ---
    @@ -69,11 +69,14 @@
     
       private Map<Object, Integer>[] localCaches;
     
    +  private boolean isStreamingFlow;
    +
       public RowConverterImpl(DataField[] fields, CarbonDataLoadConfiguration configuration,
    -      BadRecordsLogger badRecordLogger) {
    +      BadRecordsLogger badRecordLogger, boolean isStreamingFlow) {
    --- End diff --
   
    Better not mention about streaming in core layer. Change boolean to `isConvertToBinary` and add javadoc about what does it do


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2736#discussion_r219704623
 
    --- Diff: processing/src/main/java/org/apache/carbondata/processing/loading/converter/impl/RowConverterImpl.java ---
    @@ -69,11 +69,14 @@
     
       private Map<Object, Integer>[] localCaches;
     
    +  private boolean isStreamingFlow;
    +
       public RowConverterImpl(DataField[] fields, CarbonDataLoadConfiguration configuration,
    -      BadRecordsLogger badRecordLogger) {
    +      BadRecordsLogger badRecordLogger, boolean isStreamingFlow) {
    --- End diff --
   
    And also add another constructor without this filed and by default it should be false. So that we can avoid unnecessary confusions in other caller classes


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---