From 04e8d890aec8e996d568565555164a27a6a76057 Mon Sep 17 00:00:00 2001 From: Alexander Sosnovskiy Date: Sat, 7 Mar 2015 11:56:25 +0300 Subject: [PATCH] Fixed #16891 -- Made Model/QuerySet.delete() return the number of deleted objects. --- django/db/models/base.py | 2 +- django/db/models/deletion.py | 11 ++++-- django/db/models/query.py | 6 ++-- django/db/models/sql/subqueries.py | 18 ++++++---- docs/ref/models/instances.txt | 7 +++- docs/ref/models/querysets.txt | 18 +++++++--- docs/releases/1.9.txt | 4 +++ docs/topics/db/queries.txt | 17 +++++++-- tests/delete/tests.py | 57 ++++++++++++++++++++++++++++++ 9 files changed, 120 insertions(+), 20 deletions(-) diff --git a/django/db/models/base.py b/django/db/models/base.py index 27764bba84..b679d08654 100644 --- a/django/db/models/base.py +++ b/django/db/models/base.py @@ -833,7 +833,7 @@ class Model(six.with_metaclass(ModelBase)): collector = Collector(using=using) collector.collect([self], keep_parents=keep_parents) - collector.delete() + return collector.delete() delete.alters_data = True diff --git a/django/db/models/deletion.py b/django/db/models/deletion.py index 8bfba43fac..61263b569e 100644 --- a/django/db/models/deletion.py +++ b/django/db/models/deletion.py @@ -1,4 +1,4 @@ -from collections import OrderedDict +from collections import Counter, OrderedDict from itertools import chain from operator import attrgetter @@ -280,6 +280,8 @@ class Collector(object): # don't support transactions or cannot defer constraint checks until the # end of a transaction. self.sort() + # number of objects deleted for each model label + deleted_counter = Counter() with transaction.atomic(using=self.using, savepoint=False): # send pre_delete signals @@ -291,7 +293,8 @@ class Collector(object): # fast deletes for qs in self.fast_deletes: - qs._raw_delete(using=self.using) + count = qs._raw_delete(using=self.using) + deleted_counter[qs.model._meta.label] += count # update fields for model, instances_for_fieldvalues in six.iteritems(self.field_updates): @@ -308,7 +311,8 @@ class Collector(object): for model, instances in six.iteritems(self.data): query = sql.DeleteQuery(model) pk_list = [obj.pk for obj in instances] - query.delete_batch(pk_list, self.using) + count = query.delete_batch(pk_list, self.using) + deleted_counter[model._meta.label] += count if not model._meta.auto_created: for obj in instances: @@ -324,3 +328,4 @@ class Collector(object): for model, instances in six.iteritems(self.data): for instance in instances: setattr(instance, model._meta.pk.attname, None) + return sum(deleted_counter.values()), dict(deleted_counter) diff --git a/django/db/models/query.py b/django/db/models/query.py index 050994811a..343f4dc718 100644 --- a/django/db/models/query.py +++ b/django/db/models/query.py @@ -590,10 +590,12 @@ class QuerySet(object): collector = Collector(using=del_query.db) collector.collect(del_query) - collector.delete() + deleted, _rows_count = collector.delete() # Clear the result cache, in case this QuerySet gets reused. self._result_cache = None + return deleted, _rows_count + delete.alters_data = True delete.queryset_only = True @@ -602,7 +604,7 @@ class QuerySet(object): Deletes objects found from the given queryset in single direct SQL query. No signals are sent, and there is no protection for cascades. """ - sql.DeleteQuery(self.model).delete_qs(self, using) + return sql.DeleteQuery(self.model).delete_qs(self, using) _raw_delete.alters_data = True def update(self, **kwargs): diff --git a/django/db/models/sql/subqueries.py b/django/db/models/sql/subqueries.py index 14739dea0e..2dbdf2edd7 100644 --- a/django/db/models/sql/subqueries.py +++ b/django/db/models/sql/subqueries.py @@ -5,7 +5,9 @@ Query subclasses which provide extra functionality beyond simple data retrieval. from django.core.exceptions import FieldError from django.db import connections from django.db.models.query_utils import Q -from django.db.models.sql.constants import GET_ITERATOR_CHUNK_SIZE, NO_RESULTS +from django.db.models.sql.constants import ( + CURSOR, GET_ITERATOR_CHUNK_SIZE, NO_RESULTS, +) from django.db.models.sql.query import Query from django.utils import six @@ -23,7 +25,8 @@ class DeleteQuery(Query): def do_query(self, table, where, using): self.tables = [table] self.where = where - self.get_compiler(using).execute_sql(NO_RESULTS) + cursor = self.get_compiler(using).execute_sql(CURSOR) + return cursor.rowcount if cursor else 0 def delete_batch(self, pk_list, using, field=None): """ @@ -32,13 +35,16 @@ class DeleteQuery(Query): More than one physical query may be executed if there are a lot of values in pk_list. """ + # number of objects deleted + num_deleted = 0 if not field: field = self.get_meta().pk for offset in range(0, len(pk_list), GET_ITERATOR_CHUNK_SIZE): self.where = self.where_class() self.add_q(Q( **{field.attname + '__in': pk_list[offset:offset + GET_ITERATOR_CHUNK_SIZE]})) - self.do_query(self.get_meta().db_table, self.where, using=using) + num_deleted += self.do_query(self.get_meta().db_table, self.where, using=using) + return num_deleted def delete_qs(self, query, using): """ @@ -63,8 +69,7 @@ class DeleteQuery(Query): values = list(query.values_list('pk', flat=True)) if not values: return - self.delete_batch(values, using) - return + return self.delete_batch(values, using) else: innerq.clear_select_clause() innerq.select = [ @@ -73,7 +78,8 @@ class DeleteQuery(Query): values = innerq self.where = self.where_class() self.add_q(Q(pk__in=values)) - self.get_compiler(using).execute_sql(NO_RESULTS) + cursor = self.get_compiler(using).execute_sql(CURSOR) + return cursor.rowcount if cursor else 0 class UpdateQuery(Query): diff --git a/docs/ref/models/instances.txt b/docs/ref/models/instances.txt index 02903668d1..6af5ae3caa 100644 --- a/docs/ref/models/instances.txt +++ b/docs/ref/models/instances.txt @@ -537,7 +537,8 @@ Deleting objects Issues an SQL ``DELETE`` for the object. This only deletes the object in the database; the Python instance will still exist and will still have data in -its fields. +its fields. This method returns the number of objects deleted and a dictionary +with the number of deletions per object type. For more details, including how to delete objects in bulk, see :ref:`topics-db-queries-delete`. @@ -553,6 +554,10 @@ keep the parent model's data. The ``keep_parents`` parameter was added. +.. versionchanged:: 1.9 + + The return value describing the number of objects deleted was added. + Pickling objects ================ diff --git a/docs/ref/models/querysets.txt b/docs/ref/models/querysets.txt index 510fbcbbb8..100553bd87 100644 --- a/docs/ref/models/querysets.txt +++ b/docs/ref/models/querysets.txt @@ -2070,8 +2070,11 @@ delete .. method:: delete() -Performs an SQL delete query on all rows in the :class:`.QuerySet`. The -``delete()`` is applied instantly. You cannot call ``delete()`` on a +Performs an SQL delete query on all rows in the :class:`.QuerySet` and +returns the number of objects deleted and a dictionary with the number of +deletions per object type. + +The ``delete()`` is applied instantly. You cannot call ``delete()`` on a :class:`.QuerySet` that has had a slice taken or can otherwise no longer be filtered. @@ -2081,15 +2084,22 @@ For example, to delete all the entries in a particular blog:: # Delete all the entries belonging to this Blog. >>> Entry.objects.filter(blog=b).delete() + (4, {'weblog.Entry': 2, 'weblog.Entry_authors': 2}) + +.. versionchanged:: 1.9 + + The return value describing the number of objects deleted was added. By default, Django's :class:`~django.db.models.ForeignKey` emulates the SQL constraint ``ON DELETE CASCADE`` — in other words, any objects with foreign keys pointing at the objects to be deleted will be deleted along with them. For example:: - blogs = Blog.objects.all() + >>> blogs = Blog.objects.all() + # This will delete all Blogs and all of their Entry objects. - blogs.delete() + >>> blogs.delete() + (5, {'weblog.Blog': 1, 'weblog.Entry': 2, 'weblog.Entry_authors': 2}) This cascade behavior is customizable via the :attr:`~django.db.models.ForeignKey.on_delete` argument to the diff --git a/docs/releases/1.9.txt b/docs/releases/1.9.txt index 1aff56aef3..e36c4512ca 100644 --- a/docs/releases/1.9.txt +++ b/docs/releases/1.9.txt @@ -214,6 +214,10 @@ Models ` to allow deleting only a child's data in a model that uses multi-table inheritance. +* :meth:`Model.delete() ` + and :meth:`QuerySet.delete() ` return + the number of objects deleted. + * Added a system check to prevent defining both ``Meta.ordering`` and ``order_with_respect_to`` on the same model. diff --git a/docs/topics/db/queries.txt b/docs/topics/db/queries.txt index 8bb2bab482..28ddf660df 100644 --- a/docs/topics/db/queries.txt +++ b/docs/topics/db/queries.txt @@ -899,9 +899,15 @@ Deleting objects The delete method, conveniently, is named :meth:`~django.db.models.Model.delete`. This method immediately deletes the -object and has no return value. Example:: +object and returns the number of objects deleted and a dictionary with +the number of deletions per object type. Example:: - e.delete() + >>> e.delete() + (1, {'weblog.Entry': 1}) + +.. versionchanged:: 1.9 + + The return value describing the number of objects deleted was added. You can also delete objects in bulk. Every :class:`~django.db.models.query.QuerySet` has a @@ -911,7 +917,8 @@ members of that :class:`~django.db.models.query.QuerySet`. For example, this deletes all ``Entry`` objects with a ``pub_date`` year of 2005:: - Entry.objects.filter(pub_date__year=2005).delete() + >>> Entry.objects.filter(pub_date__year=2005).delete() + (5, {'webapp.Entry': 5}) Keep in mind that this will, whenever possible, be executed purely in SQL, and so the ``delete()`` methods of individual object instances will not necessarily @@ -923,6 +930,10 @@ object individually) rather than using the bulk :meth:`~django.db.models.query.QuerySet.delete` method of a :class:`~django.db.models.query.QuerySet`. +.. versionchanged:: 1.9 + + The return value describing the number of objects deleted was added. + When Django deletes an object, by default it emulates the behavior of the SQL constraint ``ON DELETE CASCADE`` -- in other words, any objects which had foreign keys pointing at the object to be deleted will be deleted along with diff --git a/tests/delete/tests.py b/tests/delete/tests.py index dd3c427824..11e780bfac 100644 --- a/tests/delete/tests.py +++ b/tests/delete/tests.py @@ -137,6 +137,7 @@ class OnDeleteTests(TestCase): class DeletionTests(TestCase): + def test_m2m(self): m = M.objects.create() r = R.objects.create() @@ -356,6 +357,62 @@ class DeletionTests(TestCase): self.assertFalse(RChild.objects.filter(id=child.id).exists()) self.assertTrue(R.objects.filter(id=parent_id).exists()) + def test_queryset_delete_returns_num_rows(self): + """ + QuerySet.delete() should return the number of deleted rows and a + dictionary with the number of deletions for each object type. + """ + Avatar.objects.bulk_create([Avatar(desc='a'), Avatar(desc='b'), Avatar(desc='c')]) + avatars_count = Avatar.objects.count() + deleted, rows_count = Avatar.objects.all().delete() + self.assertEqual(deleted, avatars_count) + + # more complex example with multiple object types + r = R.objects.create() + h1 = HiddenUser.objects.create(r=r) + HiddenUser.objects.create(r=r) + HiddenUserProfile.objects.create(user=h1) + existed_objs = { + R._meta.label: R.objects.count(), + HiddenUser._meta.label: HiddenUser.objects.count(), + A._meta.label: A.objects.count(), + MR._meta.label: MR.objects.count(), + HiddenUserProfile._meta.label: HiddenUserProfile.objects.count(), + } + deleted, deleted_objs = R.objects.all().delete() + for k, v in existed_objs.items(): + self.assertEqual(deleted_objs[k], v) + + def test_model_delete_returns_num_rows(self): + """ + Model.delete() should return the number of deleted rows and a + dictionary with the number of deletions for each object type. + """ + r = R.objects.create() + h1 = HiddenUser.objects.create(r=r) + h2 = HiddenUser.objects.create(r=r) + HiddenUser.objects.create(r=r) + HiddenUserProfile.objects.create(user=h1) + HiddenUserProfile.objects.create(user=h2) + m1 = M.objects.create() + m2 = M.objects.create() + MR.objects.create(r=r, m=m1) + r.m_set.add(m1) + r.m_set.add(m2) + r.save() + existed_objs = { + R._meta.label: R.objects.count(), + HiddenUser._meta.label: HiddenUser.objects.count(), + A._meta.label: A.objects.count(), + MR._meta.label: MR.objects.count(), + HiddenUserProfile._meta.label: HiddenUserProfile.objects.count(), + M.m2m.through._meta.label: M.m2m.through.objects.count(), + } + deleted, deleted_objs = r.delete() + self.assertEqual(deleted, sum(existed_objs.values())) + for k, v in existed_objs.items(): + self.assertEqual(deleted_objs[k], v) + class FastDeleteTests(TestCase):