From a2396a4c8f2ccd7f91adee6d8c2e9c31f13f0e3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anssi=20K=C3=A4=C3=A4ri=C3=A4inen?= Date: Wed, 24 Oct 2012 00:04:37 +0300 Subject: [PATCH] Fixed #19173 -- Made EmptyQuerySet a marker class only The guarantee that no queries will be made when accessing results is done by new EmptyWhere class which is used for query.where and having. Thanks to Simon Charette for reviewing and valuable suggestions. --- django/contrib/auth/models.py | 4 +- django/db/models/manager.py | 8 +- django/db/models/query.py | 159 +++----------------- django/db/models/sql/query.py | 9 +- django/db/models/sql/where.py | 8 + docs/ref/models/querysets.txt | 10 +- docs/releases/1.6.txt | 5 + tests/modeltests/basic/tests.py | 7 + tests/modeltests/get_object_or_404/tests.py | 2 +- tests/modeltests/lookup/tests.py | 2 +- tests/regressiontests/queries/tests.py | 59 ++++---- 11 files changed, 96 insertions(+), 177 deletions(-) diff --git a/django/contrib/auth/models.py b/django/contrib/auth/models.py index 6f20981ca6..1b63833688 100644 --- a/django/contrib/auth/models.py +++ b/django/contrib/auth/models.py @@ -473,8 +473,8 @@ class AnonymousUser(object): is_staff = False is_active = False is_superuser = False - _groups = EmptyManager() - _user_permissions = EmptyManager() + _groups = EmptyManager(Group) + _user_permissions = EmptyManager(Permission) def __init__(self): pass diff --git a/django/db/models/manager.py b/django/db/models/manager.py index 8da8af487c..da6523c89a 100644 --- a/django/db/models/manager.py +++ b/django/db/models/manager.py @@ -1,6 +1,6 @@ import copy from django.db import router -from django.db.models.query import QuerySet, EmptyQuerySet, insert_query, RawQuerySet +from django.db.models.query import QuerySet, insert_query, RawQuerySet from django.db.models import signals from django.db.models.fields import FieldDoesNotExist @@ -113,7 +113,7 @@ class Manager(object): ####################### def get_empty_query_set(self): - return EmptyQuerySet(self.model, using=self._db) + return QuerySet(self.model, using=self._db).none() def get_query_set(self): """Returns a new QuerySet object. Subclasses can override this method @@ -258,5 +258,9 @@ class SwappedManagerDescriptor(object): class EmptyManager(Manager): + def __init__(self, model): + super(EmptyManager, self).__init__() + self.model = model + def get_query_set(self): return self.get_empty_query_set() diff --git a/django/db/models/query.py b/django/db/models/query.py index d1f519aaf8..edc8cc9776 100644 --- a/django/db/models/query.py +++ b/django/db/models/query.py @@ -35,7 +35,6 @@ class QuerySet(object): """ def __init__(self, model=None, query=None, using=None): self.model = model - # EmptyQuerySet instantiates QuerySet with model as None self._db = using self.query = query or sql.Query(self.model) self._result_cache = None @@ -217,7 +216,9 @@ class QuerySet(object): def __and__(self, other): self._merge_sanity_check(other) if isinstance(other, EmptyQuerySet): - return other._clone() + return other + if isinstance(self, EmptyQuerySet): + return self combined = self._clone() combined._merge_known_related_objects(other) combined.query.combine(other.query, sql.AND) @@ -225,9 +226,11 @@ class QuerySet(object): def __or__(self, other): self._merge_sanity_check(other) - combined = self._clone() + if isinstance(self, EmptyQuerySet): + return other if isinstance(other, EmptyQuerySet): - return combined + return self + combined = self._clone() combined._merge_known_related_objects(other) combined.query.combine(other.query, sql.OR) return combined @@ -632,7 +635,9 @@ class QuerySet(object): """ Returns an empty QuerySet. """ - return self._clone(klass=EmptyQuerySet) + clone = self._clone() + clone.query.set_empty() + return clone ################################################################## # PUBLIC METHODS THAT ALTER ATTRIBUTES AND RETURN A NEW QUERYSET # @@ -981,6 +986,18 @@ class QuerySet(object): # empty" result. value_annotation = True +class InstanceCheckMeta(type): + def __instancecheck__(self, instance): + return instance.query.is_empty() + +class EmptyQuerySet(six.with_metaclass(InstanceCheckMeta), object): + """ + Marker class usable for checking if a queryset is empty by .none(): + isinstance(qs.none(), EmptyQuerySet) -> True + """ + + def __init__(self, *args, **kwargs): + raise TypeError("EmptyQuerySet can't be instantiated") class ValuesQuerySet(QuerySet): def __init__(self, *args, **kwargs): @@ -1180,138 +1197,6 @@ class DateQuerySet(QuerySet): return c -class EmptyQuerySet(QuerySet): - def __init__(self, model=None, query=None, using=None): - super(EmptyQuerySet, self).__init__(model, query, using) - self._result_cache = [] - - def __and__(self, other): - return self._clone() - - def __or__(self, other): - return other._clone() - - def count(self): - return 0 - - def delete(self): - pass - - def _clone(self, klass=None, setup=False, **kwargs): - c = super(EmptyQuerySet, self)._clone(klass, setup=setup, **kwargs) - c._result_cache = [] - return c - - def iterator(self): - # This slightly odd construction is because we need an empty generator - # (it raises StopIteration immediately). - yield next(iter([])) - - def all(self): - """ - Always returns EmptyQuerySet. - """ - return self - - def filter(self, *args, **kwargs): - """ - Always returns EmptyQuerySet. - """ - return self - - def exclude(self, *args, **kwargs): - """ - Always returns EmptyQuerySet. - """ - return self - - def complex_filter(self, filter_obj): - """ - Always returns EmptyQuerySet. - """ - return self - - def select_related(self, *fields, **kwargs): - """ - Always returns EmptyQuerySet. - """ - return self - - def annotate(self, *args, **kwargs): - """ - Always returns EmptyQuerySet. - """ - return self - - def order_by(self, *field_names): - """ - Always returns EmptyQuerySet. - """ - return self - - def distinct(self, fields=None): - """ - Always returns EmptyQuerySet. - """ - return self - - def extra(self, select=None, where=None, params=None, tables=None, - order_by=None, select_params=None): - """ - Always returns EmptyQuerySet. - """ - assert self.query.can_filter(), \ - "Cannot change a query once a slice has been taken" - return self - - def reverse(self): - """ - Always returns EmptyQuerySet. - """ - return self - - def defer(self, *fields): - """ - Always returns EmptyQuerySet. - """ - return self - - def only(self, *fields): - """ - Always returns EmptyQuerySet. - """ - return self - - def update(self, **kwargs): - """ - Don't update anything. - """ - return 0 - - def aggregate(self, *args, **kwargs): - """ - Return a dict mapping the aggregate names to None - """ - for arg in args: - kwargs[arg.default_alias] = arg - return dict([(key, None) for key in kwargs]) - - def values(self, *fields): - """ - Always returns EmptyQuerySet. - """ - return self - - def values_list(self, *fields, **kwargs): - """ - Always returns EmptyQuerySet. - """ - return self - - # EmptyQuerySet is always an empty result in where-clauses (and similar - # situations). - value_annotation = False - def get_klass_info(klass, max_depth=0, cur_depth=0, requested=None, only_load=None, from_parent=None): """ diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py index 87104f0d13..f021d571e9 100644 --- a/django/db/models/sql/query.py +++ b/django/db/models/sql/query.py @@ -25,7 +25,7 @@ from django.db.models.sql.constants import (QUERY_TERMS, ORDER_DIR, SINGLE, from django.db.models.sql.datastructures import EmptyResultSet, Empty, MultiJoin from django.db.models.sql.expressions import SQLEvaluator from django.db.models.sql.where import (WhereNode, Constraint, EverythingNode, - ExtraWhere, AND, OR) + ExtraWhere, AND, OR, EmptyWhere) from django.core.exceptions import FieldError __all__ = ['Query', 'RawQuery'] @@ -1511,6 +1511,13 @@ class Query(object): self.add_filter(('%s__isnull' % trimmed_prefix, False), negate=True, can_reuse=can_reuse) + def set_empty(self): + self.where = EmptyWhere() + self.having = EmptyWhere() + + def is_empty(self): + return isinstance(self.where, EmptyWhere) or isinstance(self.having, EmptyWhere) + def set_limits(self, low=None, high=None): """ Adjusts the limits on the rows retrieved. We use low/high to set these, diff --git a/django/db/models/sql/where.py b/django/db/models/sql/where.py index 47f4ffaba9..02847b1f54 100644 --- a/django/db/models/sql/where.py +++ b/django/db/models/sql/where.py @@ -272,6 +272,14 @@ class WhereNode(tree.Node): if hasattr(child[3], 'relabel_aliases'): child[3].relabel_aliases(change_map) +class EmptyWhere(WhereNode): + + def add(self, data, connector): + return + + def as_sql(self, qn=None, connection=None): + raise EmptyResultSet + class EverythingNode(object): """ A node that matches everything. diff --git a/docs/ref/models/querysets.txt b/docs/ref/models/querysets.txt index a8e946f8a5..2bbd895fd4 100644 --- a/docs/ref/models/querysets.txt +++ b/docs/ref/models/querysets.txt @@ -593,15 +593,17 @@ none .. method:: none() -Returns an ``EmptyQuerySet`` — a ``QuerySet`` subclass that always evaluates to -an empty list. This can be used in cases where you know that you should return -an empty result set and your caller is expecting a ``QuerySet`` object (instead -of returning an empty list, for example.) +Calling none() will create a queryset that never returns any objects and no +query will be executed when accessing the results. A qs.none() queryset +is an instance of ``EmptyQuerySet``. Examples:: >>> Entry.objects.none() [] + >>> from django.db.models.query import EmptyQuerySet + >>> isinstance(Entry.objects.none(), EmptyQuerySet) + True all ~~~ diff --git a/docs/releases/1.6.txt b/docs/releases/1.6.txt index 1f57913397..e425036839 100644 --- a/docs/releases/1.6.txt +++ b/docs/releases/1.6.txt @@ -31,6 +31,11 @@ Minor features Backwards incompatible changes in 1.6 ===================================== +* The ``django.db.models.query.EmptyQuerySet`` can't be instantiated any more - + it is only usable as a marker class for checking if + :meth:`~django.db.models.query.QuerySet.none` has been called: + ``isinstance(qs.none(), EmptyQuerySet)`` + .. warning:: In addition to the changes outlined in this section, be sure to review the diff --git a/tests/modeltests/basic/tests.py b/tests/modeltests/basic/tests.py index 1c83b980a7..dba9a686d9 100644 --- a/tests/modeltests/basic/tests.py +++ b/tests/modeltests/basic/tests.py @@ -4,6 +4,7 @@ from datetime import datetime from django.core.exceptions import ObjectDoesNotExist, MultipleObjectsReturned from django.db.models.fields import Field, FieldDoesNotExist +from django.db.models.query import EmptyQuerySet from django.test import TestCase, skipIfDBFeature, skipUnlessDBFeature from django.utils import six from django.utils.translation import ugettext_lazy @@ -639,3 +640,9 @@ class ModelTest(TestCase): Article.objects.bulk_create([Article(headline=lazy, pub_date=datetime.now())]) article = Article.objects.get() self.assertEqual(article.headline, notlazy) + + def test_emptyqs(self): + # Can't be instantiated + with self.assertRaises(TypeError): + EmptyQuerySet() + self.assertTrue(isinstance(Article.objects.none(), EmptyQuerySet)) diff --git a/tests/modeltests/get_object_or_404/tests.py b/tests/modeltests/get_object_or_404/tests.py index 3b234c6cd3..38ebeb4f8c 100644 --- a/tests/modeltests/get_object_or_404/tests.py +++ b/tests/modeltests/get_object_or_404/tests.py @@ -53,7 +53,7 @@ class GetObjectOr404Tests(TestCase): get_object_or_404, Author.objects.all() ) - # Using an EmptyQuerySet raises a Http404 error. + # Using an empty QuerySet raises a Http404 error. self.assertRaises(Http404, get_object_or_404, Article.objects.none(), title__contains="Run" ) diff --git a/tests/modeltests/lookup/tests.py b/tests/modeltests/lookup/tests.py index 98358e3d10..de7105f92d 100644 --- a/tests/modeltests/lookup/tests.py +++ b/tests/modeltests/lookup/tests.py @@ -436,7 +436,7 @@ class LookupTests(TestCase): ]) def test_none(self): - # none() returns an EmptyQuerySet that behaves like any other QuerySet object + # none() returns a QuerySet that behaves like any other QuerySet object self.assertQuerysetEqual(Article.objects.none(), []) self.assertQuerysetEqual( Article.objects.none().filter(headline__startswith='Article'), []) diff --git a/tests/regressiontests/queries/tests.py b/tests/regressiontests/queries/tests.py index e3e515025c..7d01c16255 100644 --- a/tests/regressiontests/queries/tests.py +++ b/tests/regressiontests/queries/tests.py @@ -9,7 +9,7 @@ from django.conf import settings from django.core.exceptions import FieldError from django.db import DatabaseError, connection, connections, DEFAULT_DB_ALIAS from django.db.models import Count, F, Q -from django.db.models.query import ITER_CHUNK_SIZE, EmptyQuerySet +from django.db.models.query import ITER_CHUNK_SIZE from django.db.models.sql.where import WhereNode, EverythingNode, NothingNode from django.db.models.sql.datastructures import EmptyResultSet from django.test import TestCase, skipUnlessDBFeature @@ -663,31 +663,32 @@ class Queries1Tests(BaseQuerysetTest): Item.objects.filter(created__in=[self.time1, self.time2]), ['', ''] ) - def test_ticket7235(self): # An EmptyQuerySet should not raise exceptions if it is filtered. - q = EmptyQuerySet() - self.assertQuerysetEqual(q.all(), []) - self.assertQuerysetEqual(q.filter(x=10), []) - self.assertQuerysetEqual(q.exclude(y=3), []) - self.assertQuerysetEqual(q.complex_filter({'pk': 1}), []) - self.assertQuerysetEqual(q.select_related('spam', 'eggs'), []) - self.assertQuerysetEqual(q.annotate(Count('eggs')), []) - self.assertQuerysetEqual(q.order_by('-pub_date', 'headline'), []) - self.assertQuerysetEqual(q.distinct(), []) - self.assertQuerysetEqual( - q.extra(select={'is_recent': "pub_date > '2006-01-01'"}), - [] - ) - q.query.low_mark = 1 - self.assertRaisesMessage( - AssertionError, - 'Cannot change a query once a slice has been taken', - q.extra, select={'is_recent': "pub_date > '2006-01-01'"} - ) - self.assertQuerysetEqual(q.reverse(), []) - self.assertQuerysetEqual(q.defer('spam', 'eggs'), []) - self.assertQuerysetEqual(q.only('spam', 'eggs'), []) + Eaten.objects.create(meal='m') + q = Eaten.objects.none() + with self.assertNumQueries(0): + self.assertQuerysetEqual(q.all(), []) + self.assertQuerysetEqual(q.filter(meal='m'), []) + self.assertQuerysetEqual(q.exclude(meal='m'), []) + self.assertQuerysetEqual(q.complex_filter({'pk': 1}), []) + self.assertQuerysetEqual(q.select_related('food'), []) + self.assertQuerysetEqual(q.annotate(Count('food')), []) + self.assertQuerysetEqual(q.order_by('meal', 'food'), []) + self.assertQuerysetEqual(q.distinct(), []) + self.assertQuerysetEqual( + q.extra(select={'foo': "1"}), + [] + ) + q.query.low_mark = 1 + self.assertRaisesMessage( + AssertionError, + 'Cannot change a query once a slice has been taken', + q.extra, select={'foo': "1"} + ) + self.assertQuerysetEqual(q.reverse(), []) + self.assertQuerysetEqual(q.defer('meal'), []) + self.assertQuerysetEqual(q.only('meal'), []) def test_ticket7791(self): # There were "issues" when ordering and distinct-ing on fields related @@ -1935,8 +1936,8 @@ class CloneTests(TestCase): class EmptyQuerySetTests(TestCase): def test_emptyqueryset_values(self): - # #14366 -- Calling .values() on an EmptyQuerySet and then cloning that - # should not cause an error" + # #14366 -- Calling .values() on an empty QuerySet and then cloning + # that should not cause an error self.assertQuerysetEqual( Number.objects.none().values('num').order_by('num'), [] ) @@ -1952,9 +1953,9 @@ class EmptyQuerySetTests(TestCase): ) def test_ticket_19151(self): - # #19151 -- Calling .values() or .values_list() on an EmptyQuerySet - # should return EmptyQuerySet and not cause an error. - q = EmptyQuerySet() + # #19151 -- Calling .values() or .values_list() on an empty QuerySet + # should return an empty QuerySet and not cause an error. + q = Author.objects.none() self.assertQuerysetEqual(q.values(), []) self.assertQuerysetEqual(q.values_list(), [])