Migration changing uuids from byte to hex format in db #106

Merged
reynir merged 5 commits from 20220509_migrating_uuids_to_hex_format_in_db into main 3 months ago
rand commented 3 months ago
Owner

This PR is for supporting simple querying for uuids in db - instead of going through OCaml or C plugin for sqlite.

A CLI tool for doing this was too errorprone, as shells can't handle binary values except via stdin (and then one would need a prepared statement to pass data to sqlite).

A small extension to Uuidms uuidtrip for conversion of input uuids was created in this process - it's available at https://github.com/rand00/uuidm/tree/20220505_adding_convert_option_to_uuidtrip . Though it now depends on Unix to read stdin.

This PR is for supporting simple querying for uuids in db - instead of going through OCaml or C plugin for sqlite. A CLI tool for doing this was too errorprone, as shells can't handle binary values except via stdin (and then one would need a prepared statement to pass data to sqlite). A small extension to `Uuidm`s `uuidtrip` for conversion of input uuids was created in this process - it's available at https://github.com/rand00/uuidm/tree/20220505_adding_convert_option_to_uuidtrip . Though it now depends on `Unix` to read `stdin`.
rand added 1 commit 3 months ago
rand added 1 commit 3 months ago
rand added 1 commit 3 months ago
reynir added 3 commits 3 months ago
rand added 4 commits 3 months ago
reynir force-pushed 20220509_migrating_uuids_to_hex_format_in_db from 02c3e05d8e to da2aa77b53 3 months ago
reynir added 1 commit 3 months ago
Owner

I think this PR is good to merge now.

Some comments:

I asked for recreating indices because I expected we would need to change the column type for uuid from BLOB to TEXT/VARCHAR(36). Sqlite3 has limited support for altering tables, and often you have to perform a dance of copying tables, and this dance requires one to recreate all relevant indices because it's strictly speaking a new table. However, the uuid column has been a VARCHAR(36) since the beginning by mistake, and luckily we don't seem to have suffered from any data loss or corruption due to this mistake. The current indices can be found in the definition of Builder_db.migrate.

I prefer to not rely on parts of Builder_db.Rep that might change (like uuid) in migration scripts. Things I consider safe is id, fpath and cstruct.

Remember to also update the db version in db/builder_db.ml as well :-)

@rand sorry for the force pushes

I think this PR is good to merge now. Some comments: I asked for recreating indices because I expected we would need to change the column type for `uuid` from BLOB to TEXT/VARCHAR(36). Sqlite3 has limited support for altering tables, and often you have to perform a dance of copying tables, and this dance requires one to recreate all relevant indices because it's strictly speaking a new table. However, the uuid column has been a VARCHAR(36) since the beginning by mistake, and luckily we don't seem to have suffered from any data loss or corruption due to this mistake. The current indices can be found in the definition of `Builder_db.migrate`. I prefer to not rely on parts of `Builder_db.Rep` that might change (like uuid) in migration scripts. Things I consider safe is `id`, `fpath` and `cstruct`. Remember to also update the db version in db/builder_db.ml as well :-) @rand sorry for the force pushes
Poster
Owner

Ah yes but concerning the index, there is another reason for recreating it - if the old index is used, the old values cached in the index are still kept around - so they take up space.

Ah yes but concerning the index, there is another reason for recreating it - if the old index is used, the old values cached in the index are still kept around - so they take up space.
Owner

Ah yes but concerning the index, there is another reason for recreating it - if the old index is used, the old values cached in the index are still kept around - so they take up space.

I'm not convinced that's the case. Otherwise, you would get wrong results with a 'stale' index, right?

> Ah yes but concerning the index, there is another reason for recreating it - if the old index is used, the old values cached in the index are still kept around - so they take up space. I'm not convinced that's the case. Otherwise, you would get wrong results with a 'stale' index, right?
reynir merged commit 9f5cc4d156 into main 3 months ago
hannes deleted branch 20220509_migrating_uuids_to_hex_format_in_db 3 months ago
The pull request has been merged as 9f5cc4d156.
Sign in to join this conversation.
No reviewers
No Label
No Milestone
No Assignees
2 Participants
Notifications
Due Date

No due date set.

Dependencies

This pull request currently doesn't have any dependencies.

Loading…
There is no content yet.