brianc / node-postgres

PostgreSQL client for node.js.
https://node-postgres.com
MIT License
12.16k stars 1.21k forks source link

Wrong timestamp to JavaScript date conversion #993

Open pinggi opened 8 years ago

pinggi commented 8 years ago
  1. saved 20:00 to 'timestamp without time zone' column in postgres db
  2. queried 20:00 by pgAdmin III
  3. queried 18:00 by node-postgres

node-postgress probably converts timestamp to Date object as it would be local time. However it is UTC time and should be treated like that if there is no timezone in the db.

langpavel commented 8 years ago

Again time issue :-) Perhaps it should be perfectly documented on wiki

joskuijpers commented 8 years ago

I do suggest to never use 'without time zone' but always with time zone. It does solve your problem as well.

Could you supply tests that expose this problem?

cressie176 commented 8 years ago

Also encountered this, but not sure it's a bug with node-postgres. Steps to reproduce...

describe.only('TZ Bug', () => {

    before((done) => {
        async.series([
            pool.query.bind(pool, 'DROP TABLE IF EXISTS tzbug'),
            pool.query.bind(pool, "CREATE TABLE IF NOT EXISTS tzbug (stamp timestamp NOT NULL);")  
        ], done)            
    })

    it('should return the same date that was written', (done) => {
        pool.query("INSERT INTO tzbug (stamp) values ('2016-07-15T06:00:00.000Z')", (err) => {
            assert.ifError(err)
            pool.query('SELECT stamp FROM tzbug', (err, result) => {
                assert.equal(result.rows[0].stamp.toISOString(), '2016-07-15T06:00:00.000Z')
                done()
            })
        })
    })
})

Output

  1) Import Store TZ Bug should return the same date that was written:
     Uncaught AssertionError: expected '2016-07-15T05:00:00.000Z' to equal '2016-07-15T06:00:00.000Z'

I suspect that because the column type of timestamp (without timezone) then the timestamp is stored as a 'local' value. When node-postgres reads it back there is no timezone info, and so creates the date using the default timezone of the machine running node.

Changing the column definition to (stamp timestamp with time zone NOT NULL) and the test passes.

joskuijpers commented 8 years ago

Right, and that is why you should always use 'with timezone'

astitt-ripple commented 7 years ago

I've run into this bug as well. I did some investigation and it appears the root issue is that postgres-date library used by node-postgres does not respect the database's timezone setting when parsing timestamp without time zone fields. Instead, it incorrectly uses the javascript interpreter's local timezone, which is not guaranteed to be the same as the timezone setting postgres uses internally.

