django-polymorphic
django-polymorphic copied to clipboard
jazzband
Cascading bug on deletion (Django 1.8)
I have something similar to the following situation:
A ------- B
/ \
B1 B2 ------- C
In code:
class A(models.Model):
pass
class B(polymorphic.PolymorphicModel):
a = models.ForeignKey(A)
class B1(B):
pass
class B2(B):
pass
class C(models.Model):
b = models.ForeignKey(B1)
If I try to delete an instance of A, the following error occurs:
ValueError: Cannot query "B2 object": Must be "B1" instance.
The full example and traceback:
a = A.objects.create()
b1 = B1.objects.create(a=a)
b2 = B2.objects.create(a=a)
c = C.objects.create(b=b1)
a.delete()
File "[...]/text.py", line 137, in test_cascading_bug
a.delete()
File "[...]/django/db/models/base.py", line 895, in delete
collector.collect([self])
File "[...]/django/db/models/deletion.py", line 229, in collect
field.rel.on_delete(self, field, sub_objs, self.using)
File "[...]/django/db/models/deletion.py", line 18, in CASCADE
source_attr=field.name, nullable=field.null)
File "[...]/django/db/models/deletion.py", line 225, in collect
sub_objs = self.related_objects(related, batch)
File "[...]/django/db/models/deletion.py", line 247, in related_objects
**{"%s__in" % related.field.name: objs}
File "[...]/django/db/models/query.py", line 679, in filter
return self._filter_or_exclude(False, *args, **kwargs)
File "[...]/django/db/models/query.py", line 697, in _filter_or_exclude
clone.query.add_q(Q(*args, **kwargs))
File "[...]/django/db/models/sql/query.py", line 1309, in add_q
clause, require_inner = self._add_q(where_part, self.used_aliases)
File "[...]/django/db/models/sql/query.py", line 1337, in _add_q
allow_joins=allow_joins, split_subq=split_subq,
File "[...]/django/db/models/sql/query.py", line 1178, in build_filter
self.check_related_objects(field, value, opts)
File "[...]/django/db/models/sql/query.py", line 1076, in check_related_objects
self.check_query_object_type(v, opts)
File "[...]/django/db/models/sql/query.py", line 1057, in check_query_object_type
(value, opts.object_name))
ValueError: Cannot query "B2 object": Must be "B1" instance.
I'm not 100% sure it is caused by django-polymorphic, but I could not reproduce the error without it...
Seeing this same bug on a very similar model setup (multiple levels of inheritance with django-polymorphic models, various ForeignKey fields).
Django==1.8.7 django-polymorphic==0.8.1
Our workaround is to handle cascading deletes by overriding delete(), but it's a less than ideal solution.
Let me know if there's any information I can provide, but Martin's bug report above is basically the same as what we've got on our end of things.
I'm getting this as well. Very frustrating. @dsilvers Overriding delete may well be the only solution, I'll look into it. Has it affected performance at all? @chrisglass Would you accept a pull request for delete being overridden?
@bonniejools No idea on performance. We have a total of zero users currently.
@bonniejools I'm curious what you've overwritten. Does the issue also occur on standard Django models, or only when the polymorphic models/managers are used?
Workaround looks something like this, based on @martinmaillard's example at top:
class A(models.Model):
def delete(self):
B.objects.non_polymorphic().filter(a=self).all().delete()
super(A, self).delete()
I had the same problem, using the polymorphic model with Django 1.9.2 and django-polymorphic 0.8.1 and 0.9.1.
What worked for me, was calling delete on each object instead of on the queryset.
I.e, the following approach caused integrity error: A.objects.all().delete()
whereas the following worked just fine: for x in A.objects.all(): x.delete()
where class A is a PolymorphicModel
No need for overriding the delete method
Also seeing this bug, Django 1.8.9, django-polymorphic 0.8.1
Additionally, when the delete is triggered by a cascade, overriding A's delete() method won't help.
Z <--- A <-- B
/ \
B1 B2
Z.objects.all().delete() results in ValueError: Cannot query "B1 object": Must be "B2" instance.
Potentially I could override Z's delete() as well, but it starts to seem messy.
any update on this? 1.10.1 has landed and this issue is still happening. https://code.djangoproject.com/ticket/23076 is closed (see bottom)
Really? This is still an issue, basic django functionality. Why are the dev's working on exotic problem xyz when this doesn't even work...
We just run into the same issue with Django 1.10.6 and django-polymorphic 1.1
At the moment i fixed it with:
from polymorphic.models import PolymorphicModel
from polymorphic.managers import PolymorphicManager
from polymorphic.query import PolymorphicQuerySet
class DeletablePolymorphicQuerySet(PolymorphicQuerySet):
def delete(self):
for instance in self:
instance.delete()
class DeletablePolymorphicManager(PolymorphicManager):
queryset_class = DeletablePolymorphicQuerySet
class ABC(PolymorphicModel):
objects = DeletablePolymorphicManager()
My last fix only solved the problem if the deletion starts within a Polymorphic Model. After some search my current solutions is based on #229
class ABC(PolymorphicModel):
if django.VERSION[:2] < (1, 10):
# Fix delete. Workaround for https://github.com/chrisglass/django_polymorphic/issues/34
_base_manager = models.Manager()
class Meta:
if django.VERSION[:2] >= (1, 10):
# Fix delete. Workaround for https://github.com/django-polymorphic/django-polymorphic/issues/229#issuecomment-246613138
base_manager_name = 'base_objects'
Why waste your time on workarounds instead of simply fixing it properly in the django-polymorphic codebase? PRs welcome.
So what would a "proper" fix be? I'll submit the PR if someone can explain what needs to happen.
@TZanke Thanks, It worked for me on Django 1.11 and django-polymorphic 1.13
But if we change the manager, polymorphic model looses the ability to cast children to appropriate type!
How to deal with it? Could someone explain?
I'm using this custom function for on_delete and it works perfectly now (Django 2.2).
https://github.com/django-polymorphic/django-polymorphic/issues/229#issuecomment-398434412
def NON_POLYMORPHIC_CASCADE(collector, field, sub_objs, using):
return models.CASCADE(collector, field, sub_objs.non_polymorphic(), using)
I think this should be documented as the solution so this issue can be closed.
Just want to say:
- @martinmaillard (the OP) really excellent reproducible example.
- totally agree with @expresspotato - whatever the django-polymorphic team is working on, this bug should take priority #1. It's been years, right? I see people saying "PRs are welcome" but this is a leadership question - what IS the "right" solution? Someone deeply involved with this project needs to step up and say "Overriding the delete method is the solution" or "Creating a NON_POLYMORPHIC_XYZ
on_delete=function is the right way." Then close these multi-parallel issue threads going about this same problem. I get it, people are busy. But right now, this project - django-polymorphic - is broken if you literally can't delete related objects after install without a "work-around". - @benkonrath Thanks for point me in the right direction. That solution is clean, and doesn't require overriding the delete method as is suggested.
For now, I'll go with this. For anyone wanting a more understandable solution, here it is.
models.py
from django.db import models
from polymorphic.models import PolymorphicModel
from django.contrib.auth.models import AbstractUser
class User(AbstractUser):
pass
class Content(PolymorphicModel):
user = models.ForeignKey(User, on_delete=models.CASCADE)
class Post(Content):
title = models.CharField(max_length=100)
class Note(Content):
title = models.CharField(max_length=100)
class Bookmark(models.Model):
post = models.ForeignKey(Post, on_delete=models.CASCADE)
Create a user and some posts and notes and a bookmark.
from myapp.models import User, Post, Note, Bookmark
u = User.objects.create(username='abc')
p1 = Post.objects.create(user=u, title="Some title")
p2 = Post.objects.create(user=u, title="Another title")
n1 = Note.objects.create(user=u, title="Django Polymorphic")
n2 = Note.objects.create(user=u, title="Is Broken")
b = Bookmark.objects.create(post=p1)
See the user's content:
>>> u.content_set.all()
<PolymorphicQuerySet [<Post: Post object (1)>, <Post: Post object (2)>, <Note: Note object (3)>, <Note: Note object (4)>]>
Now try to delete user instance:
u.delete()
ValueError: Cannot query "Note object (3)": Must be "Post" instance.
Solution Is To...
from django.db import models
from polymorphic.models import PolymorphicModel
from django.contrib.auth.models import AbstractUser
class User(AbstractUser):
pass
def NON_POLYMORPHIC_CASCADE(collector, field, sub_objs, using):
return models.CASCADE(collector, field, sub_objs.non_polymorphic(), using)
class Content(PolymorphicModel):
user = models.ForeignKey(User, on_delete=NON_POLYMORPHIC_CASCADE)
class Post(Content):
title = models.CharField(max_length=100)
class Note(Content):
title = models.CharField(max_length=100)
class Bookmark(models.Model):
post = models.ForeignKey(Post, on_delete=models.CASCADE)
Make migrations and migrate:
python manage.py makemigrations myapp
python manage.py migrate
Get the user again
>>> u = User.objects.get(username='abc')
>>> u.delete()
(10, {'myapp.Bookmark': 1, 'myapp.Note': 2, 'myapp.Post': 2, 'myapp.Content': 4, 'myapp.User': 1})
>>> User.objects.all()
<QuerySet []>
