wolf4ood / gremlin-rs

Gremlin Rust
Apache License 2.0
108 stars 30 forks source link

Date Type At Wrong Precision? #128

Closed criminosis closed 3 years ago

criminosis commented 3 years ago

I believe the Date is at the wrong precision?

At this location: https://github.com/wolf4ood/gremlin-rs/blob/c6936ad5981e7ce8b32ae31261e942644cd42abc/gremlin-client/src/io/mod.rs#L65

It calls d.timestamp() which Returns the number of non-leap **seconds** since January 1, 1970 0:00:00 UTC

However at least according to the Tinkerpop docs it has for Date: Representing a **millisecond**-precision offset from the unix epoch. In Java, it is simply Date.getTime().

I believe that method should instead be d.timestamp_millis()

Doing that corrected writing the timestamp so it's correct when viewed in a gremlin session.

To fix parsing it in the library I believe this needs to be changed https://github.com/wolf4ood/gremlin-rs/blob/2e18b3cb09217a1c7f9645f98ce98a90a69acfab/gremlin-client/src/io/serializer_v3.rs#L45

to Ok(GValue::from(Utc.timestamp_millis(val)))

But this will naturally have significant impact on anyone who's using this library currently for any values that were serialized as epoch seconds in conflict with the Tinkerpop spec.

wolf4ood commented 3 years ago

Hi @criminosis good catch we could release a new version for this like 0.8.x

Thanks

criminosis commented 3 years ago

@wolf4ood thanks! I just put up a PR for it so I hope that helps get 0.8.x out soon, I know I'd like to use it as soon as the 3 issues I opened are fixed with the PRs I'm working on 😃