Model subclasses cannot be deleted if a nullable foreign key relates to a model that relates back

[James Murty]

We use model subclasses in our application and we have come across a situation where Django gets confused when it tries to delete a model object and its related items. Our model contains a class that refers to a base/subclass model with a foreign key, and the base/subclass model in refers back to this class in turn.

The example below should make this clearer. Let's say we are interested in celebrities, which are categorised into subclasses including the TV Chef celebrity subclass. Celebrities can have multiple fans, but a celebrity can have only one "greatest" fan.

class Celebrity(models.Model):
    name = models.CharField("Name", max_length=20)
    greatest_fan = models.ForeignKey("Fan", null=True, unique=True)

class TvChef(Celebrity):
    pass
    

class Fan(models.Model):
    fan_of = models.ForeignKey(Celebrity)    

With these model relationships, Django will be unable to delete the TvChef subclass if it has multiple related Fans. If you run the test program below, it will try to nullify a non-existent column in the TvChef database table.

from subclass_deletion.models import *

c1 = Celebrity.objects.create(name="Madonna")
c2 = Celebrity.objects.create(name="The Queen")
c3 = TvChef.objects.create(name="Huey")

f1 = Fan.objects.create(fan_of=c3)
f2 = Fan.objects.create(fan_of=c3)

c3.greatest_fan = f1
c3.save()

# You cannot delete the TvChef subclass, it fails with the error:
#   OperationalError: no such column: greatest_fan_id
# This error occurs when it tries to run the SQL (sqlite3):
#   UPDATE "subclass_deletion_tvchef" SET "greatest_fan_id" = NULL WHERE "celebrity_ptr_id" IN (3)
c3.delete()

The attached patch performs a sanity-check before Django attempts to clear related fields, and will avoid doing so if it cannot find the expected column name in the class that is the "to" destination.

[Malcolm Tredinnick]

Could you please include a test (patch tests/regressiontests/queries/models.py) that fails before this patch is applied and passes afterwards. If at all possible, try to use the existing models in that file (or make a few small changes), rather than including another three model classes.

(And although you made "queryset-refactor" the version, I presume you mean "SVN", since the queryset-refactor branch is no longer active.)

[James Murty]

Added a test that fails before the patch is applied, and passes afterwards. Unfortunately there were no pre-existing models similar to what was needed, and I didn't want to break other's tests, so I just added my own.

[Malcolm Tredinnick]

(In [8100]) Fixed #7778 -- Fixed a tricky case of foreign key clearing with inherited
models. Patch from James Murty.

[Bob Thomas <bthomas@…>]

This change seems to completely break deleting models with related fields for me. Specifically, I'm trying to delete a User object with a profile. The profile class has a foreign key to the user table like so:

user = models.ForeignKey(User, null=True, db_column='user', blank=True)

Attempting to delete the User via the admin interface gives me an IntegrityError. The update query for the profile table which should be setting the foreign key field to null is never run.
Changing your lambda function to:

lambda f: f.column == field.rel.get_related_field().name

fixes this for me. However, I'm not sure how to run the unit test, so that may undo your fix.

[Malcolm Tredinnick]

When I try the example in comment 5, it works for me. Trying to delete the User will also cause the UserProfile to be deleted (it says this in the admin on the confirmation screen after hitting delete on the appropriate user's User page) and it works without error. Both objects are deleted.

My testing was done against [8156] with both MySQL/InnoDB and PostgreSQL. If you still see the problem there, can you please post a short, complete model that demonstrates the problem and steps to replicate. I'd like to fix it if there's a bug, but at the moment I cannot repeat.

[Bob Thomas <bthomas@…>]

I am running [8161] now on PostgreSQL. Any model whose foreign key column name is different from the column name of the model it references should reproduce this. The database must also have the foreign key constraint for the error to occur. I added a bit of debug code to my copy, which demonstrates the problem rather well:

Index: django/db/models/query.py
===================================================================
--- django/db/models/query.py   (revision 8161)
+++ django/db/models/query.py   (working copy)
@@ -836,6 +836,9 @@

         update_query = sql.UpdateQuery(cls, connection)
         for field, model in cls._meta.get_fields_with_model():
+            if field.rel and field.null and field.rel.to in seen_objs:
+                for f in field.rel.to._meta.fields:
+                    print field.column, f.column
             if (field.rel and field.null and field.rel.to in seen_objs and
                     filter(lambda f: f.column == field.column,
                     field.rel.to._meta.fields)):

When attempting to delete a user with a profile, the output I see is:

user id
user username
user first_name
user last_name
user email
user password
user is_staff
user is_active
user is_superuser
user last_login
user date_joined

The 'user' field on the profile references the 'id' field on the User model, but since they are not named the same, the filter function returns an empty list, and the update query is not run. The database then returns an error because the row in the user table is still referenced by the profile table. Of course, if the table in your database does not actually have the foreign key constraint, you may not get the error, and both objects would be deleted eventually.

[Bob Thomas]

Ok, I think I've figured out the reproduction steps, and why this happens to work for you.

When Django creates a foreign key for a model it sets it as DEFERRABLE, so it is not actually checked until the transaction is committed. If both objects are deleted in the transaction without the update query running, there will not be an error. However, I created this foreign key myself (this is a legacy database) and did not set DEFERRABLE. "Fixing" the foreign key would resolve this for me, but my fix in comment #5 still seems to be the correct behavior.

I don't know how to create an appropriate regression test for this, since it would involve modifying the foreign key in the database. However, all the existing regression tests pass with my patch (including the one added for this bug).

[Malcolm Tredinnick]

There's quite possibly a lot of Django that doesn't work if you change things like deferrable reference resolution. I'm not sure we want to open up that door right at the moment, because that makes it open season on all sorts of problem reports in that area. I'll leave the milestone for the moment, but we might punt this if there isn't time. It requires thinking through the consequences a bit (plus it wasn't immediately clear to me last time I looked at this that it wouldn't have unintended consequences, but I'm not sure why it makes me feel nervous).

[Bob Thomas]

Well, I'm not asking for a special workaround be added for me or anything. If deferred foreign keys are required, then the entire for loop in question is pointless and should be removed (in fact, all tests pass with it removed). The original bug was caused by it running an update query in one special case that it shouldn't, and the patch changed the behavior to never (except in rare circumstances) run the update query. I think it's quite reasonable for that code to continue to perform its original function without being broken by the fix for this bug.

[James Bennett]

Following up on Malcolm's comment above, I'm punting this to post-1.0.

[Bob Thomas]

Changing this back to fixed, since #9308 describes the problem better.

[(none)]

Milestone post-1.0 deleted