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

Handle bigint as string to prevent precision loss #353

Merged
merged 2 commits into from
Jun 19, 2013
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions lib/types/binaryParsers.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
var ref = require('ref');
var endian = (ref.endianness === 'LE') ? 'BE' : 'LE';

var parseBits = function(data, bits, offset, invert, callback) {
offset = offset || 0;
invert = invert || false;
Expand Down Expand Up @@ -106,11 +109,7 @@ var parseInt32 = function(value) {
};

var parseInt64 = function(value) {
if (parseBits(value, 1) == 1) {
return -1 * (parseBits(value, 63, 1, true) + 1);
}

return parseBits(value, 63, 1);
return String(ref['readInt64' + endian](value, 0));
};

var parseFloat32 = function(value) {
Expand Down
17 changes: 13 additions & 4 deletions lib/types/textParsers.js
Original file line number Diff line number Diff line change
Expand Up @@ -162,11 +162,20 @@ var parseInteger = function(val) {
return parseInt(val, 10);
};

var parseBigInteger = function(val) {
var valStr = String(val);
if (/^\d+$/.test(valStr)) { return valStr; }
return val;
};

var init = function(register) {
register(20, parseInteger);
register(21, parseInteger);
register(23, parseInteger);
register(26, parseInteger);
register(20, parseBigInteger); // int8
register(21, parseInteger); // int2
register(23, parseInteger); // int4
register(26, parseInteger); // oid
register(700, parseFloat); // float4/real
register(701, parseFloat); // float8/double
//register(1700, parseString); // numeric/decimal
register(16, parseBool);
register(1082, parseDate); // date
register(1114, parseDate); // timestamp without timezone
Expand Down
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@
"main": "./lib",
"dependencies": {
"generic-pool": "2.0.3",
"buffer-writer": "1.0.0"
"buffer-writer": "1.0.0",
"ref": "0.1.3"
},
"devDependencies": {
"jshint": "1.1.0",
Expand Down
33 changes: 25 additions & 8 deletions test/integration/client/type-coercion-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@ var testForTypeCoercion = function(type){
});

assert.emits(query, 'row', function(row) {
assert.strictEqual(row.col, val, "expected " + type.name + " of " + val + " but got " + row.col);
var expected = val + " (" + typeof val + ")";
var returned = row.col + " (" + typeof row.col + ")";
assert.strictEqual(row.col, val, "expected " + type.name + " of " + expected + " but got " + returned);
}, "row should have been called for " + type.name + " of " + val);

client.query('delete from test_type');
Expand All @@ -40,13 +42,21 @@ var testForTypeCoercion = function(type){

var types = [{
name: 'integer',
values: [1, -1, null]
values: [-2147483648, -1, 0, 1, 2147483647, null]
},{
name: 'smallint',
values: [-1, 0, 1, null]
values: [-32768, -1, 0, 1, 32767, null]
},{
name: 'bigint',
values: [-10000, 0, 10000, null]
values: [
'-9223372036854775808',
'-9007199254740992',
'0',
'9007199254740992',
'72057594037928030',
'9223372036854775807',
null
]
},{
name: 'varchar(5)',
values: ['yo', '', 'zomg!', null]
Expand All @@ -58,13 +68,20 @@ var types = [{
values: [true, false, null]
},{
name: 'numeric',
values: ['-12.34', '0', '12.34', null]
values: [
'-12.34',
'0',
'12.34',
'-3141592653589793238462643383279502.1618033988749894848204586834365638',
'3141592653589793238462643383279502.1618033988749894848204586834365638',
null
]
},{
name: 'real',
values: ['101.1', '0', '-101.3', null]
values: [-101.3, -1.2, 0, 1.2, 101.1, null]
},{
name: 'double precision',
values: ['-1.2', '0', '1.2', null]
values: [-101.3, -1.2, 0, 1.2, 101.1, null]
},{
name: 'timestamptz',
values: [null]
Expand All @@ -82,7 +99,7 @@ var types = [{
// ignore some tests in binary mode
if (helper.config.binary) {
types = types.filter(function(type) {
return !(type.name in {'real':1, 'timetz':1, 'time':1, 'numeric': 1, 'double precision': 1});
return !(type.name in {'real': 1, 'timetz':1, 'time':1, 'numeric': 1});
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure about these.

parseFloat32 (real) somtimes has loss of precision. I tried Buffer's native function readFloat1 but the same precision loss happened.

However a code snippet from dzone :P returns correct amounts but it's probably slower and I haven't tested it thoroughly.

After reading some bits about floating points and IEEE754, it seems that sometimes loss of precision is expected.

});
}

Expand Down
28 changes: 14 additions & 14 deletions test/unit/client/typed-query-results-tests.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
var helper = require(__dirname + '/test-helper');
//http://www.postgresql.org/docs/8.4/static/datatype.html
//http://www.postgresql.org/docs/9.2/static/datatype.html
test('typed results', function() {
var client = helper.client();
var con = client.connection;
Expand All @@ -18,20 +18,20 @@ test('typed results', function() {
name: 'integer/int4',
format: 'text',
dataTypeID: 23,
actual: '100',
expected: 100
actual: '2147483647',
expected: 2147483647
},{
name: 'smallint/int2',
format: 'text',
dataTypeID: 21,
actual: '101',
expected: 101
actual: '32767',
expected: 32767
},{
name: 'bigint/int8',
format: 'text',
dataTypeID: 20,
actual: '102',
expected: 102
actual: '9223372036854775807',
expected: '9223372036854775807'
},{
name: 'oid',
format: 'text',
Expand All @@ -42,20 +42,20 @@ test('typed results', function() {
name: 'numeric',
format: 'text',
dataTypeID: 1700,
actual: '12.34',
expected: '12.34'
actual: '31415926535897932384626433832795028841971693993751058.16180339887498948482045868343656381177203091798057628',
expected: '31415926535897932384626433832795028841971693993751058.16180339887498948482045868343656381177203091798057628'
},{
name: 'real/float4',
dataTypeID: 700,
format: 'text',
actual: '123.456',
expected: '123.456'
expected: 123.456
},{
name: 'double precision / float8',
format: 'text',
dataTypeID: 701,
actual: '1.2',
expected: '1.2'
actual: '12345678.12345678',
expected: 12345678.12345678
},{
name: 'boolean true',
format: 'text',
Expand Down Expand Up @@ -222,13 +222,13 @@ test('typed results', function() {
format: 'binary',
dataTypeID: 20,
actual: [0, 0, 0, 0, 0, 0, 0, 102],
expected: 102
expected: '102'
},{
name: 'binary-bigint/int8-full',
format: 'binary',
dataTypeID: 20,
actual: [1, 0, 0, 0, 0, 0, 0, 102],
expected: 72057594037928030
expected: '72057594037928038'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this wasn't correct.

1* 2567 + 102 = 72057594037928038

},{
name: 'binary-oid',
format: 'binary',
Expand Down