-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Store the schema version in the database. #1956
Conversation
@edolstra If this approach looks good to you, I'll add tests, so far I've only compile-tested. |
I don't really see the need for this. Removing the "legacy" schema would make it impossible to change to a different storage (e.g. replace SQLite with something else). |
@edolstra The need for this is evidenced by #1954. This PR handles the case where we want to move to a different storage: We keep the legacy schema around, bump it to |
Sounds a good idea to me, but I wonder if the |
According to this one SO user, yes 👅 https://stackoverflow.com/questions/7359721/sqlite-are-pragma-statements-undone-by-rolling-back-transactions/18550663#18550663 |
I wonder if this could be done simpler though: just read the schema from both places, but if the one stored in SQLite is non-zero, consider it as the authoritative one, and upgrade the flat file to match. |
Waiting on @edolstra to decide what we want to do about this, but that seems reasonable. |
Yeah that sounds good to me. |
Before this, there was a gap between the database update and changes to the schema version file, leading to NixOS#1954. Now database changes and schema bumps happen in a single transaction. Fixes NixOS#1954.
64e7d8a
to
60af4ee
Compare
@edolstra updated. |
@edolstra ping |
Before this, there was a gap between the database update and changes
to the schema version file, leading to #1954. Now database changes and
schema bumps happen in a single transaction.
To avoid gratuitous backwards incompatibility, we still write 10 to
the old version file, and will write 11 on the next schema bump. We
won't write 12 to it unless we're moving away from storing the schema
in the database.
Fixes #1954.