[ 
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)

Reply via email to