rbetts / voltdbgo

[Deprecated] VoltDB driver for Google go (golang)
Other
31 stars 8 forks source link

Timestamp support #4

Closed martinhpedersen closed 10 years ago

martinhpedersen commented 10 years ago

Hi Ryan - thanks for resolving the previous issue. I've added timestamp support, unmarshalled to a time.Time struct. In the future it could be nice to also support a plain int64 representation, but I believe a time.Time struct is more convenient.

I've also written some tests to accompany the implementation.

Let me know what you think :)

martinhpedersen commented 10 years ago

I just realized that this commit is using a func Time.Round() that is not available in golang 1.0.

I'm looking into a better solution.

martinhpedersen commented 10 years ago

time.Round() implementation is ~200 lines of code and was added in Go 1.1.

The reason for using the method is that time.Time supports nano-second resolution, but voltdb only supports microseconds.

I see the following possible solutions:

  1. Copy / re-implement Time.Round()
  2. Truncate to microseconds, and document that behaviour.
  3. Do a basic floor()/ceil() depending on last digit
  4. Require golang >= 1.1

What do you propose? My best pick atm would be truncating.

rbetts commented 10 years ago

I'm fine with only supporting Go 1.1. Thoughts?

martinhpedersen commented 10 years ago

The only concern I have with requiring go 1.1 is that it is not available in debian stable's apt repository. I am however, under the impression that it is common to use a later release as it has many improvements over 1.0. Debian testing has 1.2, as do latest ubuntu.

I personally use go 1.2, so it would not be a problem for me.

rbetts commented 10 years ago

I think it's fine. I use brew on os x and it is installing 1.2. Thanks for the pull request - will review and pull.

Ryan.

On Wed, Feb 5, 2014 at 4:49 PM, martinhpedersen notifications@github.comwrote:

The only concern I have with requiring go 1.1 is that it is not available in debian stable's apt repository. I am however, under the impression that it is common to use a later release as it has many improvements over 1.0. Debian testing has 1.2, as do latest ubuntu.

I personally use go 1.2, so it would not be a problem for me.

— Reply to this email directly or view it on GitHubhttps://github.com/rbetts/voltdbgo/pull/4#issuecomment-34253852 .

Ryan Betts, CTO VoltDB, Inc.

_E: rbetts@voltdb.com jpiekos@voltdb.com O: 508.596.2676 M: 508.596.2676 T: @ryanbetts www.voltdb.com Follow VoltDB on Twitter http://twitter.com/voltdb, Facebookhttps://www.facebook.com/VoltDB?sk=wall, and LinkedIn<http://www.linkedin.com/company/857257?goback=.fcs_GLHD_volt+db_false__2__2__2__2__2__2__2__2__2__2__2_2&trk=ncsrch_hits>

rbetts commented 10 years ago

Martin, can you please add your name to the LICENSE file copyright line and I'll pull. I need to track contributors here with a bit of care.

martinhpedersen commented 10 years ago

Done!

Have a nice weekend :)