Unable to retrieve record using datetime/timestamp column. #3519

Closed
mattscilipoti opened this Issue Nov 4, 2011 · 17 comments

Comments

Projects
None yet
9 participants
@mattscilipoti

Rails 3.1.1

I have a table called :emails.

class CreateEmails < ActiveRecord::Migration
  def self.up
    create_table :emails do |t|
      t.datetime :sent_at
      ...
      t.timestamps
    end
  end
end

The generated structure:

CREATE TABLE emails (
    id integer NOT NULL,
    ...
    sent_at timestamp without time zone,
    created_at timestamp without time zone,
    updated_at timestamp without time zone

I am unable to retrieve a record from Email using ether of the datetime fields.

> Email.create!(:sent_at => Time.now)
=> #<Email id: 39, sent_at: "2011-11-04 14:21:57">  

> Email.where(:sent_at => "2011-11-04 14:21:57")
=> [] 

The same happens with created_at and updated_at.

I believe it is a question of precision.
It finds the email where :sent_at is between 14:21:57 and 14:21:58.

Shouldn't we be able to retrieve this record using our normally formatted datetime strings?

@mattscilipoti

This comment has been minimized.

Show comment
Hide comment
@mattscilipoti

mattscilipoti Nov 4, 2011

Confirmed: it is a precision issue. When I eliminate the fractional seconds, everything works as expected.

CREATE TABLE emails (
    id integer NOT NULL,
    ...
    sent_at timestamp(0) without time zone,
end

Unfortunately, rails migrations do not support precision for datetime fields.

change_column :emails, :sent_at, :datetime, :precision => 0 #ignore fractions of seconds

This does NOT add the precision to the timestamp field.

Since this current issue is questioning what rails' convention should be, I created a new ticket for the Migration bug (#3520).

Confirmed: it is a precision issue. When I eliminate the fractional seconds, everything works as expected.

CREATE TABLE emails (
    id integer NOT NULL,
    ...
    sent_at timestamp(0) without time zone,
end

Unfortunately, rails migrations do not support precision for datetime fields.

change_column :emails, :sent_at, :datetime, :precision => 0 #ignore fractions of seconds

This does NOT add the precision to the timestamp field.

Since this current issue is questioning what rails' convention should be, I created a new ticket for the Migration bug (#3520).

@ahawkins

This comment has been minimized.

Show comment
Hide comment
@ahawkins

ahawkins Apr 29, 2012

@steveklabnik can you close this? This is longer an issue, but more of a proposal for adding options to timestamp migrations in #3520.

@steveklabnik can you close this? This is longer an issue, but more of a proposal for adding options to timestamp migrations in #3520.

@steveklabnik

This comment has been minimized.

Show comment
Hide comment
@steveklabnik

steveklabnik Apr 29, 2012

Member

Your wish is my command ;)

Member

steveklabnik commented Apr 29, 2012

Your wish is my command ;)

@ahawkins

This comment has been minimized.

Show comment
Hide comment
@ahawkins

ahawkins Apr 29, 2012

Gotta make contributions to rails issues man :)

Gotta make contributions to rails issues man :)

@mattscilipoti

This comment has been minimized.

Show comment
Hide comment
@mattscilipoti

mattscilipoti May 2, 2012

Are you saying that is not an issue? Are you saying you can find a record via datetime using (default) ActiveRecord? Or are you saying that you can't, but you don't care because the submitter (me) didn't contribute a solution (in code).

I'm not convinced that #3520 is a solution to this. It seems strange that, even if AR did support precision correctly, Rails would ask people to use it to support a where clause and a datetime field. Doesn't it?

Are you saying that is not an issue? Are you saying you can find a record via datetime using (default) ActiveRecord? Or are you saying that you can't, but you don't care because the submitter (me) didn't contribute a solution (in code).

I'm not convinced that #3520 is a solution to this. It seems strange that, even if AR did support precision correctly, Rails would ask people to use it to support a where clause and a datetime field. Doesn't it?

@steveklabnik steveklabnik reopened this May 2, 2012

@steveklabnik

This comment has been minimized.

Show comment
Hide comment
@steveklabnik

steveklabnik May 2, 2012

Member

I closed because @twinturbo said it was no longer an issue, and since this was originally against 3.1, I took him for his word.

That said, yes, if your database has a column of the right type, AR finds it. Migrations provide a limit option which allows you to create the right kind of column.

Basically, I see this as a duplicate of #3250. @tenderlove or @jonleighton might disagree. Since you feel its an issue, and I'm not Mr. ActiveRecord, I'll re-open it and see what they say.

Member

steveklabnik commented May 2, 2012

I closed because @twinturbo said it was no longer an issue, and since this was originally against 3.1, I took him for his word.

That said, yes, if your database has a column of the right type, AR finds it. Migrations provide a limit option which allows you to create the right kind of column.

Basically, I see this as a duplicate of #3250. @tenderlove or @jonleighton might disagree. Since you feel its an issue, and I'm not Mr. ActiveRecord, I'll re-open it and see what they say.

tonywok added a commit to tonywok/rails that referenced this issue Jun 22, 2012

Allow precision option for postgresql datetimes
This patch addresses the difficulty of retrieving datetime fields. By default, the database holds a higher precision than the time as a String.

