[GitHub] carbondata pull request #2287: [CARBONDATA-2418] [Presto] [S3] Fixed Presto ...

[qiuchenjian-2]

GitHub user anubhav100 opened a pull request:

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

    [CARBONDATA-2418] [Presto] [S3] Fixed Presto Can't Query CarbonData When CarbonStore is at S3

    Be sure to do all of the following checklist to help us incorporate
    your contribution quickly and easily:
   
     - [ ] Any interfaces changed?
     
     - [ ] Any backward compatibility impacted?
     
     - [ ] Document update required?
   
     - [ ] Testing done
            Please provide details on
            - Whether new unit test cases have been added or why no new tests are required?
            - How it is tested? Please attach test report.
            - Is it a performance related change? Please attach the performance test report.
            - Any additional information to help reviewers in testing this change.
           
     - [ ] 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/anubhav100/incubator-carbondata presto/s3

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

    https://github.com/apache/carbondata/pull/2287.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 #2287
   
----
commit 9706f59df0f27769c6d3416d2daa4bb2d572e134
Author: anubhav100 <anubhav.tarar@...>
Date:   2018-05-01T04:29:05Z

    Fixed Presto Can't Query CarbonData When CarbonStore is at s3

----


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2287
 
    Build Failed with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/4608/



---

[qiuchenjian-2]

Github user anubhav100 commented on the issue:

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


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/2287
 
    SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/4826/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2287#discussion_r187768917
 
    --- Diff: integration/presto/README.md ---
    @@ -82,6 +82,29 @@ Please follow the below steps to query carbondata in presto
       For example, if you have a schema named 'default' stored in hdfs://namenode:9000/test/carbondata/,
       Then set carbondata-store=hdfs://namenode:9000/test/carbondata
       
    +#### Connecting to carbondata store on s3
    + * In case you want to query carbonstore on S3 using S3A api put following additional properties inside $PRESTO_HOME$/etc/catalog/carbondata.properties
    +   ```
    +    fs.s3a.access.key={value}
    +    fs.s3a.secret.key={value}
    +    Optional: fs.s3a.endpoint={value}
    --- End diff --
   
    better to describe the properties as below to make it convenient for user to copy&paste.
    ```
    # Required properties
    A=B
    # Optional properties
    C=D
    ```


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2287
 
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/5918/



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/2287
 
    SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/4949/



---

[qiuchenjian-2]

Github user anubhav100 commented on the issue:

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


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2287#discussion_r190794987
 
    --- Diff: integration/presto/README.md ---
    @@ -82,6 +82,29 @@ Please follow the below steps to query carbondata in presto
       For example, if you have a schema named 'default' stored in hdfs://namenode:9000/test/carbondata/,
       Then set carbondata-store=hdfs://namenode:9000/test/carbondata
       
    +#### Connecting to carbondata store on s3
    + * In case you want to query carbonstore on S3 using S3A api put following additional properties inside $PRESTO_HOME$/etc/catalog/carbondata.properties
    +   ```
    +    fs.s3a.access.key={value}
    +    fs.s3a.secret.key={value}
    +    Optional: fs.s3a.endpoint={value}
    --- End diff --
   
    done


---

[qiuchenjian-2]

Github user jackylk commented on the issue:

    https://github.com/apache/carbondata/pull/2287
 
    please remove datamap.mdtfile file in the commit
    @anubhav100


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2287#discussion_r191330841
 
    --- Diff: integration/presto/src/main/java/org/apache/carbondata/presto/impl/CarbonTableConfig.java ---
    @@ -51,7 +59,8 @@
         return this;
       }
     
    -  @NotNull public String getStorePath() {
    +  @NotNull public String
    +  getStorePath() {
    --- End diff --
   
    no need to change


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2287#discussion_r191330878
 
    --- Diff: integration/presto/src/main/java/org/apache/carbondata/presto/impl/CarbonTableConfig.java ---
    @@ -60,6 +69,7 @@
         return this;
       }
     
    +
    --- End diff --
   
    remove empty line


---

[qiuchenjian-2]

Github user anubhav100 commented on the issue:

    https://github.com/apache/carbondata/pull/2287
 
    @jackylk i have done changes please review


---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/2287
 
    SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/5156/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---