add omitempty option #2
Conversation
| i -= len(m.XXX_unrecognized) | ||
| copy(dAtA[i:], m.XXX_unrecognized) | ||
| } | ||
| if !m.InnerOmitEmpty.IsEmpty() { |
There was a problem hiding this comment.
This line is pretty much what the option does.
|
Nice idea! Out of curiosity, why require an |
| } | ||
| return nil | ||
| } | ||
| func (this *OmitEmpty) Equal(that interface{}) bool { |
There was a problem hiding this comment.
nit: if gogoproto is generating a method called Equal, would it be more consistent to rename IsEmpty to Empty?
| } | ||
| return nil | ||
| } | ||
| func (this *OmitEmpty) Equal(that interface{}) bool { |
There was a problem hiding this comment.
Should we follow the pattern set by Equal of exposing a gogoproto.empty option to generate an IsEmpty/Empty method?
There was a problem hiding this comment.
I'm not supercomfortable with working in this repo, I'd rather not complicate things here. It will be trivial to write it manually wherever we want to use this.
|
I figured it could in some cases be faster than a full struct compare. For example for the timestamp I think it's enough to check the WallTime. Also it would allow non-strucr casttypes (which is the case for KVNemesisSeq in test builds). |
This change adds a `gogoproto.omitempty` option that can be used (in conjunction with `(gogoproto.nullable) = false`) for non-nullable message fields. When a message field is not nullable, its tag is always encoded - even if all message fields are unset. The `omitempty` option changes the marshalling code to check an `Empty()` method on that message, which must be defined by the user. The result is that when the message is empty, the encoding matches the one we would get if the field was nullable and the pointer is unset.
|
You claim in the commit msg:
But i'm not sure that's always true. Consider the Do you understand why the empty fields of the MVCCValueheader are not encoded in the |
|
Good question. It looks like the test disables that metamorphic constant: |
|
I'd like to merge this soon to clean up the hacks in crdb. |
|
TFTR! |
This change adds a
gogoproto.omitemptyoption that can be used (in conjunction with(gogoproto.nullable) = false) for non-nullable message fields.When a message field is not nullable, its tag is always encoded - even if all message fields are unset. The
omitemptyoption changes the marshalling code to check anIsEmpty()method on that message, which can be defined by the user. The result is that when the message is empty, the encoding matches the one we would get if the field was nullable and the pointer is unset.