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 2018/08/23 10:17:53 UTC

[GitHub] carbondata pull request #2651: [HOTFIX] Support TableProperties Map API for ...

GitHub user ajantha-bhat opened a pull request:

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

    [HOTFIX] Support TableProperties Map API for SDK

    Currently SDK supports load options as map input. But table properties is not a map. 
    So this PR supports a API that can take already supported table properties as map. 
    
    This is will help for easy configuration for end user of SDK.
    
    your contribution quickly and easily:
    
     - [ ] Any interfaces changed? NA. Added new interface.
     
     - [ ] Any backward compatibility impacted? NA
     
     - [ ] Document update required? yes, updated.
    
     - [ ] 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 unmanaged_table

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

    https://github.com/apache/carbondata/pull/2651.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 #2651
    
----
commit 5671a9ff55bb23ce678319da342245ec527b0386
Author: ajantha-bhat <aj...@...>
Date:   2018-08-23T10:09:29Z

    Support TablePropertiesMap API for SDK

----


---

[GitHub] carbondata issue #2651: [HOTFIX] Support TableProperties Map API for SDK

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

    https://github.com/apache/carbondata/pull/2651
  
    Build Success with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/6763/



---

[GitHub] carbondata issue #2651: [HOTFIX] Support TableProperties Map API for SDK

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

    https://github.com/apache/carbondata/pull/2651
  
    SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/6343/



---

[GitHub] carbondata pull request #2651: [HOTFIX] Support TableProperties Map API for ...

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

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


---

[GitHub] carbondata issue #2651: [HOTFIX] Support TableProperties Map API for SDK

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

    https://github.com/apache/carbondata/pull/2651
  
    Build Success with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/6716/



---

[GitHub] carbondata issue #2651: [HOTFIX] Support TableProperties Map API for SDK

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

    https://github.com/apache/carbondata/pull/2651
  
    SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/6387/



---

[GitHub] carbondata issue #2651: [HOTFIX] Support TableProperties Map API for SDK

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

    https://github.com/apache/carbondata/pull/2651
  
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/8041/



---

[GitHub] carbondata issue #2651: [HOTFIX] Support TableProperties Map API for SDK

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

    https://github.com/apache/carbondata/pull/2651
  
    LGTM...can be merged once build is passed


---

[GitHub] carbondata issue #2651: [HOTFIX] Support TableProperties Map API for SDK

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

    https://github.com/apache/carbondata/pull/2651
  
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/7992/



---

[GitHub] carbondata pull request #2651: [HOTFIX] Support TableProperties Map API for ...

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/2651#discussion_r212589476
  
    --- Diff: store/sdk/src/main/java/org/apache/carbondata/sdk/file/CarbonWriterBuilder.java ---
    @@ -272,6 +272,56 @@ public CarbonWriterBuilder withLoadOptions(Map<String, String> options) {
         return this;
       }
     
    +  /**
    +   * To support the table properties for sdk writer
    +   *
    +   * @param options key,value pair of create table properties.
    +   * supported keys values are
    +   * a. blocksize -- [1-2048] values in MB. Default value is 1024
    +   * b. blockletsize -- values in MB. Default value is 64 MB
    +   * c. localDictionaryThreshold -- positive value, default is 10000
    +   * d. enableLocalDictionary -- true / false. Default is false
    +   * e. sortcolumns -- comma separated column. "c1,c2". Default all dimensions are sorted.
    +   *
    +   * @return updated CarbonWriterBuilder
    +   */
    +  public CarbonWriterBuilder withTableProperties(Map<String, String> options) {
    +    Objects.requireNonNull(options, "Table properties should not be null");
    +    //validate the options.
    +    if (options.size() > 5) {
    +      throw new IllegalArgumentException("Supports only 5 options now. "
    +          + "Refer method header or documentation");
    +    }
    +
    +    for (String option: options.keySet()) {
    +      if (!option.equalsIgnoreCase("blocksize") &&
    +          !option.equalsIgnoreCase("blockletsize") &&
    +          !option.equalsIgnoreCase("localDictionaryThreshold") &&
    +          !option.equalsIgnoreCase("enableLocalDictionary") &&
    +          !option.equalsIgnoreCase("sortcolumns")) {
    +        throw new IllegalArgumentException("Unsupported options. "
    --- End diff --
    
    ok. modified.


---

[GitHub] carbondata pull request #2651: [HOTFIX] Support TableProperties Map API for ...

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

    https://github.com/apache/carbondata/pull/2651#discussion_r212582796
  
    --- Diff: store/sdk/src/main/java/org/apache/carbondata/sdk/file/CarbonWriterBuilder.java ---
    @@ -272,6 +272,56 @@ public CarbonWriterBuilder withLoadOptions(Map<String, String> options) {
         return this;
       }
     
    +  /**
    +   * To support the table properties for sdk writer
    +   *
    +   * @param options key,value pair of create table properties.
    +   * supported keys values are
    +   * a. blocksize -- [1-2048] values in MB. Default value is 1024
    +   * b. blockletsize -- values in MB. Default value is 64 MB
    +   * c. localDictionaryThreshold -- positive value, default is 10000
    +   * d. enableLocalDictionary -- true / false. Default is false
    +   * e. sortcolumns -- comma separated column. "c1,c2". Default all dimensions are sorted.
    +   *
    +   * @return updated CarbonWriterBuilder
    +   */
    +  public CarbonWriterBuilder withTableProperties(Map<String, String> options) {
    +    Objects.requireNonNull(options, "Table properties should not be null");
    +    //validate the options.
    +    if (options.size() > 5) {
    +      throw new IllegalArgumentException("Supports only 5 options now. "
    +          + "Refer method header or documentation");
    +    }
    +
    +    for (String option: options.keySet()) {
    +      if (!option.equalsIgnoreCase("blocksize") &&
    +          !option.equalsIgnoreCase("blockletsize") &&
    +          !option.equalsIgnoreCase("localDictionaryThreshold") &&
    +          !option.equalsIgnoreCase("enableLocalDictionary") &&
    +          !option.equalsIgnoreCase("sortcolumns")) {
    +        throw new IllegalArgumentException("Unsupported options. "
    --- End diff --
    
    Better to put all the allowed options in a set and then each property u can check using Set.Contains API which will be faster and a cleaner code


---