Handle bigint as string to prevent precision loss

[sevastos]

TIL dealing with bigints in javascript is a bit of a pain.

I had a weird issue of inserting correctly bigints on the db but getting off by a bit numbers on the response.

Apparently all the numbers in javascript are floating-point numbers¹ and the accuracy is only assured for integers ±2⁵³ between -9007199254740992 and 9007199254740992.
However bigint ranges ±2⁶³: -9223372036854775808 to 9223372036854775807 ².

The most common solution for handling bigint data on javascript is carry them with strings and use special libs for math operations. So strings it is.

However, I wonder if it may cause any troubles to anyone that is treating bigint as number on javascript-level and do some checks around that. Though they shouldn't since that only works till the ±2⁵³ range and is not the right way.

Anyway let me know what you think.
Thanks

[rpedela]

Related to #339.

[brianc]

thanks for bringing this up. There's been discussion around it but nothings' really completely addressed it yet. I'll keep this open & really soon get to dealing with this more properly. Sorry for the headache you ran into so far!

[sevastos]

I believe this wasn't correct.

1* 256⁷ + 102 = 72057594037928038

[sevastos]

Thanks for letting me know @rpedela, for some reason, it didn't came up to my search before.

Hey @brianc, no worries and thanks for your work on this, great stuff. I am completely with you on having a consistent strategy on data type handling, I mostly did it for my project because it's quite urgent.

[brianc]

Yeah so the only thing I can see is this requires a major version bump from 1.x to 2.x because it's a breaking change. There are a few other numeric problems execelently described by @rpedela in #339 which probably should be addressed before doing this major version bump since they're also backwards incompatible. I'll try to get all of them put in at the same time and merged. I'm not sure I'll have time for the next 10 days or so because I'm on vacation, but I'll try. If I don't get to it by then I'll try to get to it ASAP. If you were interested in appending the other number parsing changes to this pull request along with test modifications I could most likely merge it as soon as it was ready, but I understand if your time is limited & you can't get to the changes or dont want to. 😄 I'll eventually get to fixing this, it just might be 2 weeks or so.

Now, I'm off to the beach! 🏄

[sevastos]

Sure, I'll give it a try, unless @rpedela wants to take it over. Enjoy your vacations 😃 !

Hey @rpedela, I haven't dealt with decimal and numeric but from what I see decimal is just an alias to numeric, you know if that's the case? Also the serial equivalents of the integer-variants are effectively the integers with a simple constraint for the autoincrement, so no need for new handlers.

[rpedela]

Yes, decimal is an alias for numeric. And yes serial is just a column constraint. I added them for completeness relative to the Postgres number type docs.

I have been working non-stop the last couple months on a product launch set for the beginning of June. I will have more time starting mid-June. This is why I haven't done it already. Sorry about that. But if you want to do it then great!

[brianc]

@rpedela no worries at all. If I get time on this vacation I'll knock it out - otherwise it'll be when I get back. Since you can override the type parsing with relative ease it isn't a critical thing but I do appreciate and share your concern over the correctness of the out-of-box behavior.

Also, good luck with your product launch!

[sevastos]

Oops, I got interrupted.

So, the text parsers are pretty much done, but the binary parsers have some issues.

• parseFloat32 suffers from loss of precision (e.x 101.1 => 101.0999984741211) but that's seems to be expected (according to IEEE754)
• Parsing variabled-width numerics from binary responses looks quite the task to me

Unfortunately, I don't have time nor much knowledge on these to go more in depth, sorry about that.

[sevastos]

Not sure about these.

parseFloat32 (real) somtimes has loss of precision. I tried Buffer's native function readFloat¹ 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.

[brianc]

Hey @sevastos sorry for taking a long time to respond - I've been out.

First thing, don't worry about the binary parser. That was 100% community supplied by a couple of people & if they want to change their implementation they're more than welcome, but I don't support it myself. So, don't consider that a hold up by any means.

So, is anything outstanding other than the binary stuff? You can always skip the tests that are failing in binary mode. I've done it before. Whenever you're ready for me to review this without the binary stuff just let me know.

[sevastos]

No problemo @brianc. Yeap, you can go ahead and review.

Cheers

[brianc]

Okay - I reviewed this.

Just so I'm not missing anything, this is what I see as the changed datatypes

summary of changes

1. float4 -> parsed to javascript number (reverts change in v1.x, breaking change)
2. float8 -> parsed to javascript number (reverts change in v1.x, breaking change)
3. int2/4/oid -> parsed to javascript number (no change)
4. int8 -> parsed to javascript string (breaking change)
5. numeric -> parsed to javascript string (breaking change)
6. decimal -> parsed to javascript string (breaking change)

concerns

