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

Encoding zero values as null #42

Open
Stebalien opened this issue Nov 6, 2018 · 5 comments
Open

Encoding zero values as null #42

Stebalien opened this issue Nov 6, 2018 · 5 comments

Comments

@Stebalien
Copy link
Contributor

Currently, refmt:

  1. Will decode null to a zero value (i.e., MyStruct{})
  2. Can be instructed to skip zero values when encoding by specifying "omitempty".

However, there isn't a way to say "encode zero values to null" (the inverse of 1). This can be useful when we always want to have a field present in an object but would like to make it clear that the field is empty.

Specifically, this would be really useful for CIDs in IPLD. Users can currently specify "omitempty" but, if they fail to include this, encoding a zero-value CID will return an error. With some way to say "encode the zero value to null", this would "just work".

Proposal: Add a Nullable(nullable bool) option to the atlas entry builder.

@Stebalien
Copy link
Contributor Author

Alternatively, allow marshal transform func to return some marker error/value?

@warpfork
Copy link
Member

So to recap, the matrix of transforms that seem potentially reasonable here are:

  • when populating a struct from a null token:
    • A1: results in zero struct (this can be seen as "ignore" behavior);
    • A2: or, results in error
  • when marshalling a struct which is zero-valued:
    • B1: result in null (even when it's not a pointer; this is Somewhat Odd, but we can define it to be so)
    • B2: result in tokens for an empty map (this is normal)
    • B3: result in nothing at all (the omitempty behavior)

(Did I miss any?)

So we currently have A1 and B2 by default, and B3 can be configured.

We're missing the ability to configure A2 at all (and that seems wrong! It's possible A2 should be the default, really...); and we're missing B1. Yep.

And we could also try to get B1 by making marshal transform funcs support returning a sentinel value that means null, and that seems also like it would be reasonable to support but also seems like the less general case.


So then, thinking outloud, is ZeroAsNull(bool) actually enough?

  • default: A2, B2 (n.b. this would be a change to the default)
  • if omitempty: A2, B3
  • if zeroAsNull: A1, B1
  • if omitempty&&zeroAsNull: A1, B3

With just these parameters, some of the other combos like B2+A1 would be unreachable. Is that desirable? Or do we need more parameters for these behaviors?

@Stebalien
Copy link
Contributor Author

With just these parameters, some of the other combos like B2+A1 would be unreachable. Is that desirable? Or do we need more parameters for these behaviors?

Well, this is the current default. It's also what go does by default. (Not sure if it should be but users will be confused if it isn't)

One solution is to just define both ZeroAsNull (defaults to false) and NullAsZero (defaults to true).


There's actually another dimension here: tags.

Without tags (and with #37) fixed, I could just write a pair of transform functions to/from interface{}. The issue with that is I also need to be able to say "tag this only if I transform to type X, not type Y".

Really, I'm starting to become more and more convinced that using either *cid.Cid or omitempty is what we want but users may not be so happy with that.

@warpfork
Copy link
Member

Another question came to mind: Do we want these behaviors to be specified at the site of fields (as omitempty is), or on types?

@Stebalien
Copy link
Contributor Author

Well, for my usecase, types. In general, for fields, I'd just specify omitempty.

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

2 participants