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

Sequelize not handling datetimes correctly when backed by TIMESTAMP WITHOUT TIME ZONE #3000

Open
BadIdeaException opened this issue Jan 25, 2015 · 25 comments
Labels
dialect: postgres For issues and PRs. Things that involve PostgreSQL (and do not involve all dialects). topic: dates & times For issues and PRs. Things that involve dates and times. type: bug

Comments

@BadIdeaException
Copy link

The issue

Given the following

    var Entry = sequelize.define("Entry", {
         _id: {
            type: DataTypes.INTEGER,
            primaryKey: true
        },
        datetime: {
            type: DataTypes.DATE,
            allowNull: false
        } ...

the following test fails:

it('should not mess with the time', function() {
    var datetime = new Date(Date.now());
    datetime.setMilliseconds(0);

    var entry = {
        _id: 0,
        datetime: datetime
    };

    return expect(Entry
            .create(entry)
            .then(function(entry) { return Entry.find(entry._id); })
            .then(function(entry) { return entry.datetime.getTime(); }))
            .to.eventually.equal(entry.datetime.getTime());
});

The values differ by an hour (I'm on CET).

In the database, the values are stored as TIMESTAMP(0) WITHOUT TIME ZONE, which I can't change for legacy reasons.

The workaround

For the time being, I'm working around it by adding

require('pg').types.setTypeParser(1114, function(stringValue) {
    return new Date(stringValue.substring(0, 10) + 'T' + stringValue.substring(11) + '.000Z');
});

Why I'm posting

So having found a workaround, why am I posting this? For two reasons, mainly:

  1. I've spent hours trying to find out what was going wrong and how to fix it, and had a hard time finding help through google. I'm hoping it'll help somebody down the road
  2. It would be nice to see Sequelize support this without requiring ugly hacks. Because other than this, it's pretty awesome, y'know. :)
@janmeier
Copy link
Member

janmeier commented Feb 1, 2015

The workaround you showed could prettymuch verbatim be added to the pg connection manager :)

The only change would be that it should use this.sequelize.options.timezone (https://github.com/sequelize/sequelize/blob/master/lib/dialects/mysql/connection-manager.js#L31) instead of always Z

@janmeier
Copy link
Member

janmeier commented Feb 8, 2015

@chris-33 Are you up for providing a PR? As I said, this should be a pretty easy fix, you've already found the right way to fix this, so it just needs to be added to the connector manager and have a test added. This only needs to be added for postgres, since other dialects already support columns without timestamp (since they don't have TIMESTAMP WITH TIMEZONE). So you won't need to look into the other dialects

@BadIdeaException
Copy link
Author

Hey, glad my workaround is proving useful. I've forked and am looking into it for the right places to add my code, but I'm going through a lot of stuff in my professional life right now (I only develop for a hobby), so it'll probably be a long time before I can contribute. Like, a long time. So if you need it anytime soon, you'll probably be quicker off adding the changes yourself. ;) If on the other hand it can wait, I'll be happy to.

@zaptree
Copy link

zaptree commented Aug 23, 2015

This seems like a pretty big bug. Sequelize returns wrong dates when using postgres. I'm curious how nobody else has noticed this. Also the code suggested by @chris-33 is a bit wrong since it only works if there are no milliseconds. Switching it to this should work fine:

//will now work if stringValue has format like this: '0000-00-00 00:00:00.000' 
require('pg').types.setTypeParser(1114, function(stringValue) {
    return new Date(stringValue.substring(0, 10) + 'T' + stringValue.substring(11) + 'Z');
});

@mickhansen
Copy link
Contributor

@zaptree Most users probably use timezones or save it in UTC since postgres uses UTC internally. Otherwise you have to set your connection timezone to whatever timezone you're storing data from.

@zaptree
Copy link

zaptree commented Aug 23, 2015

@mickhansen you are probably correct in that most users use timestamp with timezone instead of without. I'm stuck with a db I can't change so I'm having some troubles. I will probably have to ask the db admins to change this if possible.

