Opened 18 years ago

Closed 18 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: dev
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: no UI/UX: no

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 18 years ago.
Test case for deletion problem
3215.patch (3.4 KB ) - added by Russell Keith-Magee 18 years ago.
Full patch (including tests) for deleting generic related objects

Download all attachments as: .zip

Change History (10)

comment:1 by Thomas Steinacher <tom@…>, 18 years ago

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

comment:2 by peter.kese@…, 18 years ago

Cc: peter.kese@… added

comment:3 by Chris Beaven, 18 years ago

Triage Stage: UnreviewedAccepted

comment:4 by Alex Dedul, 18 years ago

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 by gabor@…, 18 years ago

Cc: gabor@… added

comment:6 by anonymous, 18 years ago

Cc: larlet@… added

by Russell Keith-Magee, 18 years ago

Attachment: 3215-testcase.patch added

Test case for deletion problem

by Russell Keith-Magee, 18 years ago

Attachment: 3215.patch added

Full patch (including tests) for deleting generic related objects

comment:7 by Russell Keith-Magee, 18 years ago

Has patch: set
Version: SVN

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

comment:8 by Russell Keith-Magee, 18 years ago

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