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

BigNumbers/BigInt not handled #259

Open
TheKnightCoder opened this issue May 22, 2024 · 4 comments · May be fixed by #268
Open

BigNumbers/BigInt not handled #259

TheKnightCoder opened this issue May 22, 2024 · 4 comments · May be fixed by #268

Comments

@TheKnightCoder
Copy link

When you run a query on BigInt column, I would expect it to return the result in JS native BigInt type instead of regular int which is 4 bytes and therefore truncates the result

@gabegorelick
Copy link

truncates the result

This is kind of a big deal. Currently, BigInts are silently converted to Numbers.

We've had to patch @databricks/sql to not do that.

gabegorelick added a commit to gabegorelick/databricks-sql-nodejs that referenced this issue Oct 25, 2024
BigInts are currently converted to JS Numbers, which can't fit values
over 2**53.

Fixes databricks#259
@gabegorelick gabegorelick linked a pull request Oct 25, 2024 that will close this issue
@gabegorelick
Copy link

Similar issue exists for DECIMAL types. They're returned as Numbers but if they have high enough precision they won't fit.

@kravets-levko
Copy link
Contributor

@gabegorelick thank you for bringing this up. Yes, you're totally right - BigInt support would be great, and, as you've already seen from #268, it doesn't require that much changes. But we didn't make that change because it may break for some users. So we either need to add some feature flag for it, or release next major version. Both options have own pros and cons.

As for DECIMAL type - Node has no any native type that can represent decimal values. Number was the best fit, even though it may lack precision in some cases. But if you have any ideas - please share them, and let's discuss

@gabegorelick
Copy link

As for DECIMAL type - Node has no any native type that can represent decimal values. Number was the best fit, even though it may lack precision in some cases. But if you have any ideas - please share them, and let's discuss

There are third party packages for arbitrary precision numeric types. https://www.npmjs.com/package/big.js is a big one (no pun intended). It's pretty popular and used by the BigQuery JS SDK.

If you don't want an external dependency, returning DECIMALs and similar types as strings is also a common approach used by libraries like https://www.npmjs.com/package/pg. It's certainly more annoying for cases where a DECIMAL can fit in a JS Number, but it's far safer for the cases where they don't fit.

gabegorelick added a commit to gabegorelick/databricks-sql-nodejs that referenced this issue Oct 28, 2024
BigInts are currently converted to JS Numbers, which can't fit values
over 2**53.

Fixes databricks#259
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants