[
https://issues.apache.org/jira/browse/THRIFT-2232?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13801855#comment-13801855
]
Ben Sigelman commented on THRIFT-2232:
--------------------------------------
Hi [~jensg], thanks for the quick response!
There are, of course, a variety of options. Here are some things I'd like to
maintain:
- allow users to continue to treat thrift structs as simple Go structs, at
least to a first approximation
- do not force users to manually maintain a bit vector of IsSet information
- backwards compatibility would be awesome
The sad thing is that I can't imagine a solution that accomplishes all of the
above. Since these are still relatively early days (and since the current
library is broken per this bug report), I am inclined to give up the backwards
compatibility goal.
One option (which I do not favor) would be to generate Go interfaces rather
than Go structs. These interfaces would be the only way to get/set fields, and
could internally do any number of things to satisfy the various method
guarantees. The actual implementation of the thrift-generated interface could
take many forms, but it definitely wouldn't be exported from the generated
package. I don't like this idea, though, as it seems a little heavy-weight and
replaces simple struct accessors with method calls. It would be great to allow
users to treat generated thrift structs as Go structs somehow.
My proposal is to do something akin to what protobufs does to solve this
problem. (It's also similar to what other popular packages do for things like
this... e.g., the way that the `gorp` ORM package deals with nullable database
columns) In particular, we would represent an optional field as a pointer, and
use the `nil` value to represent absence. Required fields could either (a) also
be represented as pointers and make their containing structs invalid when
absent, or (b) be represented as non-pointer fields like the current thrift Go
impl does for all fields. list<>s etc would hopefully be unaffected by this
proposal... they can still be empty, and that's all we need as far as
presence/absence info (correct me if I'm mistaken!).
Also, note that it's annoying [though still totally feasible] to pass pointers
to POD and string literals in Go (*). Protobufs deals with this by providing
helpers. We would probably do the same... See these guys:
http://godoc.org/code.google.com/p/goprotobuf/proto#String
http://godoc.org/code.google.com/p/goprotobuf/proto#Uint32
The above begs the question of whether there can/should be a central lib of Go
thrift helpers that people can add to their projects (rather than simply
embedding all of these, redundantly, in every generated thrift Go package).
So, let me know what you think. FYI, I know a lot about protobufs (was at
google for 9 years), but less about Thrift per se. I'd love to know if there
are thorny things I should be aware of in advance.
Also, if there's someone else who's willing to do this work, I have a million
other things to do and would gladly wait for them if the turnaround would be
less than a month or so. Thanks for your help!
> IsSet* broken in Go
> -------------------
>
> Key: THRIFT-2232
> URL: https://issues.apache.org/jira/browse/THRIFT-2232
> Project: Thrift
> Issue Type: Bug
> Components: Go - Compiler
> Reporter: Ben Sigelman
> Labels: isset
> Fix For: 0.9.2
>
>
> The various generated IsSetXYZ() methods just check for magic values of their
> respective fields. This is obviously broken, and also diverges from the
> implementation in other languages.
> I am willing and able to fix this myself, but I don't want to start on any
> impl until we can decide on an approach.
> At this point, though, optional fields in Go are basically useless if one's
> application makes use of the magic "absence" value.
--
This message was sent by Atlassian JIRA
(v6.1#6144)
