Pig
  1. Pig
  2. PIG-3138

Decouple PigServer.executeBatch() from compilation of batch

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.12.0
    • Component/s: None
    • Labels:
      None
    • Patch Info:
      Patch Available

      Description

      executeBatch() currently does parsing and building of LogicalPlan in addition to the actual execution. It will be beneficial to separate out parsing/building from execution - that will allow us to get a handle on load/store and other operators before execution of batch. Useful for folks using PigServer API.

      1. PIG-3138_hotfix.patch
        2 kB
        Cheolsoo Park
      2. PIG-3138_1.patch
        3 kB
        Prashant Kommireddi
      3. PIG-3138.patch
        3 kB
        Prashant Kommireddi

        Activity

        Hide
        Prashant Kommireddi added a comment -

        Adding a patch, please review.

        Show
        Prashant Kommireddi added a comment - Adding a patch, please review.
        Hide
        Prashant Kommireddi added a comment -

        Ping! Can a committer please take a look?

        Show
        Prashant Kommireddi added a comment - Ping! Can a committer please take a look?
        Hide
        Cheolsoo Park added a comment -

        Hi Prashant, LGTM. I have one question:

        This method should be followed by {@link PigServer#executeBatch(boolean)} with argument as true.

        In the comment of parseAndBuild(), shouldn't you say "with argument as false" instead of "with argument as true"?

        Show
        Cheolsoo Park added a comment - Hi Prashant, LGTM. I have one question: This method should be followed by {@link PigServer#executeBatch(boolean)} with argument as true. In the comment of parseAndBuild() , shouldn't you say "with argument as false " instead of "with argument as true "?
        Hide
        Prashant Kommireddi added a comment -

        Nice catch! I will update it. Thanks Cheolsoo, again.

        Show
        Prashant Kommireddi added a comment - Nice catch! I will update it. Thanks Cheolsoo, again.
        Hide
        Prashant Kommireddi added a comment -

        Cheolsoo Park corrected the comment in this new patch.

        Show
        Prashant Kommireddi added a comment - Cheolsoo Park corrected the comment in this new patch.
        Hide
        Cheolsoo Park added a comment -

        +1.

        Show
        Cheolsoo Park added a comment - +1.
        Hide
        Cheolsoo Park added a comment -

        Committed to trunk. Thanks Prashant!

        Show
        Cheolsoo Park added a comment - Committed to trunk. Thanks Prashant!
        Hide
        Cheolsoo Park added a comment -

        Prashant Kommireddi, unfortunately, this patch broke the following test case:

        • PigRunner.simpleTest
        Show
        Cheolsoo Park added a comment - Prashant Kommireddi , unfortunately, this patch broke the following test case: PigRunner.simpleTest
        Hide
        Cheolsoo Park added a comment -

        Attaching a hotfix.

        Show
        Cheolsoo Park added a comment - Attaching a hotfix.
        Hide
        Prashant Kommireddi added a comment -

        Thanks Cheolsoo, that looks good.

        Show
        Prashant Kommireddi added a comment - Thanks Cheolsoo, that looks good.
        Hide
        Prashant Kommireddi added a comment -

        Hey Cheolsoo Park, would this require a +1 from another commit to make it in?

        Show
        Prashant Kommireddi added a comment - Hey Cheolsoo Park , would this require a +1 from another commit to make it in?
        Hide
        Prashant Kommireddi added a comment -

        Typo - committer

        Show
        Prashant Kommireddi added a comment - Typo - committer
        Hide
        Cheolsoo Park added a comment -

        Prashant Kommireddi, yes, it does.

        Show
        Cheolsoo Park added a comment - Prashant Kommireddi , yes, it does.
        Hide
        Prashant Kommireddi added a comment -

        Hello, can a committer (other than Cheolsoo Park) please take a look at this?

        Show
        Prashant Kommireddi added a comment - Hello, can a committer (other than Cheolsoo Park ) please take a look at this?
        Hide
        Jonathan Coveney added a comment -

        +1, feel free to commit, Cheolsoo

        Show
        Jonathan Coveney added a comment - +1, feel free to commit, Cheolsoo
        Hide
        Cheolsoo Park added a comment -

        Committed hotfix to trunk.

        Thank you Jonathan for the review!

        Show
        Cheolsoo Park added a comment - Committed hotfix to trunk. Thank you Jonathan for the review!

          People

          • Assignee:
            Prashant Kommireddi
            Reporter:
            Prashant Kommireddi
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development