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: Fix model subclass ___ selector for abstract/proxy models #37 - dog
  • Cat: Simplify add_type_form.html #38 - cat
• Animal: Fix model subclass ___ selector for abstract/proxy models #37 - dog
• Animal: Simplify add_type_form.html #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: Fix model subclass ___ selector for abstract/proxy models #37 - dog
    • Dog: Fix model subclass ___ selector for abstract/proxy models #37 - dog
  • Animal: Simplify add_type_form.html #38 - cat
    • Cat: Simplify add_type_form.html #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:
#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 #229 (comment). 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 #229 (comment) 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 #229 (comment) 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 #229 (comment) and it's working for now.

[mtazzari]

I'm having this issue with Django 2.1.7 and polymorphic 2.0.3. Solution #229 (comment) 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 #229 (comment) 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 #229 (comment) 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 #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.

[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

[HesseMM]

Not sure if this is the right place, but deletions seem to have multiple issue in django-polymorphic. The issues #34 and #229 appear to have similar root causes, but #34 leads here, so this is where ill post too. For my setup, i stumbled into either of them, depending on what/where i tried to delete my model instances. Most of the workarounds posted here did not help me, as some are outdated and others came with inconveniences, such as preventing polymorphic list entries in admin views. For some reason the NON_POLYMORPHIC_CASCADE did nothing for me, maybe it's for an issue i did not experience (yet). Might be useful to apply on top of the below, but my current models did not require it.

To be clear, i did have problems with deletions in admin lists, as well as with simple actions such as qs.delete() - as long as the sets contained different child types. Looking for the errors always brought me to either #229 or #34, but im not sure if this actually has anything to do with CASCADES.
Since i came up with something that worked for me and since im pretty sure people with similar issues would end up here.. ill outline my solution and the reasoning behind it in hopes to help someone in the future.

---

Neither problem occurs when deleting a single instance. The issues appear to only occur when deleting a PolymorphicQuerySet with different child types included. At the core of this appears to sit django.db.models.deletion.Collector, which does not allow deletion for differing types and is used in QuerySet.delete() as well as separately in admin delete_selected, through get_deletion_objects.

Except for the admin delete action, most deletion problems should thus be caused by/within QuerySet.delete() calls, for which no django-polymorphic specific implementation exists in the PolymorphicQuerySet. So i wrote my own. In theory these deletion problems could be avoided by deleting the objects sequentially, but that wouldnt be very efficient. Instead i attempted to write a version which splits the multi-typed PolymorphicQuerySet into multiple uniform-typed PolymorphicQuerySets (ie, each containing only instances of one polymorphic_ctype). Delete()ing those in an atomic block should both: simulate normal .delete() behavior and be decently efficient for larger deletions.

class MyPolymorphicQuerySet(PolymorphicQuerySet):

    def delete(self):
        """Delete the records in the current QuerySet."""
        # Attempt to use Default
        try:
            with transaction.atomic():
                deleted, _rows_count = super(MyPolymorphicQuerySet, self).delete()
            return deleted, _rows_count
        except (IntegrityError, ValueError):
            pass

        # Otherwise we have >= 2 types. Handle by splitting PolymorphicQuerySet per type
        distinct_types = self._chain().values('polymorphic_ctype').distinct()
        typed_qs_list: List(QuerySet) = []
        for type_entry in distinct_types:
            typed_qs = self._chain().filter(polymorphic_ctype=type_entry['polymorphic_ctype'])
            typed_qs_list.append(typed_qs)

        # Execute separate deletions per type to avoid above exceptions
        deleted = 0
        _rows_count = dict()
        with transaction.atomic():
            for typed_qs in typed_qs_list:
                deletion_counter, rows = typed_qs.delete()
                # Combine the returns of all deletion operations
                deleted += deletion_counter
                for key, value in rows.items():
                    if key in _rows_count:
                        _rows_count[key] += value
                    else:
                        _rows_count[key] = value
        return deleted, _rows_count

class MyPolymorphicModel(PolymorphicModel):
    # Adjust which QuerySet the default manager uses to include fix for QS Deletions
    objects = PolymorphicManager.from_queryset(MyPolymorphicQuerySet)()
    class Meta:
        abstract = True

With the above, simply use MyPolymorphicModel instead of the default PolymorphicModel for your definitions.

Which potentially leaves us with admin-only deletion problems, which for some reason i only encountered for one of my models, not the others. This issue could be fixed by overwriting the base_manager with the non-polymorphic variant, as others have done. However, using the above, thats not possible anymore (i think). It's also not necessary, as we can simply overwrite the get_deletion_objects method to pass non_polymorphic() to super() instead. Which should have the same effect, but without the disadvantages of not being able to have polymorphic children in admin list views, among others. This fixed my remaining admin-related problems:

class MyPolymorphicParentModelAdmin(MyModelAdmin, PolymorphicParentModelAdmin):
    def get_deleted_objects(self, objs, request):
        # Pass non_polymorphic() Queryset Contents to workaround 'Cannot Query X: must be Y instance' error
        return super(MyPolymorphicParentModelAdmin, self).get_deleted_objects(objs.non_polymorphic(), request)

[bmarinescu]

The root of the issue is that Django doesn't expect models to be automatically polymorphic during deletion and this messes up the order of the deletions leading to constraint errors.

I've implemented a workaround on my project by first including the library source code to my repo so I could change it then I added a static global_polymorphic_disabled = 0 to the PolymorphicQuerySet class and in PolymorphicModelIterable.__iter__ I altered the if to take into account global_polymorphic_disabled:

    def __iter__(self):
        base_iter = super().__iter__()
        if self.queryset.polymorphic_disabled or PolymorphicQuerySet.global_polymorphic_disabled:
            return base_iter
        return self._polymorphic_iterator(base_iter)

Then I changed my base model classes to disable auto polymorphism for all deletions. I had to set global_polymorphic_disabled even for non polymorphic models to avoid the foreign key error.

class BaseModel(models.Model):
    class Meta:
        abstract = True

    def delete(self, **kwargs):
        try:
            PolymorphicQuerySet.global_polymorphic_disabled += 1
            super().delete(**kwargs)
        finally:
            PolymorphicQuerySet.global_polymorphic_disabled -= 1


class PolymorphicBaseModel(PolymorphicModel):
    class Meta:
        abstract = True

    def delete(self, **kwargs):
        try:
            PolymorphicQuerySet.global_polymorphic_disabled += 1
            super().delete(**kwargs)
        finally:
            PolymorphicQuerySet.global_polymorphic_disabled -= 1

[robert-databooster]

While the accepted workaround works when foreign keys are defined on the parent PolymorphicModel, it will not mark foreign keys defined on the child classes.
To also accommodate these cases, I changed it up a bit:

def POLYMORPHIC_CASCADE(collector, field, sub_objs, using):
    CASCADE(collector, field, sub_objs.non_polymorphic(), using)

    for sub_class in {type(sub_obj) for sub_obj in sub_objs}:
        sub_class_objs = [sub_obj for sub_obj in sub_objs if isinstance(sub_obj, sub_class)]
        CASCADE(collector, field, sub_class_objs, using)

This will mark for deletion the foreign keys of the parent class in the first line, and then will use the default polymorphic behaviour to also go through child class foreign keys
The CASCADE is called for each subclass independently because if there's a FK defined on one of the objects, but not the others, it would throw an error

EDIT: I was using the bare-bones Collector class to find relations, using the NestedObjects collector from django.contrib.admin.utils actually works fine without this

[oabuhamdan]

The solution in comment#229 worked with
django-polymorphic==3.1.0
Django==5.0.2