Pig
  1. Pig
  2. PIG-2834

MultiStorage requires unused constructor argument

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Trivial Trivial
    • Resolution: Fixed
    • Affects Version/s: 0.10.0, 0.11
    • Fix Version/s: 0.14.0
    • Component/s: data
    • Labels:
    • Environment:

      Linux

    • Patch Info:
      Patch Available
    • Hadoop Flags:
      Reviewed
    • Release Note:
      MultiStorage no longer requires parentPathStr as at constructor argument.
    • Tags:
      MultiOutput

      Description

      each constructor in
      org.apache.pig.piggybank.storage.MultiStorage

      requires a constructor argument 'parentPathStr", that has no meaningful usage.

      1. PIG-2834-1.patch
        0.8 kB
        Daniel Dai
      2. MultiStorage.patch
        4 kB
        Danny Antonetti

        Activity

        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Open Open Patch Available Patch Available
        3m 3s 1 Danny Antonetti 22/Jul/12 08:49
        Patch Available Patch Available Open Open
        197d 9h 51m 1 Alan Gates 04/Feb/13 17:41
        Open Open Resolved Resolved
        616d 4h 21m 1 Daniel Dai 13/Oct/14 23:03
        Resolved Resolved Closed Closed
        38d 7h 55m 1 Daniel Dai 21/Nov/14 05:58
        Daniel Dai made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Daniel Dai made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Hadoop Flags Incompatible change [ 10342 ] Reviewed [ 10343 ]
        Assignee Danny Antonetti [ dannyant ]
        Resolution Fixed [ 1 ]
        Hide
        Daniel Dai added a comment -

        Patch committed to both 0.14 branch and trunk.

        Show
        Daniel Dai added a comment - Patch committed to both 0.14 branch and trunk.
        Daniel Dai made changes -
        Attachment PIG-2834-1.patch [ 12674615 ]
        Hide
        Daniel Dai added a comment -

        Since we cannot remove this parameter to break compatibility, we can only change the comments to document it.

        Show
        Daniel Dai added a comment - Since we cannot remove this parameter to break compatibility, we can only change the comments to document it.
        Hide
        Daniel Dai added a comment -

        I would say just change the parameter name to unused.

        Show
        Daniel Dai added a comment - I would say just change the parameter name to unused.
        Aniket Mokashi made changes -
        Fix Version/s 0.14.0 [ 12326954 ]
        Fix Version/s 0.13.0 [ 12324971 ]
        Daniel Dai made changes -
        Fix Version/s 0.13.0 [ 12324971 ]
        Fix Version/s 0.12.0 [ 12323380 ]
        Hide
        Danny Antonetti added a comment -

        I dont see how it is possible to do this, without breaking backwards compatibility.

        MultiStorage has 3 consturctors,

        public MultiStorage(String parentPathStr, String splitFieldIndex)
        public MultiStorage(String parentPathStr, String splitFieldIndex, String compression)
        public MultiStorage(String parentPathStr, String splitFieldIndex, String compression, String fieldDel)

        So I can not add a new constructor removing parentPathStr, because they already exist.

        Show
        Danny Antonetti added a comment - I dont see how it is possible to do this, without breaking backwards compatibility. MultiStorage has 3 consturctors, public MultiStorage(String parentPathStr, String splitFieldIndex) public MultiStorage(String parentPathStr, String splitFieldIndex, String compression) public MultiStorage(String parentPathStr, String splitFieldIndex, String compression, String fieldDel) So I can not add a new constructor removing parentPathStr, because they already exist.
        Alan Gates made changes -
        Status Patch Available [ 10002 ] Open [ 1 ]
        Hide
        Alan Gates added a comment -

        These changes break backward compatibility for users of MultiStorage. I agree the parentPathStr is unused and not required, but you need to deprecate the
        existing contructors without removing them and add new ones that don't take parentPathStr. This allows current users a path forward without breaking their
        code.

        Show
        Alan Gates added a comment - These changes break backward compatibility for users of MultiStorage. I agree the parentPathStr is unused and not required, but you need to deprecate the existing contructors without removing them and add new ones that don't take parentPathStr. This allows current users a path forward without breaking their code.
        Olga Natkovich made changes -
        Fix Version/s 0.12 [ 12323380 ]
        Fix Version/s 0.11 [ 12318878 ]
        Danny Antonetti made changes -
        Priority Minor [ 4 ] Trivial [ 5 ]
        Danny Antonetti made changes -
        Attachment MultiStorage.patch [ 12537482 ]
        Danny Antonetti made changes -
        Attachment MultiStorage.patch [ 12537481 ]
        Danny Antonetti made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Danny Antonetti made changes -
        Field Original Value New Value
        Attachment MultiStorage.patch [ 12537481 ]
        Hide
        Danny Antonetti added a comment -

        MultiStorage patch for removing an unused constructor argument.

        Show
        Danny Antonetti added a comment - MultiStorage patch for removing an unused constructor argument.
        Danny Antonetti created issue -

          People

          • Assignee:
            Danny Antonetti
            Reporter:
            Danny Antonetti
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development