AlterField migration on ForeignKey field re-creates foreign key constraints unnecessarily

[Ömer Faruk Abacı]

Let's assume that we have the following models:

class Bar(models.Model):
    pass

class Foo(models.Model):
    bar = models.ForeignKey(Bar, related_name="foos", on_delete=models.CASCADE)

The migration that creates these models will add an index to the bar field of Foo model, this is well-documented and there is no problem up until now. But when we do the following change to drop the index on the ForeignKey field:

class Foo(models.Model):
    bar = models.ForeignKey(..., db_index=False)

Then we have a migration that results in the following SQL statement:

BEGIN;
--
-- Alter field bar on foo
--
SET CONSTRAINTS "foos_foo_bar_id_efb062ad_fk_foos_bar_id" IMMEDIATE;
ALTER TABLE "foos_foo"
    DROP CONSTRAINT "foos_foo_bar_id_efb062ad_fk_foos_bar_id";

DROP INDEX IF EXISTS "foos_foo_bar_id_efb062ad";

ALTER TABLE "foos_foo" ADD CONSTRAINT "foos_foo_bar_id_efb062ad_fk_foos_bar_id"
    FOREIGN KEY ("bar_id")
    REFERENCES "bars_bar" ("id")
    DEFERRABLE INITIALLY DEFERRED;
COMMIT;

I believe that we shouldn't be needing to re-create the FK constraint, so the following SQL should suffice:

BEGIN;
--
-- Alter field bar on foo
--
DROP INDEX IF EXISTS "foos_foo_bar_id_efb062ad";
COMMIT;

Actually the main problem here is that dropping & adding constraints locks the entire table, and it might be catastrophic for some live production databases. IMHO this should either be documented or refactored. I look forward to hear your opinions on this, thank you in advance!

[Simon Charette]

Dropping the foreign key and recreating it ​is effectively not necessary on PostgreSQL but ​it is on MySQL as this backend requires an index on the referencing side of the relation and will silently default to using an existing one if its present (see #31335 for more details).

Given the contention issues associated with dropping a foreign key on PostgreSQL I think it's a worthy investment to try to make things better on databases that are not affected by this quirk.

Would you like ​to work on a patch? The ​alter_field method is where you want to start looking and you'll likely need a feature flag to denote that foreign keys must be dropped or not based on the backend.

[Ömer Faruk Abacı]

Hello Simon, thank you for your review! If that's OK for you one of my colleagues encountered this issue will work on a patch for this on Monday.

[Simon Charette]

Totally okay Ömer, thanks to you and your coworker for taking the time to submit a patch.

[Deniz Altun]

Hello Simon, just letting you know that I will be working on this patch.

[Simon Charette]

Sounds good, you'll want to assign the ticket to you then.

[Mariusz Felisiak]

​PR