Reconsider automatic number/BigInt promotion behaviour

[jeremyBanks]

👋 Hello! Thank you for the excellent library. I have a suggestion for your consideration.

Currently, this library converts SQLite INTEGER values to a JavaScript number if the they satisfy Number.isSafeInteger(…), or else a bigint, to ensure that precision is not lost. This is a simple and practical choice for many cases, and it reminds me of what Python does. However, JavaScript doesn't allow you to use bigint and numbers fully interchangeably, so the mixed return types can create bugs that won't be apparently until the certain combinations of data are encountered.

For a simple example, if we're returning a result from an API...

#!/usr/bin/env -S deno run
import { DB } from "https://deno.land/x/sqlite@v2.3.0/mod.ts";

const db = new DB();

db.query(`CREATE TABLE T (X)`);
db.query(`INSERT INTO T VALUES (1), (12345), (9007199254740991), (9876543210123456)`);

let sum = 0;

for (const [x] of db.query(`SELECT * FROM T`)) {
  console.log('sum +=', x);
  sum += x;
  console.log('sum ==', sum);
  console.log();
}

sum += 1
sum == 1

sum += 12345
sum == 12346

sum += 9007199254740991
sum == 9007199254753336

sum += 9876543210123456n
error: Uncaught TypeError: Cannot mix BigInt and other types, use explicit conversions
  sum += x;

One possible alternative is to use number by default for all INTEGER values, and throw an error if you encounter a value that can't safely fit, but have an option (on the DB handle, maybe?) to use bigint for INTEGER instead, that users can enable when they know they may need to handle such values.

Any change would probably be breaking and require a new major version.

[dyedgreen]

Hi @jeremyBanks thank you for the suggestion. You are right, that mixing big ints with numbers causes errors. However, I don’t think this is necessarily a bad thing. Let me outline the reasons the library choose to promote to big int in the first place:

The most common use case for really large integers is tables with lots of entries (leading to large ids). For these, we never actually want to do any math on the values, and so should be fine having the mixed type (things will just work).

Most (although not all) computations in JavaScript which involve big numbers will probably involve floating points, so those are not effected.

For any other case, if you don’t think about overflow errors, this library will just work if either all or none of to values hit this case (this is good, since the uses didn’t have to think but the result is correct), or it will throw, if they end up mixing. But if that happens, you should be handling the case with greater care anyways (so the error is a good thing imho).

I believe the current behavior will ‘just work’ for must users, and error instead of producing unexpected results. (While eg returning only numbers will sometimes produces silent errors.)

[dyedgreen]

I’m closing this issue for now, please feel free to re-open it if you believe your concern was not addressed appropriately by my comment 😊

[jeremyBanks]

Very reasonable, thank you for the reply. 🙂 It's an design trade-off, and you make a good case that this is better for most users, even if my preferences may be different.