Hi Henrique,

Since the i16 side wouldn't be able to read the i32's
that would came either as an input or output of your
function. Therefore, to make it work we would have
to be able to read larger numbers too, and cast them
down.

Agree. The size of the internal data field is the limit that counts, as the data have to be stored somewhere. So the last one would become then something like:

       case ::apache::thrift::protocol::T_I64:
           int64_t  tmp4712;
           xfer += iprot->readI16(tmp4712);
           if ( (tmp4712 < std::numeric_limits<int32_t>::min()) ||
                 (tmp4712 > std::numeric_limits<int32_t>::max())) {
             throw TProtocolException( TProtocolException::SIZE_LIMIT);
           }
           this->thing = (int32_t)tmp4712;
           this->__isset.thing = true;
           break;

All of this should be achieveable by means of a specialized protocol wrapper, so we don't even have to bloat (and modify) the generated code. Looking for a good name .... do you have one?

Jens


-----Ursprüngliche Nachricht----- From: Henrique Mendonça
Sent: Tuesday, March 26, 2013 11:03 PM
To: u...@thrift.apache.org ; dev@thrift.apache.org
Subject: Re: updating to longer ints

Hi Jens,

That's an interesting proposition, sorry for the delayed answer, but if I'm
not wrong with your example it would generate exceptions independently of
the side you update first.

Since the i16 side wouldn't be able to read the i32's that would came
either as an input or output of your function. Therefore, to make it work
we would have to be able to read larger numbers too, and cast them down.
This could be a little tricky for an user to understand the results, and
increase the complexity of the compiler quite a bit.

However, it's theoretically possible and I don't think it would affect the
performance that much.

- Henrique


On 15 March 2013 19:57, Jens Geyer <jensge...@hotmail.com> wrote:

I think TCompactProtocol uses var length ints, achieving
something similar to what you mentioned.


Yep, but good point anyway.


 Not that this would help w/ the original issue.


Of course, because it is only "something similar" and suffers from the
same limitations. What the OP suggests and/or what I was thinking of is
something like this (this would be generated C++ code for one field) :

     case 1:
       switch( ftype) {
       case ::apache::thrift::protocol::T_BYTE:
           int8_t   tmp4710;
           xfer += iprot->readByte(tmp4710);
           this->thing = tmp4710;
           this->__isset.thing = true;
           break;
       case ::apache::thrift::protocol::T_I16:
           int16_t  tmp4711;
           xfer += iprot->readI16(tmp4711);
           this->thing = tmp4711;
           this->__isset.thing = true;
           break;

       case ::apache::thrift::protocol::T_I32:
           xfer += iprot->readI32(this->thing);
           this->__isset.thing = true;
           break;
       case ::apache::thrift::protocol::T_I64:
           throw TProtocolException( TProtocolException::SIZE_LIMIT);
       else
         xfer += iprot->skip(ftype);
           break;
       }
       break;

and similarly with the generated write() code. This way all protocols
would profit form it (except for those where it makes no difference with
regard to serialized data, such as JSON).

The two biggest problem that I see are a) it breaks compatibility, so we
have to increase the VERSION number, and b) it might affect overall
serialization/deserialization performance.

Jens











Reply via email to