I am not sure I understand when you suggest saving it in UTC though? I thought that was the problem. Sequelize converts the date to UTC time and saves it in the db, but because the fields are timestamp without timezone when the pg module reads the data from the database it wrongly assumes that the date is in local time and creates a date object that is using the UTC time as if it was local time.

Also just to note that the fix suggested by @chris-33 will not actually work properly because it assumes that the sequelize option for timezone is left at the default '+00:00'. If that is changed to something different then converting the stringValue to a UTC date string is no longer valid. What is needed is to take into account the timezone setting and the timezone offset and then convert the returned date appropriately.

@mickhansen
Copy link
Contributor

@zaptree You should be able to set the sequelize timezone option to whatever timezone your client/PG server is in and it should work as intended - @janmeier knows more about this than me though.

@zaptree
Copy link

zaptree commented Aug 24, 2015

@mickhansen I tried that already but what it does is it saves the data in the db in your local timezone. I need to save it in UTC since existing data is already like that (this is a db used by a legacy system) plus the different servers accessing the db are on different timezones. I can see that the only sensible solution is to change to timestamp with timezone.

I really appreciate your help and quick responses.

@janmeier
Copy link
Member

@zaptree You are right about the fix mentioned here only working for UTC - that's why I mentioned options.timezone previously.

Your diagnosis of the problem is correct - the problem is exactly that sequelize converts the date to UTC, but when its returned, node-pg converts it into a date in the local timezone of the machine. I'm going to fix this in #4186, which allows for a more abstract way to hook into the parsing system than playing around with pg-types

@stale stale bot added the stale label Jun 29, 2017
@stale
Copy link

stale bot commented Jun 29, 2017

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If this is still an issue, just leave a comment 🙂

@forrest-akin
Copy link

Could someone clarify exactly how to implement the workaround? Where does this code go?

@stale stale bot removed the stale label Jun 30, 2017
@stale stale bot added the stale label Aug 29, 2017
@stale
Copy link

stale bot commented Aug 29, 2017

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If this is still an issue, just leave a comment 🙂

@stale stale bot closed this as completed Sep 5, 2017
@Gpia
Copy link

Gpia commented Mar 14, 2018

no problem resolved, but issue closed.

@sanjaykhanal
Copy link

The problem still exists.

@maxwellsmart84
Copy link

Bump

@papb
Copy link
Member

papb commented Aug 2, 2019

Hello, I will reopen this, but since the original code is 4 years old (and not self-contained), I need someone to provide a SSCCE (also known as MCVE/reprex) that can be simply copy-pasted and executed to see the problem.

@papb papb reopened this Aug 2, 2019
@stale stale bot removed the stale label Aug 2, 2019
@papb papb added the status: awaiting response For issues and PRs. OP must respond (or change something, if it is a PR). Maintainers have no action label Aug 2, 2019
@papb papb added the topic: dates & times For issues and PRs. Things that involve dates and times. label Oct 30, 2019
@nestorvanz
Copy link

nestorvanz commented Dec 11, 2019

Solution for PostgreSQL

This seems like a problem with the module used to connect to PosgreSQL databases. I solved this issue changing the date formatter from pg library for date types:

const pg = require('pg');
pg.types.setTypeParser(1114, (str:string) => new Date((str.split(' ').join('T'))+'Z'));

I am running this code before create the database connection.

The explanation is:

  1. Split the plain string value from the database date column
  2. Join the result with the T to indicate the time to Date class
  3. Add the UTC time zone with the Z flag

@papb
Copy link
Member

papb commented Jan 16, 2020

@nestorvanz Thanks for sharing the workaround!!

@papb papb removed the status: awaiting response For issues and PRs. OP must respond (or change something, if it is a PR). Maintainers have no action label Jan 16, 2020
@papb papb added status: awaiting investigation dialect: postgres For issues and PRs. Things that involve PostgreSQL (and do not involve all dialects). labels Jan 16, 2020
@papb papb self-assigned this Jan 16, 2020
@claudia-ochoa
Copy link

