Make pedantic checks less fatal, fix an upgrade related null field access#3191
Merged
niftynei merged 4 commits intoElementsProject:masterfrom Oct 21, 2019
Merged
Conversation
Collaborator
|
thanks, this fixed it |
Checking on whether we access a null field is ok, but should we crash right away? Probably not. This reduces the access to a warning on sqlite3 and let's it continue. We can look for occurences and fix them as they come up and then re-arm the asserts once we addressed all cases.
The case where this is needed is when the wallet had a forwarded payment somewhere between commits 66a47d2 (which started tracking forwardings) and d901304 (which added the `received_time` column). This just emulates the behavior of sqlite3 for postgres as well. Signed-off-by: Christian Decker <@cdecker>
cdecker
force-pushed
the
fix-postgres-for-real
branch
from
11de3ce to
e86c5ba
Compare
Collaborator
|
ACK 037aad2 |
4 tasks
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Reverts the
assertinto a warning that we can watch for in the tests and logs, so we can fix them as they come up and then re-arm the asserts once we're happy that we have caught all of them. On postgres we'll still catch them given the field length check,We also fix an upgrade related null field (which was the real cause for #3189), where we'd have a null field if a payment was forwarded inbetween commits 66a47d2 (which started tracking forwardings) and d901304 (which added the received_time column). Since this is a rare thing, and we can't really just remove a compat check later, I opted to just set these cases to 0.
Fixes #3189