The postgres manual is unfortunately not very direct about what how timestamp without time zone are interpreted, but at the end of the section on timestamps (https://www.postgresql.org/docs/9.1/static/datatype-datetime.html) there is some clarification:

Conversions between timestamp without time zone and timestamp with time zone normally assume that the timestamp without time zone value should be taken or given as timezone local time.

Therefore, for the driver to be consistent with the behavior of postgres, the node-postgres timestamp parsing should use that same timezone setting for timestamp without time zone.

The bug happens in the following code from the postgres-date module, which is actually used for both timestamp datatypes:

  var offset = timeZoneOffset(isoDate)
  if (offset != null) {
    var utc = Date.UTC(year, month, day, hour, minute, second, ms)
    date = new Date(utc - offset)
  } else {
    date = new Date(year, month, day, hour, minute, second, ms)
  }

In the above code, for a timestamp without time zone datatype, timeZoneOffset() will return undefined, causing the flow to enter the else clause. Calling new Date(year, month, day, hour, minute, second, ms) will then apply the operating system's local timezone offset. What should happen is the offset should be inferred from the database settings (eg current_setting('TIMEZONE')) instead, since the two are not guaranteed to be the same.

That way, if for example, someone wants to forego timezone's in their application's database entirely and work in exclusively UTC (or use a different timezone then their operating system), they can set their database timezone setting to UTC, and timestamp without time zone will work consistently for both operations performed inside queries, and timestamps returned from the driver for that datatype.

alter database database_name set timezone to 'UTC';

I've found the following works as a simple workaround for making timestamp without time zone be interpreted as UTC (assuming the default postgres timestamp output formatting is used) when initializing the driver at runtime:

  pg.types.setTypeParser(1114, function(stringValue) {
    return new Date(stringValue + 'Z')
  })

Adding the zero offset 'Z' suffix to the postgres default timestamp output format is sufficient to convince new Date to interpret the date as UTC. (The special value 1114 is the oid for timestamp without time zone (documented here https://doxygen.postgresql.org/include_2catalog_2pg__type_8h.html#a1ac4c664545022e308e9d78a040114a9))

jlumia commented 6 years ago

MY GOD! I was losing my mind. Many, many expletives later, I cannot thank you enough. @astitt-ripple

Telokis commented 5 years ago

@astitt-ripple Would it be possible to submit a PR or open an issue on https://github.com/bendrucker/postgres-date? This doesn't seem to be a difficult one.

salatielosorno commented 4 years ago

I've run into this bug as well. I did some investigation and it appears the root issue is that postgres-date library used by node-postgres does not respect the database's timezone setting when parsing timestamp without time zone fields. Instead, it incorrectly uses the javascript interpreter's local timezone, which is not guaranteed to be the same as the timezone setting postgres uses internally.

The postgres manual is unfortunately not very direct about what how timestamp without time zone are interpreted, but at the end of the section on timestamps (https://www.postgresql.org/docs/9.1/static/datatype-datetime.html) there is some clarification:

Conversions between timestamp without time zone and timestamp with time zone normally assume that the timestamp without time zone value should be taken or given as timezone local time.

Therefore, for the driver to be consistent with the behavior of postgres, the node-postgres timestamp parsing should use that same timezone setting for timestamp without time zone.

The bug happens in the following code from the postgres-date module, which is actually used for both timestamp datatypes:

  var offset = timeZoneOffset(isoDate)
  if (offset != null) {
    var utc = Date.UTC(year, month, day, hour, minute, second, ms)
    date = new Date(utc - offset)
  } else {
    date = new Date(year, month, day, hour, minute, second, ms)
  }

In the above code, for a timestamp without time zone datatype, timeZoneOffset() will return undefined, causing the flow to enter the else clause. Calling new Date(year, month, day, hour, minute, second, ms) will then apply the operating system's local timezone offset. What should happen is the offset should be inferred from the database settings (eg current_setting('TIMEZONE')) instead, since the two are not guaranteed to be the same.

That way, if for example, someone wants to forego timezone's in their application's database entirely and work in exclusively UTC (or use a different timezone then their operating system), they can set their database timezone setting to UTC, and timestamp without time zone will work consistently for both operations performed inside queries, and timestamps returned from the driver for that datatype.

alter database database_name set timezone to 'UTC';

I've found the following works as a simple workaround for making timestamp without time zone be interpreted as UTC (assuming the default postgres timestamp output formatting is used) when initializing the driver at runtime:

  pg.types.setTypeParser(1114, function(stringValue) {
    return new Date(stringValue + 'Z')
  })

Adding the zero offset 'Z' suffix to the postgres default timestamp output format is sufficient to convince new Date to interpret the date as UTC. (The special value 1114 is the oid for timestamp without time zone (documented here https://doxygen.postgresql.org/include_2catalog_2pg__type_8h.html#a1ac4c664545022e308e9d78a040114a9))

Here is the snapshot about this documentation

https://web.archive.org/web/20160613215445/http://doxygen.postgresql.org/include_2catalog_2pg__type_8h_source.html#l00507

I don't know why this still has not been fixed it if the documentation has already changed. I think this is using a library deprecated or something like that