feat (client): client-side support for int8

[kevin-dp]

This PR adds client-side support for int8 aka bigint.
Supporting BigInts is tricky because of inconsistencies in the sqlite drivers.
For example, better-sqlite3 requires BigInts to be explicitly enabled but then all integers are returned as BigInts because SQLite does not distinguish between them (they are all just 64bit integers). On the other hand, wa-sqlite returns a regular JS number if the integer fits, otherwise it returns a BigInt.

To avoid these problems, we always cast BigInt to text when reading from the DB and then let the DAL turn it into a BigInt;
i.e., when the Postgres type of a column "foo" is bigint, we read as follows: SELECT cast(foo as TEXT) AS foo FROM table. Similarly, we cast returned values: INSERT INTO table VALUES (9223372036854775807) RETURNING cast(foo as TEXT) AS foo.

A similar problem occurs in the oplog triggers. These triggers insert an oplog entry for every insert/update/delete. This oplog entry contains the old and new rows as JSON objects. However, JSON does not support BigInts:

sqlite> SELECT json_valid('{"a": 123n}');
0

We also can't write it as a regular number because when parsing the JSON it is read as a number and rounding errors occur:

js> JSON.parse('{"a": 9223372036854775807}')
{ a: 9223372036854776000 }

Hence, the triggers also need to cast BigInts to text and the deserialisation function for oplog entries (deserialiseRow) needs to correctly deserialise them as BigInts (when the column type is int8/bigint).

[linear]

VAX-1325 DAL: add support for int8 type

Reference: https://electric-sql.slab.com/posts/data-validation-supporting-more-postgres-types-tpegouhj#h1sz5-supporting-more-basic-postgres-types-in-electric.

*NOTE: it is only possible to implement a mapping between 64-bit integers in SQLite and *bigint in JavaScript if the used SQLite supports reading 64-bit integers. I have checked wa-sqlite and it does correctly map large integers to bigint values when reading from SQLite.

const { db } = useElectric()!

> await db.raw({sql: "SELECT 180143985094819849"})
[{int64: 180143985094819849n}]

• Add client-side mapping between the integer value stored in SQLite and TypeScript's bigint type.
• Add client-side validations to ensure only integers that fall into int8's range get written to SQLite.

[alco]

Stellar job! ⭐ 💪

My only piece of feedback is that I'm not seeing any tests that verify the validation is working for bigints. I.e. trying to write a value that would exceed the range of what a 64-bit integer can hold.

[alco]

This string should be returned from a function if it's used in multiple places like this. If, say, we add quoting for these field identifiers later, it'll be easier to make the change once and not worry about missing some occurrences that will be left unquoted.

[kevin-dp]

You want to introduce a local getField() function that returns the field?
something like:

private returnAllFields<T extends QueryBuilder & ReturningMixin>(
    query: T
  ): T {
    const getField = (field: string) => field 
    return this._fields.reduce((query, field) => {
      // if field is of type BigInt cast the result to TEXT
      // because not all adapters deal well with BigInts
      // the DAL will convert the string into a BigInt in the `fromSqlite` function from `../conversions/sqlite.ts`.
      const pgType = this._tableDescription.fields.get(field)
      if (pgType === PgBasicType.PG_INT8) {
        return query.returning(`cast(${getField()} as TEXT) AS ${getField()}`)
      }
      return query.returning(getField())
    }, query)
  }
}

I don't really see the value of adding this now as we merely use the field without any changes.
I do see the value once we do more than just using the field name, e.g. add quoting, but now it is literally the identity function.

[alco]

I was only referring to the act of interpolating a variable into a string which is then passsed as SQL for execution by SQLite. Depending on the context, this can lead to SQL injection attacks or merely client-side fatal errors.

What I'm saying is that we should properly quote all identifiers and values when interpolating them into strings of SQL. It doesn't take much effort for a particular field name to break our client and as a consequence the app that depends on it. Here's a quick example:

$ sqlite3
SQLite version 3.36.0 2021-06-18 18:36:39
Enter ".help" for usage hints.
Connected to a transient in-memory database.
Use ".open FILENAME" to reopen on a persistent database.
sqlite> create table foo(cast integer);
sqlite> insert into foo values (1), (2), (3);
sqlite> select * from foo;
1
2
3
sqlite> select cast(cast as text) from foo;
Error: near "as": syntax error
sqlite> select cast("cast" as text) from foo;
1
2
3

Perhaps the value in the field variable is already quoted? In that case, disregard this comment.

[kevin-dp]

Indeed, the arguments we pass to the function are already quoted, e.g.:

castIfNeeded(col, `"${col}"`)

[alco]

Seeing this for the first time now, it's surprising we're not using a disciplined approach for interpolating fargments into SQL strings. I know it's out of scope for this PR but I wanted to hear your opinion on it: shouldn't we audit all table column references that are interpolated into strings like this and replace them with a centralized function that can quote all identifiers uniformly?

[kevin-dp]

We could indeed change all places were we quote explicitly by calling a quote function that does the quoting. That improves maintainability if at some later point in time we want to change the way we quote identifiers but i don't think we will ever change the quoting as it seems the only way to quote identifiers is to use double quotes.

[kevin-dp]

Stellar job! ⭐ 💪

My only piece of feedback is that I'm not seeing any tests that verify the validation is working for bigints. I.e. trying to write a value that would exceed the range of what a 64-bit integer can hold.

Thanks, i added a unit test for values that are out of range :-)