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

SELECT COUNT() return typeof string #378

Closed
hoopsoul opened this issue Jun 25, 2013 · 13 comments · Fixed by #427
Closed

SELECT COUNT() return typeof string #378

hoopsoul opened this issue Jun 25, 2013 · 13 comments · Fixed by #427

Comments

@hoopsoul
Copy link

using postgres 9.2 and pg moudle 2.0.0

script like following, the count type is number in 1.1.3, but string in 2.0.0, is it by design?

pgClient.query "SELECT COUNT(*) AS cnt FROM abc;", (err, results) ->
    count = results.rows[0].cnt
    console.log typeof(count)
    pgClient.end()

Thanks
Qiang

@caseywebdev
Copy link
Contributor

Getting the same problem with a string being returned for an id column (big serial). I'm using the native adapter and downgrading back to 1.3.0 works.

@brianc
Copy link
Owner

brianc commented Jun 25, 2013

Unfortunately those are going to return strings now since the return type of the COUNT operator is bigint which, by definition, can exceed the maximum value of an int in JavaScript. I know it's extremely unlikely you'd ever have more than one hundred billion rows or whatever the actual Number.MAX_VALUE is, but it's the only way to be correct. We've gone back and forth and back and forth on number type conversions, and converting numbers to integers when they can possibly loose precision is the wrong way to do things.

It really sucks it introduces such a large backwards compatibility issue, but that's why I bumped the major version. I would suggest if you know you're never going to have more than Number.MAX_VALUE data in your database you provide a custom type parser for the number types you'd like to receive as integers. I've created an "example" module for reference here:

https://github.com/brianc/node-pg-parse-float

@brianc
Copy link
Owner

brianc commented Jun 25, 2013

One other thing. If you would like I can produce a module which "reverts" the type parsing changes of v2.x by parsing everything it possibly can parse to numbers. This would be pretty much identical to pg-parse-float except it would parse all integer types (even if they could potentially overflow) to integers. I was thinking this could check the string length of the unparsed number before parsing it and throw an error if the string length could potentially indicate the unparsed contents are greater than Number.MAX_VALUE.

example:

var numberFromPostgres = "1234567890";

var parse = function(stringNumber) {
  //check against some length that makes sense
  //any whole number of more than 20 digits is likely to 
  //exceed the maximum number value in javascript
  if(stringNumber.length > 20) {
    throw new Error('Cannot parse ' + stringNumber + ' to an integer. Sorry about JavaScripts lack of 64bit numbers');
  }
  return parseInt(stringNumber);
};

parse(numberFromPostgres);

Does that make sense? I was planning on doing this in my code base once I upgraded to v2.0 (sometime after nodeconf).

@caseywebdev
Copy link
Contributor

Damn, that's really unfortunate but the choice makes sense. As for your check, it could simply be

var parse = function (str) {
  var n = +str;
  if (n == str) return n;
  throw new Error('Cannot parse ' + str + ' to an integer because it is too big');
};

@brianc
Copy link
Owner

brianc commented Jun 25, 2013

Sweet. Good call. I'll work up this module so I can un-break backwards compatibility as an opt-in module.

@dresende
Copy link

mysql driver had this problem also (people complaining about twitter ids). We added an option. Default was to parse everything as numbers. The few people needing large IDs would activate that option and get every number as string (and would probably use lib BigNumber to work with them).

@brianc
Copy link
Owner

brianc commented Jul 23, 2013

@dresende thanks for your help! Yeah making it an option probably would have been a good move. I'm going to work on an add-on module and document how to use it to parse everything as numbers instead of text.

@spollack
Copy link
Contributor

spollack commented Aug 8, 2013

Hi @brianc, checking in on status of the add on module to parse everything as numbers? (i'm using node-pg-parse-float already, so its really for bigints as returned from functions like count(), as discussed in this issue). thanks for all the good work here!

@trevorwilliams
Copy link

I'm in the same boat as spollack. Eager to upgrade to the latest version of pg, but will have to wait until the add-on module is ready. Thanks, @brianc - you've been doing an amazing job on this lib.

@brianc
Copy link
Owner

brianc commented Aug 29, 2013

@trevorwilliams thanks. :) Shipping a new minor version now with the feature you're looking for! 🚢

@spollack
Copy link
Contributor

Yeah! thanks @brianc.

@strk
Copy link
Contributor

strk commented Nov 19, 2013

Should this be considered closed, then ?

@brianc
Copy link
Owner

brianc commented Nov 19, 2013

I think so, yeah. Sometimes I like to leave these kinda issues open as informative. But...github search is a lot better than it used to be so that's probably a waste now. 😄

@brianc brianc closed this as completed Nov 19, 2013
twistedvisions pushed a commit to twistedvisions/anaximander that referenced this issue Jan 20, 2014
…S ints, node-postgres handles them as strings. Opting-in to handling them as ints because it isn't really a problem here. This makes the event counts be handled in order properly.

See brianc/node-postgres#378
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants