[GitHub] carbondata pull request #2758: Debug Logs and function added for Adaptive En...

[qiuchenjian-2]

GitHub user manishnalla1994 opened a pull request:

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

    Debug Logs and function added for Adaptive Encoding

    Added a function to get the type of encoding used. Added the debug log for checking which type of encoding is used.
   
     - [ ] Any interfaces changed?
     
     - [ ] Any backward compatibility impacted?
     
     - [ ] Document update required?
   
     - [x] Testing done
           
     - [ ] For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.
   


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/manishnalla1994/carbondata adaptive_encoding_fix

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/carbondata/pull/2758.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #2758
   
----
commit dc5e93252ec85fde6acc5d00992f41cdf030452a
Author: Manish Nalla <manishnalla1994@...>
Date:   2018-09-25T12:14:49Z

    Debug Logs and function added for Adaptive Encoding

----


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user manishnalla1994 commented on the issue:

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


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user manishnalla1994 commented on the issue:

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


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user shardul-cr7 commented on the issue:

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


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2758#discussion_r220531471
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/datastore/page/encoding/ColumnPageEncoder.java ---
    @@ -78,6 +78,13 @@ public DataType getTargetDataType(ColumnPage inputPage) {
         }
       }
     
    +  public Encoding getTypeOfEncoding() {
    +    if (CarbonUtil.isEncodedWithMeta(getEncodingList())) {
    +      return getEncodingList().get(0);
    --- End diff --
   
    1. `getEncodingList()` is called 2 times. Hold its value in one variable and use at both the places
    2. Change the method name from `getTypeOfEncoding()` to `getEncodingType()`


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2758#discussion_r220542843
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/datastore/page/encoding/ColumnPageEncoder.java ---
    @@ -78,6 +78,14 @@ public DataType getTargetDataType(ColumnPage inputPage) {
         }
       }
     
    +  public Encoding getEncodingType() {
    +    List<Encoding> currEncodingList = getEncodingList();
    +    if (CarbonUtil.isEncodedWithMeta(currEncodingList)) {
    +      return currEncodingList.get(0);
    +    }
    +    return null;
    --- End diff --
   
    better not returning null, why not return the list itself?


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2758#discussion_r220553863
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/datastore/page/encoding/ColumnPageEncoder.java ---
    @@ -78,6 +78,14 @@ public DataType getTargetDataType(ColumnPage inputPage) {
         }
       }
     
    +  public Encoding getEncodingType() {
    +    List<Encoding> currEncodingList = getEncodingList();
    +    if (CarbonUtil.isEncodedWithMeta(currEncodingList)) {
    +      return currEncodingList.get(0);
    +    }
    +    return null;
    --- End diff --
   
    This function will only return value in case of Adaptive Encoding, for any other case it will return null. Also we just want the first element of the list to check the type of encoding.


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---