This issue is discussed at length at the following links:
- [#3519](rails#3519)
- [#3520](rails#3520)

Also, kudos to @mattscilipoti
@rafaelfranca

This comment has been minimized.

Show comment
Hide comment
@rafaelfranca

rafaelfranca Jun 22, 2012

Member

@mattscilipoti can you confirm if 96ce1f2 closes this issue?

Thanks.

Member

rafaelfranca commented Jun 22, 2012

@mattscilipoti can you confirm if 96ce1f2 closes this issue?

Thanks.

@ghost ghost assigned rafaelfranca Jun 22, 2012

@steveklabnik

This comment has been minimized.

Show comment
Hide comment
@steveklabnik

steveklabnik Sep 15, 2012

Member

@mattscilipoti ping! Any thoughts on 96ce1f2?

Member

steveklabnik commented Sep 15, 2012

@mattscilipoti ping! Any thoughts on 96ce1f2?

@mattscilipoti

This comment has been minimized.

Show comment
Hide comment
@mattscilipoti

mattscilipoti Sep 19, 2012

Sorry for the delay. Thanks for the ping.

You can say that commit 96ce1f2 solves this problem, if you have made the decision that, by convention, Rails developers can not retrieve a model by datetime. They will be able to, if they know enough to override convention by adding :precision => 0 in their migration.

I think that is the real question here. Yes, that commit makes it possible. But, it does not allow a developer to retrieve a model by datetime, by default.

I don't think that sounds like Rails.

To accomplish this, I think the postgres adapter should assign a default precision of 0 to datetime fields. Unfortunately, that means datetime fields will have a limited precision, by default. But, I assume that, those people that need a finer precision will be testing for it and correcting appropriately. They know that they need a special precision to handle their business needs. I expect that most developers would be surprised (like I was) to find they need to override the default precision in order to simply find a record.

What do you guys think?

Sorry for the delay. Thanks for the ping.

You can say that commit 96ce1f2 solves this problem, if you have made the decision that, by convention, Rails developers can not retrieve a model by datetime. They will be able to, if they know enough to override convention by adding :precision => 0 in their migration.

I think that is the real question here. Yes, that commit makes it possible. But, it does not allow a developer to retrieve a model by datetime, by default.

I don't think that sounds like Rails.

To accomplish this, I think the postgres adapter should assign a default precision of 0 to datetime fields. Unfortunately, that means datetime fields will have a limited precision, by default. But, I assume that, those people that need a finer precision will be testing for it and correcting appropriately. They know that they need a special precision to handle their business needs. I expect that most developers would be surprised (like I was) to find they need to override the default precision in order to simply find a record.

What do you guys think?

@PikachuEXE

This comment has been minimized.

Show comment
Hide comment
@PikachuEXE

PikachuEXE Jan 13, 2013

Contributor

I agree with @mattscilipoti
The convention should be the one that more people would use

I am not sure "looking up by time" or "using fractional seconds" is more common
It would be a document about explaining the decided convention

Contributor

PikachuEXE commented Jan 13, 2013

I agree with @mattscilipoti
The convention should be the one that more people would use

I am not sure "looking up by time" or "using fractional seconds" is more common
It would be a document about explaining the decided convention

@parndt

This comment has been minimized.

Show comment
Hide comment
@parndt

parndt May 17, 2013

Contributor

So what will we do?

Contributor

parndt commented May 17, 2013

So what will we do?

@vipulnsward

This comment has been minimized.

Show comment
Hide comment
@vipulnsward

vipulnsward Sep 2, 2013

Member

@steveklabnik @mattscilipoti would this be solved by simply documenting this behaviour? Or changing the default precision? Please let me know so I workout a fix for this.

Member

vipulnsward commented Sep 2, 2013

@steveklabnik @mattscilipoti would this be solved by simply documenting this behaviour? Or changing the default precision? Please let me know so I workout a fix for this.

@steveklabnik

This comment has been minimized.

Show comment
Hide comment
@steveklabnik

steveklabnik Sep 4, 2013

Member

I think documenting the current behavior is just fine, for now, and if someone wants to make the case for changing the default in 4.1, they can.

Member

steveklabnik commented Sep 4, 2013

I think documenting the current behavior is just fine, for now, and if someone wants to make the case for changing the default in 4.1, they can.

@JonRowe

This comment has been minimized.

Show comment
Hide comment
@JonRowe

JonRowe Mar 21, 2014

Contributor

Did anyone get a chance to document this then?

Contributor

JonRowe commented Mar 21, 2014

Did anyone get a chance to document this then?

@Silex

This comment has been minimized.

Show comment
Hide comment
@Silex

Silex May 13, 2014

This is not working for mysql versions over 5.6.4 which also supports fractional datetimes. And when I say not working, I mean that it's impossible to create a DATETIME(6) column in mysql.

[EDIT]: also, trying to save datetime with fractional values to the db doesn't seem to work

Silex commented May 13, 2014

This is not working for mysql versions over 5.6.4 which also supports fractional datetimes. And when I say not working, I mean that it's impossible to create a DATETIME(6) column in mysql.

[EDIT]: also, trying to save datetime with fractional values to the db doesn't seem to work

@Silex

This comment has been minimized.

Show comment
Hide comment
@Silex

Silex May 13, 2014

Apparently some of it is fixed in #14359

Silex commented May 13, 2014

Apparently some of it is fixed in #14359

@rafaelfranca

This comment has been minimized.

Show comment
Hide comment
@rafaelfranca

rafaelfranca May 13, 2014

Member

I'm closing so

Member

rafaelfranca commented May 13, 2014

I'm closing so

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