My only concern is including the ref dependency for the binary parser. I do appreciate the binary parser is correct, but like I said before I didn't write any of it and don't support it myself. Not a huge deal, but if ref not compiling starts giving windows users errors or something, I'll likely rip the binary stuff out completely into another module. For the mean time we can leave it in.

thoughts

I really appreciate everyone's help on this issue and the careful thought put into correct type handling. I apologize for not doing a better job of it myself: my own use of numeric types in applications I have done has never hit any of these pain points. Thankfully the open source & node community is full of friendly helpful folks to get things like this figured out for the greater good! 🎸

@rpedela since you were the originator of the well thought out issue #339 would you mind chiming in on this as well?
@booo any thoughts?

Once I hear from @rpedela I'll go ahead & do a major version bump, documentation update, and do what I can to broadcast this change out on twitter & IRC.

[rpedela]

Yeah I think it looks great! I don't have any comment on the binary parser dependency.

Sorry I could not get to this sooner. On the bright side, my product was just released and it uses node-postgres.
https://www.datalanche.com/

[brianc]

awesome! Congrats! No worries on taking a while. I'll get this merged this weekend & do the whole release song & dance.

[defunctzombie]

Here is a small gist I created for wrapping numbers to avoid users accidentally "adding" them.

https://gist.github.com/shtylman/5483285

Not sure if this is a great solution but it is something to think about when working with numbers in js. I think we just need to be more vocal about the number problems for folks with more "accounting" sensitive applications.

[brianc]

That's great @shtylman - I'll put big red letters somewhere about node-postgres returning a string if it's a big number and tell them to watch out and remind them to optionally drop in your exception throwing goodness.

[mariusa]

Would it be possible to please add a preference/setting, so that all bigint are parsed with parseInt(), knowing there will be data loss on really big numbers?

We constantly find bugs because in some place we forget data is transformed to string.

Thanks!

[eric-brechemier]

Apparently all the numbers in javascript are floating-point numbers1 and the accuracy is only assured for integers ±2⁵³ between -9007199254740992 and 9007199254740992.
However bigint ranges ±2⁶³: -9223372036854775808 to 9223372036854775807 2.

The most common solution for handling bigint data on javascript is carry them with strings and use special libs for math operations. So strings it is.

Why not keep BIGINT values as numbers up to ±2⁵³, and only use strings for values between ±2⁵³ and ±2⁶³?

This would be less confusing in most cases, and only confusing when truly required.

[rpedela]

@mariusa Personally, I am not opposed to a config option. In the meantime, you can override the default type parser.

var pg = require('pg');

// bigint
pg.types.setTypeParser(20, function (value) {
    return parseInt(value);
});

// numeric
pg.types.setTypeParser(1700, function (value) {
    return parseFloat(value);
});

[mariusa]

Thanks, that works!

We'll trust the magic "20" number :) (saw the internal mapping, hope that won't change)

[rpedela]

That magic number comes from SELECT oid FROM pg_type WHERE typname = 'int8'; and is unlikely to change.

[1mike12]

I guess this still hasn't really been addressed after all this time?

Here's some copy pasta based on the discussion above that finally gets count operations return as numbers, falling back to string if they are really big.

// 20 refers to BigInt, pg by default returns count subqueries as strings instead of
// numbers. We override that behavior so counts less than 2^53 
// (IE practically anything we would ever do) are returned as numbers
pg.types.setTypeParser(20, function(value) {
  if (isStringASafeInteger(value)) {
    return parseInt(value)
  } else {
    return value
  }
})

pg.types.setTypeParser(1700, function(value) {
  return parseFloat(value)
})

/**
 * In JavaScript, numbers are represented using the IEEE 754 double-precision floating-point format, also known as "double." This format uses 64 bits to store a number: 1 bit for the sign, 11 bits for the exponent, and 52 bits for the fraction (or mantissa), which results in JavaScript having a single Number type for both integer and floating-point values.
 *
 * Although numbers larger than 2^53 - 1 or smaller than -2^53 + 1 can be loosely represented, they are not safe integers and there can be rounding errors after operations push outside the safe bounds
 * @param str
 */
export function isStringASafeInteger(str: string) {
  const num = Number(str)
  if (isNaN(num)) return false
  return Number.isSafeInteger(num)
}

[charmander]

@1mike12 The issue of precision loss has been addressed by this pull request. The suggestion I guess you’re referring to hasn’t been implemented because quietly changing representations based on a number’s value is a really bad idea. Expressions like n + 1 will appear to work correctly until they suddenly don’t. (Even the current implementation where the JavaScript type only depends on the PostgreSQL type is a bit of a trap.)

Anyway, JavaScript has the much more efficient and better-behaved BigInt type now, which can be used consistently and solves the problem elegantly. The only issue is breaking backward compatibility, but it should happen in an upcoming major version.