Details

    • Type: Task
    • Status: Reopened
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: None
    • Labels:
    • old issue number:
      200

      Description

      I see a pig storage has been added it would be a great feature for a hive one as well.

      1. PHOENIX-331.patch
        183 kB
        nicolas maillard
      2. PHOENIX-331-1.patch
        154 kB
        Sergey Soldatov

        Activity

        Hide
        jamestaylor James Taylor added a comment -

        Duplicate of PHOENIX-946

        Show
        jamestaylor James Taylor added a comment - Duplicate of PHOENIX-946
        Hide
        nmaillard nicolas maillard added a comment -

        thanks james I had started working on a phoenix hive storage handler but the other threads seems like a more complicte discussion

        Show
        nmaillard nicolas maillard added a comment - thanks james I had started working on a phoenix hive storage handler but the other threads seems like a more complicte discussion
        Hide
        nmaillard nicolas maillard added a comment -

        Hi james
        With our last discussion I have started upo on this work again.
        I have a first working copy of the hivephoenix storage handler.
        I need to iron out predicate pushdowns and splits but the work is nearing the end

        Show
        nmaillard nicolas maillard added a comment - Hi james With our last discussion I have started upo on this work again. I have a first working copy of the hivephoenix storage handler. I need to iron out predicate pushdowns and splits but the work is nearing the end
        Hide
        nmaillard nicolas maillard added a comment -

        Hello james I took ages to get a first working copy, so sorry for the delay.
        I will propose a first version shortly could you assign me to this task.

        Show
        nmaillard nicolas maillard added a comment - Hello james I took ages to get a first working copy, so sorry for the delay. I will propose a first version shortly could you assign me to this task.
        Hide
        jamestaylor James Taylor added a comment -

        No problem, nicolas maillard. Take a look at ravi's latest pull for stuff you may be able to leverage: https://github.com/apache/phoenix/pull/22 or the Pig loader support we now have.

        Show
        jamestaylor James Taylor added a comment - No problem, nicolas maillard . Take a look at ravi 's latest pull for stuff you may be able to leverage: https://github.com/apache/phoenix/pull/22 or the Pig loader support we now have.
        Hide
        nmaillard nicolas maillard added a comment -

        Hello James
        I already did essentially to make sure we have a code consistency and reuse and also to make my life easier.
        I have the hive loader working to some extent but the predicate pushdown is limited and the split is suboptimal right now. So I will push a first version allowing to create and write in phoenix while I polish up the loader part, it would work but the performance would be terrible.

        Show
        nmaillard nicolas maillard added a comment - Hello James I already did essentially to make sure we have a code consistency and reuse and also to make my life easier. I have the hive loader working to some extent but the predicate pushdown is limited and the split is suboptimal right now. So I will push a first version allowing to create and write in phoenix while I polish up the loader part, it would work but the performance would be terrible.
        Hide
        nmaillard nicolas maillard added a comment -

        This issue had been closed duplicate, but inital work can be done here

        Show
        nmaillard nicolas maillard added a comment - This issue had been closed duplicate, but inital work can be done here
        Hide
        nmaillard nicolas maillard added a comment -

        Initial patch for this feature

        Show
        nmaillard nicolas maillard added a comment - Initial patch for this feature
        Hide
        nmaillard nicolas maillard added a comment -

        Here ta first patch for this feature a more in depth documentation can be found here
        https://github.com/nmaillard/Phoenix-Hive

        Show
        nmaillard nicolas maillard added a comment - Here ta first patch for this feature a more in depth documentation can be found here https://github.com/nmaillard/Phoenix-Hive
        Hide
        maghamravikiran@gmail.com maghamravikiran added a comment -

        Thanks nicolas maillard for the patch. As the patch includes a lot files, do you mind doing a Pull Request on git so we can have an effective discussion there. Sorry for the trouble.

        Show
        maghamravikiran@gmail.com maghamravikiran added a comment - Thanks nicolas maillard for the patch. As the patch includes a lot files, do you mind doing a Pull Request on git so we can have an effective discussion there. Sorry for the trouble.
        Hide
        jamestaylor James Taylor added a comment -

        Agree, ravi, a pull request would make it quite a bit easier to review. Would you mind doing that, nicolas maillard?

        Show
        jamestaylor James Taylor added a comment - Agree, ravi , a pull request would make it quite a bit easier to review. Would you mind doing that, nicolas maillard ?
        Hide
        nmaillard nicolas maillard added a comment -

        Yes, makes sense and I wanted to get a couple more UT in there. Sorry for the trouble, I'll do a pull request asap

        Show
        nmaillard nicolas maillard added a comment - Yes, makes sense and I wanted to get a couple more UT in there. Sorry for the trouble, I'll do a pull request asap
        Hide
        nmaillard nicolas maillard added a comment - - edited

        PR sent out, with a couple more UTs
        https://github.com/apache/phoenix/pull/74
        Hope this makes it easier to review

        Show
        nmaillard nicolas maillard added a comment - - edited PR sent out, with a couple more UTs https://github.com/apache/phoenix/pull/74 Hope this makes it easier to review
        Hide
        nmaillard nicolas maillard added a comment -

        Updated the pull request with initial feedback.
        Should I do second pull request or this update the way to go.
        Have not seen if this update has been seen

        Show
        nmaillard nicolas maillard added a comment - Updated the pull request with initial feedback. Should I do second pull request or this update the way to go. Have not seen if this update has been seen
        Hide
        devaraj Devaraj Das added a comment -

        nicolas maillard it'd be good to attach the patch here so that the precommit build can run on the patch and catch issues if any.

        Show
        devaraj Devaraj Das added a comment - nicolas maillard it'd be good to attach the patch here so that the precommit build can run on the patch and catch issues if any.
        Hide
        nmaillard nicolas maillard added a comment -

        Ok i'll attach a the patch with the updated PR as well.
        For the updated PR, let me know if I have addressed the more important issues and any feedback

        Show
        nmaillard nicolas maillard added a comment - Ok i'll attach a the patch with the updated PR as well. For the updated PR, let me know if I have addressed the more important issues and any feedback
        Hide
        jamestaylor James Taylor added a comment -

        Thanks for the revisions, nicolas maillard. I've left some feedback for you on the pull request.

        Show
        jamestaylor James Taylor added a comment - Thanks for the revisions, nicolas maillard . I've left some feedback for you on the pull request.
        Hide
        leDominik Dominik Wagenknecht added a comment -

        I'm so looking forward to the PhoenixStorageHandler – just awesome, no matter how bad the predicate push down may be initially. Just a question nicolas maillard: Is it really necessary to specify the primary-key fields, especially if it's an existing table

        Show
        leDominik Dominik Wagenknecht added a comment - I'm so looking forward to the PhoenixStorageHandler – just awesome, no matter how bad the predicate push down may be initially. Just a question nicolas maillard : Is it really necessary to specify the primary-key fields, especially if it's an existing table
        Hide
        sergey.soldatov Sergey Soldatov added a comment -

        Cleaned a bit the patch. Let's discuss what else we need to add to get it committed.

        Show
        sergey.soldatov Sergey Soldatov added a comment - Cleaned a bit the patch. Let's discuss what else we need to add to get it committed.
        Hide
        githubbot ASF GitHub Bot added a comment -

        GitHub user ss77892 opened a pull request:

        https://github.com/apache/phoenix/pull/150

        PHOENIX-331 impementation

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

        $ git pull https://github.com/ss77892/phoenix PHOENIX-331

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

        https://github.com/apache/phoenix/pull/150.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 #150


        commit 5545a6fdddbb12a621a1db1a4b5f2cebc37b9530
        Author: Sergey Soldatov <sergey.soldatov@gmail.com>
        Date: 2016-02-19T08:32:52Z

        Initial Patch


        Show
        githubbot ASF GitHub Bot added a comment - GitHub user ss77892 opened a pull request: https://github.com/apache/phoenix/pull/150 PHOENIX-331 impementation You can merge this pull request into a Git repository by running: $ git pull https://github.com/ss77892/phoenix PHOENIX-331 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/phoenix/pull/150.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 #150 commit 5545a6fdddbb12a621a1db1a4b5f2cebc37b9530 Author: Sergey Soldatov <sergey.soldatov@gmail.com> Date: 2016-02-19T08:32:52Z Initial Patch
        Hide
        warwithin YoungWoo Kim added a comment -

        Sergey Soldatov I'm interested in this. please point me out the quick start for installation and configuration. Thanks!

        Show
        warwithin YoungWoo Kim added a comment - Sergey Soldatov I'm interested in this. please point me out the quick start for installation and configuration. Thanks!
        Hide
        jamestaylor James Taylor added a comment - - edited

        Thanks for picking up this important work, Sergey Soldatov. There still appears to be a fair amount of copy/paste code from our MR integration. For example:

        • HivePhoenixInputFormat extends PhoenixInputFormat but seems to copy/paste much of the code. Is that necessary? Can some refactoring be done instead?
        • Same thing with HivePhoenixRecordReader which extends PhoenixRecordReader.

        ravi, [~ndimiduk, and Gabriel Reid - would be great if you could review.

        Show
        jamestaylor James Taylor added a comment - - edited Thanks for picking up this important work, Sergey Soldatov . There still appears to be a fair amount of copy/paste code from our MR integration. For example: HivePhoenixInputFormat extends PhoenixInputFormat but seems to copy/paste much of the code. Is that necessary? Can some refactoring be done instead? Same thing with HivePhoenixRecordReader which extends PhoenixRecordReader. ravi , [~ndimiduk, and Gabriel Reid - would be great if you could review.

          People

          • Assignee:
            Unassigned
            Reporter:
            nmaillard nicolas maillard
          • Votes:
            1 Vote for this issue
            Watchers:
            12 Start watching this issue

            Dates

            • Created:
              Updated:

              Development