parseFloat unsuitable for NUMERIC type

[chrisfarms]

I think columns of type NUMERIC should be returned as Strings rather than native javascript numbers.

The NUMERIC type is for arbitrary precision numbers. Numbers which will fall way outside of what the native max size from V8 will be. Currently if I have huge integers in my database, then fetch them out with node-postgres, it gets run through parseFloat, truncated and I'm left with a different number:

var pg = require('pg');

var crazyBigInt = '294733346389144765940638005275322203805';

pg.connect("tcp://localhost/test", function(err,client){ 
    if(err) throw err; 
    // make an example table with numeric col of arbitary precision
    client.query("CREATE TABLE example (id numeric)");
    client.query("INSERT INTO example (id) VALUES ($1)", [crazyBigInt]);
    client.query("SELECT * FROM example", function(err,res){
        var n = res.rows[0].id;
        // check string equality
        console.log("Expected result from numeric column to be", crazyBigInt, "but got", n.toString());
        // clean up
        client.query("DROP TABLE example", function(err,res){
            process.exit();
        });
    });
});

I think including some kind of BigInt implementation within node-postrges would just cause more problems, so I think it would be best if it just always returned strings for numeric columns.

[chrisfarms]

For anyone else is in this situation here is some nice unstable duct tape:

////////////////////////////////////////////////////////
// MONKEY PATCH PG TO HANDLE NUMERIC COLS AS STRINGS
var pgTypes = require('pg/lib/types'),
    pgGTP = pgTypes.getTypeParser;
pgTypes.getTypeParser = function(oid, format){ 
    if(format=='text' && oid==1700)
        return String;
    return pgGTP.call(this, oid, format); 
};
////////////////////////////////////////////////////////

[freewil]

This seems like a reasonable solution, you could always take the string and use some sort of bigint lib outside of node-postgres if you needed to.

[brianc]

yeah I think this is a good idea. Question: are numerics used as the sort of 'default' for number columns? I generally stick to int. I could see this breaking backward compatibility of sorts if many people already depend on numerics coming out as javascript numbers. What if the converting function for numerics returned a string only if it was too large for the javascript number type? That's kinda annoying maybe?

Also, some pull request I didn't do enough research into has removed the ability for users of node-postgres to easily register their own type converters. Having to do the monkey patch you did above is nasty. I'll re-introduce an easier way to override the built-in converters.

[chrisfarms]

Agree with the backward compat issue. Having a documented API to register converters would be ideal.

[freewil]

What if the converting function for numerics returned a string only if it was too large for the javascript number type? That's kinda annoying maybe?

I think just leaving the existing behavior of parseFloat() might be ok in the interest of backward-compatibility, with the added ability to override the converters.

Maybe in the next major release change it to return a string by default?

[brianc]

I agree with both of you. I'll punch a hole back where there should be one to override and provide custom converters, and put a warning in whenever a numeric type comes back containing a size greater than Number.MAX_VALUE or less than Number.MIN_VALUE saying it will be deprecated in a future release, and you better check yourself before your wreck yourself so they say. :)

[brianc]

Okay - I added back the ability to override type converters. You can see an example override here:

https://github.com/brianc/node-postgres/blob/master/test/integration/client/huge-numeric-tests.js#L6

I've also included a warning to stderr if the numeric value retrieved from the database would be truncated if it were passed through parseFloat. This should allow for deprecation & eventually replacing the built-in numeric converter with one having more "smarts."

In the meantime, it's totally possible to provide your own numeric type conversion. Thanks for the help & discussion.

[brianc]

The setTypeParser function is exported, so you should be able to access it
by requiring types.js

https://github.com/brianc/node-postgres/blob/master/lib/types.js#L43

On Tue, Jul 24, 2012 at 7:19 PM, Lalit Kapoor <
reply@reply.github.com

wrote:

@brianc is the setTypeParser function (in type.js) being exposed
anywhere? I don't think I'm able to access it without modifying the module
itself. Thanks.

---

Reply to this email directly or view it on GitHub:
#107 (comment)

[lalitkapoor]

Indeed you are correct, I realized that shortly after my comment and then promptly deleted my comment :) Thanks for the feedback though!

Cheers,
Lalit

On Tuesday, July 24, 2012 at 10:37 PM, Brian Carlson wrote:

The setTypeParser function is exported, so you should be able to access it
by requiring types.js

https://github.com/brianc/node-postgres/blob/master/lib/types.js#L43

On Tue, Jul 24, 2012 at 7:19 PM, Lalit Kapoor <
reply@reply.github.com (mailto:reply@reply.github.com)

wrote:

@brianc is the setTypeParser function (in type.js) being exposed
anywhere? I don't think I'm able to access it without modifying the module
itself. Thanks.

---

Reply to this email directly or view it on GitHub:
#107 (comment)

---

Reply to this email directly or view it on GitHub:
#107 (comment)

[brianc]

ah I was wondering why I couldn't see your comment. Was confused so I just responded via email.