You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@carbondata.apache.org by ajantha-bhat <gi...@git.apache.org> on 2019/01/07 15:15:26 UTC

[GitHub] carbondata pull request #3055: [HOTFIX] presto can't read dictionary include...

GitHub user ajantha-bhat opened a pull request:

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

    [HOTFIX] presto can't read dictionary include decimal column 

    problem: decimal column with dictionary include cannot be read in
    presto
    
    cause: int is typecasted to decimal for dictionary columns in decimal stream reader.
    
    solution: keep original data type as well as new data type for decimal
    stream reader.
    
    Be sure to do all of the following checklist to help us incorporate 
    your contribution quickly and easily:
    
     - [ ] Any interfaces changed? NA
     
     - [ ] Any backward compatibility impacted? NA
     
     - [ ] Document update required? NA
    
     - [ ] Testing done. 
    done
     - [ ] For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.  NA
    


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

    $ git pull https://github.com/ajantha-bhat/carbondata issue_fix

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

    https://github.com/apache/carbondata/pull/3055.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 #3055
    
----
commit 205189d121f80aa87598a6c5f5e34562036c03c5
Author: ajantha-bhat <aj...@...>
Date:   2019-01-07T09:20:11Z

    dictionary include decimal column type cast issue
    
    problem: decimal column with dictionary include cannot be read in
    presto
    
    cause: int is typecasted to decimal for dictionary columns.
    
    solution: keep original data type as well as new data type for decimal
    slice reader

----


---

[GitHub] carbondata issue #3055: [CARBONDATA-3237] Fix presto carbon issues in dictio...

Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/3055
  
    LGTM


---

[GitHub] carbondata pull request #3055: [CARBONDATA-3237] Fix presto carbon issues in...

Posted by ajantha-bhat <gi...@git.apache.org>.
Github user ajantha-bhat commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/3055#discussion_r246319596
  
    --- Diff: integration/presto/src/main/java/org/apache/carbondata/presto/readers/SliceStreamReader.java ---
    @@ -95,22 +105,14 @@ public SliceStreamReader(int batchSize, DataType dataType,
         dictOffsets[dictOffsets.length - 1] = size;
         dictionaryBlock = new VariableWidthBlock(dictionary.getDictionarySize(),
             Slices.wrappedBuffer(singleArrayDictValues), dictOffsets, Optional.of(nulls));
    -    values = (int[]) ((CarbonColumnVectorImpl) getDictionaryVector()).getDataArray();
    +    this.isLocalDict = true;
       }
    -
       @Override public void setBatchSize(int batchSize) {
    +
    --- End diff --
    
    done


---

[GitHub] carbondata issue #3055: [CARBONDATA-3237] Fix presto carbon issues in dictio...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #3055: [CARBONDATA-3237] Fix presto carbon issues in dictio...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata pull request #3055: [CARBONDATA-3237] Fix presto carbon issues in...

Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/3055#discussion_r246277431
  
    --- Diff: integration/presto/src/main/java/org/apache/carbondata/presto/readers/SliceStreamReader.java ---
    @@ -142,5 +144,17 @@ public SliceStreamReader(int batchSize, DataType dataType,
     
       @Override public void reset() {
         builder = type.createBlockBuilder(null, batchSize);
    +    this.isLocalDict = false;
    +  }
    +
    +  @Override public void putInt(int rowId, int value) {
    +    Object data = DataTypeUtil
    --- End diff --
    
    Direct overriding does not create problem to local dictionary? how do you handle local dictionary here?


---

[GitHub] carbondata pull request #3055: [CARBONDATA-3237] Fix presto carbon issues in...

Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/3055#discussion_r246277337
  
    --- Diff: integration/presto/src/main/java/org/apache/carbondata/presto/readers/SliceStreamReader.java ---
    @@ -95,22 +105,14 @@ public SliceStreamReader(int batchSize, DataType dataType,
         dictOffsets[dictOffsets.length - 1] = size;
         dictionaryBlock = new VariableWidthBlock(dictionary.getDictionarySize(),
             Slices.wrappedBuffer(singleArrayDictValues), dictOffsets, Optional.of(nulls));
    -    values = (int[]) ((CarbonColumnVectorImpl) getDictionaryVector()).getDataArray();
    +    this.isLocalDict = true;
       }
    -
       @Override public void setBatchSize(int batchSize) {
    +
    --- End diff --
    
    remove empty space


---

[GitHub] carbondata issue #3055: [CARBONDATA-3237] Fix presto carbon issues in dictio...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/3055
  
    Build Success with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/10481/



---

[GitHub] carbondata issue #3055: [CARBONDATA-3237] Fix presto carbon issues in dictio...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #3055: [CARBONDATA-3237] Fix presto carbon issues in dictio...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #3055: [HOTFIX] presto can't read dictionary include decima...

Posted by ajantha-bhat <gi...@git.apache.org>.
Github user ajantha-bhat commented on the issue:

    https://github.com/apache/carbondata/pull/3055
  
    @ravipesala : please check


---

[GitHub] carbondata pull request #3055: [CARBONDATA-3237] Fix presto carbon issues in...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

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


---

[GitHub] carbondata issue #3055: [CARBONDATA-3237] Fix presto carbon issues in dictio...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/3055
  
    Build Failed  with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/10476/



---

[GitHub] carbondata issue #3055: [HOTFIX] presto can't read dictionary include decima...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/3055
  
    Build Success with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/10458/



---

[GitHub] carbondata issue #3055: [CARBONDATA-3237] Fix presto carbon issues in dictio...

Posted by ajantha-bhat <gi...@git.apache.org>.
Github user ajantha-bhat commented on the issue:

    https://github.com/apache/carbondata/pull/3055
  
    @ravipesala : please check


---

[GitHub] carbondata issue #3055: [HOTFIX] presto can't read dictionary include decima...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #3055: [CARBONDATA-3237] Fix presto carbon issues in dictio...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/3055
  
    Build Success with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/10495/



---

[GitHub] carbondata issue #3055: [CARBONDATA-3237] Fix presto carbon issues in dictio...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #3055: [CARBONDATA-3237] Fix presto carbon issues in dictio...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #3055: [CARBONDATA-3237] Fix presto carbon issues in dictio...

Posted by kumarvishal09 <gi...@git.apache.org>.
Github user kumarvishal09 commented on the issue:

    https://github.com/apache/carbondata/pull/3055
  
    LGTM


---

[GitHub] carbondata issue #3055: [HOTFIX] presto can't read dictionary include decima...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata pull request #3055: [CARBONDATA-3237] Fix presto carbon issues in...

Posted by ajantha-bhat <gi...@git.apache.org>.
Github user ajantha-bhat commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/3055#discussion_r246318548
  
    --- Diff: integration/presto/src/main/java/org/apache/carbondata/presto/readers/SliceStreamReader.java ---
    @@ -142,5 +144,17 @@ public SliceStreamReader(int batchSize, DataType dataType,
     
       @Override public void reset() {
         builder = type.createBlockBuilder(null, batchSize);
    +    this.isLocalDict = false;
    +  }
    +
    +  @Override public void putInt(int rowId, int value) {
    +    Object data = DataTypeUtil
    --- End diff --
    
    putInt() will not be called incase of local dictionary as setDictionary() itself is filling all the values array.  Hence no impact with change to local dictionary. 
    Also local dictionary UT are present and running fine after the changes


---