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

datetime drops milliseconds from the value #62

Open
snappa opened this issue Mar 24, 2015 · 13 comments
Open

datetime drops milliseconds from the value #62

snappa opened this issue Mar 24, 2015 · 13 comments

Comments

@snappa
Copy link

snappa commented Mar 24, 2015

First let me say that this is a great gem. I found it today and it's really nice and makes hstore use much cleaner.

I need to store DateTime values in an hstore column and defining as follows:
hstore_accessor :properties,
queued_at: :datetime,
started_at: :datetime,
finished_at: :datetime

and setting queued_at, started_at, and finished_at drops off the millisecond portion of the DateTime value. I had gotten this to work in my model with the following and was looking forward to removing this code:

properties["queued_at"] = qd_value.strftime("%Q").to_i

and reconstituting using:

Time.strptime(qd_value.to_s, '%Q').to_datetime

This preserved the millisecond value of the timestamp.

@snappa snappa changed the title datetime drops milliseconds from the valie datetime drops milliseconds from the value Mar 24, 2015
@crismali
Copy link
Contributor

Is there a step missing in your description above? I played with this a bit locally in a Rails 4.1 console:

x = Time.now
#=> 2015-03-24 11:56:28 -0500
x.to_i
#=> 1427216188
x.strftime("%Q").to_i
#=> 0
Time.strptime(x.to_s, '%Q').to_datetime
#=> Wed, 31 Dec 1969 18:00:02 -0600

@snappa
Copy link
Author

snappa commented Mar 24, 2015

The goal is to be able to compute time deltas in milliseconds.

# Create and store the value in the property
datetime_value = DateTime.now
properties['queued_at'] = datetime_value.strftime("%Q").to_i)

#Retrieve the value and use the DateTime instance to compare against another to get a delta in ms
timeval_str = properties['queued_at']
datetime_value =  Time.strptime(timeval_str, '%Q').to_datetime

# Notice the difference in values when you do:
datetime_value.to_i == 1427218442
# and 
datetime_value.strftime("%Q").to_i == 1427218442783

Does this help? Notice my serializer converts time to microseconds. The serializer in hstore_accessor does just a to_i on the DateTime instance yielding different values. Just using .to_i gives you the number of seconds since the Unix epoch but my method of serialization / deserialization gives you to the millisecond

@crismali
Copy link
Contributor

This issue is difficult to solve without another major version release. We could store datetimes as the number of milliseconds and hack in a solution to make it work with legacy systems that have already stored milliseconds. In theory we could use the number of digits to switch, but that feels a bit hacky (at least at the moment).

Could you try opening up a pull request with possible implementation that does not require a major version release?

@jhirn
Copy link
Contributor

jhirn commented Mar 24, 2015

Alternatively, we could introduce another type, perhaps timestamp, which
has greater precision and consider deprecating one.

Thoughts?

On Tue, Mar 24, 2015 at 1:18 PM, Michael [email protected] wrote:

This issue is difficult to solve without another major version release. We
could store datetimes as the number of milliseconds and hack in a solution
to make it work with legacy systems that have already stored milliseconds.
In theory we could use the number of digits to switch, but that feels a bit
hacky (at least at the moment).

Could you try opening up a pull request with possible implementation that
does not require a major version release?


Reply to this email directly or view it on GitHub
#62 (comment)
.

@crismali
Copy link
Contributor

That could work well. That way when we hit the next major release we could swap drop datetime and just rename timestamp to datetime.

@snappa
Copy link
Author

snappa commented Mar 24, 2015

As a consumer of this gem, I'm good with whatever direction you choose. Personally, since it's a type of "datetime" and is associated with ActiveRecord I was expecting it to behave like saving a DateTime instance to a datetime column in Postgres would. Therefore I'd opt to make it save in the finer granularity and keep a single type. Then again, I don't know if this would break other users of your gem. I'm guessing it wouldn't but don't want to presume.

