[
https://issues.apache.org/jira/browse/THRIFT-2429?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13962575#comment-13962575
]
Ben Sigelman commented on THRIFT-2429:
--------------------------------------
Using default values as a convenience is one thing (your first example). It's a
matter of taste, and I don't really have any super-strong feelings. That said,
I was at google for a while (perhaps too long!) working on core infrastructure,
and default values were generally frowned upon, though certainly not forbidden
or anything similar. I agree with your point about their convenience when
adding a new field and supporting older peers (or older serialized data).
I could be missing something, but in the table you outlined
[here|https://issues.apache.org/jira/browse/THRIFT-2429?focusedCommentId=13957788&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13957788],
it seemed to suggest that optional fields set to their default value would not
serialize. (When I said "the wire protocol", btw, I just meant "the wire" --
mea culpa on that front, this is certainly a protocol-independent discussion,
as you observed!)
Given those proposed semantics, it is unsafe to change the default value in the
future; it becomes as essential to the field as its tag number. Rather than a
programming convenience (i.e., rather than what it was intended to be), it
quite literally becomes part of the protocol...
E.g., using your example above, if the value of `y` is set, serialize it
*unless* it's set to `5`, in which case *don't* serialize it. Now if we wish to
change or remove the default in the future, if `y` isn't set in a serialized
message, we can't be sure if it was actually intended to be un-set, or whether
it was intended to be set to `5`.
I do applaud the effort to make these semantics well-defined, though. Here's
what I'd propose:
- I like your "deserializer" table; no proposal on that front!
- I also like your treatment of required fields (i.e., always serialize)
- For optional field serialization: a default-constructed IDL-generated type
initializes the optional field with its default value. The user then has the
option to clear that field (i.e., un-set it) if they so choose, which would
prevent it from serializing. Of course, if the optional field takes on a
non-default value, it is serialized [surely we can all agree on that :)]. But
if the optional field is (a) not cleared, and (b) set to its default value, it
*is* serialized.
This allows the user of the thrift type to remove the default value in the
future without creating uncertainty for all future readers of the optional
field; which is what I'm principally concerned by.
I am admittedly baffled by the idea that "optional is an optimization", by the
way. If that's a widely-held belief, then I should probably bow out of this
discussion because I'm clearly missing too much context! I essentially never
use `required` (when I was at google there was even a movement to ban
`required`, as so many critical protocols had to swallow bitter pills when they
realized required things weren't actually required after one migration or
another), "opting" (pun!) for `optional` since it's harder to paint yourself
into a corner with it. (Even if it requires more annoying boilerplate and
validation in the layer above the protobuf/thrift/etc objects)
Anyway, those are my thoughts. I think this proposal is sensible/desirable(*)
with the exception of the case I outlined above. I am ok with the fact that
others will disagree, but I do want to make sure that I'm at least
communicating the problem I'm concerned about. For my own projects, I just
won't use default values :) So I'm not personally going to be affected by
this regardless.
Thanks for the clear and thoughtful response!
Ben
(*): I will admit to not-understanding the `-` case (not-optional,
not-required). I know that it's historical, and I know I don't grok it... I am
using thrift out of necessity due to the [poor] quality of protobuf bindings in
one of my target languages, and I only ever tried to understand enough of
Thrift to recreate what I got out of protobufs. Shame on me!
PS: I recognize that users (at fb or otherwise) may be clamoring for this (or
whatever else). Users of infrastructure software sometimes unknowingly ask for
things that will harm them in the long run; it is our job to understand their
actual requirements and offer solutions that address them, but not necessarily
to simply give them what they want in the immediate. This is especially true
for things (like this default-optional thing) that have a "back-loaded" cost:
e.g., in this case, the feature is incredibly convenient for the migration *to*
the new field, yet equally painful for the migration away from it years later.
The busy application developer is understandably more focused on the immediate
need and thus suggests a feature without necessarily thinking through all of
the implications. </sermon> I'm sure none of the above is controversial, but
hearing the bit about this feature being "popular" prompted me to reiterate it
:)
> 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)
