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

Add a REINDEX NOCASE v2.15.1 migration and update v2.15.0 migration #3533

Merged
merged 3 commits into from
Oct 18, 2024

Conversation

mikiher
Copy link
Contributor

@mikiher mikiher commented Oct 18, 2024

This is a migration aimed at fixing potential hidden old database index corruptiuons due to the faulty introduction of the unicode extension in v2.12.0. It perform a reindexing of all NOCASE indices in the database.

Migration v.2.15.0 is also updated to include the same fix, since we've seen that migration fail due these corruptions.

@mikiher mikiher changed the title Add REINDEX NOCASE migration and update v2.15.0 migration Add a REINDEX NOCASE v2.15.1 migration and update v2.15.0 migration Oct 18, 2024
@mikiher mikiher marked this pull request as ready for review October 18, 2024 06:35
@advplyr
Copy link
Owner

advplyr commented Oct 18, 2024

Since we've identified a few indexes that should be added I wonder if we should try to put them in a migration or just put them in to sequelize like #3528

From my experience I have seen that sequelize will create the index if it doesn't exist but I'm not sure how full-proof that is. What do you think?

I'm leaning towards putting it in a migration to be safe but might be unnecessary.

@mikiher
Copy link
Contributor Author

mikiher commented Oct 18, 2024

I would prefer a migration.

It's true that additional indices usually do not change the functionality of the database, just its performance characteristics, but it is still considered a good practice to have a migration - this keeps the database schema consistent with code versions, which is part of the point of using migrations.

For example, you know that when server version 2.17.0 is running, function X should be fast because it uses an index added in version 2.16.0 migration, but when the server version is 2.15.0, you expect X to be slow, because either the index was not yet added then, or because it was reverted due to a downward migration.

@advplyr
Copy link
Owner

advplyr commented Oct 18, 2024

My thoughts as well.
Tested this migration and looks good. Thanks!

@advplyr advplyr merged commit 5079787 into advplyr:master Oct 18, 2024
5 checks passed
@mikiher mikiher deleted the reindex-nocase-migraton branch November 18, 2024 07:05
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