@snappa
Copy link
Author

snappa commented Mar 24, 2015

Regardless of the direction you choose, I'm grateful as I was literally about to create a gem like this for a project I'm working on so thank you for your work!

@crismali
Copy link
Contributor

Thanks for using the gem and opening up this issue. We'll play around with this a bit more but I'm leaning towards the new data type option. Seems like the easiest way to go.

@jhirn
Copy link
Contributor

jhirn commented Mar 24, 2015

I agree with @snappa on mimicking the natural ActiveRecord behavior in lieu
of a new datatype.

Perhaps we can add the new functionality disabled by default which can be
enabled via an initializer. This seems like the win-win for all parties.

On Tue, Mar 24, 2015 at 1:39 PM, Michael [email protected] wrote:

Thanks for using the gem and opening up this issue. We'll play around with
this a bit more but I'm leaning towards the new data type option. Seems
like the easiest way to go.


Reply to this email directly or view it on GitHub
#62 (comment)
.

@snappa
Copy link
Author

snappa commented Mar 24, 2015

Thanks guys. Ideally I'd make the datetime type perform exactly like ActiveRecord's datetime column but if that's going to potentially break code that's out there then that's not good for the masses.

I agree with you looking at the number of digits to determine how the data is saved is a bit of a hack to an already clean gem.

I'd opt for the new type or an be find with an initializer option. I'd also suggest providing a migration for people to update their systems IF you choose to keep one type and make it finer granularity. I'll take a look at this later today when I have a few. Right now I've coded around this and basically made methods that will be removed once hstore_accessor has a fix.

@snappa
Copy link
Author

snappa commented Mar 24, 2015

I think the confusion comes from the fact that there's a :datetime type that in ActiveRecord maps to a DateTime instance and to datetime column in most databases. The exception is that db2 and postgresql map them to timestamp columns. Regardless, Rails treats them as DateTime instances.

You have :datetime mapped to :time which all databases with the exception of sqlite and oracle map to a time type column. Oracle maps it to date and sqlite to datetime.

The more I think of this the more I'd have separate types for :time, :datetime, :timestamp, and :date that map to corresponding ActicveRecord types if your goal is to make it easy to have active record like behavior for types stored in an hstore. The list is ":primary_key, :string, :text, :integer, :float, :decimal, :datetime, :time, :date, :binary, :boolean"

Just my $.02 worth.

@jhirn
Copy link
Contributor

jhirn commented Mar 24, 2015

Appreciate the input. We'll talk it over and let you know what we come up
with.

On Tue, Mar 24, 2015 at 2:46 PM, snappa [email protected] wrote:

I think the confusion comes from the fact that there's a :datetime type
that in ActiveRecord maps to a DateTime instance and to datetime column in
most databases. The exception is that db2 and postgresql map them to
timestamp columns. Regardless, Rails treats them as DateTime instances.

You have :datetime mapped to :time which all databases with the exception
of sqlite and oracle map to a time type column. Oracle maps it to date and
sqlite to datetime.

The more I think of this the more I'd have separate types for :time,
:datetime, :timestamp, and :date that map to corresponding ActicveRecord
types if your goal is to make it easy to have active record like behavior
for types stored in an hstore. The list is ":primary_key, :string, :text,
:integer, :float, :decimal, :datetime, :time, :date, :binary, :boolean"

Just my $.02 worth.


Reply to this email directly or view it on GitHub
#62 (comment)
.

@snappa
Copy link
Author

snappa commented Mar 24, 2015

I'm good with another type. If you add :timestamp for example just make it clear in the documentation what is expected to be stored there. The same goes for the :datetime type as it currently returns a Time instance and not a DateTime as the name implies. This can be solved with documentation and a new type. Your documentation does show the use of Time for datetime but I glossed over it thinking that since this was ActiveRecord column representation I made the assumption it was the same types based on name :datetime

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

No branches or pull requests

3 participants