Add json columns in Postgres #1865
Merged
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
Codecov Report
@@ Coverage Diff @@
## master #1865 +/- ##
==========================================
- Coverage 88.94% 87.09% -1.85%
==========================================
Files 150 156 +6
Lines 11239 11666 +427
Branches 447 464 +17
==========================================
+ Hits 9997 10161 +164
- Misses 1242 1505 +263
|
t-bast
reviewed
Jul 7, 2021
Good catch, done with c5804ad. Note that this test is pretty subtle and brings strong non-reg guarantees, see the commit message. |
|
ACK c5804ad Very nice to improve these tests, adding json serialization will indeed make that kind of tests more readable in the future, we should leverage it more. |
|
Rebased on parent PR and squashed. |
A json column has been added to the few tables that contains an opaque serialized blob: - `local_channels.data` - `nodes.data` - `channels.channel_announcement`, `channels.channel_update_x` We can now access all the individual data fields from SQL. For the serialization, we use the same serializers than the one that were previously used by the API. They have been moved to the `eclair-core` module and simplified a bit. There are two json data types in Postgres: `JSON` and `JSONB`. We use the latter one, which is more recent, and allows indexing. An alternative to this PR would have been to use columns, but: - there would have been a *lot* of columns for the channel data - every modification of our types would have required a db migration NB: to handle non-backwards compatible changes in the json serializersi, all the json columns can be recomputed on restart by setting `eclair.db.reset-json-columns=true`. Change in in ChannelCodecsSpec: The goal of this test is to make sure that, in addition to successfully decoding data that encoded with an older codec, we actually read the correct data. Just because there is no error doesn't mean that we interpreted the data properly. For example we could invert a `payment_hash` and a `payment_preimage`. We can't compare object to object, because the current version of the class has probably changed too. That's why we compare using the json representation of the data, that we amend to ignore new or modified fields. After doing a manual comparison, I updated the test to use the current json serializers, and replaced the test data with the latest json serialization. This allows us to remove all the tweaks that we added over time to take into account new and updated fields.
|
Parent PR merged, rebased on master. |
t-bast
approved these changes
Jul 8, 2021
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.
A json column has been added to the few tables that contains an
opaque serialized blob:
local_channels.datanodes.datachannels.channel_announcement,channels.channel_update_xWe can now access all the individual data fields from SQL.
For the serialization, we use the same serializers than the one
that were previously used by the API. They have been moved to the
eclair-coremodule and simplified a bit.There are two json data types in Postgres:
JSONandJSONB. We usethe latter one, which is more recent, and allows indexing.
An alternative to this PR would have been to use columns, but:
NB: to handle non-backwards compatible changes in the json serializersi,
all the json columns can be recomputed on restart by setting
eclair.db.reset-json-columns=true.