Skip to content

fix: use write transaction in SpkiHashStore.cleanup()#8135

Merged
link2xt merged 1 commit intomainfrom
link2xt/mlouyrnzotow
Apr 19, 2026
Merged

fix: use write transaction in SpkiHashStore.cleanup()#8135
link2xt merged 1 commit intomainfrom
link2xt/mlouyrnzotow

Conversation

@link2xt
Copy link
Copy Markdown
Collaborator

@link2xt link2xt commented Apr 18, 2026

query_map_vec() uses read-only connection,
so it cannot be used to delete rows.

Follow-up to #8086

query_map_vec() uses read-only connection,
so it cannot be used to delete rows.
@link2xt
Copy link
Copy Markdown
Collaborator Author

link2xt commented Apr 18, 2026

I think we need to make housekeeping stop ignoring errors at some point. There is only one test failing if we do this, it is testing housekeeping on closed database 2a39dc0 (#2255). I looked at the PR and it seems the test was introduced when housekeeping was mostly enumerating used files, and now because of the test we ignore all sort of SQL errors that are better not ignored.

Copy link
Copy Markdown
Collaborator

@iequidoo iequidoo left a comment

Choose a reason for hiding this comment

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

Not ignoring errors makes sense if next steps depend on the current one, otherwise debug assertions look better to me. If an error occurs in the release configuration, it will be an untested condition anyway because a part of the code has already run and caused side effects, skipping the remaining steps won't be safer probably

@link2xt
Copy link
Copy Markdown
Collaborator Author

link2xt commented Apr 19, 2026

Not ignoring errors makes sense if next steps depend on the current one, otherwise debug assertions look better to me.

Debug assertion would also be fine. I cannot add it into this PR as well because then housekeeping test will fail with panic.

@link2xt link2xt merged commit 8cd06bb into main Apr 19, 2026
31 checks passed
@link2xt link2xt deleted the link2xt/mlouyrnzotow branch April 19, 2026 09:15
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.

2 participants