#25882 closed Bug (fixed)
Deletion on ForeignKey raises TypeError
Reported by: | Markus Gerards | Owned by: | Simon Charette |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | 1.9 |
Severity: | Release blocker | Keywords: | mysql |
Cc: | raphael.merx@… | 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
Consider following model constellation:
class Catalog(models.Model): name = models.CharField(max_length=255) ... class Reader(models.Model): name = models.CharField(max_length=255) catalog = models.ForeignKey(Catalog) ... class ReaderHasMedia(models.Model): reader = models.ForeignKey(Reader) ...
Now in some cases I need to perform following command
ReaderHasMedia.objects.filter(catalog=Catalog.objects.get(pk=123)).delete()
With this command, I get following TypeError:
Traceback (most recent call last): File "<console>", line 1, in <module> File "/usr/local/lib/python2.7/site-packages/django/db/models/query.py", line 600, in delete deleted, _rows_count = collector.delete() File "/usr/local/lib/python2.7/site-packages/django/db/models/deletion.py", line 293, in delete deleted_counter[qs.model._meta.label] += count TypeError: unsupported operand type(s) for +=: 'int' and 'NoneType'
With Django 1.8.7 there wasn't a count variable in deletion.py and so I believe, this is a fresh bug with Django 1.9...
Change History (13)
comment:1 by , 9 years ago
Severity: | Normal → Release blocker |
---|---|
Triage Stage: | Unreviewed → Accepted |
comment:2 by , 9 years ago
Giving it more thought I strongly believe this should be fixed in DeleteQuery.delete_qs
by simply returning 0
instead of None
.
follow-up: 5 comment:3 by , 9 years ago
In your example, ReaderHasMedia
does not have a FK to Catalog
, so ReaderHasMedia.objects.filter(catalog=Catalog.objects.get(pk=123)).delete()
isn't valid.
comment:4 by , 9 years ago
Cc: | added |
---|
comment:5 by , 9 years ago
Replying to raphaelmerx:
In your example,
ReaderHasMedia
does not have a FK toCatalog
, soReaderHasMedia.objects.filter(catalog=Catalog.objects.get(pk=123)).delete()
isn't valid.
Ah! Sorry - I meant: ReaderHasMedia.objects.filter(reader__catalog=Catalog.objects.get(pk=123)).delete()
comment:6 by , 9 years ago
I can't capture the bug in a test, neither on master nor on 1.9. Am I understanding something wrong?
-
tests/delete_regress/models.py
diff --git a/tests/delete_regress/models.py b/tests/delete_regress/models.py index f0145de..6d77c31 100644
a b class OrderedPerson(models.Model): 139 139 140 140 class Meta: 141 141 ordering = ['name'] 142 143 144 class Catalog(models.Model): 145 name = models.CharField(max_length=255) 146 147 class Reader(models.Model): 148 name = models.CharField(max_length=255) 149 catalog = models.ForeignKey(Catalog, models.CASCADE) 150 151 class ReaderHasMedia(models.Model): 152 reader = models.ForeignKey(Reader, models.CASCADE) -
tests/delete_regress/tests.py
diff --git a/tests/delete_regress/tests.py b/tests/delete_regress/tests.py index 2128733..8b36509 100644
a b class OrderedDeleteTests(TestCase): 345 345 OrderedPerson.objects.create(name='Bob', lives_in=h) 346 346 OrderedPerson.objects.filter(lives_in__address='Foo').delete() 347 347 self.assertEqual(OrderedPerson.objects.count(), 0) 348 349 class DoubleForeignKeyTests(TestCase): 350 def test_double_foreign_key(self): 351 from .models import Catalog, Reader, ReaderHasMedia 352 c = Catalog.objects.create(name='aaa') 353 r = Reader.objects.create(name='bbb', catalog=c) 354 ReaderHasMedia.objects.create(reader=r) 355 ReaderHasMedia.objects.filter(reader__catalog=Catalog.objects.get(pk=1)).delete()
comment:7 by , 9 years ago
From looking at the stacktrace it looks like you need to run your tests against a backend that doesn't have the update_can_self_select feature.
The only core backend that has this feature turned off is MySQL.
comment:8 by , 9 years ago
Keywords: | mysql added |
---|
comment:9 by , 9 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:10 by , 9 years ago
Created a PR.
@raphaelmerx thanks for the investigation. I looks like the easiest way to reproduce the issue on MySQL is the following.
class A(models.Model): text = models.TextField() class B(models.Model): a = models.ForeignKey(A, models.CASCADE) B.objects.delete(a__text='missing')
I suppose the reported test case didn't work because you created a Reader
object. The following should do:
Reader.objects.filter(catalog__name='missing')
In order to trigger the bug you have to generate a delete query that issues a join but doesn't match any row on MySQL.
comment:11 by , 9 years ago
Has patch: | set |
---|---|
Triage Stage: | Accepted → Ready for checkin |
It looks like the fix for #16891 didn't account for the fact
django.db.models.sql.subqueries.DeleteQuery.delete_qs
can return None.Not sure if this should be fixed at the
Collector
orDeleteQuery
level.