Opened 10 years ago

Closed 10 years ago

#3215 closed defect (fixed)

Deletion of objects with a GenericRelation(X) deletes unrelated X objects with the same object_id!

Reported by: Thomas Steinacher <tom@…> Owned by: Adrian Holovaty
Component: Database layer (models, ORM) Version: master
Severity: critical Keywords:
Cc: peter.kese@…, gabor@…, larlet@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

Take a look at the following models file:

from django.db import models
from django.contrib.contenttypes.models import ContentType

class Pointer(models.Model):
    object = models.GenericForeignKey()
    object_id = models.IntegerField()
    content_type = models.ForeignKey(ContentType)

class A(models.Model):
    pointers = models.GenericRelation(Pointer)

class B(models.Model):
    pointers = models.GenericRelation(Pointer)

Let's create an A, a B and two Pointer objects:

In [1]: from test import models
In [2]: a = models.A.objects.create()
In [3]: b = models.B.objects.create()
In [4]: pa = models.Pointer.objects.create(object=a)
In [5]: pb = models.Pointer.objects.create(object=b)
In [6]: [(p.content_type, p.object_id) for p in models.Pointer.objects.all()]
Out[6]: [(<ContentType: a>, 1), (<ContentType: b>, 1)]

Now let's delete ONE object. Notice that ALL pointer objects with this object_id will be deleted. Django DOES NOT take a look at the content_type!

In [7]: a.delete()
In [8]: [(p.content_type, p.object_id) for p in models.Pointer.objects.all()]
Out[8]: []

It looks like the deletion happens in django/db/models/query.py (lines 935-940):

        for f in cls._meta.many_to_many:
            for offset in range(0, len(pk_list), GET_ITERATOR_CHUNK_SIZE):
                cursor.execute("DELETE FROM %s WHERE %s IN (%s)" % \
                    (qn(f.m2m_db_table()), qn(f.m2m_column_name()),
                    ','.join(['%s' for pk in pk_list[offset:offset+GET_ITERATOR_CHUNK_SIZE]])),
                    pk_list[offset:offset+GET_ITERATOR_CHUNK_SIZE])

The pointer class is in the object's many_to_many list, because I added "pointers = models.GenericRelation(Pointer)" to the model:

In [9]: models.A._meta.many_to_many[0].m2m_db_table()
Out[9]: 'test_pointer'
In [10]: models.A._meta.many_to_many[0].m2m_column_name()
Out[10]: 'object_id'

I'm using Django SVN (rev 4269).

Attachments (2)

3215-testcase.patch (1.3 KB) - added by Russell Keith-Magee 10 years ago.
Test case for deletion problem
3215.patch (3.4 KB) - added by Russell Keith-Magee 10 years ago.
Full patch (including tests) for deleting generic related objects

Download all attachments as: .zip

Change History (10)

comment:1 Changed 10 years ago by Thomas Steinacher <tom@…>

Looks like this bug is related to #2749. This should be fixed as soon as possible as it deletes unrelated objects...

comment:2 Changed 10 years ago by peter.kese@…

Cc: peter.kese@… added

comment:3 Changed 10 years ago by Chris Beaven

Triage Stage: UnreviewedAccepted

comment:4 Changed 10 years ago by Alex Dedul

I've already tried to implement fix for this in #3081, but it needs improvements - for now it couples contrib content types to django database wrapper, uses generator expressions available only in python 2.4, lacks tests etc. And if generic relations will be moved to contrib apps too seems like it would be problematic to do proper deletion at all.. But this should be investigated further.

comment:5 Changed 10 years ago by gabor@…

Cc: gabor@… added

comment:6 Changed 10 years ago by anonymous

Cc: larlet@… added

Changed 10 years ago by Russell Keith-Magee

Attachment: 3215-testcase.patch added

Test case for deletion problem

Changed 10 years ago by Russell Keith-Magee

Attachment: 3215.patch added

Full patch (including tests) for deleting generic related objects

comment:7 Changed 10 years ago by Russell Keith-Magee

Has patch: set
Version: SVN

Patch needs a sanity check from a pair of eyes that aren't jetlagged.

comment:8 Changed 10 years ago by Russell Keith-Magee

Resolution: fixed
Status: newclosed

(In [4428]) Fixed #3215, #3081, #2749 -- Fixed problem with mistaken deletion of objects when a GenericRelation? is involved. Thanks to Thomas Steinacher for helping to narrow down the problem (#3215), and Alex Dedul (#3081) for the starting point of a working patch.

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