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 JSONB column type #661

Merged
merged 6 commits into from
Nov 29, 2023

Conversation

alco
Copy link
Member

@alco alco commented Nov 13, 2023

No description provided.

Copy link

linear bot commented Nov 13, 2023

VAX-825 Implement support for json and jsonb types

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

  • Implement server-side validation that ensures a given string contains a valid JSON object
  • Add json and jsonb to the list of supported data types
  • Mention json and jsonb in the data type docs
  • Add an E2E test that verifies syncing of json and jsonb values

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.

cool. weirdly much simpler than the int8 support...

@@ -512,6 +512,11 @@ defmodule Electric.Satellite.Serialization do
val
end

def decode_column_value!(val, type) when type in [:json, :jsonb] do
_ = Jason.decode!(val)
Copy link
Contributor

Choose a reason for hiding this comment

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

shame to have to do a full decode with all those allocations just to test the syntax but for now it makes sense

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh yes, this is tragic. Ideally we'd use a native library that only validates JSON, without any allocations. It's surprisingly difficult to find one because googling for it yields results about validating json schema.

Alternatively, and I don't know how feasible it is, but maybe we could pull the JSON validation code (directly from Postgres)[https://github.com/postgres/postgres/blob/50c67c2019ab9ade8aa8768bfe604cd802fe8591/src/backend/utils/adt/json.c#L1625] and wrap it into a NIF. The maintanence burden of that would be low, especially when taking into account that we currently have uncovered edge cases where Jason.decode!() would treat a given JSON as valid while Postgres would reject it.

alco and others added 5 commits November 29, 2023 13:44
This PR adds client-side support for the JSON type.
The DAL accepts JS values representing JSON and serialises them to a
string that is stored in SQLite.
When reading a JSON value from the database the string is deserialised
back into a JS value.
A corner case arises when having an optional column of type JSON because
we need to differentiate between the database NULL value and the JSON
null value. We treat the regular JS null value as a database NULL (to be
consistent with how null values are interpreted for other column types)
and require users to pass a special JsonNull object in order to store a
top-level JSON null value.

Still need to add an E2E test for json values.

---------

Co-authored-by: Oleksii Sholik <oleksii@sholik.dev>
@alco alco force-pushed the alco/vax-825-add-support-for-jsonb branch from b1a30e0 to 5a0a033 Compare November 29, 2023 12:35
@alco alco merged commit 4ad7df4 into main Nov 29, 2023
14 checks passed
@alco alco deleted the alco/vax-825-add-support-for-jsonb branch November 29, 2023 13:05
@barbalex
Copy link

Did you see that sqlite will soon support jsonb? https://sqlite.org/draft/jsonb.html

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.

4 participants