Skip to content

Commit

Permalink
Fixed #9479 -- Corrected an edge case in bulk queryset deletion that …
Browse files Browse the repository at this point in the history
…could cause an infinite loop when using MySQL InnoDB.

git-svn-id: http://code.djangoproject.com/svn/django/branches/soc2009/http-wsgi-improvements@10994 bcc190cf-cafb-0310-a4f2-bffc1f526a37
  • Loading branch information
ccahoon committed Jun 13, 2009
1 parent ae2dd58 commit cb847aa
Show file tree
Hide file tree
Showing 4 changed files with 78 additions and 2 deletions.
3 changes: 2 additions & 1 deletion django/db/models/query.py
Expand Up @@ -355,10 +355,11 @@ def delete(self):


# Delete objects in chunks to prevent the list of related objects from # Delete objects in chunks to prevent the list of related objects from
# becoming too long. # becoming too long.
seen_objs = None
while 1: while 1:
# Collect all the objects to be deleted in this chunk, and all the # Collect all the objects to be deleted in this chunk, and all the
# objects that are related to the objects that are to be deleted. # objects that are related to the objects that are to be deleted.
seen_objs = CollectedObjects() seen_objs = CollectedObjects(seen_objs)
for object in del_query[:CHUNK_SIZE]: for object in del_query[:CHUNK_SIZE]:
object._collect_sub_objects(seen_objs) object._collect_sub_objects(seen_objs)


Expand Down
15 changes: 14 additions & 1 deletion django/db/models/query_utils.py
Expand Up @@ -32,11 +32,21 @@ class CollectedObjects(object):
This is used for the database object deletion routines so that we can This is used for the database object deletion routines so that we can
calculate the 'leaf' objects which should be deleted first. calculate the 'leaf' objects which should be deleted first.
previously_seen is an optional argument. It must be a CollectedObjects
instance itself; any previously_seen collected object will be blocked from
being added to this instance.
""" """


def __init__(self): def __init__(self, previously_seen=None):
self.data = {} self.data = {}
self.children = {} self.children = {}
if previously_seen:
self.blocked = previously_seen.blocked
for cls, seen in previously_seen.data.items():
self.blocked.setdefault(cls, SortedDict()).update(seen)
else:
self.blocked = {}


def add(self, model, pk, obj, parent_model, nullable=False): def add(self, model, pk, obj, parent_model, nullable=False):
""" """
Expand All @@ -53,6 +63,9 @@ def add(self, model, pk, obj, parent_model, nullable=False):
Returns True if the item already existed in the structure and Returns True if the item already existed in the structure and
False otherwise. False otherwise.
""" """
if pk in self.blocked.get(model, {}):
return True

d = self.data.setdefault(model, SortedDict()) d = self.data.setdefault(model, SortedDict())
retval = pk in d retval = pk in d
d[pk] = obj d[pk] = obj
Expand Down
1 change: 1 addition & 0 deletions tests/regressiontests/delete_regress/__init__.py
@@ -0,0 +1 @@

61 changes: 61 additions & 0 deletions tests/regressiontests/delete_regress/models.py
@@ -0,0 +1,61 @@
from django.conf import settings
from django.db import models, backend, connection, transaction
from django.db.models import sql, query
from django.test import TransactionTestCase

class Book(models.Model):
pagecount = models.IntegerField()

# Can't run this test under SQLite, because you can't
# get two connections to an in-memory database.
if settings.DATABASE_ENGINE != 'sqlite3':
class DeleteLockingTest(TransactionTestCase):
def setUp(self):
# Create a second connection to the database
self.conn2 = backend.DatabaseWrapper({
'DATABASE_HOST': settings.DATABASE_HOST,
'DATABASE_NAME': settings.DATABASE_NAME,
'DATABASE_OPTIONS': settings.DATABASE_OPTIONS,
'DATABASE_PASSWORD': settings.DATABASE_PASSWORD,
'DATABASE_PORT': settings.DATABASE_PORT,
'DATABASE_USER': settings.DATABASE_USER,
'TIME_ZONE': settings.TIME_ZONE,
})

# Put both DB connections into managed transaction mode
transaction.enter_transaction_management()
transaction.managed(True)
self.conn2._enter_transaction_management(True)

def tearDown(self):
# Close down the second connection.
transaction.leave_transaction_management()
self.conn2.close()

def test_concurrent_delete(self):
"Deletes on concurrent transactions don't collide and lock the database. Regression for #9479"

# Create some dummy data
b1 = Book(id=1, pagecount=100)
b2 = Book(id=2, pagecount=200)
b3 = Book(id=3, pagecount=300)
b1.save()
b2.save()
b3.save()

transaction.commit()

self.assertEquals(3, Book.objects.count())

# Delete something using connection 2.
cursor2 = self.conn2.cursor()
cursor2.execute('DELETE from delete_regress_book WHERE id=1')
self.conn2._commit();

# Now perform a queryset delete that covers the object
# deleted in connection 2. This causes an infinite loop
# under MySQL InnoDB unless we keep track of already
# deleted objects.
Book.objects.filter(pagecount__lt=250).delete()
transaction.commit()
self.assertEquals(1, Book.objects.count())

0 comments on commit cb847aa

Please sign in to comment.