[ https://issues.apache.org/jira/browse/THRIFT-3650?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jens Geyer updated THRIFT-3650: ------------------------------- Description: With C++ the following {{union}} generates incorrect serialization code: {code} union type_ex { 1 : string foo 2 : i32 bar 3 : double baz } {code} This results in all three fields being *unconditionally* written, which not only wastes space on the wire, but also sets all {{__isset}} bits during deserialization. This makes it a bit hard to the other side to find out which {{union}} field is the right one. {code} uint32_t type_ex::write(::apache::thrift::protocol::TProtocol* oprot) const { uint32_t xfer = 0; apache::thrift::protocol::TOutputRecursionTracker tracker(*oprot); xfer += oprot->writeStructBegin("type_ex"); xfer += oprot->writeFieldBegin("STRING", ::apache::thrift::protocol::T_STRING, 1); xfer += oprot->writeString(this->STRING); xfer += oprot->writeFieldEnd(); xfer += oprot->writeFieldBegin("INT64", ::apache::thrift::protocol::T_I64, 2); xfer += oprot->writeI64(this->INT64); xfer += oprot->writeFieldEnd(); xfer += oprot->writeFieldBegin("DOUBLE", ::apache::thrift::protocol::T_DOUBLE, 3); xfer += oprot->writeDouble(this->DOUBLE); xfer += oprot->writeFieldEnd(); xfer += oprot->writeFieldStop(); xfer += oprot->writeStructEnd(); return xfer; } {code} *Note* that the code above is just right (due to "default" requiredness) if we talk about structs. But these rules do not really apply to union members where *by definition* only one value is used. Hence, union members implicitly have {{optional}} requiredness *Workaround* Add explicit {{optionals}}: {code} union type_ex { 1 : optional string foo 2 : optional i32 bar 3 : optional double baz } {code} was: With C++ the following {{union}} generates incorrect serialization code: {code} union type_ex { 1 : string foo 2 : i32 bar 3 : double baz } {code} This results in all three fields being *unconditionally* written, which not only wastes space on the wire, but also sets all {{__isset}} bits during deserialization. This makes it a bit hard to the other side to find out which {{union}} field is the right one. {code} uint32_t type_ex::write(::apache::thrift::protocol::TProtocol* oprot) const { uint32_t xfer = 0; apache::thrift::protocol::TOutputRecursionTracker tracker(*oprot); xfer += oprot->writeStructBegin("type_ex"); xfer += oprot->writeFieldBegin("STRING", ::apache::thrift::protocol::T_STRING, 1); xfer += oprot->writeString(this->STRING); xfer += oprot->writeFieldEnd(); xfer += oprot->writeFieldBegin("INT64", ::apache::thrift::protocol::T_I64, 2); xfer += oprot->writeI64(this->INT64); xfer += oprot->writeFieldEnd(); xfer += oprot->writeFieldBegin("DOUBLE", ::apache::thrift::protocol::T_DOUBLE, 3); xfer += oprot->writeDouble(this->DOUBLE); xfer += oprot->writeFieldEnd(); xfer += oprot->writeFieldStop(); xfer += oprot->writeStructEnd(); return xfer; } {code} *Note* that the code above is just right (due to "default" requiredness) if we talk about structs. But these rules do not really apply to union members where *by definition* only one value is used. Hence, union members implicitly have {{optional}} requiredness *Workaround* Add explicit {{optionals}}: {code} union type_ex { 1 : optional string STRING, 2 : optional i64 INT64, 3 : optional double DOUBLE } {code} > incorrect union serialization > ------------------------------ > > Key: THRIFT-3650 > URL: https://issues.apache.org/jira/browse/THRIFT-3650 > Project: Thrift > Issue Type: Bug > Components: C++ - Compiler > Affects Versions: 0.9.3 > Reporter: Jens Geyer > > With C++ the following {{union}} generates incorrect serialization code: > {code} > union type_ex { > 1 : string foo > 2 : i32 bar > 3 : double baz > } > {code} > This results in all three fields being *unconditionally* written, which not > only wastes space on the wire, but also sets all {{__isset}} bits during > deserialization. This makes it a bit hard to the other side to find out which > {{union}} field is the right one. > {code} > uint32_t type_ex::write(::apache::thrift::protocol::TProtocol* oprot) const { > uint32_t xfer = 0; > apache::thrift::protocol::TOutputRecursionTracker tracker(*oprot); > xfer += oprot->writeStructBegin("type_ex"); > xfer += oprot->writeFieldBegin("STRING", > ::apache::thrift::protocol::T_STRING, 1); > xfer += oprot->writeString(this->STRING); > xfer += oprot->writeFieldEnd(); > xfer += oprot->writeFieldBegin("INT64", ::apache::thrift::protocol::T_I64, > 2); > xfer += oprot->writeI64(this->INT64); > xfer += oprot->writeFieldEnd(); > xfer += oprot->writeFieldBegin("DOUBLE", > ::apache::thrift::protocol::T_DOUBLE, 3); > xfer += oprot->writeDouble(this->DOUBLE); > xfer += oprot->writeFieldEnd(); > xfer += oprot->writeFieldStop(); > xfer += oprot->writeStructEnd(); > return xfer; > } > {code} > *Note* that the code above is just right (due to "default" requiredness) if > we talk about structs. But these rules do not really apply to union members > where *by definition* only one value is used. Hence, union members implicitly > have {{optional}} requiredness > *Workaround* > Add explicit {{optionals}}: > {code} > union type_ex { > 1 : optional string foo > 2 : optional i32 bar > 3 : optional double baz > } > {code} -- This message was sent by Atlassian JIRA (v6.3.4#6332)