tapdb: fix bug w.r.t null bool handling, allow InsertScriptKey to allow flipping known to true #1185
Conversation
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
Roasbeef
requested review from
a team,
ffranr and
jharveyb
and removed request for
a team
Roasbeef
force-pushed
the
script-key-upsert
branch
from
951109c to
2b288da
Compare
Pull Request Test Coverage Report for Build 11808369738Details
💛 - Coveralls |
This only works for bools that are always set? Vs. other bools that are optional. I think this would be correct for any spot where we insert data created with |
jharveyb
reviewed
Nov 11, 2024
Roasbeef
force-pushed
the
script-key-upsert
branch
2 times, most recently
from
c27ea4d to
922448d
Compare
Roasbeef
commented
Nov 12, 2024
Roasbeef
requested review from
guggero and
jharveyb
and removed request for
ffranr
Roasbeef
force-pushed
the
script-key-upsert
branch
from
922448d to
08c182d
Compare
In this commit, we add a new `extractBool` helper function. In an upcoming commit, we'll use this to fix incorrect usage of the `sql.NullBool` type.
In this commit, we fix some incorrect handling on null bool in SQL, that looks to be mostly benign. Before this commit, instead of doing `.Bool`, we did `.Valid`. The latter is only useful to check if something is zero, or NULL. In this case, we can just go with the bool value directly.
In this commit, we update `InsertScriptKey` to allow flipping known to true, if it was false before. This is useful as at times a script key from a smart contract might have been inserting on disk, but with declared known as false. Then if we tried to insert it again, with known as true, the upsert logic would end up making no change on disk.
Roasbeef
force-pushed
the
script-key-upsert
branch
from
08c182d to
d453f81
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 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.
In this commit, we fix some incorrect handling on null bool in SQL, that
looks to be mostly benign. Before this commit, instead of doing
.Bool,we did
.Valid. The latter is only useful to check if something iszero, or NULL. In this case, we can just go with the bool value
directly.
Then, we update
InsertScriptKeyto allow flipping known totrue, if it was false before.
This is useful as at times a script key from a smart contract might have
been inserting on disk, but with declared known as false. Then if we
tried to insert it again, with known as true, the upsert logic would end
up making no change on disk.
Note that without the initial bug fix, the test added in the last commit
would fail.