Opened 13 years ago

Closed 13 years ago

Last modified 12 years ago

#15776 closed Bug (fixed)

Delete Regression in Django 1.3

Reported by: aaron.l.madison@… Owned by: Luke Plant
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: no

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@… 13 years ago.
Sample project file to recreate the problem
15776.deletion_order.diff (1.1 KB ) - added by Johannes Dollinger 13 years ago.
15776.deletion_order.2.diff (668 bytes ) - added by Johannes Dollinger 13 years ago.

Download all attachments as: .zip

Change History (18)

by aaron.l.madison@…, 13 years ago

Attachment: project.zip added

Sample project file to recreate the problem

comment:1 by Jacob, 13 years ago

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 by aaron.l.madison@…, 13 years ago

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.

by Johannes Dollinger, 13 years ago

Attachment: 15776.deletion_order.diff added

comment:3 by Johannes Dollinger, 13 years ago

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 by aaron.l.madison@…, 13 years ago

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`))')

in reply to:  2 comment:5 by anonymous, 13 years ago

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 by Adam Nelson, 13 years ago

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`))')

by Johannes Dollinger, 13 years ago

Attachment: 15776.deletion_order.2.diff added

comment:7 by Johannes Dollinger, 13 years ago

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 by anonymous, 13 years ago

Triage Stage: UnreviewedAccepted

comment:9 by Ramiro Morales, 13 years ago

Easy pickings: unset
Needs tests: set

comment:10 by Luke Plant, 13 years ago

Owner: changed from nobody to Luke Plant

comment:11 by Luke Plant, 13 years ago

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 by Luke Plant, 13 years ago

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 by Luke Plant, 13 years ago

Resolution: fixed
Status: newclosed

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 by Luke Plant, 13 years ago

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 by Jacob, 12 years ago

milestone: 1.3

Milestone 1.3 deleted

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