From 82efb4840311d549989ccf224fa78765226f6040 Mon Sep 17 00:00:00 2001 From: Russell Keith-Magee Date: Sun, 4 Apr 2010 17:05:43 +0000 Subject: [PATCH] Fixed #12328 -- Corrected the handling of subqueries with ordering and slicing, especially when used in delete subqueries. Thanks to Walter Doekes for the report. This fixes a feature that isn't available under MySQL and Oracle (Refs #10099). git-svn-id: http://code.djangoproject.com/svn/django/trunk@12912 bcc190cf-cafb-0310-a4f2-bffc1f526a37 --- django/db/backends/__init__.py | 1 + django/db/backends/mysql/base.py | 1 + django/db/backends/oracle/base.py | 1 + django/db/models/query.py | 9 +++-- django/db/models/sql/compiler.py | 8 +++-- tests/regressiontests/queries/tests.py | 50 ++++++++++++++++++++++---- 6 files changed, 58 insertions(+), 12 deletions(-) diff --git a/django/db/backends/__init__.py b/django/db/backends/__init__.py index 791d1fb3d1..5918935899 100644 --- a/django/db/backends/__init__.py +++ b/django/db/backends/__init__.py @@ -95,6 +95,7 @@ class BaseDatabaseFeatures(object): # If True, don't use integer foreign keys referring to, e.g., positive # integer primary keys. related_fields_match_type = False + allow_sliced_subqueries = True class BaseDatabaseOperations(object): """ diff --git a/django/db/backends/mysql/base.py b/django/db/backends/mysql/base.py index 66c13ed035..2ade58bc65 100644 --- a/django/db/backends/mysql/base.py +++ b/django/db/backends/mysql/base.py @@ -123,6 +123,7 @@ class DatabaseFeatures(BaseDatabaseFeatures): update_can_self_select = False allows_group_by_pk = True related_fields_match_type = True + allow_sliced_subqueries = False class DatabaseOperations(BaseDatabaseOperations): compiler_module = "django.db.backends.mysql.compiler" diff --git a/django/db/backends/oracle/base.py b/django/db/backends/oracle/base.py index f87a482609..f3f46e35d3 100644 --- a/django/db/backends/oracle/base.py +++ b/django/db/backends/oracle/base.py @@ -52,6 +52,7 @@ class DatabaseFeatures(BaseDatabaseFeatures): interprets_empty_strings_as_nulls = True uses_savepoints = True can_return_id_from_insert = True + allow_sliced_subqueries = False class DatabaseOperations(BaseDatabaseOperations): diff --git a/django/db/models/query.py b/django/db/models/query.py index 3567671a1a..b8b2340b3f 100644 --- a/django/db/models/query.py +++ b/django/db/models/query.py @@ -3,6 +3,7 @@ The main QuerySet implementation. This provides the public API for the ORM. """ from copy import deepcopy +from itertools import izip from django.db import connections, router, transaction, IntegrityError from django.db.models.aggregates import Aggregate @@ -429,11 +430,13 @@ class QuerySet(object): # becoming too long. seen_objs = None while 1: - # Collect all the objects to be deleted in this chunk, and all the + # Collect a chunk of objects to be deleted, and then all the # objects that are related to the objects that are to be deleted. + # The chunking *isn't* done by slicing the del_query because we + # need to maintain the query cache on del_query (see #12328) seen_objs = CollectedObjects(seen_objs) - for object in del_query[:CHUNK_SIZE]: - object._collect_sub_objects(seen_objs) + for i, obj in izip(xrange(CHUNK_SIZE), del_query): + obj._collect_sub_objects(seen_objs) if not seen_objs: break diff --git a/django/db/models/sql/compiler.py b/django/db/models/sql/compiler.py index 84c28a0d35..8a24c62fe5 100644 --- a/django/db/models/sql/compiler.py +++ b/django/db/models/sql/compiler.py @@ -120,13 +120,15 @@ class SQLCompiler(object): """ Perform the same functionality as the as_sql() method, returning an SQL string and parameters. However, the alias prefixes are bumped - beforehand (in a copy -- the current query isn't changed) and any - ordering is removed. + beforehand (in a copy -- the current query isn't changed), and any + ordering is removed if the query is unsliced. Used when nesting this query inside another. """ obj = self.query.clone() - obj.clear_ordering(True) + if obj.low_mark == 0 and obj.high_mark is None: + # If there is no slicing in use, then we can safely drop all ordering + obj.clear_ordering(True) obj.bump_prefix() return obj.get_compiler(connection=self.connection).as_sql() diff --git a/tests/regressiontests/queries/tests.py b/tests/regressiontests/queries/tests.py index 5699b60241..ee79a9f85a 100644 --- a/tests/regressiontests/queries/tests.py +++ b/tests/regressiontests/queries/tests.py @@ -1,27 +1,65 @@ import unittest -from models import Tag, Annotation + +from django.db import DatabaseError, connections, DEFAULT_DB_ALIAS from django.db.models import Count +from django.test import TestCase + +from models import Tag, Annotation, DumbCategory class QuerysetOrderedTests(unittest.TestCase): """ Tests for the Queryset.ordered attribute. """ - + def test_no_default_or_explicit_ordering(self): self.assertEqual(Annotation.objects.all().ordered, False) def test_cleared_default_ordering(self): self.assertEqual(Tag.objects.all().ordered, True) self.assertEqual(Tag.objects.all().order_by().ordered, False) - + def test_explicit_ordering(self): self.assertEqual(Annotation.objects.all().order_by('id').ordered, True) - + def test_order_by_extra(self): self.assertEqual(Annotation.objects.all().extra(order_by=['id']).ordered, True) - + def test_annotated_ordering(self): qs = Annotation.objects.annotate(num_notes=Count('notes')) self.assertEqual(qs.ordered, False) self.assertEqual(qs.order_by('num_notes').ordered, True) - \ No newline at end of file + + +class SubqueryTests(TestCase): + def setUp(self): + DumbCategory.objects.create(id=1) + DumbCategory.objects.create(id=2) + DumbCategory.objects.create(id=3) + + def test_ordered_subselect(self): + "Subselects honor any manual ordering" + try: + query = DumbCategory.objects.filter(id__in=DumbCategory.objects.order_by('-id')[0:2]) + self.assertEquals(set(query.values_list('id', flat=True)), set([2,3])) + + query = DumbCategory.objects.filter(id__in=DumbCategory.objects.order_by('-id')[:2]) + self.assertEquals(set(query.values_list('id', flat=True)), set([2,3])) + + query = DumbCategory.objects.filter(id__in=DumbCategory.objects.order_by('-id')[2:]) + self.assertEquals(set(query.values_list('id', flat=True)), set([1])) + except DatabaseError: + # Oracle and MySQL both have problems with sliced subselects. + # This prevents us from even evaluating this test case at all. + # Refs #10099 + self.assertFalse(connections[DEFAULT_DB_ALIAS].features.allow_sliced_subqueries) + + def test_sliced_delete(self): + "Delete queries can safely contain sliced subqueries" + try: + DumbCategory.objects.filter(id__in=DumbCategory.objects.order_by('-id')[0:1]).delete() + self.assertEquals(set(DumbCategory.objects.values_list('id', flat=True)), set([1,2])) + except DatabaseError: + # Oracle and MySQL both have problems with sliced subselects. + # This prevents us from even evaluating this test case at all. + # Refs #10099 + self.assertFalse(connections[DEFAULT_DB_ALIAS].features.allow_sliced_subqueries)