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

question: Do we support Timestamp type? #94

Closed
amitguptagwl opened this issue Sep 10, 2018 · 8 comments
Closed

question: Do we support Timestamp type? #94

amitguptagwl opened this issue Sep 10, 2018 · 8 comments
Labels

Comments

@amitguptagwl
Copy link

Here is the complete list supported by protobuf 3: https://developers.google.com/protocol-buffers/docs/proto3#json

I face an issue in compiling proto file with Timestamp field. So I was wondering if I'm using the wrong syntax or it is not supported.

message Signals {
    Timestamp start = 2;
    repeated sint32 signal = 1;
}
@mourner
Copy link
Member

mourner commented Sep 10, 2018

Yeah, we don't currently support importing google protobuf extensions. Same as #92

As a workaround, you can inline Timestamp definition from Google:

https://github.com/protocolbuffers/protobuf/blob/f8d56e929e35e822f62912fa0c91aa1e640cc373/src/google/protobuf/timestamp.proto#L123-L135

@amitguptagwl
Copy link
Author

Great thanks

@amitguptagwl
Copy link
Author

@mourner One more question. For float field, If I input 987654321 I get 987654336 back. I know that is because of 'ieee754' encoding. But just want to ensure that if this problem persist with actual google library as well? (TBH I didn't tested that yet)

@amitguptagwl
Copy link
Author

@mourner I've created gist extracted from google protobuf implementation. However it doesn't work for any number. So I'm not sure what's wrong I'm doing.

@kjvalencik
Copy link
Collaborator

kjvalencik commented Sep 11, 2018

@amitguptagwl float in protobuf corresponds to a 32-bit float. Single precision floats can only represent integers up to 16777216 accurately.

const assert = require('assert');

const num = 987654321;
const expected = 987654336;
const buf = Buffer.alloc(4);

buf.writeFloatLE(num);

const actual = buf.readFloatLE();

assert.strictEqual(actual, expected);

If you use a double precision float (64-bit, double in protobuf), it can accurately represent that number.

@amitguptagwl
Copy link
Author

Thanks @kjvalencik . I actually noticed that google protobuf implementation stops user to write data which is out of range or can't read same. Shouldn't we do the same?

@kjvalencik
Copy link
Collaborator

I don't think so. Those bounds check could impact performance. Also, there are some intentional inaccuracies. For example, int64 and uint64 both use a JS Number which can't represent the range.

@amitguptagwl
Copy link
Author

Hmm

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

No branches or pull requests

3 participants