Pig
  1. Pig
  2. PIG-2732

Let's get rid of the deprecated Tuple methods

    Details

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

      Description

      In my favorite comment ever, this is the comment on isNull and setNull

          /** 
           * This is an unsupported method.
           * It never really worked. Don't use it!
           * @param isNull (but it will be ignored)
           */
      

      Simply delightful. Let's get rid of it!

      And let's @deprecate reference, which never really was used to begin with

      1. PIG-2732-0.patch
        4 kB
        Jonathan Coveney
      2. fix_CSVExcelStorage_0.patch
        0.9 kB
        Jonathan Coveney
      3. PIG-2732-fix-compiliation-errors.patch
        7 kB
        Jie Li

        Issue Links

          Activity

          Jonathan Coveney created issue -
          Jonathan Coveney made changes -
          Field Original Value New Value
          Assignee Jonathan Coveney [ jcoveney ]
          Jonathan Coveney made changes -
          Attachment PIG-2732-0.patch [ 12530284 ]
          Hide
          Jonathan Coveney added a comment -

          Here 'tis

          Show
          Jonathan Coveney added a comment - Here 'tis
          Jonathan Coveney made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          Julien Le Dem made changes -
          Link This issue is duplicated by PIG-2731 [ PIG-2731 ]
          Jonathan Coveney made changes -
          Link This issue duplicates PIG-2731 [ PIG-2731 ]
          Julien Le Dem made changes -
          Link This issue duplicates PIG-2731 [ PIG-2731 ]
          Hide
          Julien Le Dem added a comment -

          +1 it's been deprecated in the previous version of Pig. Nothing uses it inside Pig.
          Let's remove it.

          Show
          Julien Le Dem added a comment - +1 it's been deprecated in the previous version of Pig. Nothing uses it inside Pig. Let's remove it.
          Hide
          Jonathan Coveney added a comment -

          DONE! r1344897

          Show
          Jonathan Coveney added a comment - DONE! r1344897
          Jonathan Coveney made changes -
          Status Patch Available [ 10002 ] Resolved [ 5 ]
          Resolution Fixed [ 1 ]
          Hide
          Jie Li added a comment -

          contrib/piggybank/java/src/main/java/org/apache/pig/piggybank/storage/CSVExcelStorage.java is using isNull(), and there may be some other classes using it. We need to clean them as well, otherwise the e2e test doesn't compile.

          Show
          Jie Li added a comment - contrib/piggybank/java/src/main/java/org/apache/pig/piggybank/storage/CSVExcelStorage.java is using isNull(), and there may be some other classes using it. We need to clean them as well, otherwise the e2e test doesn't compile.
          Jie Li made changes -
          Resolution Fixed [ 1 ]
          Status Resolved [ 5 ] Reopened [ 4 ]
          Hide
          Jonathan Coveney added a comment -

          Jie, good call. I think that that particular use is just an overzealous use of the isNull method, as no other storage class uses or needs it.

          Which e2e tests did you see failing?

          Show
          Jonathan Coveney added a comment - Jie, good call. I think that that particular use is just an overzealous use of the isNull method, as no other storage class uses or needs it. Which e2e tests did you see failing?
          Hide
          Jonathan Coveney added a comment -

          I've attached a fix for CSVExcelStorage. It just involved getting rid of the call to isNull(), which is superfluous.

          Side note: I've noticed that the test for IndexedStorage has been failing in piggybank (independent of the Tuple changes). I will submit a fix separately.

          Show
          Jonathan Coveney added a comment - I've attached a fix for CSVExcelStorage. It just involved getting rid of the call to isNull(), which is superfluous. Side note: I've noticed that the test for IndexedStorage has been failing in piggybank (independent of the Tuple changes). I will submit a fix separately.
          Jonathan Coveney made changes -
          Attachment fix_CSVExcelStorage_0.patch [ 12530676 ]
          Hide
          Jie Li added a comment -

          Hi Jonathan, I've found some other places that involve this method. See attached patch.

          Show
          Jie Li added a comment - Hi Jonathan, I've found some other places that involve this method. See attached patch.
          Jie Li made changes -
          Status Reopened [ 4 ] Patch Available [ 10002 ]
          Jie Li made changes -
          Attachment PIG-2732-fix-compiliation-errors.patch [ 12530817 ]
          Hide
          Jonathan Coveney added a comment -

          Oh, right, Zebra. Good call. I always forget that it exists! +1, will commit shortly

          Show
          Jonathan Coveney added a comment - Oh, right, Zebra. Good call. I always forget that it exists! +1, will commit shortly
          Hide
          Daniel Dai added a comment -

          PIG-2732-fix-compiliation-errors.patch committed to trunk. Thanks Jie!

          Show
          Daniel Dai added a comment - PIG-2732 -fix-compiliation-errors.patch committed to trunk. Thanks Jie!
          Daniel Dai made changes -
          Status Patch Available [ 10002 ] Resolved [ 5 ]
          Hadoop Flags Reviewed [ 10343 ]
          Resolution Fixed [ 1 ]
          Bill Graham made changes -
          Status Resolved [ 5 ] Closed [ 6 ]

            People

            • Assignee:
              Jonathan Coveney
              Reporter:
              Jonathan Coveney
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development