-
Notifications
You must be signed in to change notification settings - Fork 143
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(DAL): client-side support for JSON type #633
Merged
kevin-dp
merged 11 commits into
alco/vax-825-add-support-for-jsonb
from
kevindp/vax-863-json-support
Nov 29, 2023
Merged
feat(DAL): client-side support for JSON type #633
kevin-dp
merged 11 commits into
alco/vax-825-add-support-for-jsonb
from
kevindp/vax-863-json-support
Nov 29, 2023
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
VAX-863 DAL: add support for json type
|
kevin-dp
force-pushed
the
kevindp/vax-863-json-support
branch
3 times, most recently
from
November 7, 2023 14:29
fb8581e
to
bd1b8cc
Compare
alco
reviewed
Nov 13, 2023
alco
force-pushed
the
kevindp/vax-863-json-support
branch
from
November 13, 2023 22:50
f2a86a1
to
c1d63d4
Compare
alco
changed the base branch from
alco/json
to
alco/vax-825-add-support-for-jsonb
November 13, 2023 22:50
alco
reviewed
Nov 27, 2023
generator/src/functions/tableDescriptionWriters/writeTableSchemas.ts
Outdated
Show resolved
Hide resolved
kevin-dp
merged commit Nov 29, 2023
b1a30e0
into
alco/vax-825-add-support-for-jsonb
11 checks passed
alco
added a commit
that referenced
this pull request
Nov 29, 2023
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 <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.