brianc / node-postgres

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

Possible bug with handling bigints? #166

Closed jonasacres closed 12 years ago

jonasacres commented 12 years ago

Hi!

I'm admittedly new to Node.js and Postgres, so perhaps there is an environment issue that I am simply unaware of. Further, I'm a stranger to the project and am not sure if this is the appropriate place to file something like this. If I'm putting this in the wrong spot, my apologies, and I'd be grateful if you could direct me to where I ought to go.

I'm working with a dataset that uses bigints. It seems like there's an issue where node-postgres loses the lowest two bits when the MSB of a bigint is set. Here's an example:

mydb=# select version(); PostgreSQL 9.1.4 on x86_64-apple-darwin, compiled by i686-apple-darwin10-gcc-4.2.1 (GCC) 4.2.1 (Apple Inc. build 5666) (dot 3), 64-bit (1 row) mydb=# create table bigints ( number bigint ); CREATE TABLE mydb=# insert into bigints (number) values (0), (1), (2), (3), (4), (5), (6), (7), (8), (9), (10), (11), (12), (13), (14), (15), (25227580722380800+0), (25227580722380800+1), (25227580722380800+2), (25227580722380800+3), (25227580722380800+4), (25227580722380800+5), (25227580722380800+6), (25227580722380800+7), (25227580722380800+8), (25227580722380800+9), (25227580722380800+10), (25227580722380800+11), (25227580722380800+12), (25227580722380800+13), (25227580722380800+14), (25227580722380800+15); INSERT 0 32 mydb=# select * from bigints; 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 25227580722380800 25227580722380801 25227580722380802 25227580722380803 25227580722380804 25227580722380805 25227580722380806 25227580722380807 25227580722380808 25227580722380809 25227580722380810 25227580722380811 25227580722380812 25227580722380813 25227580722380814 25227580722380815 (32 rows)

So far, so good. Let's try it in node.js using node-postgres:

var pg = require('pg');
function retrieve(req, res)
{
    var conString = "tcp://postgres:mypassword@localhost/mydb";
    var client = new pg.Client(conString);
    client.connect(function(error)
        {
            if(error != null)
            {
                logger.error("Error connecting: " + error);
            } else
            {
                client.query("SELECT * FROM bigints", function(error, result)
                    {
                        rowOutput = [];
                        for(var i = 0; i < result.rows.length; i++)
                        {
                            rowOutput.push(result.rows[i].number.toString(2));
                        }

                        res.send(rowOutput.join("\n"));
                    });
            }
        });
}

$ curl http://localhost:3000/ 0 1 10 11 100 101 110 111 1000 1001 1010 1011 1100 1101 1110 1111 1011001101000000101100111011011100000000000000000000000 1011001101000000101100111011011100000000000000000000000 1011001101000000101100111011011100000000000000000000000 1011001101000000101100111011011100000000000000000000100 1011001101000000101100111011011100000000000000000000100 1011001101000000101100111011011100000000000000000000100 1011001101000000101100111011011100000000000000000001000 1011001101000000101100111011011100000000000000000001000 1011001101000000101100111011011100000000000000000001000 1011001101000000101100111011011100000000000000000001000 1011001101000000101100111011011100000000000000000001000 1011001101000000101100111011011100000000000000000001100 1011001101000000101100111011011100000000000000000001100 1011001101000000101100111011011100000000000000000001100 1011001101000000101100111011011100000000000000000010000 1011001101000000101100111011011100000000000000000010000

Notice that for the small values, the least significant bits are fine; but for the big values, the two LSBs are always 0. I see the same behavior happen in other tables, using both select *, as well as select column_name, and INSERT...RETURNING. I'm using node-postgres v0.8.1.

For the sake of completeness, I tried again using Python and psycopg2:

#!/usr/bin/python

import psycopg2;

conn = psycopg2.connect("dbname=mydb user=postgres password=mypassword")
cur = conn.cursor()

cur.execute("SELECT * FROM bigints;")
for record in cur:
    print record

$ /tmp/bigint-test.py (0L,) (1L,) (2L,) (3L,) (4L,) (5L,) (6L,) (7L,) (8L,) (9L,) (10L,) (11L,) (12L,) (13L,) (14L,) (15L,) (25227580722380800L,) (25227580722380801L,) (25227580722380802L,) (25227580722380803L,) (25227580722380804L,) (25227580722380805L,) (25227580722380806L,) (25227580722380807L,) (25227580722380808L,) (25227580722380809L,) (25227580722380810L,) (25227580722380811L,) (25227580722380812L,) (25227580722380813L,) (25227580722380814L,) (25227580722380815L,)

The output here matches the correct values, suggesting that this issue is specific to node-postgres. Any guidance would be tremendously appreciated. Thanks!

P.S.: Here's a summary of all the software versions I'm using.

For completeness' sake:

booo commented 12 years ago

I think this is a normal precision problem when dealing with big integers (64bit) in node. Maybe this link helps understanding the problem:

http://stackoverflow.com/questions/2983206/bitwise-and-in-javascript-with-a-64-bit-integer

jonasacres commented 12 years ago

Yikes. That would certainly explain it. Thanks!

brianc commented 12 years ago

Yeah, we had an issue on this before as well. In 95% of the cases people don't need the big-num support, so we default to returning numeric types as integers. You can and should override the type-parser for the oid of the type of column (numeric oid = 1700) to return the number as a string instead of an integer. The type converter functions are given a string value as input so you can just return the input. In the test I set the type converter to always return the string 'yes.' Not sure why I did it that way, but it should still work as an example for you:

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

mjackson commented 12 years ago

Thanks @brianc. What is the best way to do this? I see that I can pass in a custom config object to pg.Client.query, but I'd rather set it at a higher level so that I don't need to pass it in every time. Is that possible?

brianc commented 12 years ago

@mjijackson from the link I posted in my last comment there's an example. Basically, this:

var types = require(__dirname + '/../../../lib/types');
  //1231 = numericOID
  types.setTypeParser(1700, function(){
    return 'yes';
  })
  types.setTypeParser(1700, 'binary', function(){
    return 'yes';
  })
mjackson commented 12 years ago

@brianc Sorry, I should have been more specific. My question is how do you actually require the types module from user code? As far as I can see I would need to do a require('pg/lib/types') in order to access it, which is ok, but it feels like something that could break in the future. All node modules that I've used have had a single entry point on which everything else hangs, exactly like the native module hangs on the pg module in this package. Could we also hang the types module on it as well?

Edit: Also, I'm not 100% sure that require('pg/lib/types') is the blessed way to do a require. Seems to me that this kind of require used to not be allowed in node at all...

brianc commented 12 years ago

No worries. I believe require('module/subpath') is an a-okay way to bring in sub-files of a module, but if you would rather you could do: var path = require('path'); require(path.join(require.resolve('pg'), 'lib/types'))). or something like that.

You're right on though w/ saying it could/should be hung off the main module require as an object instead of a separate file. I'll leave this open until I can get to that.

Also, you're right that require('pg/lib/types') didn't use to be allowed, which might be one reason why so many modules hang all their objects off the main require point including this one. :smile:

mjackson commented 12 years ago

For anyone else that comes across this thread, the following worked for me:

var pg = require('pg');
pg.types.setTypeParser(20, String);

This tells node-postgres to parse all bigints that come back from the database as strings, so you won't lose digits when parseInt tries to parse bigints as JavaScript integers.

Thanks @brianc!