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

Provide option to interpret dates as UTC #783

Open
nareshbhatia opened this issue May 18, 2015 · 11 comments
Open

Provide option to interpret dates as UTC #783

nareshbhatia opened this issue May 18, 2015 · 11 comments

Comments

@nareshbhatia
Copy link

Looking at #510, it appears that dates used to be interpreted as UTC, but then there was a decision made to interpret them as system time. In my experience interpreting dates and timestamps in system timezone is very fragile. Even having to set the system clock to UTC is not always possible. It's an even bigger problem when the application needs to handle multiple timezones.

The rule of thumb we use is to always use UTC internally. Only the user interface layer deals with local timezone and converts it to UTC on input and local timezone on output. This allows the app to be extremely portable - no need to worry about system time or local timezone of the client machine.

I was wondering if there is a way to provide an option to interpret dates as UTC or system timezone. This will make sure that backward compatibility is maintained but make the library flexible to cater for different use cases.

@lroal
Copy link

lroal commented Jun 10, 2015

I agree with @nareshbhatia, dates / timestamps should not worry about system time or local timezone of the client machine.

@lroal
Copy link

lroal commented Jun 10, 2015

Hmm..thinking more about it...if we have a date with timezone, it really doesn't matter if date object is utc or system timezone. But with for date without timezone, it could be reasonable to have it in utc.
There should be an option though.

@brianc
Copy link
Owner

brianc commented Jun 10, 2015

If you'd like to provide the ability to provide an option in a pull request I'll happily take a look at it. The current suggested fix is to implement your own custom date/time parser and register it with pg-types.

@bendrucker
Copy link
Contributor

The correct way to handle dates is in the system timezone. That's the behavior of the Date constructor.

I think the best solution here would be a separate package rather than an option:

require('node-postgres-utc')(require('node-postgres').types)

Then override the parser there.

@brianc
Copy link
Owner

brianc commented Jun 10, 2015

+1 for that, absolutely.

On Wed, Jun 10, 2015 at 1:10 PM, Ben Drucker notifications@github.com
wrote:

The correct way to handle dates is in the system timezone. That's the
behavior of the Date constructor.

I think the best solution here would be a separate package rather than an
option:

require('node-postgres-utc')(require('node-postgres').types)

Then override the parser there.


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

@abenhamdine
Copy link
Contributor

Hmmm... just bumped into the same problem.
Any new on this issue ?
I'm surprised it's wasn't already solved because it seems pretty probable that everyone run into this...

@teburd
Copy link

teburd commented Jan 20, 2016

I just ran in to this the other day and ended up casting my date column types to text just to avoid the weird situation of postgres dates getting converted to js Date objects, where they have a bunch of added and incorrect information about time and timezone, especially when serializing these value to json.

My vote for how node-postgres deals with dates is just treat them as plain strings when decoding them. Its safer than the alternative, and easy enough to convert the plain strings to Date objects if desired.

@luggage66
Copy link

I think I am having a related issue. I am storing date (no time) in postgres and am getting a different date in windows. Edit: This was a previous version of pg-types, actually. 1.8.0 changed the behavior with: brianc/node-pg-types@468dfda

// pg-types 1.8+
client.query("SELECT '2016-01-01'::date as date", ...); // Thu Dec 31 2015 19:00:00 GMT-0500 (Eastern Standard Time)

//  pg-types 1.7
client.query("SELECT '2016-01-01'::date as date", ...); // Fri Jan 1 2016 00:00:00 GMT-0500 (EST)

@spollack
Copy link
Contributor

i just spent some time digging into this, since we use the postgres date column type in our schema, and our code treats all dates and times as UTC.

In our test and production environments, everything works great, since the machines are running in UTC. However, locally, where our machines are in various timezones, you get rather unexpected results. For example, running on my machine in PST:

var testDate = new Date('2016-02-22T00:00:00.000Z');
var resultDate = db.query("select $1::date as result", [testDate], _).rows[0].result;
console.log(testDate.toString()); // "Sun Feb 21 2016 16:00:00 GMT-0800 (PST)"
console.log(resultDate.toString()); // "Sat Feb 20 2016 16:00:00 GMT-0800 (PST)"

So, round-tripping a javascript Date in and out of a postgres date can give you a different result (the wrong day, i.e. a different number of milliseconds since the epoch).

The specific problem here has to do with the translation of input data that happens in pg's prepareValue function. (not the output data translation via pg-types.)

I'm going to make a fix locally, but @brianc if you are open to it i can open a PR to make this behavior configurable. I.e. optionally parse input javascript Dates as UTC instead of as local time with offset in prepareValue. Please let me know your thoughts.

@jonahbron
Copy link

This is a good option, it really needs to be configurable. As of now, it cannot be changed 👎 .

@guikubivan
Copy link

Setting that default did not work for, but this did in case anybody else is lost: To be explicit, do something similar to this post: #1746 (comment)

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

No branches or pull requests

10 participants