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

Automatically parse DB bigints as JavaScript Ints #83

Merged
merged 3 commits into from
Dec 10, 2020

Conversation

Cruikshanks
Copy link
Member

https://trello.com/c/RIATlpAS

In the charging-module-api we hit an issue caused by integer overflow. We hold all our monetary values as pennies rather than pounds and pence. This is common practise in financial systems as it avoids some issues with rounding in equations.

The issue was all our monetary fields were set as integer but we found examples of bill runs with values larger than 2147483647 (the max an integer can hold). To solve the issue we needed to set the DB fields to bigint. Bigint's can hold values in the range of -9223372036854775808 to +9223372036854775807 in PostgreSQL.

JavaScript on the other hand uses Number to manage integers. An integer in JavaScript has a range of -9007199254740991 to 9007199254740991. If you need to go as large as PostgreSQL you need to use a JavaScript BigInt.

Because of this discrepancy, the PostgreSQL driver for Node by default returns all BigInt values as strings. This is a design decision by PostgreSQL to avoid any loss of precision.

However, if you are confident you won't be affected by this precision loss (which we are) you can tell the PostgreSQL driver to return an integer instead.

So as we do not expect to generate bill runs with values greater than 9.0071993e+13 (I don't even know how to say what that number is!) this change adds the config needed so we can forget about BigInt's in the rest of our code.

https://trello.com/c/RIATlpAS

In the [charging-module-api](https://github.com/defra/charging-module-api) we hit an issue caused by integer overflow. We hold all our monetary values as pennies rather than pounds and pence. This is common practise in financial systems as it avoids some issues with rounding in equations.

The issue was all our monetary fields were set as `integer` but we found examples of bill runs with values larger than 2147483647 (the max an integer can hold). To solve the issue we needed to set the DB fields to `bigint`. [Bigint's](https://www.postgresql.org/docs/10/datatype-numeric.html#DATATYPE-INT) can hold values in the range of -9223372036854775808 to +9223372036854775807 in PostgreSQL.

JavaScript on the otherhand uses [Number](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number) to manage integers. An integer in JavaScript has a range of -9007199254740991 to 9007199254740991. If you need to go as large as PostgreSQL you need to use a [JavaScript BigInt](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/BigInt).

Because of this discrepency the PostgreSQL driver for Node by default returns all BigInt values as strings. This is a [design decision by PostgreSQL](brianc/node-postgres#353) to avoid any loss of precision.

However, if you are confident you won't be effected by this precision loss (which we are) you can tell the PostgreSQL driver to return an integer instead.

So as we do not expect to generate bill runs with values greater than 9.0071993e+13 (I don't even know how to say what that number is!) this change adds the config needed so we can forget about BigInt's in the rest of our code.
@Cruikshanks Cruikshanks added the enhancement New feature or request label Dec 10, 2020
@Cruikshanks Cruikshanks self-assigned this Dec 10, 2020
@Cruikshanks Cruikshanks marked this pull request as ready for review December 10, 2020 10:08
@Cruikshanks Cruikshanks merged commit 19b1f9a into main Dec 10, 2020
@Cruikshanks Cruikshanks deleted the parse-db-big-ints-as-integers branch December 10, 2020 12:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants