Pig
  1. Pig
  2. PIG-3090

Introduce a syntax to be able to easily refer to the previously defined relation

    Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.12.0
    • Component/s: None
    • Labels:
      None

      Description

      Sometimes I feel like swimming with ANTLRs. This particular feature isn't too hard to add... and supports syntax like this:

      a = load 'thing' as (x:int);
      b = foreach @ generate x;
      c = foreach @ generate x;
      d = foreach @ generate x;
      

      I have a patch, though I need to make sure it doesn't change anything (it shouldn't) and I need to add tests.

      1. PIG-3090-1.patch
        52 kB
        Jonathan Coveney
      2. PIG-3090-0.patch
        45 kB
        Jonathan Coveney

        Issue Links

          Activity

          Daniel Dai made changes -
          Status Resolved [ 5 ] Closed [ 6 ]
          Jonathan Coveney made changes -
          Status Patch Available [ 10002 ] Resolved [ 5 ]
          Resolution Fixed [ 1 ]
          Jonathan Coveney made changes -
          Link This issue is depended upon by PIG-3129 [ PIG-3129 ]
          Hide
          Cheolsoo Park added a comment -

          +1.

          I am comfortable with opening a blocker jira for documentation.

          Show
          Cheolsoo Park added a comment - +1. I am comfortable with opening a blocker jira for documentation.
          Hide
          Jonathan Coveney added a comment -

          +1's welcome.

          Cheolsoo, are you comfortable with making a blocking documentation patch for this feature, or do you want it to be a part of this?

          Show
          Jonathan Coveney added a comment - +1's welcome. Cheolsoo, are you comfortable with making a blocking documentation patch for this feature, or do you want it to be a part of this?
          Hide
          Daniel Dai added a comment -

          I am fine to do it progressively. We can create another issue on top of this to further get rid of the relation name left to the equal sign.

          Show
          Daniel Dai added a comment - I am fine to do it progressively. We can create another issue on top of this to further get rid of the relation name left to the equal sign.
          Hide
          Cheolsoo Park added a comment -

          Hi Jonathan, I am fine as long as it's well-documented. Doing something special will make the implementation complicated.

          Show
          Cheolsoo Park added a comment - Hi Jonathan, I am fine as long as it's well-documented. Doing something special will make the implementation complicated.
          Hide
          Jonathan Coveney added a comment -

          Daniel,

          I think that that is a good idea, but should be a separate patch. It would not be hard to create a syntax which implicitly uses some reserved pig-only alias (____reserved or something), which => would implicitly become. If you make the ticket and assign it to me I will work on it. That said, I think this patch can and should be evaluated separately.

          Cheolsoo,

          I think that's a matter for documentation. Or do you think we should do something special with the @ inside of a nested foreach?

          Show
          Jonathan Coveney added a comment - Daniel, I think that that is a good idea, but should be a separate patch. It would not be hard to create a syntax which implicitly uses some reserved pig-only alias (____reserved or something), which => would implicitly become. If you make the ticket and assign it to me I will work on it. That said, I think this patch can and should be evaluated separately. Cheolsoo, I think that's a matter for documentation. Or do you think we should do something special with the @ inside of a nested foreach?
          Hide
          Daniel Dai added a comment -

          I do prefer a simplified syntax. My only concern is the relation name left to the equal sign is not used in most cases, which make the scriptlet look ugly. How about get rid of them as well? My preferred syntax is similar to the below:

          load 'thing' as (x:int);
          => foreach @ generate x;
          => foreach @ generate x;
          => foreach @ generate x;
          => d
          
          Show
          Daniel Dai added a comment - I do prefer a simplified syntax. My only concern is the relation name left to the equal sign is not used in most cases, which make the scriptlet look ugly. How about get rid of them as well? My preferred syntax is similar to the below: load 'thing' as (x: int ); => foreach @ generate x; => foreach @ generate x; => foreach @ generate x; => d
          Hide
          Cheolsoo Park added a comment -

          Hi Jonathan,

          This is very convenient. Thank you very much!

          My only concern is that people might try this inside a nested block. Since an alias inside a nested block is not a relation, it doesn't work. Nevertheless, it may cause confusion to some people.

          A = LOAD '1.txt';
          B = GROUP A ALL;
          C = FOREACH B {
              D = FILTER A BY ($0 > 1);
              E = FOREACH @ GENERATE $0; -- Can @ refer to D?
              GENERATE E;
          }
          DUMP C;
          

          Also we should document this if we're going to commit it.

          Show
          Cheolsoo Park added a comment - Hi Jonathan, This is very convenient. Thank you very much! My only concern is that people might try this inside a nested block. Since an alias inside a nested block is not a relation, it doesn't work. Nevertheless, it may cause confusion to some people. A = LOAD '1.txt'; B = GROUP A ALL; C = FOREACH B { D = FILTER A BY ($0 > 1); E = FOREACH @ GENERATE $0; -- Can @ refer to D? GENERATE E; } DUMP C; Also we should document this if we're going to commit it.
          Jonathan Coveney made changes -
          Attachment PIG-3090-1.patch [ 12565533 ]
          Hide
          Jonathan Coveney added a comment -

          Updated patch, and here is the RB: https://reviews.apache.org/r/9019/

          Show
          Jonathan Coveney added a comment - Updated patch, and here is the RB: https://reviews.apache.org/r/9019/
          Hide
          Jonathan Coveney added a comment -

          This works with grunt. I'm merging master and will put a review up shortly.

          Show
          Jonathan Coveney added a comment - This works with grunt. I'm merging master and will put a review up shortly.
          Hide
          Russell Jurney added a comment -

          But wait - does this work with grunt?

          Russell Jurney http://datasyndrome.com

          Show
          Russell Jurney added a comment - But wait - does this work with grunt? Russell Jurney http://datasyndrome.com
          Hide
          Russell Jurney added a comment -

          Does this work with grunt? Maybe I'll review it.

          Show
          Russell Jurney added a comment - Does this work with grunt? Maybe I'll review it.
          Jonathan Coveney made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          Fix Version/s 0.12 [ 12323380 ]
          Jonathan Coveney made changes -
          Assignee Jonathan Coveney [ jcoveney ]
          Hide
          Jonathan Coveney added a comment -

          Poke one of your colleagues into reviewing it

          Show
          Jonathan Coveney added a comment - Poke one of your colleagues into reviewing it
          Hide
          Russell Jurney added a comment -

          I like this.

          Show
          Russell Jurney added a comment - I like this.
          Jonathan Coveney made changes -
          Field Original Value New Value
          Attachment PIG-3090-0.patch [ 12560477 ]
          Hide
          Jonathan Coveney added a comment -

          This implements the proposed solution. Should make some scripts a lot less verbose.

          Show
          Jonathan Coveney added a comment - This implements the proposed solution. Should make some scripts a lot less verbose.
          Jonathan Coveney created issue -

            People

            • Assignee:
              Jonathan Coveney
              Reporter:
              Jonathan Coveney
            • Votes:
              1 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development