Generic Relation not cascading with Multi table inheritance.

[thomaspurchas]

Generic Relations don't cascade on delete if you are using the generic relation of a superclass in a subclass, then deleting the subclass does not result in the normal cascade behaviour.

For example using the following models:

class TaggedItem(models.Model):
    tag = models.SlugField()
    content_type = models.ForeignKey(ContentType)
    object_id = models.PositiveIntegerField()
    content_object = generic.GenericForeignKey('content_type', 'object_id')

    def __unicode__(self):
        return self.tag

class Post(models.Model):
    title = models.TextField(blank=True)

    relation = generic.GenericRelation(TaggedItem)

    def __unicode__(self):
        return self.title

class ParentPost(Post):

    description = models.TextField(blank=True)

    def __unicode__(self):
        return self.description  

If you create a ParentPost and a TaggedItem related using the GenericForeignKey:

>>> p = ParentPost()
>>> p.title = "This is a title"
>>> p.description = "This is a description"
>>> p.save()

>>> t = TaggedItem(content_object=p, tag="This is a tag")
>>> t.save()
>>> TaggedItem.objects.all()
[<TaggedItem: Ypo>, <TaggedItem: This is a tag>]

and then delete the ParentPost, the TaggedItem is not also deleted, rather it just points to a None object.

>>> p.delete()
>>> TaggedItem.objects.all()
[<TaggedItem: Ypo>, <TaggedItem: This is a tag>]
>>> print t.content_object
None

If you repeat the same procedure with the Post model you get the expected behaviour where the TaggedItem is deleted.

[Nicolas Lara]

This should be a relatively easy fix. At the moment the delete query is adding a WHERE clause with the id of the parent (in this case Post). We need to change that to use the id of the actual object being deleted.

[Nicolas Lara]

​https://github.com/django/django/pull/526

[Simon Charette]

Don't mark your own patch as RFC.

[Anssi Kääriäinen]

The patch isn't correct - doing if not objs: return [] doesn't return a queryset.

[miracle2k]

This fixes the merge conflict and returns an empty queryset: ​https://github.com/miracle2k/django/compare/19149-generic-rel

[Anssi Kääriäinen]

The patch doesn't work correctly for parent model associations. I modified the test case to this:

    def test_inherited_models_delete(self):
        """
        Test that when deleting a class that inherits a GenericRelation,
        the correct related object is deleted on cascade.
        """
        p = Post.objects.create(title="This is a title",
            description="This is a description")
        ppost = ParentPost.objects.get(pk=p.pk)
        t1 = TaggedItem.objects.create(content_object=p, tag="This is a tag")
        t2 = TaggedItem.objects.create(content_object=ppost,
                                       tag="This is anoter tag")
        ppost_ct = ContentType.objects.get_for_model(ParentPost)
        self.assertEqual(list(TaggedItem.objects.all().order_by('tag')),
                         [t1, t2])
        self.assertEqual(
            list(TaggedItem.objects.filter(content_type=ppost_ct)),
            [t2])
        self.assertEqual(list(Post.objects.all()), [p])
        p.delete()
        self.assertEqual(list(TaggedItem.objects.all()), [])

The last line doesn't pass - the parent model's taggeditem isn't deleted. So, the patch just moves the problem around. At minimum both parent and child model deletion should delete the taggeditems for both parent and child.

There are more problems in parent <-> child deletions in general If there are more than one child for a parent and you delete one of the childs, then the other childs will not be deleted. Also, each parent model retrieval executes a separate query. A more generic solution to all of the problems would be welcome, though just solving this ticket's issue is a good approach, too...

[Anssi Kääriäinen]

The proper deletion of inheritance chains is somewhat ugly problem. It seems the best approach is to start the deletion from base parents (that is, from those parent models which do not have any concrete parents), then cascade down from there. This should result in correct, though not totally optimal code. OTOH it doesn't matter how fast the deletion code is if it isn't correct...

I will see if I can write a patch using the above idea. I think it will be faster than current code even if it isn't optimal, as each parent model fetch will issue separate query at the moment.

[Vajrasky Kok]

This is the PR: ​https://github.com/django/django/pull/1970

I based my unit test on akaariai's unit test. I did not try to fix other problems (each parent model retrieval executes a separate query, if there are more than one child for a parent and you delete one of the childs, then the other childs will not be deleted), because I think those deserve separate tickets.

[Tim Graham]

I rebased the PR, but there's a failing test:

======================================================================
FAIL: test_cascade_delete_proxy_model_admin_warning (proxy_models.tests.ProxyModelAdminTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/tim/code/django/tests/proxy_models/tests.py", line 386, in test_cascade_delete_proxy_model_admin_warning
    collector.collect(ProxyTrackerUser.objects.all())
  File "/home/tim/code/django/django/test/testcases.py", line 110, in __exit__
    query['sql'] for query in self.captured_queries
AssertionError: 13 queries executed, 7 expected

Be sure to uncheck "Patch needs improvement" if you can fix it so the ticket shows up for review.