brianc / node-postgres

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

lose precision when querying large double precision values #730

Open yacoobk opened 9 years ago

yacoobk commented 9 years ago

When querying large double precision values they appear to be rounded. e.g. the following values are all returned as '2.09999999769e+15' 2099999997689999 2099999997690000 2099999997690001

I believe that is because _extra_floatdigits should be set to 3 (ideally at protocol level when a connection to the DB is opened): http://www.postgresql.org/docs/9.3/static/runtime-config-client.html

This appears to be how the PostgreSQL JDBC driver works: https://github.com/pgjdbc/pgjdbc/search?utf8=%E2%9C%93&q=extra_float_digits

PostgreSQL JDBC used to do it at SQL level. This isn't ideal, but I think preferable to getting incorrect values. https://github.com/pgjdbc/pgjdbc/issues/168 http://stackoverflow.com/questions/24680696/set-extra-float-digits-3-in-postgresql

sehrope commented 9 years ago

I've put together a WIP patch that corrects this behavior. It adds an extra_float_digits connection parameter defaulted to 3. Branch for it is here: https://github.com/sehrope/node-postgres/tree/extra-float-digits

Couple of points/issues as it's definitely not ready to be merged in:

  1. It requires a PR to pg-connection-string for parsing the extra_float_digits parameter
  2. It breaks node-postgres connecting to older (<=8.4) databases unless they explicitly disable setting extra_float_digits (as it sends the value in the startup packet and <=8.4 doesn't accept that).
  3. It breaks a number of existing type-coercion tests
  4. Right now a value of "0" is used to disable the setting ... not sure I really like that.

Real question is: _Should we be making this the default (i.e. extra_floatdigits=3) vs making it an explicit setting?

Normally, I'd side with doing our best to maintain backwards compatibility. The annoying part with that is it means everybody would need to add an ?extra_float_digits=3 to every connection URL. I'm pretty sure most people are using node-postgres with a 9.0+ server so they'd all be fine. Only people connecting to older databases would be impacted. It could go into a non-compat 5.x release.

Either way, I'd like however it's implemented to have the value sent in the startup packet vs being sent afterwards in a separate "SET extra_float_digits = ..." query. The JDBC driver does that for fallback handling and it's both slower (extra roundtrip for connection setup) and annoying (cruft in pg_stat_activity).

phated commented 9 years ago

@brianc any thoughts on this?

yacoobk commented 9 years ago

@brianc any comment at all?

cjnqt commented 8 years ago

I think this one deserves some attention, maybe @brianc would like to chime in?

brianc commented 7 years ago

node-postgres shouldn't be parsing numbers longer than JavaScript can handle into number objects by default. They should return as strings unless that behavior is manually over-ridden in your app. If this is still an issue please open an issue at http://github.com/brianc/node-pg-types

brianc commented 7 years ago

FYI: I think this has been fixed.

xajhqffl commented 6 years ago

Can I change extra_float_digits to 6, if available? @brianc you said this has been fixed?

charmander commented 6 years ago

@Fangmingdu It hasn’t been fixed, but you can set extra_float_digits manually: https://github.com/brianc/node-postgres/issues/1393#issuecomment-319308021

Should we be making this the default (i.e. extra_float_digits=3) vs making it an explicit setting?

Yes, I think it should be a default.