jazzband / django-polymorphic

Improved Django model inheritance with automatic downcasting
https://django-polymorphic.readthedocs.io
Other
1.65k stars 279 forks source link

Cannot delete or update a parent row: a foreign key constraint fails #34

Open thenewguy opened 11 years ago

thenewguy commented 11 years ago

Using the delete_selected admin action can cause an error similar to the following:

IntegrityError: (1451, 'Cannot delete or update a parent row: a foreign key constraint fails (foo2.bar, CONSTRAINT format_ptr_id_refs_id_270cb9d612063f8e FOREIGN KEY (format_ptr_id) REFERENCES foo1 (id))')

vdboor commented 11 years ago

Hi, can you tell me how to reproduce this issue? I've tried everything in the example project, and didn't get this issue.

dsaradini commented 11 years ago

Hello,

We have the same issue when deleting list of polymorphic elements. What is strange is that, when running tests, we are unable to reproduce the problem. ( no constraint violations )

Here is the example 1) we have an "Group" object containing Question objects of 3 types ( "Rating" "FreeText" and "Choice" all subclass of "Question"). 2) "Question" holds the ForeignKey on "Group". 3a) If we create a "Group" with all questions of the same type, deleting the "Group" works fine. 3b) If we create mixed "Question" types ( one FreeText and one Rating in this case ), deleting the "Group" will fail with the following error "update or delete on table "sample_question" violates foreign key constraint "questionptr_id_refs_id_b59c6fec" on table "sample_rating""

We are using postgresql and it seems that the delete statements generated during the delete will depend on the FIRST type found in the deleted list.

"sample_rating" elements are never deleted. Indeed, if we create a test, the result of Rating.objects.count(), will be 1 NOT 0.

If we create Rating first, then FreeText, it is the "sample_freetext" that is never delete...

As soon as we have time, we will investigating the code, to see where ( and how ) the delete query set is "generate".

chrisglass commented 11 years ago

Hum that is very strange. I will look as well.

chrisglass commented 11 years ago

@dsaradini you mention writing a test for it - would you mind sharing it in a gist or as a PR? What version of the software are you running? Can you try using trunk if you're using a released version?

dsaradini commented 11 years ago

@chrisglass We're using the Pypi version 0.5.1. I will try with the trunk as soon as I have a moment, and also provide a running code that demonstrate the problem.

stephrdev commented 11 years ago

We tracked down the error to a problem in the deletion collector of the Django ORM.

The first problem is django/db/models/deletion.py:102. This method assumes that all objects in the list are the same model class (which is correct, if you use the normal Django manager). django-polymorphic is different. Therefore the code adds all objects with the first model class it finds in the list of objects.

The objects which get passed to the add method are fetched in the related_objects method (line 240, same file). This method uses _base_manager of the related model (which is the base model of the multitable model construction).

We fixed this bug by manually adding a _base_manager = models.Manager() to the PolymorhpicModel subclass, but this doesn't feel right.

Another note: this only happens with foreignkeys/relations on the polymorphic base model. If you have foreignkeys in one of the submodels, everything works just fine. Hope the remarks help to find a clean solution for this issue.

vdboor commented 11 years ago

Thanks @stephrdev for the great help here. Can you post some example code (or gist) to demonstrate this issue? I'd love to fire up a project in the debugger to figure this out, but am keenly looking for model code.

vdboor commented 11 years ago

I also ran into this issue today, and it was caused by a GenericRelation object on the model. The collector has special code for that. I fixed it in my code by returning the non-polymorphic query:

class ContentItemRelation(GenericRelation):
    def __init__(self, **kwargs):
        super(ContentItemRelation, self).__init__(to=ContentItem,
            object_id_field='parent_id', content_type_field='parent_type', **kwargs)

    def bulk_related_objects(self, objs, using=DEFAULT_DB_ALIAS):
        # Fix delete screen. Workaround for https://github.com/chrisglass/django_polymorphic/issues/34
        return super(ContentItemRelation, self).bulk_related_objects(objs).non_polymorphic()

