databricks / databricks-sql-nodejs

Databricks SQL Connector for Node.js
Apache License 2.0
24 stars 34 forks source link

BigNumbers/BigInt not handled #259

Open TheKnightCoder opened 5 months ago

TheKnightCoder commented 5 months ago

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 commented 2 weeks ago

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 commented 2 weeks ago

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

kravets-levko commented 2 weeks ago

@gabegorelick thank you for bringing this up. Yes, you're totally right - BigInt support would be great, and, as you've already seen from databricks/databricks-sql-nodejs#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 commented 2 weeks ago

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.