Pig
  1. Pig
  2. PIG-2834

MultiStorage requires unused constructor argument

    Details

    • Type: Improvement Improvement
    • Status: Open
    • Priority: Trivial Trivial
    • Resolution: Unresolved
    • Affects Version/s: 0.10.0, 0.11
    • Fix Version/s: 0.13.0
    • Component/s: data
    • Labels:
    • Environment:

      Linux

    • Patch Info:
      Patch Available
    • Hadoop Flags:
      Incompatible change
    • 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. MultiStorage.patch
        4 kB
        Danny Antonetti

        Activity

        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.
        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.
        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.

          People

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

            Dates

            • Created:
              Updated:

              Development