Skip to content
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

database_schema_version table stores current database schema version. #3105

Merged
merged 3 commits into from
Feb 11, 2022

Conversation

Unisay
Copy link
Contributor

@Unisay Unisay commented Feb 4, 2022

Issue Number

ADP-1135

This PR adds a database table database_metadata with 1 row and 1 column in it: version (from here DV).
The idea is that wallet library knows (hardcodes) version of the database file format that it expects to work with (EV), and then:

  • if DV > EV then its a "database from the future" and no changes should be made to it. Following migrations are aborted.
  • if DV < EV then its a database from the past, and it needs to be migrated (the new mechanism to apply migrations isn't implemented yet)
  • if DV = EV then its all good.

@Unisay Unisay self-assigned this Feb 4, 2022
@Unisay Unisay force-pushed the ADP-1135-add-table-file-format-to-database branch 4 times, most recently from 187839e to ceb5357 Compare February 4, 2022 11:57
Copy link
Contributor

@rvl rvl left a comment

Choose a reason for hiding this comment

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

👍

lib/core/src/Cardano/Wallet/DB/Sqlite/Migration.hs Outdated Show resolved Hide resolved
lib/core/test/unit/Cardano/Wallet/DB/SqliteSpec.hs Outdated Show resolved Hide resolved
@Unisay Unisay force-pushed the ADP-1135-add-table-file-format-to-database branch from f5bbf7c to 1018cd2 Compare February 7, 2022 14:55
@Unisay Unisay marked this pull request as ready for review February 7, 2022 14:55
@Unisay Unisay force-pushed the ADP-1135-add-table-file-format-to-database branch 2 times, most recently from 4781217 to f7f5e42 Compare February 8, 2022 10:26
@Unisay
Copy link
Contributor Author

Unisay commented Feb 8, 2022

I made the following changes to this PR:

  • database table renamed to database_schema_version to avoid clashes with other usages of the word metadata.
  • removed traces from migration as it seems to have a complexity costs not worth the value. Verification in the unit test is based on the SQL query instead of traces now.

@HeinrichApfelmus do you think this PR is ready to be merged or is there anything else you'd like to add? Thanks.

@Unisay Unisay changed the title database_metadata table stores current database schema (file format) version. database_schema_version table stores current database schema version. Feb 8, 2022
@Unisay Unisay requested a review from rvl February 9, 2022 10:03
Copy link
Contributor

@HeinrichApfelmus HeinrichApfelmus left a comment

Choose a reason for hiding this comment

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

😊

@Unisay Unisay force-pushed the ADP-1135-add-table-file-format-to-database branch from 0b9b257 to bda3702 Compare February 10, 2022 11:21
@Unisay Unisay force-pushed the ADP-1135-add-table-file-format-to-database branch from bda3702 to bb41b55 Compare February 11, 2022 10:05
@Unisay Unisay enabled auto-merge February 11, 2022 10:14
@rvl rvl disabled auto-merge February 11, 2022 10:17
@rvl rvl merged commit e6b0585 into master Feb 11, 2022
@rvl rvl deleted the ADP-1135-add-table-file-format-to-database branch February 11, 2022 10:17
@Unisay
Copy link
Contributor Author

Unisay commented Feb 11, 2022

🎆

WilliamKingNoel-Bot pushed a commit that referenced this pull request Feb 11, 2022
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.

3 participants