[patch] Deleting object with generic relation does not work with one to one

[Chris Long]

Jay Parlar came across this bug while testing the POP branch. It appears that this code, when using generic relations:

      for related in cls._meta.get_all_related_many_to_many_objects():
            for offset in range(0, len(pk_list), GET_ITERATOR_CHUNK_SIZE):
                cursor.execute("DELETE FROM %s WHERE %s IN (%s)" % \
                    (qn(related.field.m2m_db_table()),
                        qn(related.field.m2m_reverse_name()), 

For the m2m_reverse_name returns the one-to-one id name (in the example Jay Parlar gives it's user_id) instead of the id used in the generic relation (in POP branch it's model_id or owner_id). The below patch seems to have fixed this problem.

Patch:

--- django/db/models/fields/generic.py  (revision 3668)
+++ django/db/models/fields/generic.py  (working copy)
@@ -117,7 +117,7 @@
         return self.object_id_field_name

     def m2m_reverse_name(self):
-        return self.model._meta.pk.attname
+        return self.object_id_field_name

     def contribute_to_class(self, cls, name):
         super(GenericRelation, self).contribute_to_class(cls, name)

[Malcolm Tredinnick]

(In [3787]) Fixed #2749 -- Get the correct m2m_reverse_name() in generic relations. Thanks
to Jay Parlar and Chris Long for some good debugging here.

[Favo <favo@…>]

the old action just remove only one record, the patch version will remove all
records who's object_id = xxx

Class Tag:
   object_id = ...
   content_type = ...
   content_object = GenericFK()

Class Photo:
   tags = GenericRelation(Tag)
   ...

photo1 = Photo()
tag1 = Tag(GFK=photo1); tag1.save()
tag2 = Tag(GFK=photo1); tag2.save()

tag1.delete() will cause:

"DELETE FROM tag WHERE object_id in (1);"

then both tag1 and tag2 have been deleted.

in fact the correct delete sql for generic FK should be:

   DELETE FROM tag WHERE object_id =1 and content_type = xxx;

but it's not easy to implement for current query backend, so I suggest
we just revert the changeset.

    # revert it.
    def m2m_reverse_name(self):
-        return self.model._meta.pk.attname
+        return self.object_id_field_name

[peter.kese@…]

I am also encountering the same problem and have LOST heaps of data due to it and had to ask site administrators to avoid any deletion of any data for the while.

I have noticed, that by trying to remove models.GenericRelation(...) from models in which objects are often deleted improves situation, but data stil gets lost on models, where GenericRelation fields can't be removed.

Also in the Favo's suggeston, he says that correct deletion should be like this:

DELETE FROM tag WHERE object_id =1 and content_type = xxx;

but I think, if you wish to delete a specific tag, then it would be best to just delete the tag by tag's
tag1.id like this:

DELETE FROM tag WHERE id =''tag1.id'';

I believe Favo's example would still delete all tags that relate to photo1 instead of just the selected tag1.

---

This ticket is related to #3215, it is clearly a bug, it causes unexpected data loss and is critical.

[Favo <favo@…>]

The implement for GenericRelation is totally wrong. it should not inherit from M anytoManyRelation -- that cause serious delete bug. because I have not time to implement a new relation class for generic, and the current support generic look up search with our patch. we just hide the delete function for short term.

For long term we need help django implement a new GenericRelation which support generic lookup and sync delete(with test).

I paste the patch here because it's not a real solution

Index: django/db/models/query.py
===================================================================
--- django/db/models/query.py   (revision 5321)
+++ django/db/models/query.py   (revision 5322)
@@ -926,6 +926,8 @@

         pk_list = [pk for pk,instance in seen_objs[cls]]
         for related in cls._meta.get_all_related_many_to_many_objects():
+            if related.field.__class__.__name__ == 'GenericRelation':
+                continue
             for offset in range(0, len(pk_list), GET_ITERATOR_CHUNK_SIZE):
                 cursor.execute("DELETE FROM %s WHERE %s IN (%s)" % \
                     (qn(related.field.m2m_db_table()),
@@ -933,6 +935,8 @@
                         ','.join(['%s' for pk in pk_list[offset:offset+GET_ITERATOR_CHUNK_SIZE]])),                     pk_list[offset:offset+GET_ITERATOR_CHUNK_SIZE])
         for f in cls._meta.many_to_many:
+            if f.__class__.__name__ == 'GenericRelation':
+                continue
             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()),

[Russell Keith-Magee]

(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.

[Malcolm Tredinnick]

(In [6900]) Fixed #3906 -- Fixed the reverse_m2m_name for a generic relation. Refs #2749.