Uploaded image for project: 'Flink'
  1. Flink
  2. FLINK-3866

StringArraySerializer claims type is immutable; shouldn't

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 1.0.3
    • Fix Version/s: 1.2.0, 1.1.2
    • Component/s: Core
    • Labels:
      None

      Description

      Looking at default `TypeSerializer` instances I noticed what looks like a minor flaw, unless I am missing something.
      Whereas all other array serializers indicate that type is not immutable (since in Java, arrays are not immutable), `StringArraySerializer` has:

      ```
      @Override
      public boolean isImmutableType()

      { return true; }

      ```

      and I think it should instead return `false`. I could create a PR, but seems like a small enough thing that issue report makes more sense.
      I tried looking for deps to see if there's a test for this, but couldn't find one; otherwise could submit a test fix.

        Issue Links

          Activity

          Hide
          ivan.mushketyk Ivan Mushketyk added a comment -

          I'll fix this.

          Show
          ivan.mushketyk Ivan Mushketyk added a comment - I'll fix this.
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user mushketyk opened a pull request:

          https://github.com/apache/flink/pull/2289

          FLINK-3866 StringArraySerializer type should be mutable

          Thanks for contributing to Apache Flink. Before you open your pull request, please take the following check list into consideration.
          If your changes take all of the items into account, feel free to open your pull request. For more information and/or questions please refer to the [How To Contribute guide](http://flink.apache.org/how-to-contribute.html).
          In addition to going through the list, please provide a meaningful description of your changes.

          • [x] General
          • The pull request references the related JIRA issue ("[FLINK-XXX] Jira title text")
          • The pull request addresses only one issue
          • Each commit in the PR has a meaningful commit message (including the JIRA id)
          • [x] Documentation
          • Documentation has been added for new functionality
          • Old documentation affected by the pull request has been updated
          • JavaDoc for public methods has been added
          • [x] Tests & Build
          • Functionality added by the pull request is covered by tests
          • `mvn clean verify` has been executed successfully locally or a Travis build has passed

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/mushketyk/flink mutable-array

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/flink/pull/2289.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #2289


          commit 4edb12cfc13c06ca1fb19dfb7042795cf883719c
          Author: Ivan Mushketyk <ivan.mushketik@gmail.com>
          Date: 2016-07-23T09:39:24Z

          FLINK-3866 StringArraySerializer type should be mutable


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user mushketyk opened a pull request: https://github.com/apache/flink/pull/2289 FLINK-3866 StringArraySerializer type should be mutable Thanks for contributing to Apache Flink. Before you open your pull request, please take the following check list into consideration. If your changes take all of the items into account, feel free to open your pull request. For more information and/or questions please refer to the [How To Contribute guide] ( http://flink.apache.org/how-to-contribute.html ). In addition to going through the list, please provide a meaningful description of your changes. [x] General The pull request references the related JIRA issue (" [FLINK-XXX] Jira title text") The pull request addresses only one issue Each commit in the PR has a meaningful commit message (including the JIRA id) [x] Documentation Documentation has been added for new functionality Old documentation affected by the pull request has been updated JavaDoc for public methods has been added [x] Tests & Build Functionality added by the pull request is covered by tests `mvn clean verify` has been executed successfully locally or a Travis build has passed You can merge this pull request into a Git repository by running: $ git pull https://github.com/mushketyk/flink mutable-array Alternatively you can review and apply these changes as the patch at: https://github.com/apache/flink/pull/2289.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #2289 commit 4edb12cfc13c06ca1fb19dfb7042795cf883719c Author: Ivan Mushketyk <ivan.mushketik@gmail.com> Date: 2016-07-23T09:39:24Z FLINK-3866 StringArraySerializer type should be mutable
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user mushketyk commented on the issue:

          https://github.com/apache/flink/pull/2289

          Hi, Could someone please review this small bug-fix?

          Show
          githubbot ASF GitHub Bot added a comment - Github user mushketyk commented on the issue: https://github.com/apache/flink/pull/2289 Hi, Could someone please review this small bug-fix?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user rmetzger commented on the issue:

          https://github.com/apache/flink/pull/2289

          Hi @mushketyk, I'll try to find somebody to review this

          Show
          githubbot ASF GitHub Bot added a comment - Github user rmetzger commented on the issue: https://github.com/apache/flink/pull/2289 Hi @mushketyk, I'll try to find somebody to review this
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user mushketyk commented on the issue:

          https://github.com/apache/flink/pull/2289

          Hi @rmetzger, Thank you for helping with this!

          Show
          githubbot ASF GitHub Bot added a comment - Github user mushketyk commented on the issue: https://github.com/apache/flink/pull/2289 Hi @rmetzger, Thank you for helping with this!
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user fhueske commented on the issue:

          https://github.com/apache/flink/pull/2289

          Thanks for the fix @mushketyk!

          Looks good to merge

          Show
          githubbot ASF GitHub Bot added a comment - Github user fhueske commented on the issue: https://github.com/apache/flink/pull/2289 Thanks for the fix @mushketyk! Looks good to merge
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user mushketyk commented on the issue:

          https://github.com/apache/flink/pull/2289

          @fhueske Thank you for your review!
          Could you please merge it? I don't have write permissions.

          Show
          githubbot ASF GitHub Bot added a comment - Github user mushketyk commented on the issue: https://github.com/apache/flink/pull/2289 @fhueske Thank you for your review! Could you please merge it? I don't have write permissions.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user fhueske commented on the issue:

          https://github.com/apache/flink/pull/2289

          Sorry for the delay @mushketyk!
          Will merge it tomorrow. Thanks, Fabian

          Show
          githubbot ASF GitHub Bot added a comment - Github user fhueske commented on the issue: https://github.com/apache/flink/pull/2289 Sorry for the delay @mushketyk! Will merge it tomorrow. Thanks, Fabian
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

          https://github.com/apache/flink/pull/2289

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/flink/pull/2289
          Hide
          fhueske Fabian Hueske added a comment -

          Fixed for 1.2.0 with 502ae585c13b67a2510ca27d5b02fb09f288e15b
          Fixed for 1.1.2 with e352ce4be8e8bc1cb75b40682aa8b9b4c99fb63e

          Thanks for the fix!

          Show
          fhueske Fabian Hueske added a comment - Fixed for 1.2.0 with 502ae585c13b67a2510ca27d5b02fb09f288e15b Fixed for 1.1.2 with e352ce4be8e8bc1cb75b40682aa8b9b4c99fb63e Thanks for the fix!
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user mushketyk commented on the issue:

          https://github.com/apache/flink/pull/2289

          Thank you for your help @fhueske!

          Show
          githubbot ASF GitHub Bot added a comment - Github user mushketyk commented on the issue: https://github.com/apache/flink/pull/2289 Thank you for your help @fhueske!

            People

            • Assignee:
              ivan.mushketyk Ivan Mushketyk
              Reporter:
              cowtowncoder Tatu Saloranta
            • Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development