>> Still I wonder why it was designed this way. If I compare Thrift >> generated files with those created by Protobuf, it becomes immediately >> obvious how much smaller is the Thrift generated code. Maybe that is >> the reason why it was named "Thrift", because it is parsimonious! So >> maybe it was designed this way to keep the generated code to the bare >> minimum (for example, by not causing an exception on transmission if >> the field is not set, as that would require one check of variable >> every write).
Your technical analysis is correct, as well as the historical inference! The "required" and "optional" modifiers were not present in the first version of the IDL, and the original intent was to make using Thrift as lightweight and transparent as possible. There was an assumption that many clients would be written in scripting languages, where wrapping everything up in setters/getters would feel unnatural - we didn't want to make modifying fields require a function call (via a setter) or require the programmer to manually maintain all the isset flags. So all fields would be sent regardless of isset, with one important exception - languages with nil/null don't send empty fields. The required/optional keywords were added later to make it easier to automate some of this checking, and to make it possible for clients in null-less languages (i.e. C++) to explicitly omit optional fields. The downside to this approach is that it does make the default behavior more opaque. Hope that sheds some light on it. On 4/5/12 8:00 AM, "Piscium" <[email protected]> wrote: >On 5 April 2012 15:05, Rush Manbert <[email protected]> wrote: > >> Hi Piscium, >> >> I'm using an older version of Thrift, but my code is all C++. >> >> Try it with your Thrift IDL written like this and I think you'll see >>what I meant: >> >>> ================ >>> namespace cpp test >>> >>> struct Test { >>> 1: optional i32 npotatoes, // optional field because it has the >>>optional keyword >> 2: i32 nonions, // required field (but will be sent >>regardless of the __isset state) >>> } > >Thanks, Rush. You are right, an optional field is sent only if __isset is >set. > >But there is more. I realise now that there are actually three types >of fields: optional, required and "half-way" (when not explicitly >marked as either optional or required). "half-way" fields are a bit >like half-pregnant, somewhere in-between optional and required! > >A "half-way" field is like a required field in that it is always sent >on transmission, and it is like an optional field in that it does not >cause an exception if not present on reception. > >For obvious reasons a "half-way" field is not symmetrical. Moreover a >required field is not symmetrical either, because there is no >exception on transmission if it is not set. The apparent conclusion is >that the only type of field that is symmetrical is the optional. > >Still I wonder why it was designed this way. If I compare Thrift >generated files with those created by Protobuf, it becomes immediately >obvious how much smaller is the Thrift generated code. Maybe that is >the reason why it was named "Thrift", because it is parsimonious! So >maybe it was designed this way to keep the generated code to the bare >minimum (for example, by not causing an exception on transmission if >the field is not set, as that would require one check of variable >every write). > >Below is the modified Thrift file and generated code. > >========================= > >namespace cpp test > >struct CountrySoup { > 1: optional i32 npotatoes, > 2: i32 nonions, // "half-way" field - neither required nor optional > 3: required i32 cabbages, >} > >================== > > >namespace test { > >const char* CountrySoup::ascii_fingerprint = >"A5D9FD141E77B608AA21FD4BD8487AC1"; >const uint8_t CountrySoup::binary_fingerprint[16] = >{0xA5,0xD9,0xFD,0x14,0x1E,0x77,0xB6,0x08,0xAA,0x21,0xFD,0x4B,0xD8,0x48,0x7 >A,0xC1}; > >uint32_t CountrySoup::read(::apache::thrift::protocol::TProtocol* iprot) { > > uint32_t xfer = 0; > std::string fname; > ::apache::thrift::protocol::TType ftype; > int16_t fid; > > xfer += iprot->readStructBegin(fname); > > using ::apache::thrift::protocol::TProtocolException; > > bool isset_cabbages = false; > > while (true) > { > xfer += iprot->readFieldBegin(fname, ftype, fid); > if (ftype == ::apache::thrift::protocol::T_STOP) { > break; > } > switch (fid) > { > case 1: > if (ftype == ::apache::thrift::protocol::T_I32) { > xfer += iprot->readI32(this->npotatoes); > this->__isset.npotatoes = true; > } else { > xfer += iprot->skip(ftype); > } > break; > case 2: > if (ftype == ::apache::thrift::protocol::T_I32) { > xfer += iprot->readI32(this->nonions); > this->__isset.nonions = true; > } else { > xfer += iprot->skip(ftype); > } > break; > case 3: > if (ftype == ::apache::thrift::protocol::T_I32) { > xfer += iprot->readI32(this->cabbages); > isset_cabbages = true; > } else { > xfer += iprot->skip(ftype); > } > break; > default: > xfer += iprot->skip(ftype); > break; > } > xfer += iprot->readFieldEnd(); > } > > xfer += iprot->readStructEnd(); > > if (!isset_cabbages) > throw TProtocolException(TProtocolException::INVALID_DATA); > return xfer; >} > >// =========== > >uint32_t CountrySoup::write(::apache::thrift::protocol::TProtocol* >oprot) const { > uint32_t xfer = 0; > xfer += oprot->writeStructBegin("CountrySoup"); > if (this->__isset.npotatoes) { > xfer += oprot->writeFieldBegin("npotatoes", >::apache::thrift::protocol::T_I32, 1); > xfer += oprot->writeI32(this->npotatoes); > xfer += oprot->writeFieldEnd(); > } > xfer += oprot->writeFieldBegin("nonions", >::apache::thrift::protocol::T_I32, 2); > xfer += oprot->writeI32(this->nonions); > xfer += oprot->writeFieldEnd(); > xfer += oprot->writeFieldBegin("cabbages", >::apache::thrift::protocol::T_I32, 3); > xfer += oprot->writeI32(this->cabbages); > xfer += oprot->writeFieldEnd(); > xfer += oprot->writeFieldStop(); > xfer += oprot->writeStructEnd(); > return xfer; >} > >} // namespace
