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 duplicate inferred migrations when dropping columns outside of a migration #305

Merged
merged 7 commits into from
Mar 6, 2024

Conversation

andrew-farries
Copy link
Collaborator

Ensure that only one inferred migration is created in the pgroll.migrations table when a column is dropped outside of a migration.

From the Postgres docs:

The sql_drop event occurs just before the ddl_command_end event trigger for any operation that drops database objects

This means that when the raw_migration function is run in response to sql_drop and ddl_command_end, duplicate entries will be created in pgroll.migrations; once as the function is run for sql_drop and again when it's run for ddl_command_end.

Change the definition of the pg_roll_handle_drop event trigger to only run on those kinds of drops that won't result in duplicates when the pg_roll_handle_ddl trigger runs for the same change. DROP TABLE and DROP VIEW won't result in duplicate migrations because their schema can't be inferred by the ddl_command_event trigger because the object has already been dropped when the trigger runs.

Update the inferred migration tests with two new testcases covering dropping tables and columns.

Fixes #304

Generalize the test function to allow for multiple SQL statements and
multiple expected `inferred` migrations.

Add testcases to:
* Ensure that dropping a table creates exactly one inferred migration
* Ensure that dropping a column creates exactly one inferred migration
pkg/state/state.go Outdated Show resolved Hide resolved
Copy link
Member

@exekias exekias left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change LGTM, but want to make sure we know we are covering all objects

`ddl_command_end` event triggers also run on `sql_drop` and in most cases
contain enough information for the `pgroll.migrations` table to be
populated. For example, when a column is dropped from a table, the
schema name is available via `pg_catalog.pg_event_trigger_ddl_commands`.

Only run `raw_migration` on `sql_drop` for those kinds of drops that do
not populate the schema name on `ddl_command_end` to ensure that
duplicate entries in the `pgroll.migations` table are not created.
@andrew-farries andrew-farries force-pushed the fix-duplicate-drop-column-migrations branch from 0f77390 to 801c56f Compare March 6, 2024 09:02
@andrew-farries andrew-farries merged commit 162bd06 into main Mar 6, 2024
42 checks passed
@andrew-farries andrew-farries deleted the fix-duplicate-drop-column-migrations branch March 6, 2024 09:56
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.

Dropping columns results in duplicate inferred migrations
2 participants