postgresql to aurora db migration fix #392
Conversation
|
|
mdm-core/grails-app/conf/db/migration/core/V5_1_3__delete_invalid_breadcrumb_trees.sql
Show resolved
Hide resolved
There was a problem hiding this comment.
I think there are a few issues, could you have a look at the below?
- The triggers are used to enforce the constraints (foreign keys etc.) in the DB so it can definitely cause a problem deleting without disabling them, I've tried on a test DB and got:
DELETE FROM core.breadcrumb_tree
[2023-02-06 21:19:04] [23503] ERROR: update or delete on table "breadcrumb_tree" violates foreign key constraint "fksiu83nftgdvb7kdvaik9fghsj" on table "data_type"
[2023-02-06 21:19:04] Detail: Key (id)=(8f7aed00-1a0d-4f47-9c9b-3b4230a5ad6e) is still referenced from table "data_type".
- Deleting the schema entry will mean it runs again on already migrated DBs which isn't ideal as BreadcrumbTrees have been a sensitive area in the past, it would be best to update the checksum by using the flyway CLI (
flyway repair) to get the updated checksum for the file. - Looks like a typo in the version number (5.1.3)
|
If old database data won't be imported into Aurora, maybe you could use a conditional to exclude this rebuild/delete of the BreadcrumbTrees from running on Aurora at all? |
|
Thanks @oleksiiRybak, I haven't tested this myself yet but I have some comments. |
|
If you do need a solution for Aurora and Postgres DBs with existing data in the future, I think what should work is using the information tables to look up the names of the constraints from foreign keys etc., and then use dynamic (plpg)SQL to disable/enable them by name, which should avoid the superuser issue. |
|
@joe-crawford I've done some further changes, please review them... |
There was a problem hiding this comment.
Using session_replication_role looks good if that works on Aurora too.
However the migration will fail on existing Postgres DBs due to the index already being created, could you change the DELETE flyway entry to UPDATE with the checksum?
Could you also have a look at the formatting comment.
Thanks 👍
mdm-core/grails-app/conf/db/migration/core/V5_1_3__delete_invalid_breadcrumb_trees.sql
Outdated
Show resolved
Hide resolved
| FROM core.flyway_schema_history | ||
| WHERE version = '5.1.3' AND | ||
| description = 'delete invalid breadcrumb trees'; |
There was a problem hiding this comment.
Could you use the flyway CLI to calculate the checksum and turn this into an update like the one at the top of the file? This will make sure the migration doesn't run on previously migrated systems, which is safer.
There was a problem hiding this comment.
@joe-crawford I've added an additional protection in case we run create index in a db which already has it and similarly with the temporary valid_breadcrumb_trees_temp so it shouldn't cause any problems so I think it is OK to go with delete in this case.
There was a problem hiding this comment.
I think it should still be UPDATE as rerunning the migrations in general isn't really safe, and if it fails it stops the system from starting. In this case it will still cause problems on existing Postgres DBs because the temporary table is dropped per session so it will still try to insert rows.
There was a problem hiding this comment.
ok, I will look into that. thanks
There was a problem hiding this comment.
From memory you have to install the flyway CLI, run flyway repair and then that updates the value in the flyway_schema_migration table with the new checksum.
mdm-core/grails-app/conf/db/migration/core/V5_1_3__delete_invalid_breadcrumb_trees.sql
Outdated
Show resolved
Hide resolved
| DO | ||
| $$ | ||
| BEGIN | ||
| IF EXISTS (SELECT aurora_version()) THEN | ||
| SET session_replication_role = replica; | ||
| DELETE FROM core.breadcrumb_tree; | ||
| SET session_replication_role = DEFAULT; | ||
| ELSE | ||
| ALTER TABLE core.breadcrumb_tree DISABLE TRIGGER ALL; | ||
| DELETE FROM core.breadcrumb_tree; | ||
| ALTER TABLE core.breadcrumb_tree ENABLE TRIGGER ALL; | ||
| END IF; | ||
|
|
||
| exception | ||
| when undefined_function then | ||
| ALTER TABLE core.breadcrumb_tree DISABLE TRIGGER ALL; | ||
| DELETE FROM core.breadcrumb_tree; | ||
| ALTER TABLE core.breadcrumb_tree ENABLE TRIGGER ALL; | ||
| END | ||
| $$; |
There was a problem hiding this comment.
This looks good to me and I have tested this on Postgres.
|
Great, this looks good to me. Have tested on a Postgres instance with existing data and a new blank Postgres database, both successful. |
No description provided.