[GitHub] carbondata pull request #2705: [CARBONDATA-2926] fixed ArrayIndexOutOfBoundE...

[qiuchenjian-2]

GitHub user ajantha-bhat opened a pull request:

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

    [CARBONDATA-2926] fixed ArrayIndexOutOfBoundException with varchar columns and empty sort_columns

    problem:
    ArrayIndexOutOfBoundException if varchar column is present before dictionary columns along with empty sort_columns.
   
    root cause:
    CarbonFactDataHandlerColumnar.isVarcharColumnFull() method uses model.getVarcharDimIdxInNoDict()
    and index of varchar column in no dictonary array became negative.
    currently index was calculated based on ordinal-number of dictionary columns. This can go negative in no_sort column case,
   
    solution:
    **take the varchar dimension index from no dictionary array from at runtime based on schema.**
   
    **Also hotfix: removed number validation in table properties of sdk, now we support 7 properties**
   
    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
    updated UT      
     - [ ] 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 master

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

    https://github.com/apache/carbondata/pull/2705.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 #2705
   
----
commit 38b6e13d5e1b0362756301383f184505c615edd0
Author: ajantha-bhat <ajanthabhat@...>
Date:   2018-09-10T17:38:15Z

    [CARBONDATA-2926] fixed ArrayIndexOutOfBoundException if varchar column is present before dictionary columns along with empty sort_columns.

----


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2705
 
    Build Failed  with Spark 2.3.1, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.3/8451/



---

[qiuchenjian-2]

Github user ajantha-bhat commented on the issue:

    https://github.com/apache/carbondata/pull/2705
 
    @ravipesala : please review


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2705#discussion_r216620144
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/datastore/TableSpec.java ---
    @@ -36,6 +37,14 @@
       private DimensionSpec[] dimensionSpec;
       private MeasureSpec[] measureSpec;
     
    +  // Many places we might have to access no-dictionary column spec.
    +  // but no-dictionary column spec are not always in below order like,
    +  // dictionary + no dictionary + complex + measure
    +  // when sort_columns are empty, no columns are selected for sorting.
    +  // so, spec will not be in above order.
    +  // Hence NoDictionaryDimensionSpec will be useful and it will be subset of dimensionSpec.
    +  private List<DimensionSpec> NoDictionaryDimensionSpec;
    --- End diff --
   
    Better change name to `noDictionaryDimensionSpec`


---

[qiuchenjian-2]

Github user ajantha-bhat commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2705#discussion_r216634097
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/datastore/TableSpec.java ---
    @@ -36,6 +37,14 @@
       private DimensionSpec[] dimensionSpec;
       private MeasureSpec[] measureSpec;
     
    +  // Many places we might have to access no-dictionary column spec.
    +  // but no-dictionary column spec are not always in below order like,
    +  // dictionary + no dictionary + complex + measure
    +  // when sort_columns are empty, no columns are selected for sorting.
    +  // so, spec will not be in above order.
    +  // Hence NoDictionaryDimensionSpec will be useful and it will be subset of dimensionSpec.
    +  private List<DimensionSpec> NoDictionaryDimensionSpec;
    --- End diff --
   
    done.


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2705
 
    Build Failed  with Spark 2.3.1, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.3/8471/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2705
 
    Build Failed  with Spark 2.3.1, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.3/8498/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user asfgit closed the pull request at:

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


---