Avro
  1. Avro
  2. AVRO-1512

Error serializing TUnion with avro-thrift

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.7.6
    • Fix Version/s: 1.7.7
    • Component/s: java
    • Labels:
      None

      Description

      Attempting to serialize a Thrift union type (TUnion) with avro-thrift produces an error like the following:

      org.apache.avro.AvroRuntimeException: Unknown datum type org.apache.avro.thrift.test.FooOrBar: <FooOrBar foo:foo>
      	at org.apache.avro.generic.GenericData.getSchemaName(GenericData.java:639)
      	at org.apache.avro.generic.GenericData.resolveUnion(GenericData.java:604)
      	at org.apache.avro.generic.GenericDatumWriter.resolveUnion(GenericDatumWriter.java:151)
      	at org.apache.avro.generic.GenericDatumWriter.write(GenericDatumWriter.java:71)
      	at org.apache.avro.generic.GenericDatumWriter.writeField(GenericDatumWriter.java:114)
      	at org.apache.avro.generic.GenericDatumWriter.writeRecord(GenericDatumWriter.java:104)
      	at org.apache.avro.generic.GenericDatumWriter.write(GenericDatumWriter.java:66)
      	at org.apache.avro.generic.GenericDatumWriter.write(GenericDatumWriter.java:58)
      	at org.apache.avro.thrift.TestThrift.check(TestThrift.java:83)
      	at org.apache.avro.thrift.TestThrift.testStruct(TestThrift.java:60)
      

      Upon investigation it appears that ThriftData#isRecord intentionally does not accept TUnion types as Avro records and so GenericData is unable to determine an Avro type for the datum. However, ThriftData#getSchema does generate an Avro record schema for Thrift union types.

      1. AVRO-1512.patch
        19 kB
        Doug Cutting
      2. test.thrift
        1 kB
        Will Gorman

        Activity

        Hide
        Will Gorman added a comment -

        Attaching the Thrift IDL used to reproduce the issue. It was compiled with Thrift 0.7.0, matching the version of libthrift currently used by avro-thrift.

        Show
        Will Gorman added a comment - Attaching the Thrift IDL used to reproduce the issue. It was compiled with Thrift 0.7.0, matching the version of libthrift currently used by avro-thrift.
        Hide
        Doug Cutting added a comment -

        Avro doesn't yet support Thrift unions. Simply removing that check doesn't cause unions to work correctly: a union is treated as an Avro record with optional fields, and it fails to read when it tries to set the non-selected branches of the Thrift union to null.

        I can see a few options:

        • Change getSchema() to fail for Thrift unions.
        • Detect which Thrift unions can be implemented in Avro and implement those as Avro unions. The example you provide would translate to the Avro schema ["string", "string"], which is not legal and would result in an error, since Avro doesn't permit multiple union branches of the same type.
        • Implement Thrift unions as Avro records with optional fields. In ThriftData#setField(), if the value is null and the object is a TUnion, ignore it.
        Show
        Doug Cutting added a comment - Avro doesn't yet support Thrift unions. Simply removing that check doesn't cause unions to work correctly: a union is treated as an Avro record with optional fields, and it fails to read when it tries to set the non-selected branches of the Thrift union to null. I can see a few options: Change getSchema() to fail for Thrift unions. Detect which Thrift unions can be implemented in Avro and implement those as Avro unions. The example you provide would translate to the Avro schema ["string", "string"] , which is not legal and would result in an error, since Avro doesn't permit multiple union branches of the same type. Implement Thrift unions as Avro records with optional fields. In ThriftData#setField(), if the value is null and the object is a TUnion, ignore it.
        Hide
        Doug Cutting added a comment -

        Here's a patch that implements the third option mentioned above, where Thrift unions are implemented as Avro records.

        Show
        Doug Cutting added a comment - Here's a patch that implements the third option mentioned above, where Thrift unions are implemented as Avro records.
        Hide
        Will Gorman added a comment -

        The third option was also the approach I had tried as a temporary workaround. The only problem I can see is if Java classes were compiled by Avro from the schema obtained from the Thrift class. In that case the record created from the Thrift union would allow multiple fields to be set and data serialized from the Avro class couldn't always be deserialized back into the Thrift class. I think that still feels preferable to only supporting unions that don't have multiple branches of the same type though.

        Show
        Will Gorman added a comment - The third option was also the approach I had tried as a temporary workaround. The only problem I can see is if Java classes were compiled by Avro from the schema obtained from the Thrift class. In that case the record created from the Thrift union would allow multiple fields to be set and data serialized from the Avro class couldn't always be deserialized back into the Thrift class. I think that still feels preferable to only supporting unions that don't have multiple branches of the same type though.
        Hide
        ASF subversion and git services added a comment -

        Commit 1598159 from Doug Cutting in branch 'avro/trunk'
        [ https://svn.apache.org/r1598159 ]

        AVRO-1512. Java: Support Thrift unions.

        Show
        ASF subversion and git services added a comment - Commit 1598159 from Doug Cutting in branch 'avro/trunk' [ https://svn.apache.org/r1598159 ] AVRO-1512 . Java: Support Thrift unions.
        Hide
        Doug Cutting added a comment -

        I committed this.

        Show
        Doug Cutting added a comment - I committed this.
        Hide
        Hudson added a comment -

        FAILURE: Integrated in AvroJava #453 (See https://builds.apache.org/job/AvroJava/453/)
        AVRO-1512. Java: Support Thrift unions. (cutting: rev 1598159)

        • /avro/trunk/CHANGES.txt
        • /avro/trunk/lang/java/thrift/src/main/java/org/apache/avro/thrift/ThriftData.java
        • /avro/trunk/lang/java/thrift/src/test/java/org/apache/avro/thrift/TestThrift.java
        • /avro/trunk/lang/java/thrift/src/test/java/org/apache/avro/thrift/test/FooOrBar.java
        • /avro/trunk/lang/java/thrift/src/test/java/org/apache/avro/thrift/test/Test.java
        • /avro/trunk/lang/java/thrift/src/test/thrift/test.thrift
        Show
        Hudson added a comment - FAILURE: Integrated in AvroJava #453 (See https://builds.apache.org/job/AvroJava/453/ ) AVRO-1512 . Java: Support Thrift unions. (cutting: rev 1598159) /avro/trunk/CHANGES.txt /avro/trunk/lang/java/thrift/src/main/java/org/apache/avro/thrift/ThriftData.java /avro/trunk/lang/java/thrift/src/test/java/org/apache/avro/thrift/TestThrift.java /avro/trunk/lang/java/thrift/src/test/java/org/apache/avro/thrift/test/FooOrBar.java /avro/trunk/lang/java/thrift/src/test/java/org/apache/avro/thrift/test/Test.java /avro/trunk/lang/java/thrift/src/test/thrift/test.thrift

          People

          • Assignee:
            Doug Cutting
            Reporter:
            Will Gorman
          • Votes:
            1 Vote for this issue
            Watchers:
            6 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development