[GitHub] carbondata pull request #1940: [CARBONDATA-2140 ] Refactoring code to improv...

[qiuchenjian-2]

GitHub user bhavya411 opened a pull request:

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

    [CARBONDATA-2140 ] Refactoring code to improve performance and removing unnecessary code in Presto Integration

    This PR is for optimizing Presto performance and refactoring the code to remove unnecessary classes and making it simpler.
   
    Be sure to do all of the following checklist to help us incorporate
    your contribution quickly and easily:
   
     - [X] Any interfaces changed?  No
     
     - [X] Any backward compatibility impacted?   No
     
     - [X] Documentation Remains Same
   
     - [X] Testing done
         - No new test cases needed as the functionality remain same
         - All existing test cases are passing.
         - Ran the TPCH queries to check the performance .
           
     - [X] 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/bhavya411/incubator-carbondata CARBONDATA-2140

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

    https://github.com/apache/carbondata/pull/1940.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 #1940
   
----
commit 533c1d2db0d4608081ed65bb98ea3bbe94e189ea
Author: Bhavya <bhavya@...>
Date:   2018-02-06T13:27:42Z

    Reafactored Code to remove Cursor and RecordSetProvider

----


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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



---

[qiuchenjian-2]

Github user bhavya411 commented on the issue:

    https://github.com/apache/carbondata/pull/1940
 
    retest sdv please.


---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user anubhav100 commented on the issue:

    https://github.com/apache/carbondata/pull/1940
 
    In this PR we have removed CarbonDataRecordset, CarbonDataRecordSetProvider, and CarbonDataRecordCursor classes because now our column vectors are filled in CarbonDataPageSource, so there is no need to iterate on the record using recordcursor


---

[qiuchenjian-2]

Github user bhavya411 commented on the issue:

    https://github.com/apache/carbondata/pull/1940
 
    This PR Removes the following classes from the PrestoCode
    -CarbondataRecordCursor   - This class used to get the records row by row from the iterator, we were using this just as a placeholder for values so this class was not needed.
    -CarbondataRecordSet  - This class was used to create the RecordCursor and since we were not using the Row by Row reader this class is also removed
    -CarbondataRecordSetProvider - All the functionality of this class has been modularized and move to CarbondataPageSourceProvider.
    - CarbondataUtil - This class was not used in the actual code.



---

[qiuchenjian-2]

Github user chenliang613 commented on the issue:

    https://github.com/apache/carbondata/pull/1940
 
    @bhavya411 @anubhav100  Thanks for your explanation.
    LGTM


---

[qiuchenjian-2]

Github user asfgit closed the pull request at:

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


---