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

feat(electric): Add support for the int8 column type #653

Merged
merged 6 commits into from
Nov 29, 2023

Conversation

alco
Copy link
Member

@alco alco commented Nov 12, 2023

No description provided.

Copy link

linear bot commented Nov 12, 2023

VAX-820 Implement 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.

The caveat with int8 is that it cannot be represented natively by the Number type in JS. There is, however, the BigInt type.

  • Implement server-side validation that ensures a given string-formatted number can fit into int8
  • Add int8 to the list of supported data types
  • Mention int8 in the data type docs
  • Add an E2E test that verifies syncing of int8 values

@alco alco force-pushed the alco/vax-820-add-support-for-int8 branch from 8701eca to bd6bb90 Compare November 12, 2023 11:16
@alco alco force-pushed the alco/vax-820-add-support-for-int8 branch from bd6bb90 to 4fca4bd Compare November 13, 2023 15:55
Copy link
Contributor

@magnetised magnetised left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks great. good to get those big-ol ints back.

kevin-dp and others added 3 commits November 28, 2023 10:20
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:
```sql
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
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).

---------

Co-authored-by: David Martos <[email protected]>
@alco alco merged commit 0dc6166 into main Nov 29, 2023
14 checks passed
@alco alco deleted the alco/vax-820-add-support-for-int8 branch November 29, 2023 10:10
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 this pull request may close these issues.

3 participants