Again guys, please provide a usable example project. My situation can be completely different from yours, so I won't be able to track all edge-cases without an example project.

habfast commented 9 years ago

I ran into the problem today. What is the status?

svengt commented 9 years ago

I am also running into this problem, is the GenericRelation still a valid solution? The documentation of Django says this object is deprecated since 1.7 and will be completely removed in 1.9.

mlncn commented 9 years ago

The _base_manager = Manager() fix from @stephrdev worked for us too. Seems something in the PolymorphicModel's manager isn't quite right?

egamonal commented 9 years ago

mincn, can you elaborate more about that solution? I'm having the same problem with version 0.6.1. I have a polymorphic class called Action, which has a foreign key to Campaign. I can't delete campaigns from the admin view because this integrity error raises. I have to delete the child classes first.

so far I fixed the issue with a pre_delete signal in the Campaign class. I iterate over the relation and delete objects one at a time. I guess it's awful for performance, but at least it works.

jmurty commented 9 years ago

We have encountered this issue and chased it down to a bug in Django core's handling of reverse M2M relationships that target proxy models. In brief, the issue isn't specific to polymorphic but affects Django as well. It's probably just much more likely to occur if you use polymorphic.

We have followed up in the relevant Django Track ticket: https://code.djangoproject.com/ticket/23076

Also created a new, more specific Django ticket: https://code.djangoproject.com/ticket/25520

vdboor commented 9 years ago

Thanks a lot - that really helps!

jmurty commented 8 years ago

Because others continue to struggle with this I have created a gist of the (truly awful and hacky) work-around we are using for this issue, which monkey-patches Django's deletion collector to include proxy models: https://gist.github.com/jmurty/2034c24b6f91a3eaf51a

I had intended to package this up more nicely in an installable project and include some related unit tests we have, but as always it's been hard to find the time.

Please read the comments in that gist and use at your own risk. It works for us... for you, maybe not so much.

mkjpryor-stfc commented 7 years ago

I am also hitting this problem. The context I have seen it is when using the 'Delete selected' action in the admin. It only seems to happen if I try and bulk-delete objects of multiple polymorphic subclasses at once.

I.e. if I have classes ModelA(PolymorphicModel), ModelB(ModelA) and ModelC(ModelA), then I can successfully bulk delete if I select all ModelB or all ModelC objects, but if I select a combination of ModelB and ModelC objects, the delete fails with a FK constraint violation.

chance-focalcast commented 7 years ago

@mkjpryor-stfc We're seeing this same issue with the exact use-case that you described in your example. We're currently figuring out a workaround.

jmurty commented 7 years ago

To follow up on the monkey patch work-around I supplied earlier, the patch is now available in a more official location as part of our open-sourced ICEkit project. This approach has been working for us in production systems for months now, so while it is ugly it does work for us and it might help you too.

There are monkey-patches for Django versions 1.8 and 1.7 starting in the code here: https://github.com/ic-labs/django-icekit/blob/0.16/icekit/publishing/monkey_patches.py#L76

The patch functions must be called to apply them, here's how we do it in an AppConfig.ready method: https://github.com/ic-labs/django-icekit/blob/0.16/icekit/publishing/apps.py#L56

The patch should not be necessary in the latest versions of Django, so upgrading is probably the best option if you can, but in the meantime try the monkey-patches above.

jstray commented 7 years ago

I'm still seeing this with Django 1.11. What is the recommended workaround at this time?

I have re-opened the Django bug, because the previous "fix" didn't fix it (though may have fixed some other problems.) https://code.djangoproject.com/ticket/23076

jstray commented 7 years ago

Found an excellent workaround here https://github.com/django-polymorphic/django-polymorphic/issues/229#issuecomment-246613138

Seems to be a duplicate bug.

jmurty commented 7 years ago

