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

Make sqlite-compatible migration step #477

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

AndrewFerr
Copy link
Contributor

The schema upgrade added in #464 uses ALTER COLUMN, so it's incompatible with SQLite.

This PR uses the same strategy as the v16 & v17 upgrades for the main store to add a SQLite-compatible version of this migration.

@tulir
Copy link
Member

tulir commented Mar 12, 2024

I thought EMS used Postgres 🤔

@AndrewFerr
Copy link
Contributor Author

I thought EMS used Postgres 🤔

It does, but my local dev environment does not 🙂

@tulir
Copy link
Member

tulir commented Mar 12, 2024

You could just manually fix it :P The v7 -> v8 migration is exclusively for users of your PR, the main branch jumped from v6 to v8 and the alter tables there are already scoped to postgres only (because they're less important).

Anyway, if you really want sqlite support for that migration: there are no foreign keys referencing signalmeow_contacts, so disabling foreign keys and transactions isn't necessary and therefore a normal dialect-split upgrade works perfectly fine. The way to do that is to have two files with the same name and header comment, but one file name ending in .sqlite.sql and the other in .postgres.sql. https://github.com/mautrix/discord/blob/main/database/upgrades/13-merge-emoji-and-file.sqlite.sql + https://github.com/mautrix/discord/blob/main/database/upgrades/13-merge-emoji-and-file.postgres.sql is an example of that

as there are no foreign keys referencing signalmeow_contacts
@AndrewFerr
Copy link
Contributor Author

Thanks for the tips! I wasn't aware of the file-extension-based dialect splitting. Part of my reasoning in writing this PR is to learn the "proper" way to do mautrix schema upgrades, so this is a good outcome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants