Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#29016 closed Bug (fixed)

Reuse of UpdateQuery breaks some delete updates

Reported by: Étienne Loks Owned by: Étienne Loks
Component: Database layer (models, ORM) Version: 1.11
Severity: Release blocker Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Étienne Loks)

On a model A, when deleting a foreign key pointing to a model B, some other foreign key of the model A pointing to the same model B may be nullified.

I have isolated this behaviour on a simple project:

models.py:

from django.db import models


class ChildModel(models.Model):
    name = models.CharField(max_length=200)


class ParentModel(models.Model):
    name = models.CharField(max_length=200)
    child_1 = models.ForeignKey(ChildModel, on_delete=models.SET_NULL,
                                related_name='parents_1', null=True)
    child_2 = models.ForeignKey(ChildModel, on_delete=models.SET_NULL,
                                related_name='parents_2', null=True)

Django shell session:

from testapp.models import ParentModel, ChildModel

child_1 = ChildModel.objects.create(name="child_1")
child_2 = ChildModel.objects.create(name="child_2")
parent_1 = ParentModel.objects.create(name="parent 1", child_1=child_1, child_2=child_2)
parent_2 = ParentModel.objects.create(name="parent 2", child_1=child_2, child_2=child_1)

child_1.delete()
parent_1 = ParentModel.objects.get(pk=parent_1.pk)
parent_2 = ParentModel.objects.get(pk=parent_2.pk)
# parent_1.child_2 and parent_2.child_1 should be normaly equal to child_2 but...
parent_1.child_2 is not None and parent_2.child_1 is not None
# False is returned

This simple project has been tested on an SQLite database. The same behaviour has been first discovered on a PostgreSQL database.

A mis-reuse of an UpdateQuery seems to be the cause of this bug.

After search on the django bug tracker I have found another issue with the same patch attached #28099.
I have opened this new ticket because the issue seems to be more severe (I have experienced large data loss) and more general.

This issue has been found on version 1.11 and 2.0 of Django.

I have created a new branch on my github account with patch and test associated: https://github.com/Nimn/django/tree/ticket_29016

Attachments (1)

fix_delete_on_update.patch (997 bytes) - added by Étienne Loks 6 years ago.
Simple patch version (no regression test yet)

Download all attachments as: .zip

Change History (9)

Changed 6 years ago by Étienne Loks

Attachment: fix_delete_on_update.patch added

Simple patch version (no regression test yet)

comment:1 Changed 6 years ago by Étienne Loks

Summary: Reuse of UpdateQuery breaks delete some updatesReuse of UpdateQuery breaks some delete updates

comment:2 Changed 6 years ago by Étienne Loks

Description: modified (diff)
Has patch: set

comment:3 Changed 6 years ago by Tim Graham

Severity: NormalRelease blocker
Triage Stage: UnreviewedAccepted

I think that qualifies for a backport based on the "data loss" criteria. Rather than adding entirely new models, can you reuse an existing model (perhaps another ForeignKey will need to be added on an existing model) either in delete or delete_regress?

comment:4 Changed 6 years ago by Étienne Loks

Owner: changed from nobody to Étienne Loks
Status: newassigned

I have changed a model in delete_regress to add two foreign keys (it seems clearer to me). I have sent a pull request for the master branch.

If it really qualifies to a backport (I agree with this qualification), how can I proceed to propose a patch? I miss a proper documentation.

comment:5 Changed 6 years ago by Tim Graham

Triage Stage: AcceptedReady for checkin

Looks good, I made some cosmetic edits and added release notes.

comment:6 Changed 6 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: assignedclosed

In 9a621edf:

Fixed #29016 -- Fixed incorrect foreign key nullification on related instance deletion.

comment:7 Changed 6 years ago by Tim Graham <timograham@…>

In 8f2e3857:

[2.0.x] Fixed #29016 -- Fixed incorrect foreign key nullification on related instance deletion.

Backport of 9a621edf624a4eb1f1645fca628a9e432f0de776 from master

comment:8 Changed 6 years ago by Tim Graham <timograham@…>

In 419705bb:

[1.11.x] Fixed #29016 -- Fixed incorrect foreign key nullification on related instance deletion.

Backport of 9a621edf624a4eb1f1645fca628a9e432f0de776 from master

Note: See TracTickets for help on using tickets.
Back to Top