Pig
  1. Pig
  2. PIG-1967

deprecate current syntax for casting relation as scalar, to use explicit cast to tuple

    Details

    • Type: Improvement Improvement
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 0.8.0, 0.9.0
    • Fix Version/s: 0.14.0
    • Component/s: None
    • Labels:
      None

      Description

      When the feature added in PIG-1434, there was a proposal to cast it to tuple, to be able to use as a scalar. But for some reason, in the implementation this cast was not required.
      See -
      https://issues.apache.org/jira/browse/PIG-1434?focusedCommentId=12888449&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-12888449

      The current syntax which does not need this cast seems to lead to lot of confusion among users, who end up using this feature unintentionally. This usually happens because the user is referring to the bag column(s) in output of (co)group using a wrong name, which happens to be another relation. Often, users realize the mistake only at runtime. New users, will have trouble figuring out what was wrong.

      I think we should support the use of the cast as originally proposed, and deprecate the current syntax. The warning issued when the deprecated syntax is used is likely to help users realize that they have unintentionally used this feature.

      1. PIG-1967.0.patch
        7 kB
        Thejas M Nair

        Activity

        Hide
        Olga Natkovich added a comment -

        Delaying till 10 since we do not have an alternative in 0.9

        Show
        Olga Natkovich added a comment - Delaying till 10 since we do not have an alternative in 0.9
        Hide
        Thejas M Nair added a comment -

        Initial patch, tests need to be updated.

        Show
        Thejas M Nair added a comment - Initial patch, tests need to be updated.
        Hide
        Thejas M Nair added a comment -

        The syntax in the initial patch is confusing , it supports (tuple)A.$0 (where A is a relation. But that seems to imply the casting of first column of A as tuple. So the supported syntax should only be - ((tuple)A).$0 .

        Show
        Thejas M Nair added a comment - The syntax in the initial patch is confusing , it supports (tuple)A.$0 (where A is a relation. But that seems to imply the casting of first column of A as tuple. So the supported syntax should only be - ((tuple)A).$0 .
        Hide
        Dmitriy V. Ryaboy added a comment -

        Can I suggest a different syntax that would make it extra clear to the user that he is doing something unusual, such as

        ratios = foreach (group mytable by dimension) 
          generate group as dimension,
                   COUNT(mytable) / (scalar long) totals.num_rows;
        
        Show
        Dmitriy V. Ryaboy added a comment - Can I suggest a different syntax that would make it extra clear to the user that he is doing something unusual, such as ratios = foreach (group mytable by dimension) generate group as dimension, COUNT(mytable) / (scalar long ) totals.num_rows;
        Hide
        Thejas M Nair added a comment -

        I am for making that more clear. But "scalar long" sounds like a new type of long. Also, based on the current precedence of operators, i think it would be read as

        (scalar long) (totals.num_rows)

        How about using -

        ((scalar)totals).num_rows

        or

         ((scalar tuple)totals).num_rows

        .

        Show
        Thejas M Nair added a comment - I am for making that more clear. But "scalar long" sounds like a new type of long. Also, based on the current precedence of operators, i think it would be read as (scalar long ) (totals.num_rows) How about using - ((scalar)totals).num_rows or ((scalar tuple)totals).num_rows .
        Hide
        Julien Le Dem added a comment -

        This will go in a future release

        Show
        Julien Le Dem added a comment - This will go in a future release

          People

          • Assignee:
            Thejas M Nair
            Reporter:
            Thejas M Nair
          • Votes:
            1 Vote for this issue
            Watchers:
            7 Start watching this issue

            Dates

            • Created:
              Updated:

              Development