Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#15776 closed Bug (fixed)

Delete Regression in Django 1.3

Reported by: aaron.l.madison@… Owned by: lukeplant
Component: Database layer (models, ORM) Version: 1.3
Severity: Release blocker Keywords: db mysql delete
Cc: aaron.l.madison@… Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: yes Patch needs improvement: no
Easy pickings: no UI/UX:

Description

I have found a bug the way Django 1.3 carries out its cascade delete using MySQL. (the operation works fine in django 1.2.5)

Here is a stripped example from our production data.

from django.db import models

class Policy(models.Model):
    policy_number = models.CharField(max_length=10)

class Version(models.Model):
    policy = models.ForeignKey(Policy)

class Location(models.Model):
    version = models.ForeignKey(Version, blank=True, null=True)

class Item(models.Model):
    version = models.ForeignKey(Version)
    location = models.ForeignKey(Location, blank=True, null=True)

class ItemRateCode(models.Model):
    item = models.ForeignKey(Item)

class PropertyItem(models.Model):
    item_rate_code = models.ForeignKey(ItemRateCode)

class Coverage(models.Model):
    version = models.ForeignKey(Version)
    item_rate_code = models.ForeignKey(ItemRateCode)

After you syncdb, execute the following to load the data and try to delete the policy:

from myapp.models import Policy, Version, Location, Item
from myapp.models import ItemRateCode, PropertyItem, Coverage

policy = Policy.objects.create(pk=1, policy_number="1234")
version = Version.objects.create(policy=policy)
location = Location.objects.create(version=version)

item1 = Item.objects.create(version=version, location=location)
item2 = Item.objects.create(version=version, location=location)

# one for each item
item_rate_code1 = ItemRateCode.objects.create(item=item1)
item_rate_code2 = ItemRateCode.objects.create(item=item2)

# one for each item_rate_code
coverage1 = Coverage.objects.create(version=version, item_rate_code=item_rate_code1)
coverage2 = Coverage.objects.create(version=version, item_rate_code=item_rate_code2)

# one for each item_rate_code
property_item1 = PropertyItem.objects.create(item_rate_code=item_rate_code1)
property_item2 = PropertyItem.objects.create(item_rate_code=item_rate_code2)

policy = Policy.objects.get(pk=1)
policy.delete()

(I have included a sample project with a failing/blowing up testcase... the test passes on 1.2.5)

For some reason, the delete collector I believe is trying to delete the "Version" before it has deleted the "Item"s

Attachments (3)

project.zip (8.3 KB) - added by aaron.l.madison@… 4 years ago.
Sample project file to recreate the problem
15776.deletion_order.diff (1.1 KB) - added by emulbreh 4 years ago.
15776.deletion_order.2.diff (668 bytes) - added by emulbreh 4 years ago.

Download all attachments as: .zip

Change History (18)

Changed 4 years ago by aaron.l.madison@…

Sample project file to recreate the problem

comment:1 Changed 4 years ago by jacob

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

From the django-users thread, here's the failure message:

(delete_bug)amadison@dev-aaron:~/projects/delete_bug/project$ ./manage.py test myapp
Creating test database for alias 'default'...
E
======================================================================
ERROR: test_deletes_policy_successfully (myapp.tests.DeletePolicyTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/amadison/projects/delete_bug/project/myapp/tests.py", line 34, in test_deletes_policy_successfully
    self.assertEqual(None, policy.delete())
  File "/home/amadison/projects/delete_bug/lib/python2.6/site-packages/django/db/models/base.py", line 581, in delete
    collector.delete()
  File "/home/amadison/projects/delete_bug/lib/python2.6/site-packages/django/db/models/deletion.py", line 63, in decorated
    func(self, *args, **kwargs)
  File "/home/amadison/projects/delete_bug/lib/python2.6/site-packages/django/db/models/deletion.py", line 254, in delete
    query.delete_batch(pk_list, self.using)
  File "/home/amadison/projects/delete_bug/lib/python2.6/site-packages/django/db/models/sql/subqueries.py", line 44, in delete_batch
    self.do_query(self.model._meta.db_table, where, using=using)
  File "/home/amadison/projects/delete_bug/lib/python2.6/site-packages/django/db/models/sql/subqueries.py", line 29, in do_query
    self.get_compiler(using).execute_sql(None)
  File "/home/amadison/projects/delete_bug/lib/python2.6/site-packages/django/db/models/sql/compiler.py", line 735, in execute_sql
    cursor.execute(sql, params)
  File "/home/amadison/projects/delete_bug/lib/python2.6/site-packages/django/db/backends/mysql/base.py", line 86, in execute
    return self.cursor.execute(query, args)
  File "/home/amadison/projects/delete_bug/lib/python2.6/site-packages/MySQLdb/cursors.py", line 174, in execute
    self.errorhandler(self, exc, value)
  File "/home/amadison/projects/delete_bug/lib/python2.6/site-packages/MySQLdb/connections.py", line 36, in defaulterrorhandler
    raise errorclass, errorvalue
IntegrityError: (1451, 'Cannot delete or update a parent row: a foreign key constraint fails (`test_delete_test`.`myapp_item`, CONSTRAINT `version_id_refs_id_15106961` FOREIGN KEY (`version_id`) REFERENCES `myapp_version` (`id`))')

----------------------------------------------------------------------
Ran 1 test in 0.120s

comment:2 follow-up: Changed 4 years ago by aaron.l.madison@…

I put some print statements in the django.db.models.deletion.py in the Collector.delete method where it does

        # delete instances
        for model, instances in self.data.iteritems():
            query = sql.DeleteQuery(model)
            pk_list = [obj.pk for obj in instances]
            print "Deleting:", model, pk_list
            #query.delete_batch(pk_list, self.using)

The output is:

(delete_bug)amadison@dev-aaron:~/projects/delete_bug/project$ ./manage.py test myapp
Creating test database for alias 'default'...
Deleting: <class 'myapp.models.Version'> [1L]
Deleting: <class 'myapp.models.Policy'> [1L]
Deleting: <class 'myapp.models.Location'> [1L]
Deleting: <class 'myapp.models.PropertyItem'> [2L, 1L]
Deleting: <class 'myapp.models.Coverage'> [2L, 1L]
Deleting: <class 'myapp.models.ItemRateCode'> [2L, 1L]
Deleting: <class 'myapp.models.Item'> [2L, 1L]

the SQL error is happening (i am fairly sure) because the version is trying to be deleted before the Item.

Changed 4 years ago by emulbreh

comment:3 Changed 4 years ago by emulbreh

It looks as if the fix is that simple. But I haven't checked for sideeffects yet, and in particular not with MySQL.

comment:4 Changed 4 years ago by aaron.l.madison@…

That patch seems to shift the problem. Now it blows up deleting item first when it hadn't already deleted the related item rate code. The integrity issues are not present on sqlite3... I'm not sure about postgres.

IntegrityError: (1451, 'Cannot delete or update a parent row: a foreign key constraint fails (`test_delete_test`.`myapp_itemratecode`, CONSTRAINT `item_id_refs_id_3db45024` FOREIGN KEY (`item_id`) REFERENCES `myapp_item` (`id`))')

comment:5 in reply to: ↑ 2 Changed 4 years ago by anonymous

Based on the relationship structure, the proper delete order would be

PropertyItem
Coverage
ItemRateCode
Item
Location
Version
Policy

I think the confusion is coming because a 'Location' must tie to a 'Version', and 'Item' must tie to a 'Version', but an item could also be tied to a specific 'Location'. Whatever is calculating the dependency order is getting confused with this relationship.

Replying to aaron.l.madison@…:

I put some print statements in the django.db.models.deletion.py in the Collector.delete method where it does

        # delete instances
        for model, instances in self.data.iteritems():
            query = sql.DeleteQuery(model)
            pk_list = [obj.pk for obj in instances]
            print "Deleting:", model, pk_list
            #query.delete_batch(pk_list, self.using)

The output is:

(delete_bug)amadison@dev-aaron:~/projects/delete_bug/project$ ./manage.py test myapp
Creating test database for alias 'default'...
Deleting: <class 'myapp.models.Version'> [1L]
Deleting: <class 'myapp.models.Policy'> [1L]
Deleting: <class 'myapp.models.Location'> [1L]
Deleting: <class 'myapp.models.PropertyItem'> [2L, 1L]
Deleting: <class 'myapp.models.Coverage'> [2L, 1L]
Deleting: <class 'myapp.models.ItemRateCode'> [2L, 1L]
Deleting: <class 'myapp.models.Item'> [2L, 1L]

the SQL error is happening (i am fairly sure) because the version is trying to be deleted before the Item.

comment:6 Changed 4 years ago by adamnelson

I have the same problem with a much simpler model structure in Django 1.3 and MySQL 5.1/5.5 (I tried both):

class Tag(models.Model):
    name = models.CharField(max_length=150, unique=True)
    order = models.PositiveSmallIntegerField(default=0)
    score = models.PositiveSmallIntegerField(default=0)
    slug = models.SlugField(unique=True)

class TagCombo(models.Model):
    tag = models.ForeignKey('tag.Tag')


tag = Tag.objects.get(id=1234)
tag.delete()

Exception:


Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "/Users/adam/Development/example-env/lib/python2.7/site-packages/django/db/models/base.py", line 581, in delete
    collector.delete()
  File "/Users/adam/Development/example-env/lib/python2.7/site-packages/django/db/models/deletion.py", line 63, in decorated
    func(self, *args, **kwargs)
  File "/Users/adam/Development/example-env/lib/python2.7/site-packages/django/db/models/deletion.py", line 254, in delete
    query.delete_batch(pk_list, self.using)
  File "/Users/adam/Development/example-env/lib/python2.7/site-packages/django/db/models/sql/subqueries.py", line 44, in delete_batch
    self.do_query(self.model._meta.db_table, where, using=using)
  File "/Users/adam/Development/example-env/lib/python2.7/site-packages/django/db/models/sql/subqueries.py", line 29, in do_query
    self.get_compiler(using).execute_sql(None)
  File "/Users/adam/Development/example-env/lib/python2.7/site-packages/django/db/models/sql/compiler.py", line 735, in execute_sql
    cursor.execute(sql, params)
  File "/Users/adam/Development/example-env/lib/python2.7/site-packages/django/db/backends/util.py", line 34, in execute
    return self.cursor.execute(sql, params)
  File "/Users/adam/Development/example-env/lib/python2.7/site-packages/django/db/backends/mysql/base.py", line 86, in execute
    return self.cursor.execute(query, args)
  File "/Users/adam/Development/example-env/lib/python2.7/site-packages/MySQLdb/cursors.py", line 174, in execute
    self.errorhandler(self, exc, value)
  File "/Users/adam/Development/example-env/lib/python2.7/site-packages/MySQLdb/connections.py", line 36, in defaulterrorhandler
    raise errorclass, errorvalue
IntegrityError: (1451, 'Cannot delete or update a parent row: a foreign key constraint fails (`example_local`.`tag_tagcombo`, CONSTRAINT `tag_id_refs_id_39193e2d9b20ff37` FOREIGN KEY (`tag_id`) REFERENCES `tag_tag` (`id`))')

Changed 4 years ago by emulbreh

comment:7 Changed 4 years ago by emulbreh

I could not reproduce the error with adamnelson's example. But I could reproduce it with aaron's code. My first patch was completly wrong.
But the second patch fixes the issue for me with MySQL 5.5.10. Basically, we drop all but the first dependency, which causes the bad deletion order.

comment:8 Changed 4 years ago by anonymous

  • Triage Stage changed from Unreviewed to Accepted

comment:9 Changed 4 years ago by ramiro

  • Easy pickings unset
  • Needs tests set

comment:10 Changed 4 years ago by lukeplant

  • Owner changed from nobody to lukeplant

comment:11 Changed 4 years ago by lukeplant

For other people trying to reproduce this, it seems only to be a problem with the InnoDB engine. You'll need this in your settings file:

DATABASES = {
    'default': {
        'ENGINE': 'django.db.backends.mysql',
        # ...
        'OPTIONS': {"init_command": "SET storage_engine=INNODB"},

comment:12 Changed 4 years ago by lukeplant

OK, I'm about to commit the fix. Some notes, for your information:

The models ItemRateCode, PropertyItem and Coverage are irrelevant and can be removed from the example (so I did in the tests). Same for needing multiple 'Item' objects.

To expand on emulbreh's explanation of his patch:

The bug occurs because the 'Item' object is first reached via the 'Location' object, and since the relation is nullable, the dependency is not added. The second time it is reached, directly from the 'Version' object, it is not 'new', therefore the dependency is not counted. This last bit of logic needs to be removed - an object may have already been seen, but that doesn't mean the route to the object has already been seen, and we need to include the dependency nonetheless.

comment:13 Changed 4 years ago by lukeplant

  • Resolution set to fixed
  • Status changed from new to closed

In [16295]:

Fixed #15776 - delete regression in Django 1.3 involving nullable foreign keys

Many thanks to aaron.l.madison for the detailed report and to emulbreh for
the fix.

comment:14 Changed 4 years ago by lukeplant

In [16296]:

[1.3.X] Fixed #15776 - delete regression in Django 1.3 involving nullable foreign keys

Many thanks to aaron.l.madison for the detailed report and to emulbreh for
the fix.

Backport of [16295] from trunk.

comment:15 Changed 4 years ago by jacob

  • milestone 1.3 deleted

Milestone 1.3 deleted

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