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.

Sign up for GitHub

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

Jump to bottom

BigInt incorrectly cast to int with msnodesqlv8 #1385

Closed
dsbert opened this issue Apr 18, 2022 · 2 comments · May be fixed by #1387
Closed

BigInt incorrectly cast to int with msnodesqlv8 #1385

dsbert opened this issue Apr 18, 2022 · 2 comments · May be fixed by #1387

Comments

@dsbert
Copy link

dsbert commented Apr 18, 2022 •
edited

When using the msnodesqlv8 library, values passed into a BigInt column as strings are incorrectly cast using parseInt. This results in an incorrect value.

node-mssql/lib/msnodesqlv8/request.js

Lines 42 to 48 in 8a2fc63

case TYPES.BigInt:
case TYPES.SmallInt:
if ((typeof value !== 'number') && !(value instanceof Number)) {
value = parseInt(value)
if (isNaN(value)) { value = null }
}
break

For example:

const value = "9223372036854775807";

console.log(value);
// 9223372036854775807

console.log(parseInt(value));
// 9223372036854776000

SQL Server can cast the string to a BIGINT implicitly, so I'm not really sure why a cast is required in the library at all. But no matter what the case, the current logic seems to result in bad data.
@dhensby dhensby mentioned this issue Apr 19, 2022
Open
1 task
@dhensby
Copy link
Collaborator

dhensby commented Apr 19, 2022

I've opened a PR at #1387 to try to fix the problem, but it seems that msnodesqlv8's implementation doesn't support returning bigint values as strings (TimelordUK/node-sqlserver-v8#106) and so the precision is lost unless you explicitly cast the value as a varchar. Until that is fixed in msnodesqlv8, it won't be possible to pull bigint values out without precision loss.

I have tested that my fix does allow the input to the database to retain precision, though.

@dhensby
Copy link
Collaborator

dhensby commented May 3, 2022

I'm going to close this as a bug in the upstream driver. I have a PR open to assert the support for numbers as strings but it looks like this is handled differently by both drivers and using strings consistently for BigInt won't work (as it stands). I'll try to find some time to open a PR against tedious to allow the use of strings for BigInt and it looks like the fix is in progress on msnodesqlv8.

@dhensby dhensby closed this as completed May 3, 2022
@dsbert dsbert mentioned this issue Jul 7, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

treat bigint as string in msnodesqlv8 driver dhensby/node-mssql
2 participants
@dhensby @dsbert