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

chore(electric, docs): Update docs to mention supported data types #428

Merged
merged 4 commits into from
Sep 18, 2023

Conversation

alco
Copy link
Member

@alco alco commented Sep 13, 2023

Here I'm also removing int8 from the list of types accepted by the electrify() function since it's not handled correctly on the client.

Closes VAX-1034.

@alco alco force-pushed the alco/update-docs-on-type-support branch from 653e481 to 5505c33 Compare September 15, 2023 12:40
@alco alco changed the title Alco/update docs on type support chore(electric, docs): Update docs to mention supported data types Sep 15, 2023
@linear
Copy link

linear bot commented Sep 15, 2023

VAX-1034 Update the docs to mention supported data types

…and to explain the rationale of not supporting some types, such as timetz.

Maybe drop support for BIGINT and REAL for now?

@alco alco marked this pull request as ready for review September 15, 2023 12:42
@alco alco requested a review from thruflo September 15, 2023 12:42
Copy link
Contributor

@thruflo thruflo 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 👍

docs/usage/data-modelling/types.md Outdated Show resolved Hide resolved
@davidmartos96
Copy link
Contributor

Is int8 support something that can be fixed in the client?

We are actually using it successfully with the Dart client to send timestamps. Are they being removed because of the 2^53 limit on JS?

https://api.flutter.dev/flutter/dart-core/int-class.html

We've been storing them as integers on Sqlite, so it's a bit unfortunate that they are not supported on Electric.

@alco
Copy link
Member Author

alco commented Sep 16, 2023

@davidmartos96 Since JavaScript numbers cannot represent all values that a true 64-bit integer can hold, this opens up a door to silent data corruption. Imagine inserting a bigint 4611686018427387904 in Postgres, syncing that to SQLite as a string-encoded value. No data loss so far. But then if the value is read into a JS number it will get rounded. If the client then writes it locally and it is synced up to the server, the silent and unintended change becomes permanent.

> Number("4611686018427387904")
4611686018427388000 

Repairing such silent corruption after the fact can be difficult or impossible.

So we want to be conservative in choosing the types we allow to electrify now, until we add the necessary mapping from Postgres' bigint to JS' bigint in the TypeScript client.

The Dart client will also need to account for this. We're exploring the option of setting up a generative test suite that could be used to generate semi-random data and throw it at client-side validations. Then, whatever is validated on the client would be synced to the server to verify that the value can also be written to Postgres and that it doesn't change in the process. We should come up with a way to use the same test data with different client implementations.

@davidmartos96
Copy link
Contributor

Thanks for the info! A generative test suite sounds really useful to test edge cases with different client implementations.

@alco alco merged commit 1dd9500 into main Sep 18, 2023
@alco alco deleted the alco/update-docs-on-type-support branch September 18, 2023 11:02
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