Thanks for the pointer to that workaround @jstray and for following up on the Django issue. It's unfortunate the underlying problem isn't fixed in Django 1.11. I wasn't following along closely enough on the Django side to make sure it was really fixed.

bogdanpetrea commented 7 years ago

@jstray's workaround doesn't work for me with Django 1.11.

nscaife commented 6 years ago

This appears to also be broken in Django 2.0; I have been unable to find a workaround.

iMerica commented 5 years ago

Greetings from November 2019. This is still broken.

iMerica commented 5 years ago

Update:

This fails:

Basemodel.objects.all().delete()

but this succeeds:

for i in Basemodel.objects.all(): i.delete()

Obviously it's not ideal, but it's a workaround until a fix is submitted.

bogdanpetrea commented 5 years ago

I think at this point you should search for alternative solutions. Introducing django-polymorphic as a dependency to your project is almost guaranteed to cause pain.

This (and other breaking bugs) should be stated upfront in the readme file. I think most people come here and are given the impression that everything is working fine and smooth.

I'm not saying this is a bad project, but as long as it's not part of core Django it's very likely to encounter such breaking bugs that will not be fixed anytime soon.

TomD3Wiz commented 4 years ago

Base.objects.all().non_polymorphic().delete() also works well.

sambonner commented 4 years ago

I have to agree with @bogdanpetrea, given this bug hasn't yet been addressed in Django core, and is likely to manifest when using django-polymorphic (I lost a few hours on a project today encountering and resolving this very issue), could it be noted somewhere in the docs?

Usamaraoo commented 4 years ago

i had the same problem deleting the parent model i was using the generic Delete view then i manually deleted the items connected to the parent then delete the parent item in view

ZeroCoolHacker commented 3 years ago

t's very likely to encounter such breaking bugs that will not be fixed anytime soon.

Well its 2021. How late can they get man

DeD1rk commented 2 years ago

In 2022 the problem still occurs, in Django 4.0.1 and Django-polymorphic 3.1.0. In my case, the following workaround (https://github.com/apirobot/django-rest-polymorphic/issues/29#issuecomment-652395609) seems to be good:

class Parent(PolymorphicModel):
    non_polymorphic = models.Manager()

    class Meta():
        base_manager_name = 'non_polymorphic'

This problem should really be fixed, or at least documented...

chrisglass commented 2 years ago

This problem should really be fixed, or at least documented...

PRs welcome!

Be the change you want to see in the world.

ZeroCoolHacker commented 2 years ago

Well its 2021. How late can they get man

Y2Mate is - mr bean waiting gif-Tf3Rm8NwDzU-144p-1645769346563

pablo-destacame commented 2 years ago

@ZeroCoolHacker :rofl:

0nyr commented 2 years ago

Update:

This fails:

Basemodel.objects.all().delete()

but this succeeds:

for i in Basemodel.objects.all(): i.delete()

Obviously it's not ideal, but it's a workaround until a fix is submitted.

In 2022, the solution I used is still this one. It's quite bad for performance but since I expect to almost never use delete in production, it will do the job.

Let's prey for a fix someday in the future.

pfcodes commented 2 years ago

Any fix for this?

Hafnernuss commented 1 year ago

@pfcodes There are some suggestion in #229

thenewguy commented 1 year ago

unsubscribe

On Wed, Jan 11, 2023 at 11:10 AM Philipp @.***> wrote:

@pfcodes https://github.com/pfcodes There are some suggestion in #229 https://github.com/django-polymorphic/django-polymorphic/issues/229

— Reply to this email directly, view it on GitHub https://github.com/django-polymorphic/django-polymorphic/issues/34#issuecomment-1379048845, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAS7I64ORCFLCYL7R4HDCTWR3LN7ANCNFSM4AE2JOQA . You are receiving this because you authored the thread.Message ID: @.***>

bmarinescu commented 1 year ago

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_polymorphicdisabled = 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
AngeloD2022 commented 9 months ago

image

ZeroCoolHacker commented 9 months ago

image

milenmihaylovSC commented 5 months ago

200