Opened 9 years ago

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

Download all attachments as: .zip

Change History (10)

comment:1 Changed 9 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 9 years ago by peter.kese@…

  • Cc peter.kese@… added

comment:3 Changed 9 years ago by SmileyChris

  • Triage Stage changed from Unreviewed to Accepted

comment:4 Changed 9 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 9 years ago by gabor@…

  • Cc gabor@… added

comment:6 Changed 9 years ago by anonymous

  • Cc larlet@… added

Changed 9 years ago by russellm

Test case for deletion problem

Changed 9 years ago by russellm

Full patch (including tests) for deleting generic related objects

comment:7 Changed 9 years ago by russellm

  • Has patch set
  • Version set to SVN

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

comment:8 Changed 9 years ago by russellm

  • Resolution set to fixed
  • Status changed from new to closed

(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