Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

default values and testing if a field is set in v3 #359

Closed
ekg opened this issue May 11, 2015 · 21 comments
Closed

default values and testing if a field is set in v3 #359

ekg opened this issue May 11, 2015 · 21 comments

Comments

@ekg
Copy link

ekg commented May 11, 2015

Forgive the confusion, but how does one now check if a field is set in version 3?

@alfredkcp
Copy link

All field presence check (except for an embedded message) has been removed in proto3.

@ekg
Copy link
Author

ekg commented May 13, 2015

I see, so does this mean that we need to use semantics such that a non-embedded message field (such as int32) will be 0 when "unset"? Now the default values (empty string, 0 integer) can indicate non-presence or presence and that the value was set to the default.

@cfallin
Copy link
Contributor

cfallin commented May 13, 2015

Yep, this was the design decision in proto3 -- primitive (non-message) fields are no longer nullable. It's better/more accurate to not even think of "unset"; i.e., a new message object already has every primitive field set. The intent, as I understand it, was to make a message more like a plain struct.

Nullable/optional fields can still be built by wrapping the primitives in submessages. There are predefined types for this in src/google/protobuf/wrappers.proto.

@daicoden
Copy link

I realize this is too late, but some feedback FWIW.

This makes APIs complicated for synchronizing objects between systems. You can't implement something like http PATCH semantics. Clients have to now read, echo, and detect true conflicts vs contention.

Given a contact with name & email, if client A wants to change the name, and client B wants to change the email, you have to push the smarts of dealing with concurrent updates to the clients for edge cases.

Before, you could simply look at the intent of the client by being able to tell what the client wanted changed to detect if there was a conflict. The responsibility of conflict handling in the client used to be simpler because it was only initiated when there was actually a conflict.

The workaround works... but is cumbersome if it applies to every field in your domain. If you want developers interacting through other data formats (like JSON) you need to write custom marshallers.

Again, FWIW - thanks!

@jhump
Copy link
Contributor

jhump commented Dec 17, 2016

BTW, @daicoden, you can still effectively test for absence of values (e.g. intent). The real thing missing from proto3 is that you can't define defaults. Nested messages always default to null, enums always default to the first value which must be assigned number 0 (so you could use a convention like ABSENT as the first value in every enum), strings are empty, numerics are zero, and bools are false.

If you need to distinguish absent strings/numbers/bools from ""/zero/false, there are "box" types included in proto's "well-known types". These can be checked if null to determine presence (basically just like using java.lang.Integer vs. primitive int in Java).

