Another cascading bug on CASCADE deletion

[godardth]

Hi,

I have an issue in django-polymorphic when deleting models from django-admin.

Models are defined as below

class Farm(models.Model):
    class Meta:
        pass

class Animal(PolymorphicModel):
    farm = models.ForeignKey('Farm', on_delete=models.CASCADE)
    name = models.CharField(max_length=100)
    class Meta:
        pass

class Dog(Animal):
    dog_param = models.CharField(max_length=100)
    class Meta:
        pass

class Cat(Animal):
    cat_param = models.CharField(max_length=100)
    class Meta:
        pass

Now I am creating a Farm object with both a Dog and a Cat (problem doesn't appear if all polymorphic related objects are of the same sub-class.

When trying to delete the Farm object in admin, I get the below hierarchy

• Farm: Farm number 19
  • Dog: #37 - dog
  • Cat: #38 - cat
• Animal: #37 - dog
• Animal: #38 - cat

And when confirming the deletion, I get the error below

Internal Server Error: /admin/farming/farm/
Traceback (most recent call last):
  File "/usr/local/lib/python2.7/dist-packages/django/core/handlers/base.py", line 149, in get_response
    response = self.process_exception_by_middleware(e, request)
  File "/usr/local/lib/python2.7/dist-packages/django/core/handlers/base.py", line 147, in get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/usr/local/lib/python2.7/dist-packages/django/contrib/admin/options.py", line 541, in wrapper
    return self.admin_site.admin_view(view)(*args, **kwargs)
  File "/usr/local/lib/python2.7/dist-packages/django/utils/decorators.py", line 149, in _wrapped_view
    response = view_func(request, *args, **kwargs)
  File "/usr/local/lib/python2.7/dist-packages/django/views/decorators/cache.py", line 57, in _wrapped_view_func
    response = view_func(request, *args, **kwargs)
  File "/usr/local/lib/python2.7/dist-packages/django/contrib/admin/sites.py", line 244, in inner
    return view(request, *args, **kwargs)
  File "/usr/local/lib/python2.7/dist-packages/django/utils/decorators.py", line 67, in _wrapper
    return bound_func(*args, **kwargs)
  File "/usr/local/lib/python2.7/dist-packages/django/utils/decorators.py", line 149, in _wrapped_view
    response = view_func(request, *args, **kwargs)
  File "/usr/local/lib/python2.7/dist-packages/django/utils/decorators.py", line 63, in bound_func
    return func.__get__(self, type(self))(*args2, **kwargs2)
  File "/usr/local/lib/python2.7/dist-packages/django/contrib/admin/options.py", line 1512, in changelist_view
    response = self.response_action(request, queryset=cl.get_queryset(request))
  File "/usr/local/lib/python2.7/dist-packages/django/contrib/admin/options.py", line 1245, in response_action
    response = func(self, request, queryset)
  File "/usr/local/lib/python2.7/dist-packages/django/contrib/admin/actions.py", line 49, in delete_selected
    queryset.delete()
  File "/usr/local/lib/python2.7/dist-packages/django/db/models/query.py", line 600, in delete
    deleted, _rows_count = collector.delete()
  File "/usr/local/lib/python2.7/dist-packages/django/db/models/deletion.py", line 308, in delete
    for model, instances in six.iteritems(self.data):
  File "/usr/local/lib/python2.7/dist-packages/django/db/transaction.py", line 223, in __exit__
    connection.commit()
  File "/usr/local/lib/python2.7/dist-packages/django/db/backends/base/base.py", line 242, in commit
    self._commit()
  File "/usr/local/lib/python2.7/dist-packages/django/db/backends/base/base.py", line 211, in _commit
    return self.connection.commit()
  File "/usr/local/lib/python2.7/dist-packages/django/db/utils.py", line 95, in __exit__
    six.reraise(dj_exc_type, dj_exc_value, traceback)
  File "/usr/local/lib/python2.7/dist-packages/django/db/backends/base/base.py", line 211, in _commit
    return self.connection.commit()
IntegrityError: update or delete on table "farming_animal" violates foreign key constraint "farming_cat_animal_ptr_id_77dae4b8_fk_farming_animal_id" on table "farming_cat"
DETAIL:  Key (id)=(38) is still referenced from table "farming_cat".

I tracked the problem down to be a problem in the deletion Collector class where related object to Farm (i.e. Animal) are of the sub-classes either Dog or Cat. So in the case above, we try to delete the Cat object from the Dog table.

I solved my problem by using the _base_manager property of the Model class . And referring to a non polymorphic object manager like below

class Animal(PolymorphicModel):
    instance = models.ForeignKey('Farm', on_delete=models.CASCADE)
    name = models.CharField(max_length=100)
    _base_manager = models.Manager()
    class Meta:
        pass

Now when trying to delete the Farm object in admin I get the correct hierarchy

• Farm: Farm number 19
  • Animal: #37 - dog
    • Dog: #37 - dog
  • Animal: #38 - cat
    • Cat: #38 - cat

I am using the below

psql 9.3.11
python 2.7.6
django 1.9.8
django-polymorphic 0.9.2

My thought would be to add _base_manager = models.Manager() in the django-polymorphic PolymorphicModel. What do you guy think about this ?

Thanks, Theo

[patient]

Have the problem too, but in Django 1.10 solution with _base_manager does not work. Now it works with:

class Animal(PolymorphicModel):
    ...
    class Meta:
        base_manager_name = 'base_objects'

[x603]

I've faced the same issue when trying to delete objects. Adding base manager solved it. What does it actually do, can somebody explain please? And why without base manager it throws an error? Thanks.

[c0d3z3r0]

... same issue here with Django 1.10. Adding the base_manager_name solved the problem, too. This seems to be related to #84

[gunnar2k]

Thank you! @patient

[jstray]

This is a horrifying bug -- and took up about two days of my team's effort to understand before we found this workaround.

These seem to be the same bug: https://github.com/django-polymorphic/django-polymorphic/issues/34 https://code.djangoproject.com/ticket/23076#comment:16

The latter has lots of intricate detail on what is going on. It's complicated. I'm not sure if the correct solution would be to change django or django-polymorphic.

Also, it is likely to be difficult to write a failing test for this bug as sqlite does not enforce constraints, so it only fails on Postgres (and therefore only on a production server, not a development environment!)

Nonetheless it would be really great to fix this.

[meshy]

It's worth noting that tests are now running against postgres.

[benrudolph]

When I updated from 2.0 to 2.0.1 the workaround with base_manager_name no longer works. Had to bump the version back down to 2.0

[mumumumu]

I'm on django 1.11 and django-polymorphic 2.0.2. This is the workaround I'm currently using:

class ABC(PolymorphicModel):

    non_polymorphic = models.Manager()

    class Meta
        base_manager_name = 'non_polymorphic'

[sephii]

I'm getting the same issue as well (Django 1.11, django-polymorphic 2.0.2) and used the fix proposed in https://github.com/django-polymorphic/django-polymorphic/issues/229#issuecomment-370954612. I don't know if this has any other implication so it would be nice to have a proper fix for that.

[chrismeyersfsu]

I worked around by setting my own SET_NULL function that is essentially a wrapper to call sub_objs.non_polymorphic() on Django 1.11.x

from django.db import models

def SET_NULL(collector, field, sub_objs, using):
    return models.SET_NULL(collector, field, sub_objs.non_polymorphic(), using)

    books = models.ForeignKey(
        'Book',
        blank=True,
        null=True,
        default=None,
        on_delete=SET_NULL,
    )

[otech-nl]

Thanks @mumumumu Worked for me in that configuration Other workarounds resulted in e.g. "Model has no manager named 'base_objects'"

[flinz]

The workaround of @mumumumu https://github.com/django-polymorphic/django-polymorphic/issues/229#issuecomment-370954612 worked for me, however now django admin generally uses the PolymorphicModel classes' __str__ for display, and does not cast to the child classes (works if I uncomment the fix).

Edit: on second note, even on the frontend now all classes fail to display the child classes properly.

[alex2304]

I have the same issue as @flinz

@mumumumu could you suggest, how to preserve a main functionality of django-polymorphic and at the same time solve the problem?

[mumumumu]

I'm actually using the solution provided by https://github.com/django-polymorphic/django-polymorphic/issues/229#issuecomment-379084407 now. I'm on Django 2.0.6 and django-polymorphic 2.0.2.

def NON_POLYMORPHIC_CASCADE(collector, field, sub_objs, using):
    return models.CASCADE(collector, field, sub_objs.non_polymorphic(), using)

...

class MyModel(PolymorphicModel):
    fk_field = models.ForeignKey(on_delete=NON_POLYMORPHIC_CASCADE)

[guy881]

Any update on this one? I'm facing the same issue in Django 2.0.6 and django-polymorphic 2.0.3. The workarounds provided above are not working in my case.

[dmptrluke]

I'm having this issue on the latest version of both Django and polymorphic. I've used the code from https://github.com/django-polymorphic/django-polymorphic/issues/229#issuecomment-398434412 and it's working for now.

[mtazzari]

I'm having this issue with Django 2.1.7 and polymorphic 2.0.3. Solution https://github.com/django-polymorphic/django-polymorphic/issues/229#issuecomment-398434412 works!

[HJEGeorge]

Could someone explain how to implement #299?

Do I place this on the base polymorphic model, or the inheriting one, or the one referencing the base and what is the 'to' required argument?

def NON_POLYMORPHIC_CASCADE(collector, field, sub_objs, using):
    return models.CASCADE(collector, field, sub_objs.non_polymorphic(), using)

class BaseModel(PolymorphicModel):
    fk_field = models.ForeignKey(on_delete=NON_POLYMORPHIC_CASCADE) # here?

class ChildModel(BaseModel):
    fk_field = models.ForeignKey(on_delete=NON_POLYMORPHIC_CASCADE) # or here?

class OtherModel(models.Model):
    polymorphic = models.ForeignKey(to=BaseModel, on_delete=NON_POLYMORPHIC_CASCADE) # or here

[wkoot]

Proposed workarounds did not work for me on Django==1.11.20 & django-polymorphic==2.0.3. Instead, I use the following CascadeDeletePolymorphicManager on BaseModel(PolymorphicModel):

from polymorphic.managers import PolymorphicManager, PolymorphicQuerySet


class CascadeDeletePolymorphicQuerySet(PolymorphicQuerySet):
    """
    Patch the QuerySet to call delete on the non_polymorphic QuerySet, avoiding models.deletion.Collector typing problem

    Based on workarounds proposed in: https://github.com/django-polymorphic/django-polymorphic/issues/229
    See also: https://github.com/django-polymorphic/django-polymorphic/issues/34,
              https://github.com/django-polymorphic/django-polymorphic/issues/84
    Related Django ticket: https://code.djangoproject.com/ticket/23076
    """
    def delete(self):
        if not self.polymorphic_disabled:
            return self.non_polymorphic().delete()
        else:
            return super().delete()


class CascadeDeletePolymorphicManager(PolymorphicManager):
    queryset_class = CascadeDeletePolymorphicQuerySet

[jpotterm]

I worked around this by reverting the default objects manager to models.Manager and attaching PolymorphicManager separately under the name polymorphic. In this configuration Animal.objects.all() gives a non-polymorphic queryset and if you want the polymorphic queryset you have to do Animal.polymorphic.all().

This fixes the cascade delete issue because Django's cascade deletion uses the default manager and needs the non-polymorphic queryset. However, we need to add a custom get_queryset method to the admin class to pull the polymorphic queryset when editing.

This was tested with Django 2.0.7 and django-polymorphic 2.0.2.

# models.py
class Animal(PolymorphicModel):
    farm = models.ForeignKey('Farm', on_delete=models.CASCADE)
    name = models.CharField(max_length=100)
    
    objects = models.Manager()
    polymorphic = PolymorphicManager()


# admin.py
class AnimalInline(GenericStackedPolymorphicInline):
    ...

    def get_queryset(self, request):
        qs = self.model.polymorphic.get_queryset()

        ordering = self.get_ordering(request)
        if ordering:
            qs = qs.order_by(*ordering)

        if not self.has_change_permission(request):
            qs = qs.none()

        return qs

[dmastylo]

Want to add here that we're on Django 3.0.2, Django-polymorphic 2.1.2 and https://github.com/django-polymorphic/django-polymorphic/issues/229#issuecomment-398434412 this solution from mumumumu works for us.

We have a structure like so:

ModelA (Base Model)
ModelB (Child Model of A)
ModelC, ModelD, ... (Child Models of A)

ModelA():
  link_to = models.ForeignKey(ModelB, on_delete=NON_POLYMORPHIC_CASCADE)

so a foreign key exists on the parent, base model, and it points at ModelB. Every child instance of ModelA has a foreign key to ModelB, and an instance of ModelB points at disparate types of child models through the reverse relation. This fix allowed us to cascade delete all the associated child instances to an instance or query set of ModelB.

[prime51]

the above solution worked for me with Django 3.1.1, django-polymorphic 3.0.0

[suprmat95]

Hello everyone.

I've the same structure of the first comment.

Thanks to the solution of mumumumu I'm able to delete Farm object.

My problem is now to UPDATE a Farm object adding and deleting Cat and Dog objects.

the error is the same of the first comment.

Someone can help me?

[kohlab]

Noting that a tweak to @mumumumu's workaround works for SET_NULL too (which was the problem in my case).

def NON_POLYMORPHIC_SET_NULL(collector, field, sub_objs, using):
    return models.SET_NULL(collector, field, sub_objs.non_polymorphic(), using)


class Data(PolymorphicModel):
    parent = models.ForeignKey('Data', null=True, blank=True, on_delete=NON_POLYMORPHIC_SET_NULL, related_name="children")

I'm using Django 3.1 and django-polymorphic 3.0.0.

[stfl]

workaround from https://github.com/django-polymorphic/django-polymorphic/issues/229#issuecomment-398434412 Works on Django 3.2.7 and django-polymorphic 3.0.0

[CelestialGuru]

For anyone coming here from the web, here's a full example of how to implement https://github.com/django-polymorphic/django-polymorphic/issues/229#issuecomment-492175425

from polymorphic.managers import PolymorphicManager, PolymorphicQuerySet
from polymorphic.models import PolymorphicModel


class CascadeDeletePolymorphicQuerySet(PolymorphicQuerySet):
    """
    Patch the QuerySet to call delete on the non_polymorphic QuerySet, avoiding models.deletion.Collector typing problem

    Based on workarounds proposed in: https://github.com/django-polymorphic/django-polymorphic/issues/229
    See also: https://github.com/django-polymorphic/django-polymorphic/issues/34,
              https://github.com/django-polymorphic/django-polymorphic/issues/84
    Related Django ticket: https://code.djangoproject.com/ticket/23076
    """

    def delete(self):
        if not self.polymorphic_disabled:
            return self.non_polymorphic().delete()
        else:
            return super().delete()


class CascadeDeletePolymorphicManager(PolymorphicManager):
    queryset_class = CascadeDeletePolymorphicQuerySet


class Parent(PolymorphicModel):
    objects = CascadeDeletePolymorphicManager()


class ChildA(Parent):
    pass


class ChildB(Parent):
    pass

Honestly this looks like adding the updated delete method here on PolymorphicQuerySet would fix things. I'll see if I can turn this into a PR.

[Hafnernuss]

Noting that a tweak to @mumumumu's workaround works for SET_NULL too (which was the problem in my case).

def NON_POLYMORPHIC_SET_NULL(collector, field, sub_objs, using):
    return models.SET_NULL(collector, field, sub_objs.non_polymorphic(), using)


class Data(PolymorphicModel):
    parent = models.ForeignKey('Data', null=True, blank=True, on_delete=NON_POLYMORPHIC_SET_NULL, related_name="children")

I'm using Django 3.1 and django-polymorphic 3.0.0.

I can confirm that this still works on polymorphic 3.1.0.

I'd be happy to raise a PR for this, although it seems hacky. Any other suggestion on how to nicely integrate this into polymorphic?

[Hafnernuss]

For anyone coming here from the web, here's a full example of how to implement #229 (comment)

from polymorphic.managers import PolymorphicManager, PolymorphicQuerySet
from polymorphic.models import PolymorphicModel


class CascadeDeletePolymorphicQuerySet(PolymorphicQuerySet):
    """
    Patch the QuerySet to call delete on the non_polymorphic QuerySet, avoiding models.deletion.Collector typing problem

    Based on workarounds proposed in: https://github.com/django-polymorphic/django-polymorphic/issues/229
    See also: https://github.com/django-polymorphic/django-polymorphic/issues/34,
              https://github.com/django-polymorphic/django-polymorphic/issues/84
    Related Django ticket: https://code.djangoproject.com/ticket/23076
    """

    def delete(self):
        if not self.polymorphic_disabled:
            return self.non_polymorphic().delete()
        else:
            return super().delete()


class CascadeDeletePolymorphicManager(PolymorphicManager):
    queryset_class = CascadeDeletePolymorphicQuerySet


class Parent(PolymorphicModel):
    objects = CascadeDeletePolymorphicManager()


class ChildA(Parent):
    pass


class ChildB(Parent):
    pass

Honestly this looks like adding the updated delete method here on PolymorphicQuerySet would fix things. I'll see if I can turn this into a PR.

For some reason, this does not work for me. The delete method is never called when deleting all Farms. It would have seemed like a nice solution though.

[Hafnernuss]

It seems, that this works as well:

class Farm(PolymorphicModel):
    .....

    base_manager = models.Manager()
    class Meta:
        base_manager_name = 'base_manager'

[pfcodes]

setting base_manager_name breaks the "save" button for me. at least when using inline polymorphic form sets. is there another work around?

update:

the NON_POLYMORPHIC_CASCADE method above works for me on django 4.1.7 and polymorphic 3.1.0