Jens,
YES! That's why I sent the email, rather than diving into code! *grin*
(1) the current implementation in C++ and Ocaml treats a union as a struct
with all-optional fields.
(2) this means that if code sets two fields of a union, both fields get
sent on-the-wire, and the receiving end deserializes both fields. So it
really isn't a union, and it certianly doesn't "provide a means to
transport exactly one field of a possible set of fieldsprovide a means to
transport exactly one field of a possible set of fields"
(3) It's easy to fix this -- each set_field clears all the other ones. I
won't worry for the moment about making that efficient (though it probably
doesn't need to be) but it can be made efficient using C unions (yes?)
(4) But this *will* be a breaking change. B/c for instance, in
plugin.thrift, t_const_value has fields
6: optional string identifier_val
7: optional t_type_id enum_val
and those fields are -not- alternatives: they are BOTH SET, or NEITHER ARE
SET.
(5) So at a minimum, we'd need to replace that wiith something like
struct t_const_identifier_value {
1: required string identifier_val
2: required t_type_id enum_val
}
and then replace those two fields with
8: optional t_const_identifier_value const_identifier_val
(6) This will break any code people have written that uses this plugin
interface (b/c types change in non-compatible ways)
(7) More generally, anybody who uses unions, and relies on them really
being "structs with all optional fields" will find that they can't rely on
that anymore.
I would note that neither of these is a big problem. But a breaking change
is a breaking change, right?
Anyway, that's the story. I think you can see, that writing the code is
the easy part.
If you think this is OK to do, I can probably prepare a PR quickly.
--chet--
P.S. Why did I look into this subject? B/c I want to write a new ocaml
library, that will use "idiomatic" Ocaml types, and hopefully be much more
efficient. But certainly be easier-to-use for Ocaml programmers. Thrift
unions should be mapped to Ocaml union types ("constructor datatypes").
And those .... well, they really *are* exactly one field can be set at any
time. So I figure, yes I can write my new compiler and library. But it'll
be incompatible with the current Thrift contract. So I figured, I should
look into whether that contract can be fixed, while I hack my code.
On Fri, Dec 1, 2017 at 2:48 PM, Jens Geyer <[email protected]> wrote:
>
>
> If you're not sure, you can discuss it. If you are sure, provide a PR
> (ideally one per language) and we look at it.
>
> From my feelings, I'm not really sure if I fully understand the problem and
> how you plan to fix it. Maybe it is a good idea to sketch the idea behind
> and get some comments before you deep-dive in the code.
>