The downside is that box types are more verbose on the wire and likely to result in a little more garbage in Java (e.g. proto2's use of a field bitmask and primitives produces less garbage than using boxed types). Also, it's opt-in: you have to explicitly choose a box type vs. the primitive alternative when defining your messages.

@zackangelo
Copy link

@jhump I don't think the wrapper types get you there. PATCH semantics still require the ability to distinguish between a value that is absent and a value that is null (otherwise, how do you explicitly set a value to null?).

@daicoden
Copy link

@zackangelo good point. I guess you really need true tri-state fields for PATCH (omitted, clear, set). Proto2 had the null issue too, it only supported (omitted, set) because there is no null in proto.

I would be happy with (omitted, set) to allow partial document updates... I couldn't find a good way to do partial updates in proto3 aside from box-types which is really verbose in the code and makes others on the project cranky about using protos... but we will survive! ;) Thanks @jhump

@kskalski
Copy link

Yap, wrapper is of little help for PATCH/OVERRIDE semantics, since the problem just moves inside the nested message.
When you want to express request "set X to V" basically V cannot be a default value (receiver will not know if X wasn't set or was set to V==default).
With boxed values e.g. BoolValue(false) could be considered as "set X to false", however if you want to use MergeFrom it will still fail terribly, since BoolValue(false) merged on top of BoolValue(true) will still result in BoolValue(true)

In this scenario either default values chosen in proto3 should be excluded from domain of values ever used by your system, or as can easily be the case with e.g. "bool" you need to use some other type like:
enum NullableBool {
NullableBoolAbsent = 0; // unique name to not pollute namespace
True = 1;
False = 2;
}
which is acceptable, but makes user code more ugly (value() == True, value() == False, can have bugs when doing if (value()))

@adematte
Copy link

just stumbled on this old thread. It seems like the preferred way to handle PATH type requests is to use a field mask as provided by the well known types:
https://developers.google.com/protocol-buffers/docs/reference/google.protobuf#google.protobuf.FieldMask
Still would love a better alternative.

taoso pushed a commit to taoso/protobuf that referenced this issue Aug 1, 2018
)

Disable benchmarks using testing.B.Run for pre-go1.7.

Fix unmarshaling Any for pre-1.8.  Unmarshaling code for Any assumed
json.RawMessage value receiver on json.Marshaler interface, but it was a
pointer receiver pre-1.8.
@house1909
Copy link

BTW, @daicoden, you can still effectively test for absence of values (e.g. intent). The real thing missing from proto3 is that you can't define defaults. Nested messages always default to null, enums always default to the first value which must be assigned number 0 (so you could use a convention like ABSENT as the first value in every enum), strings are empty, numerics are zero, and bools are false.
[...]

its not allowed to have the same value for Enums in one File

Protofile.proto:96:9: "NULLVALUE" is already defined in "<package/scope>".
Protofile.proto:96:9: Note that enum values use C++ scoping rules, meaning that enum values are siblings of their type, not children of it. Therefore, "NULLVALUE" must be unique within "<package/scope>", not just within "".

@mrzacarias
Copy link

Today is 05/31/2019 and me and my team are still facing problems because of this design decision. 👏

@seantcanavan
Copy link

I'm still pretty new to this whole proto / protobuf thing but for me it definitely seems confusing. What I'm struggling to understand is how to differentiate between go default values and optional fields not put in the request.

For instance ints default to 0 so by the time go sees it the value is 0. There's no way to know whether the value 0 was sent or if it defaulted to 0 to the best of my understanding. This inherently creates situations that, to use JavaScripty syntax, require 'Nullable' primitives like int / bool in order to understand what the original intent or value was sent by the client.

@siepkes
Copy link

siepkes commented Mar 29, 2020

enums always default to the first value which must be assigned number 0 .

I'm genuinely curious on the background of this design decision. This also means that if you add something to an enum older clients will suddenly default to the default enum value (ie. the one defined as 0) if they encounter the new value.

If an enum is essentially an required field (since it can't be empty) then why for example does Java protobuf not throw an exception when one has not explicitly set the field? The current design seems like a great way to set your self up for a Knight Capital Group-style wipeout.

@dwsutherland
Copy link

dwsutherland commented May 3, 2020

Silly decision... I want to send/set a zero/default value and know which fields were sent/set when received (i.e. fields that don't get sent/updated every delta/data-push)...

I don’t want clunky workarounds.. Like using alternate values to interpret back to the default, or ancillary boolean fields to tell me if a field was set before send...

Byebye Protobuf

@ObsidianMinor
Copy link
Contributor

Explicit optional fields are coming to proto3. See this doc

@dwsutherland
Copy link

Explicit optional fields are coming to proto3. See this doc

Brilliant! ... Thanks for the quick reply..

(and Hello again Protobuf 😄 )

@dwsutherland
Copy link

@ObsidianMinor - Thanks 3.12.0-rc-1 works great! However I did open an issue about a mixing optional with normal singular message fields:
#7463

@ravisanwal
Copy link

Another thing different in proto3 is they way reflection responds to whether fields are set or not.
Explicitly setting an enum to default value (first ordinal) and then introspecting by message.hasField(enumFieldDescriptor) returns false.

This almost makes the first ordinal of every enum useless, more of a meta marker.

@jhump
Copy link
Contributor

jhump commented Apr 6, 2021

@ravisanwal, the new proto3 optional stuff changes this. It allows you to use an explicit optional keyword on fields in a proto3 message to indicate that the supports "presence" -- ability to distinguish the field being present or absent, even if it has the zero value for the type. So you could mark your enum field as optional and get different behavior.

This was added behind a flag (--experimental_allow_proto3_optional) in protoc 3.12. And it's no longer experimental as of 3.15.

@ravisanwal
Copy link

ravisanwal commented Apr 7, 2021

@jhump that is interesting.
Although, the way we are using it (reflection in proto) this may not help us. We are not necessarily doing the has check on optional fields only. Basically we have a merge mechanism where we take two proto messages (same type of message) and do a nested merge of fields based on certain rules (overwrite, append, ignore, add, subtract etc.) To make the merge generic, so it can work on any message, we use proto reflection semantics to detect what field is set (or not set) and this where the proro3 behavior is not favorable.

e.g. while merging an enum field between two messages to overwrite, if the later message (the one being merged) has default value of that enum (first ordinal) message.hasField(enumFieldDescriptor) will return false. If the original message (the one we are merging the later message into) has this enum set to a non default value (non 0 ordinal) then the merge basically breaks and becomes a noop. There are reasons why we skip merging fields that are not set.

We have a workaround so this is not a deal-breaker, it is just that we discovered this now and have some inconsistent data.

On second thoughts, yeah this may be helpful. If the behavior of message.hasField(enumFieldDescriptor) changes in 3.15, it may work for us.

@kskalski
Copy link

kskalski commented Apr 8, 2021

@ravisanwal I have implemented similar functionality (using protobuf messages as state and state-diff where I can apply diff to state or even diff to diff getting a collated diff) using protobuf reflection. With optional fields in proto3 you can achieve almost full functionality (the biggest exception is handling of repeated fields for which you will need some kind of strategy - append or replace), here is a crucial piece of code that is allowed with proto3 optionals:

      foreach (var field in dst.Descriptor.Fields.InFieldNumberOrder()) {
        var access = field.Accessor;
        if (field.HasPresence && !access.HasValue(src))
          continue;
        ...
     }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests