[jira] [Comment Edited] (THRIFT-2429) Provide option to not write default values, rely on receiver default construction instead

["Randy Abernethy (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/THRIFT-2429?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13957674#comment-13957674 ] 

Randy Abernethy edited comment on THRIFT-2429 at 4/5/14 3:00 PM:
-----------------------------------------------------------------

Hey Chris,

I think your suggestion is a good compromise: 

bq. whether __set or __unset an optional/default field holding the default value on the serializing side will never be written to the stream

To make sure we are all clear on the trades we will make to adopt this stance, I note the following implied considerations:

*Given that the fields in question will be optional, setters and getters will need to be used*

Non optional fields of a struct are usually set directly like this:

bq. {{myObj.x = 5;}}

as opposed to:

bq. {{myObj.__set_x(5);}}

The later is required to set an optional field in C++ (and many other languages) so any automatic data binding or other users of optional/default fields would need to be modified to use the setter. 

*The default value can not be serialized, period*

The ability lost here is that if you want to serialize the default value for readers which have a version of the IDL without a default (or those with an out of date default), you can not. For example:
{code:title=OLD|borderStyle=solid}
struct test {
   1: optional i32 x = 5
}
{code}
A program serializing x with IDL OLD cannot serialize the value 5. 
{code:title=NEW|borderStyle=solid}
struct test {
   1: optional i32 x = 10
}
{code}
Therefore the reader deserializing with IDL NEW sets x to 10, creating a disparity between the reader and the writer. 

*Objects serialized to disk may have a long shelf life and lose default information over time due to interface evolution*

This is really the same as the prior point but more acute in scenarios where people are serializing to disk for permanent object storage. Since this logic takes place at the serialization level (Protocol level) we are impacting long living objects serialized to disk as well as RPC calls. Structs serialized to disk via Thrift are more likely to face interface evolution over the long haul, and possibly changing defaults. We are in essence saying:

bq. Once defined at the interface level, a default value for a field should never change

Making defaults much like the field ordinals. Upon reflection this is probably a reasonable bit of guidance for Thrift interface design across the board.

----

Let me know what you think about these caveats. I am pretty upbeat. This will be a great core feature to add to Thrift. It will of course impact every language which is why I want to make sure that we are really clear on what we are doing and what it means to all of the stake holders.

Very best,
Randy


was (Author: codesf):
Hey Chris,

I think your suggestion is a good compromise: 

bq. whether __set or __unset an optional/default field holding the default value on the serializing side will never be written to the stream

To make sure we are all clear on the trades we will make to adopt this stance, I note the following implied considerations:

*Given that the fields in question will be optional, setters and getters will need to be used*

Non optional fields of a struct are usually set directly like this:

bq. {{myObj.x = 5;}}

as opposed to:

bq. {{myObj.__set_x(5);}}

The later is required to set an optional field in C++ (and many other languages) so any automatic data binding or other users of optional/default fields would need to be modified to use the setter. 

*The default value can not be serialized, period*

The ability lost here is that if you want to serialize the default value for readers which have a version of the IDL without a default (or those with an out of date default), you can not. For example:
{code:title=OLD|borderStyle=solid}
struct test {
   1: optional i32 x = 5
}
{code}
A program serializing x with IDL OLD cannot serialize the value 5. 
{code:title=NEW|borderStyle=solid}
struct test {
   1: optional i32 x = 10
}
{code}
Therefore the reader deserializing with IDL NEW sets x to 10, creating a disparity between the reader and the writer. 

*Objects serialized to disk may have a long shelf life and loose default information over time due to interface evolution*

This is really the same as the prior point but more acute in scenarios where people are serializing to disk for permanent object storage. Since this logic takes place at the serialization level (Protocol level) we are impacting long living objects serialized to disk as well as RPC calls. Structs serialized to disk via Thrift are more likely to face interface evolution over the long haul, and possibly changing defaults. We are in essence saying:

bq. Once defined at the interface level, a default value for a field should never change

Making defaults much like the field ordinals. Upon reflection this is probably a reasonable bit of guidance for Thrift interface design across the board.

----

Let me know what you think about these caveats. I am pretty upbeat. This will be a great core feature to add to Thrift. It will of course impact every language which is why I want to make sure that we are really clear on what we are doing and what it means to all of the stake holders.

Very best,
Randy

> Provide option to not write default values, rely on receiver default construction instead
> -----------------------------------------------------------------------------------------
>
>                 Key: THRIFT-2429
>                 URL: https://issues.apache.org/jira/browse/THRIFT-2429
>             Project: Thrift
>          Issue Type: Improvement
>          Components: C++ - Compiler
>    Affects Versions: 0.9.1
>            Reporter: Chris Stylianou
>            Assignee: Randy Abernethy
>              Labels: default, optional, required
>
> Would there be any objections to a patch that does not write default values (essentially the same logic as the optional attributes). This obviously relies on the receiving application using the same IDL version to ensure the defaults used on object construction match the senders.



--
This message was sent by Atlassian JIRA
(v6.2#6252)