-
Notifications
You must be signed in to change notification settings - Fork 25
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
we can't _just_ do the same process as 0467 because there are check constraints that depend on these. When you say "alter column template_type set type = new_type using old_type::text::new_type", it takes that "using" statement as a way of translating the prev enum into the new enum, in our case just casting to a str as an intermediate step. however, the template_type column has several check constraints against it (to say only letters can have a letter_attachment for example). Postgres isn't smart enough to take the `using` clause and apply it to these constraints. There are two ways around this: * You could create a comparator function that tells postgres "if you have to compare old_type with new_type, here's a custom function"[^1] * You can drop the old constraints and recreate them after the migration is done We've gone with the second option as it's a bit simpler and I could copy and paste from our previous scripts [^1]: https://stackoverflow.com/a/57952345 was very helpful for this
- Loading branch information
1 parent
ecf40a6
commit 8781f8a
Showing
3 changed files
with
159 additions
and
1 deletion.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
0465_delete_broadcast_data | ||
0468_remove_broadcast_type |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,132 @@ | ||
""" | ||
Create Date: 2024-10-01 11:08:46.900469 | ||
""" | ||
|
||
from alembic import op | ||
import sqlalchemy as sa | ||
from sqlalchemy.dialects import postgresql | ||
|
||
revision = "0468_remove_broadcast_type" | ||
down_revision = "0467_remove_broadcast_perms" | ||
|
||
|
||
template_type_new_values = ["email", "sms", "letter"] | ||
template_type_new = postgresql.ENUM(*template_type_new_values, name="template_type") | ||
|
||
|
||
def drop_check_constraints(): | ||
op.drop_constraint("chk_templates_letter_languages", "templates") | ||
op.drop_constraint("chk_templates_history_letter_languages", "templates_history") | ||
|
||
op.drop_constraint("ck_templates_letter_attachments", "templates") | ||
op.drop_constraint("ck_templates_history_letter_attachments", "templates_history") | ||
|
||
op.drop_constraint("ck_templates_non_email_has_unsubscribe_false", "templates") | ||
op.drop_constraint("ck_templates_history_non_email_has_unsubscribe_false", "templates_history") | ||
|
||
|
||
def add_check_constraints(): | ||
op.create_check_constraint( | ||
"chk_templates_letter_languages", | ||
"templates", | ||
""" | ||
CASE WHEN template_type = 'letter' THEN | ||
letter_languages is not null | ||
ELSE | ||
letter_languages is null | ||
END | ||
""", | ||
postgresql_not_valid=True, | ||
) | ||
op.create_check_constraint( | ||
"chk_templates_history_letter_languages", | ||
"templates_history", | ||
""" | ||
CASE WHEN template_type = 'letter' THEN | ||
letter_languages is not null | ||
ELSE | ||
letter_languages is null | ||
END | ||
""", | ||
postgresql_not_valid=True, | ||
) | ||
|
||
op.create_check_constraint( | ||
"ck_templates_letter_attachments", | ||
"templates", | ||
"template_type = 'letter' OR letter_attachment_id IS NULL", | ||
postgresql_not_valid=True, | ||
) | ||
op.create_check_constraint( | ||
"ck_templates_history_letter_attachments", | ||
"templates_history", | ||
"template_type = 'letter' OR letter_attachment_id IS NULL", | ||
postgresql_not_valid=True, | ||
) | ||
|
||
op.create_check_constraint( | ||
"ck_templates_non_email_has_unsubscribe_false", | ||
"templates", | ||
"template_type = 'email' OR has_unsubscribe_link IS false", | ||
postgresql_not_valid=True, | ||
) | ||
op.create_check_constraint( | ||
"ck_templates_history_non_email_has_unsubscribe_false", | ||
"templates_history", | ||
"template_type = 'email' OR has_unsubscribe_link IS false", | ||
postgresql_not_valid=True, | ||
) | ||
|
||
|
||
def upgrade(): | ||
""" | ||
there are three check constraints on the template_type column (across both templates and templates_history). | ||
These check constraints say things like "if type == 'letter'" - this is an enum comparison, so if we change the | ||
type of the column, we get an error | ||
> (psycopg2.errors.UndefinedFunction) operator does not exist: template_type = template_type_old | ||
> HINT: No operator matches the given name and argument types. You might need to add explicit type casts. | ||
This appears confusing because we're passing in a `using` clause to tell postgres exactly how to do this, but | ||
this using clause only applies to the column type, and not other usages of that column within the check constraints. | ||
To get round this, we "simply" drop all the constraints and then recreate them afterwards, referring to the new | ||
""" | ||
conn = op.get_bind() | ||
conn.execute("ALTER TYPE template_type RENAME TO template_type_old;") | ||
template_type_new.create(conn) | ||
|
||
drop_check_constraints() | ||
|
||
op.alter_column( | ||
table_name="templates", | ||
column_name="template_type", | ||
type_=template_type_new, | ||
postgresql_using="template_type::text::template_type", | ||
) | ||
op.alter_column( | ||
table_name="templates_history", | ||
column_name="template_type", | ||
type_=template_type_new, | ||
postgresql_using="template_type::text::template_type", | ||
) | ||
op.alter_column( | ||
table_name="service_contact_list", | ||
column_name="template_type", | ||
type_=template_type_new, | ||
postgresql_using="template_type::text::template_type", | ||
) | ||
|
||
conn.execute("DROP TYPE template_type_old;") | ||
# note: need to revalidate these constraints in a separate migration to avoid a lengthy access exclusive lock | ||
add_check_constraints() | ||
|
||
|
||
def downgrade(): | ||
# don't need to do the constraints dance, or the enum type dance, when adding a new value | ||
|
||
# ALTER TYPE must be run outside of a transaction block (see link below for details) | ||
# https://alembic.sqlalchemy.org/en/latest/api/runtime.html#alembic.runtime.migration.MigrationContext.autocommit_block | ||
with op.get_context().autocommit_block(): | ||
op.execute("ALTER TYPE template_type ADD VALUE 'broadcast'") |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
""" | ||
Create Date: 2024-10-01 11:08:46.900469 | ||
""" | ||
|
||
revision = "0469_validate_template_ck" | ||
down_revision = "0468_remove_broadcast_type" | ||
|
||
from alembic import op | ||
import sqlalchemy as sa | ||
from sqlalchemy.dialects import postgresql | ||
|
||
|
||
def upgrade(): | ||
|
||
op.execute("ALTER TABLE templates VALIDATE CONSTRAINT chk_templates_letter_languages") | ||
op.execute("ALTER TABLE templates_history VALIDATE CONSTRAINT chk_templates_history_letter_languages") | ||
|
||
op.execute("ALTER TABLE templates VALIDATE CONSTRAINT ck_templates_letter_attachments") | ||
op.execute("ALTER TABLE templates_history VALIDATE CONSTRAINT ck_templates_history_letter_attachments") | ||
|
||
op.execute("ALTER TABLE templates VALIDATE CONSTRAINT ck_templates_non_email_has_unsubscribe_false") | ||
op.execute("ALTER TABLE templates_history VALIDATE CONSTRAINT ck_templates_history_non_email_has_unsubscribe_false") | ||
|
||
|
||
def downgrade(): | ||
pass |