Avro
  1. Avro
  2. AVRO-720

Generated code for arrays should accept extensions of array content type

    Details

    • Type: Improvement Improvement
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: java
    • Labels:
      None

      Description

      Discovered when I upgraded a server from 1.3 to 1.4: trying to stuff a GenericData.Array<Utf8> object into a List<CharSequence> does not work; Avro should generate List<? implements CharSequence> instead. The same holds for other array types.

      1. AVRO-720.patch
        0.9 kB
        Doug Cutting

        Issue Links

          Activity

          Hide
          Jeff Hammerbacher added a comment -

          Fine by me. Add a line in the CHANGES.txt with a concrete example of the change and move on, I guess.

          Show
          Jeff Hammerbacher added a comment - Fine by me. Add a line in the CHANGES.txt with a concrete example of the change and move on, I guess.
          Doug Cutting made changes -
          Status Patch Available [ 10002 ] Open [ 1 ]
          Fix Version/s 1.5.0 [ 12315282 ]
          Hide
          Doug Cutting added a comment -

          I'd like to remove this from the 1.5.0 bug list. The messages in CHANGES.txt for 1.4's AVRO-637 and AVRO-605 are listed in the "incompatible" section. Combine those too, and it's already documented that types that were generated as 'GenericData.Array<Utf8>' will now be declared with 'List<CharSequence>'. Constructing a value that can be assigned to that can be done with 'new GenericData.Array<CharSequence>()' but no longer with 'new GenericData.Array<Utf8>()', due to the way Java generics work. Should we add that information to CHANGES.txt, should we add an FAQ or should we simply close this?

          Show
          Doug Cutting added a comment - I'd like to remove this from the 1.5.0 bug list. The messages in CHANGES.txt for 1.4's AVRO-637 and AVRO-605 are listed in the "incompatible" section. Combine those too, and it's already documented that types that were generated as 'GenericData.Array<Utf8>' will now be declared with 'List<CharSequence>'. Constructing a value that can be assigned to that can be done with 'new GenericData.Array<CharSequence>()' but no longer with 'new GenericData.Array<Utf8>()', due to the way Java generics work. Should we add that information to CHANGES.txt, should we add an FAQ or should we simply close this?
          Hide
          Jeff Hammerbacher added a comment -

          The workaround was not difficult, no. We should add some documentation someplace for those making the 1.3 -> 1.4 upgrade, but otherwise, I think we're good. I should not be the final arbiter of Java style, however.

          Show
          Jeff Hammerbacher added a comment - The workaround was not difficult, no. We should add some documentation someplace for those making the 1.3 -> 1.4 upgrade, but otherwise, I think we're good. I should not be the final arbiter of Java style, however.
          Hide
          Doug Cutting added a comment -

          If we change a generated field to be something like:

          java.util.List<? extends java.lang.CharSequence> strings;

          Then we can do things like:

          x.strings = new ArrayList<Utf8>();

          But we can no longer do things like:

          x.strings.add(new Utf8("foo"));

          Rather, this would have to become:

          ((List<Utf8)x.strings).add(new Utf8("foo"));

          The problem as I understand it is that List<? extends java.lang.CharSequence> declares all elements to be of some unknown type that implements CharSequence. Utf8 may or may not be that type, so we cannot directly insert without casting. So, on the whole, I think we're better of keeping the field declared with:

          java.util.List<java.lang.CharSequence> strings;

          This means we need to create instances with:

          x.strings = new ArrayList<CharSequence>();

          Which isn't really so bad, is it?

          Show
          Doug Cutting added a comment - If we change a generated field to be something like: java.util.List<? extends java.lang.CharSequence> strings; Then we can do things like: x.strings = new ArrayList<Utf8>(); But we can no longer do things like: x.strings.add(new Utf8("foo")); Rather, this would have to become: ((List<Utf8)x.strings).add(new Utf8("foo")); The problem as I understand it is that List<? extends java.lang.CharSequence> declares all elements to be of some unknown type that implements CharSequence. Utf8 may or may not be that type, so we cannot directly insert without casting. So, on the whole, I think we're better of keeping the field declared with: java.util.List<java.lang.CharSequence> strings; This means we need to create instances with: x.strings = new ArrayList<CharSequence>(); Which isn't really so bad, is it?
          Hide
          Jeff Hammerbacher added a comment -

          I get a build failure:

          [INFO] ------------------------------------------------------------------------
          [ERROR] BUILD FAILURE
          [INFO] ------------------------------------------------------------------------
          [INFO] Compilation failure

          /Users/hammer/codebox/avro-trunk/lang/java/ipc/src/main/java/org/apache/avro/ipc/trace/Util.java:[112,15] cannot find symbol
          symbol : method add(org.apache.avro.ipc.trace.TimestampedEvent)
          location: interface java.util.List<capture#611 of ? extends org.apache.avro.ipc.trace.TimestampedEvent>

          /Users/hammer/codebox/avro-trunk/lang/java/ipc/src/main/java/org/apache/avro/ipc/trace/SpanAggregator.java:[115,22] cannot find symbol
          symbol : method add(org.apache.avro.ipc.trace.TimestampedEvent)
          location: interface java.util.List<capture#551 of ? extends org.apache.avro.ipc.trace.TimestampedEvent>

          Show
          Jeff Hammerbacher added a comment - I get a build failure: [INFO] ------------------------------------------------------------------------ [ERROR] BUILD FAILURE [INFO] ------------------------------------------------------------------------ [INFO] Compilation failure /Users/hammer/codebox/avro-trunk/lang/java/ipc/src/main/java/org/apache/avro/ipc/trace/Util.java: [112,15] cannot find symbol symbol : method add(org.apache.avro.ipc.trace.TimestampedEvent) location: interface java.util.List<capture#611 of ? extends org.apache.avro.ipc.trace.TimestampedEvent> /Users/hammer/codebox/avro-trunk/lang/java/ipc/src/main/java/org/apache/avro/ipc/trace/SpanAggregator.java: [115,22] cannot find symbol symbol : method add(org.apache.avro.ipc.trace.TimestampedEvent) location: interface java.util.List<capture#551 of ? extends org.apache.avro.ipc.trace.TimestampedEvent>
          Doug Cutting made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          Assignee Doug Cutting [ cutting ]
          Fix Version/s 1.5.0 [ 12315282 ]
          Hide
          Doug Cutting added a comment -

          Probably we should add a test case.

          Show
          Doug Cutting added a comment - Probably we should add a test case.
          Doug Cutting made changes -
          Attachment AVRO-720.patch [ 12467138 ]
          Hide
          Doug Cutting added a comment -

          Jeff, does this do the trick for you?

          Show
          Doug Cutting added a comment - Jeff, does this do the trick for you?
          Jeff Hammerbacher made changes -
          Link This issue relates to HBASE-3393 [ HBASE-3393 ]
          Jeff Hammerbacher made changes -
          Link This issue is related to AVRO-605 [ AVRO-605 ]
          Hide
          Jeff Hammerbacher added a comment -

          Discovered this issue due to API changes introduced by AVRO-637 and AVRO-605

          Show
          Jeff Hammerbacher added a comment - Discovered this issue due to API changes introduced by AVRO-637 and AVRO-605
          Jeff Hammerbacher made changes -
          Field Original Value New Value
          Link This issue is related to AVRO-637 [ AVRO-637 ]
          Jeff Hammerbacher created issue -

            People

            • Assignee:
              Doug Cutting
              Reporter:
              Jeff Hammerbacher
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:

                Development