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

Fix change_column_null when raising an error in safe mode #271

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

chaadow
Copy link
Contributor

@chaadow chaadow commented Sep 1, 2024

When changing a column null value from true to false (namely, the column becomes NOT NULL) and safe mode is enabled:

  • we add a NOT VALID check constraint
  • then commit the transaction
  • and finally validate the check constraint in another migration.

The issue arises when the validation of check constraint fails. This leaves an invalid check constraint in the database and it is NOT dropped in a cleanup phase.
After making sure that all values are NOT NULL), we then retry to run rails db:migrate. However this fails, because, since the check constraint already exists, it will result in an error raised stating that said check constraint already exists.

This commit fixes the issue in both ways:

  • Firstly, if we're already in this unstable state of a NOT VALID check constraint already existing in the database, we avoid the recreation of the constraint and simply ignore this step
  • Secondly, in case the check validation fails, we rescue the exception and execute the drop of the check constraint to avoid leaving the database in an unstable state.

When changing a column null value from true to false (namely, the column
becomes NOT NULL) and `safe mode` is enabled:
-  we add a NOT VALID check constraint
- then commit the transaction
-  and finally validate the check constraint in another migration.

The issue arises when the validation of check constraint fails. This
leaves an invalid check constraint in the database and it is NOT
dropped in a cleanup phase.
After making sure that all values are NOT NULL), we then retry to run
`rails db:migrate`. However this fails, because, since the check
constraint already exists, it will result in an error raised stating
that said check constraint **already** exists.

This commit fixes the issue in both ways:
- Firstly, if we're already in this unstable state of a NOT VALID check
  constraint already existing in the database, we avoid the recreation
  of the constraint and simply ignore this step
- Secondly, in case the check validation fails, we rescue the exception
  and execute the drop of the check constraint to avoid leaving the
  database in an unstable state.
chaadow added a commit to chaadow/strong_migrations that referenced this pull request Sep 1, 2024
Followup to ankane#271

When adding a CHECK constraint and safe mode is enabled:

- we add a NOT VALID check constraint
- then commit the transaction
- and finally validate the check constraint in another migration.

If the validation step fails, this leaves a NOT VALID check constraint
in the database.
If we retry after having cleaned up the database, the creation of the
NOT VALID check constraint will fail because it will already be in the
DB.

This commit makes the migration safely retriable by checking if said
check constraint already exists, and if so, skips its creation again.
chaadow added a commit to chaadow/strong_migrations that referenced this pull request Sep 1, 2024
Followup to ankane#271

When adding a CHECK constraint and safe mode is enabled:

- we add a NOT VALID check constraint
- then commit the transaction
- and finally validate the check constraint in another migration.

If the validation step fails, this leaves a NOT VALID check constraint
in the database.
If we retry after having cleaned up the database, the creation of the
NOT VALID check constraint will fail because it will already be in the
DB.

This commit makes the migration safely retriable by checking if said
check constraint already exists, and if so, skips its creation again.
chaadow added a commit to chaadow/strong_migrations that referenced this pull request Sep 1, 2024
Followup to ankane#271

When adding a CHECK constraint and safe mode is enabled:

- we add a NOT VALID check constraint
- then commit the transaction
- and finally validate the check constraint in another migration.

If the validation step fails, this leaves a NOT VALID check constraint
in the database.
If we retry after having cleaned up the database, the creation of the
NOT VALID check constraint will fail because it will already be in the
DB.

This commit makes the migration safely retriable by checking if said
check constraint already exists, and if so, skips its creation again.
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.

1 participant