brianc / node-postgres

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

Setting `intervalstyle = 'iso_8601'` causes intervals to be returned with all 0s #3156

Open Zevgon opened 7 months ago

Zevgon commented 7 months ago

pg version: 8.11.3

How to reproduce:

Create a table with an interval field and add a row:

CREATE TABLE my_table (
    my_interval_field INTERVAL
);
INSERT INTO my_table (my_interval_field) VALUES ('3 months');

Run the following statements:

import { Client } from 'pg';

const pgClient = new Client(yourConfig);
await pgClient.connect();
await pgClient.query(`SET intervalstyle = 'iso_8601'`);
const result = await pgClient.query('SELECT my_interval_field FROM my_table');
const interval = result.rows[0].my_interval_field;
console.log(interval); // PostgresInterval {} <-- should indicate an interval of 3 months, i.e. "PostgresInterval { years: 0, months: 3, days: 0, hours: 0, minutes: 0, milliseconds: 0 }"
console.log(interval.toPostgres()); // '0' <-- should be '3 months'
console.log(interval.months); // 0 <-- should be 3

Intervals are returned correctly if you remove the SET intervalstyle line.

Commentary

IIUC this is because node-postgres uses postgres-interval under the hood, and postgres-interval doesn't know how to handle ISO 8601 values. This is called out in their docs:

If you have changed [intervalstyle](https://www.postgresql.org/docs/current/runtime-config-client.html#GUC-INTERVALSTYLE), you will need to set it back to the default

You can see this behavior more directly by going to npm playground and running the following snippet:

const parse = require('postgres-interval');
const interval1 = parse("P3M");
console.log(interval1);
const interval2 = parse("3 months");
console.log(interval2);

I'm filing an issue here rather than in postgres-interval because it's debatable what the expected behavior should be for postgres-interval users, but it's more clear for node-postgres users that the intervalstyle shouldn't affect the return value. Let me know if I'm off base though, and I can file this ticket there instead

Workarounds

Open to suggestions. In my team's case, we can't just remove the SET intervalstyle statement, since we run that statement it in the pool's on-connect listener, and we want iso_8601 for all our queries. The only option I can think of would be to create a custom type parser:

import { types, Client } from 'pg';

// Also should work with `new Pool`
const pgClient = new Client({
  ...yourConfig
  types: {
    getTypeParser: (oid) => (val) => {
      if (oid === types.builtins.INTERVAL) {
        // custom logic
      }
      return types.getTypeParser(oid)(val as string);
    }
  }
});
brianc commented 7 months ago

Yeah that makes sense....I think ideally it'd be handled by postgres-interval since that's what's doing the parsing there (I assume via pg-types)...but you're right a custom type parser would also get you by in this situation. I'll think about other things that might help though and get back to ya in some time. Finishing up a contract engagement ATM and am super busy, but should be free soon!

Zevgon commented 7 months ago

Sounds great thanks for the quick reply!

benjamin-chang commented 4 months ago

Our workaround is to cast interval to text in the query.