django-pgtrigger icon indicating copy to clipboard operation
django-pgtrigger copied to clipboard

migration error: cannot alter type of a column used in a trigger definition

Open rrauenza opened this issue 3 years ago • 2 comments

More of a question than a bug -- is there a best practice for doing migrations when triggers exist?

Right now I'm just doing a manage pgtrigger uninstall before I manage migrate. I don't like that this leaves a window where triggers are disabled, though, and I don't want to shut down the entire app.

Wondering if anyone else has found a better way for dealing with the issue.

rrauenza avatar Dec 20 '21 06:12 rrauenza

@rrauenza yea this is a tricky issue. In the latest version of pgtrigger, it is integrated with the migration system, which should help some of these use cases. Unfortunately I don't think it will work for altering column types out of the box though.

I'm going to play around with this use case a bit and report back. I believe you can manually handle this by doing a pgtrigger.migrations.RemoveTrigger before the column altering followed by a pgtrigger.migrations.AddTrigger. The important thing to note here is that they should be in the same migration file as an atomic operation to ensure that no table updates can happen when the triggers are disabled.

Again, will play around with this to try it out with the new migration integration. If there's no way I can automagically detect this, I can add an example to the "Advanced Installation" section.

wesleykendall avatar Aug 09 '22 20:08 wesleykendall

@rrauenza just verified that this is a bug even with integration in the migration system.

I'm going to follow up on this one next week and try to tackle it as follows:

  1. Try to autodetect the field change in the migration so that the appropriate drop/recreate can happen with the field type change
  2. Add some docs to let people know that migrations should be atomic whenever doing drop and recreates. They are by default, but there are consequences like you mentioned if you don't use atomic migrations
  3. Add the ability for people to more easily add trigger operations in their migration files. There are going to be some scenarios where people are going to have to manually drop/recreate them in mirations.

for now the workaround is to use pgtrigger.uninsall("trigger_uri") and pgtrigger.install("trigger_uri") in RunPython operations around the altered column in the migration file

wesleykendall avatar Aug 14 '22 02:08 wesleykendall

@rrauenza this is fixed in https://github.com/Opus10/django-pgtrigger/pull/83

To fix this, I patched the schema editor to temporarily drop/recreate the affected triggers when the column type is changed.

I made a test case for this that dynamically alters the column type of a column that's used in multiple trigger conditions. If for any reason the patching of the schema editor causes issues, there are instructions in the FAQ of the docs on how to turn it off.

Will be deployed in v4.1

wesleykendall avatar Aug 17 '22 18:08 wesleykendall

Note that this fix works with both the migration system and with the legacy way of installing all triggers after migrations.

wesleykendall avatar Aug 17 '22 18:08 wesleykendall

Version 4.1 is now deployed!

wesleykendall avatar Aug 17 '22 19:08 wesleykendall