Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#25685 closed Bug (fixed)

Model.delete() issues extra queries after a deferred queryset

Reported by: Pantelis Petridis Owned by: Simon Charette
Component: Database layer (models, ORM) Version: 1.8
Severity: Release blocker Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Pantelis Petridis)

Consider the following models

from django.db import models

class ModelA(models.Model):
        a = models.CharField(max_length=15)

class ModelB(models.Model):
        fk = models.ForeignKey(ModelA)

and the following test

from django.test import TestCase
from .models import ModelA

class DeferProblemTestCase(TestCase):
        def test_defer(self):
                a = ModelA.objects.create(a='test')
                with self.assertNumQueries(2):
                        a.delete()

                unrelated_qs = list(ModelA.objects.defer('a'))

                # let's try once again
                a = ModelA.objects.create(a='test')
                with self.assertNumQueries(2):
                        a.delete()

The above test fails under django 1.8.6, as the second call to .delete() will produce 3 queries instead of 2. It seems django somehow caches the deferred model and tries to delete any related ModelB records twice. Except from the overhead, this can lead to random test fails based on the execution order.

Change History (12)

comment:1 Changed 4 years ago by Pantelis Petridis

Description: modified (diff)

comment:2 Changed 4 years ago by Simon Charette

Has patch: set
Owner: changed from nobody to Simon Charette
Status: newassigned
Triage Stage: UnreviewedAccepted
Version: 1.8master

This has been fixed in the process of solving #18012 (8bdfabed6563f2ae136ad43e05bb254c9c15811a).

I guess adding this regression test case to tests/delete wouldn't hurt at this point.

Last edited 4 years ago by Simon Charette (previous) (diff)

comment:3 Changed 4 years ago by Pantelis Petridis

Is there any chance this gets backported to django 1.8, since #18012 is a Cleanup/optimization but this one's a bug?

comment:4 Changed 4 years ago by Simon Charette

I don't think back porting should apply here since this isn't a bug in a new feature.

comment:5 Changed 4 years ago by Pantelis Petridis

I came up across this issue while upgrading from django 1.7 to 1.8. I can confirm the above test works in 1.7.XX and the issue was introduced in 1.8. But since I'm not familiar with the release cycle I don't know if that would justify a back port. Thanks anyway!

comment:6 Changed 4 years ago by Simon Charette

Severity: NormalRelease blocker

If it's a regression in 1.8 then I think a fix should be backported.

I think we should avoid backporting the #18012 related patches and provide a more localized fix.

comment:7 Changed 4 years ago by Tim Graham

Has patch: unset

As far as I can tell, there isn't a patch to review yet.

comment:8 Changed 4 years ago by Simon Charette

Has patch: set
Version: master1.8

comment:9 Changed 4 years ago by Tim Graham

Summary: Deferred model leakModel.delete() issues extra queries after a deferred queryset
Triage Stage: AcceptedReady for checkin

comment:10 Changed 4 years ago by Simon Charette <charette.s@…>

Resolution: fixed
Status: assignedclosed

In 7c3ef19:

[1.8.x] Fixed #25685 -- Fixed a duplicate query regression on deletion of proxied models.

Thanks to Trac alias ppetrid for the report and Tim for the review.

comment:11 Changed 4 years ago by Simon Charette <charette.s@…>

In eb7a329:

[1.9.x] Fixed #25685 -- Fixed a duplicate query regression on deletion of proxied models.

Thanks to Trac alias ppetrid for the report and Tim for the review.

Conflicts:

tests/delete/tests.py

Forward port of 7c3ef19978b36b61db88a519f799f1ce8d019679 from stable/1.8.x

comment:12 Changed 4 years ago by Simon Charette <charette.s@…>

In 6d03bc1:

Fixed #25685 -- Fixed a duplicate query regression on deletion of proxied models.

Thanks to Trac alias ppetrid for the report and Tim for the review.

Conflicts:

django/db/models/deletion.py
tests/delete/tests.py

Forward port of 7c3ef19978b36b61db88a519f799f1ce8d019679 from stable/1.8.x

Note: See TracTickets for help on using tickets.
Back to Top