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

Add an option to wrap primitives to Option[] #54

Closed
onsails opened this issue Nov 13, 2015 · 8 comments
Closed

Add an option to wrap primitives to Option[] #54

onsails opened this issue Nov 13, 2015 · 8 comments

Comments

@onsails
Copy link

onsails commented Nov 13, 2015

In some cases zeros does not make sence. For example, I have a model Event, stored in relational database:

message Event {
  bytes data = 1;
  int64 deleted_at = 5 [(scalapb.field).type = "org.joda.time.DateTime"];
}

In the past Event did not have a deleted_at, but it had a boolean is_deleted. When deleted_at appeared, it had been set to the epoch start for every deleted event and to NULL for the rest. So, if deleted_at is not an Option, there is no way to determine if event was deleted.

@thesamet
Copy link
Contributor

Two options:

Option A:

int64 deleted_at = 5 [(scalapb.field).type = "Option[org.joda.time.DateTime]"];

and define a TypeMapper[Int, Option[org.joda.time.DateTime]] that maps 0 to None.

Option B:

you can define:

message NullableTime {
  int64 timestamp = 1
}

message Event {
  bytes data = 1;
  NullableTime deleted_at = 5 [(scalapb.field).type = "org.joda.time.DateTime"];
}

Message fields are always in options, and you can then use a custom type for it.

@onsails
Copy link
Author

onsails commented Nov 13, 2015

I like NullableTime idea, thanks. It totally covers my case.

@onsails onsails closed this as completed Nov 13, 2015
@zackangelo
Copy link
Contributor

@thesamet What's the reasoning behind not using Option for optional types in messages? Is it something related to how protobuf messages are encoded (e.g., they can't convey absent values)?

@thesamet
Copy link
Contributor

Exactly. The receiving end has no way of telling if an int32 field was assigned to zero or whether it has not been assigned. See note here: https://developers.google.com/protocol-buffers/docs/proto3?hl=en#default

@zackangelo
Copy link
Contributor

@thesamet this seems like a pretty big regression compared to proto2, am I missing something? :)

@zackangelo
Copy link
Contributor

@thesamet it looks like proto3 betas will include built-in nullable primitives: protocolbuffers/protobuf#159 (comment).

Could we map these to Option types in the generated code?

That being said, I can't find any trace of them in the repo yet.

@zackangelo
Copy link
Contributor

@zackangelo
Copy link
Contributor

Trying to use these built-in wrapped types results in compile-time errors:

[error] /Users/zack.angelo/git/galaxy/interfaces/target/scala-2.11/src_managed/main/compiled_protobuf/com/bigcommerce/services/catalog/Product.scala:37: value serializedSize is not a member of com.google.protobuf.Int32Value

[error]       if (id.isDefined) { __size += 1 + com.google.protobuf.CodedOutputStream.computeRawVarint32Size(id.get.serializedSize) + id.get.serializedSize }

Any idea how to fix?

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

3 participants