Solution for PostgreSQL

This seems like a problem with the module used to connect to PosgreSQL databases. I solved this issue changing the date formatter from pg library for date types:

const pg = require('pg');
pg.types.setTypeParser(1114, (str:string) => new Date((str.split(' ').join('T'))+'Z'));

I am running this code before create the database connection.

The explanation is:

  1. Split the plain string value from the database date column
  2. Join the result with the T to indicate the time to Date class
  3. Add the UTC time zone with the Z flag

Where do i need to put this?

@nestorvanz
Copy link

Solution for PostgreSQL

This seems like a problem with the module used to connect to PosgreSQL databases. I solved this issue changing the date formatter from pg library for date types:

const pg = require('pg');
pg.types.setTypeParser(1114, (str:string) => new Date((str.split(' ').join('T'))+'Z'));

I am running this code before create the database connection.
The explanation is:

  1. Split the plain string value from the database date column
  2. Join the result with the T to indicate the time to Date class
  3. Add the UTC time zone with the Z flag

Where do i need to put this?

Hi @claudia-ochoa;

As I mention, this need to be set before create the database connection.

pg.types.setTypeParser(1114, (str:string) => new Date((str.split(' ').join('T'))+'Z'));

const dbConn = new Sequelize(...);

@osvaldokalvaitir
Copy link

I'm using mysql, do you have any solution?

SELECT * FROM _tables WHERE last_update < Tue Feb 09 2021 08:51:12 GMT-0300 (Horário Padrão de Brasília)

I wonder if there is an alternative because as a problem six years ago, no one has solved it yet.

How would people be using it?

I am grateful that the sequelize is free, but I am also disappointed to try to do something simple and have no solution for it.

I would like to make a pull request and help with this, but also for now I have no experience to contribute in this matter.

@Angatupyry
Copy link

Angatupyry commented Apr 7, 2021

If I do "SELECT now();" a it brings me the following answer in my postgres client 2021-04-07 10:41:50. It is the correct time according to my time zone.
However, when running it in node with sequelize the result is this

{
  "now": "2021-04-07T14:42:01.595Z"
}

I added to my config timezone: '-4:00', but see no change, any suggestions?

@Angatupyry
Copy link

Angatupyry commented Apr 9, 2021

Solution for PostgreSQL

This seems like a problem with the module used to connect to PosgreSQL databases. I solved this issue changing the date formatter from pg library for date types:

const pg = require('pg');
pg.types.setTypeParser(1114, (str:string) => new Date((str.split(' ').join('T'))+'Z'));

I am running this code before create the database connection.
The explanation is:

  1. Split the plain string value from the database date column
  2. Join the result with the T to indicate the time to Date class
  3. Add the UTC time zone with the Z flag

Where do i need to put this?

Hi @claudia-ochoa;

As I mention, this need to be set before create the database connection.

pg.types.setTypeParser(1114, (str:string) => new Date((str.split(' ').join('T'))+'Z'));

const dbConn = new Sequelize(...);

I'm still a bit confused about this.
What is the "string" value supposed to add?
I'd appreciate your help

@gxmxni
Copy link

gxmxni commented Dec 16, 2021

Experiencing this -- the workaround works. However, this issue has been open for 6 years. To me, this is quite critical functionality.

@jcmalek
Copy link

jcmalek commented Mar 29, 2022

As @nestorvanz noted, this is a problem in the upstream node-postgres library. This problem has been "fixed" in the further upstream node-postgres-types library. The upstream library now assumes UTC for TIMESTAMP WITHOUT TIME ZONE. However, node-postgres is still using a very old version of node-postgres-types. There is already an issue open to bump the version. You can upvote it or poke the author here:
brianc/node-postgres#2547.

For Sequelize to respect its "timezone" parameter, this issue would also still have to be fixed within Sequelize.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dialect: postgres For issues and PRs. Things that involve PostgreSQL (and do not involve all dialects). topic: dates & times For issues and PRs. Things that involve dates and times. type: bug
Projects
None yet
Development

No branches or pull requests