Incorrect and/or confusing behaviour with nullable OneToOneField

[theevilgeek]

Attempting to "null" out a nullable OneToOneField before deleting the related object fails to prevent a cascading delete (i.e., both objects are still deleted whereas it seems only the related object ought to be deleted).

Example code:

# Note: using Django trunk

###### MODELS ######

class Person(models.Model):
  age = models.PositiveIntegerField()

  def die(self):
    self.soul.become_ghost()
    self.delete()


class Soul(models.Model):
  person = models.OneToOneField(Person, null=True)
  is_alive = models.BooleanField(default=True)

  def become_ghost(self):
    self.person = None
    self.is_alive = False                                                                                                                                                    
    self.save()


###### TESTCASE (INTERACTIVE) ######

# Type a few commands in "python manage.py shell"

>>> from app.models import Person, Soul
>>>
>>> bob = Person.objects.create(age=34)
>>> bobs_soul = Soul.objects.create(person=bob)

# Let's see what's happening in MySQL (switching programs...)

mysql> select * from app_person;
+----+-----+
| id | age |
+----+-----+
|  2 |  34 |
+----+-----+
1 row in set (0.00 sec)

mysql> select * from app_soul;
+----+-----------+----------+
| id | person_id | is_alive |
+----+-----------+----------+
|  2 |         2 |        1 |
+----+-----------+----------+
1 row in set (0.00 sec)

# Okay, that looks good; let's kill him (switching programs again...)

>>> bob.die()

# Back to MySQL

mysql> select * from app_person;
Empty set (0.00 sec)

mysql> select * from app_soul;
Empty set (0.00 sec)

### Huh!??! Why is app_soul being deleted?

[George Sakkis]

Confirmed here too, it's definitely non obvious, if not outright a bug. Also the SQL being issued on bob.delete() hints that something is fishy:

In [11]: bob.delete()
UPDATE `lib_soul`
SET `person_id` = NULL
WHERE `id` IN (4)

DELETE
FROM `lib_soul`
WHERE `id` IN (4)

DELETE
FROM `lib_person`
WHERE `id` IN (2)

Why do an update if it is going to be deleted right after ?

[George Sakkis]

Ugh, selecting "accept ticket" makes me the ticket owner. What's the way to say "I confirm the ticket is legit" without marking me as responsible for it ?

[Russell Keith-Magee]

@gsakkis - You use the triage stage, not the action. Confusing, I know... :-)

[George Sakkis]

Replying to gsakkis:

Why do an update if it is going to be deleted right after ?

Ugh, scratch that, I missed the self.save() in become_ghost(), that's where the UPDATE comes from; nothing to do with self.delete().

Back to the topic, it comes down to Django's indiscriminate ON DELETE CASCADE behavior, regardless of whether the ForeignKey/OneToOneField is nullable. IMO it would make more sense to treat nullable keys as ON DELETE SET NULL but that would most likely be backwards incompatible at this point. Changing to design decision needed, just in case.

[George Sakkis]

Moved tests from select_related_onetoone to one_to_one_regress

[Arthur Hebert]

Looks like this is solved here. I'm adding a little more information for anyone who is trying to work around this in a pre-1.3 version of Django. The following modification fixed a similar problem on my model.

class Person(models.Model):
  age = models.PositiveIntegerField()

  def die(self):
    self.soul.become_ghost()
    # Update self here, so that self.soul is unavailalbe in the object passed to delete()
    # If self is left alone, then self.soul points to a python object representing a relationship
    # that was deleted from the database in the call to become_ghost().
    self = Person.objects.get(pk=self.pk)
    # OR, the following to work more generally:
    # self = self.__class__.objects.get(pk=self.pk)
    self.delete()


class Soul(models.Model):
  person = models.OneToOneField(Person, null=True)
  is_alive = models.BooleanField(default=True)

  def become_ghost(self):
    self.person = None
    self.is_alive = False                                                                                                                                                    
    self.save()

I'd suggest it as a documentation change for earlier versions, but it's so hack-y.

[Russell Keith-Magee]

The provided test case doesn't validate the problem described. If you revert the change to related.py, the test case doesn't fail.

[Jacob]

Milestone 1.3 deleted

[Aymeric Augustin]

Change UI/UX from NULL to False.

[Aymeric Augustin]

Change Easy pickings from NULL to False.

[Anssi Kääriäinen]

I can confirm that this does still happen in master. I tested with this in one_to_one_regress:

    def test_nullable_o2o_delete(self):
        u = UndergroundBar.objects.create(place=self.p1)
        self.p1.delete()
        self.assertTrue(UndergroundBar.objects.filter(pk=u.pk).exists())
        self.assertIsNone(UndergroundBar.objects.get(pk=u.pk).place)

The attached patch isn't correct at all, it is explicitly breaking the link before delete and then confirming that delete doesn't cascade after that. That isn't the problem, the problem is that OneToOneField that is nullable should be set to None on delete, not cascaded.

[Anssi Kääriäinen <akaariai@…>]

In b53ed351b34918e337cbf26773998dafc6f82f4d:

Fixed #14043 -- Made sure nullable o2o delete works as expected

There was an old complaint about nullable one-to-one field cascading
even when the o2o field was saved to None value before the deletion.
Added an test to verify this doesn't happen.

Also some PEP 8 cleanup.

[Andrew Godwin <andrew@…>]

In b773ef8fa0ef09641abf376ca8d4083d8966ec52:

Fixed #14043 -- Made sure nullable o2o delete works as expected

There was an old complaint about nullable one-to-one field cascading
even when the o2o field was saved to None value before the deletion.
Added an test to verify this doesn't happen.

Also some PEP 8 cleanup.