diff options
| -rw-r--r-- | django/db/models/expressions.py | 26 | ||||
| -rw-r--r-- | django/db/models/query_utils.py | 10 | ||||
| -rw-r--r-- | django/db/models/sql/query.py | 9 | ||||
| -rw-r--r-- | docs/releases/3.2.1.txt | 4 | ||||
| -rw-r--r-- | tests/queries/test_q.py | 19 | ||||
| -rw-r--r-- | tests/queries/test_query.py | 28 |
6 files changed, 31 insertions, 65 deletions
diff --git a/django/db/models/expressions.py b/django/db/models/expressions.py index 528d988e85..0bbf8cfade 100644 --- a/django/db/models/expressions.py +++ b/django/db/models/expressions.py @@ -147,7 +147,6 @@ class Combinable: ) -@deconstructible class BaseExpression: """Base class for all query expressions.""" @@ -389,6 +388,11 @@ class BaseExpression: return self.output_field.select_format(compiler, sql, params) return sql, params + +@deconstructible +class Expression(BaseExpression, Combinable): + """An expression that can be combined with other expressions.""" + @cached_property def identity(self): constructor_signature = inspect.signature(self.__init__) @@ -409,7 +413,7 @@ class BaseExpression: return tuple(identity) def __eq__(self, other): - if not isinstance(other, BaseExpression): + if not isinstance(other, Expression): return NotImplemented return other.identity == self.identity @@ -417,11 +421,6 @@ class BaseExpression: return hash(self.identity) -class Expression(BaseExpression, Combinable): - """An expression that can be combined with other expressions.""" - pass - - _connector_combinators = { connector: [ (fields.IntegerField, fields.IntegerField, fields.IntegerField), @@ -1103,7 +1102,7 @@ class Case(Expression): return super().get_group_by_cols(alias) -class Subquery(Expression): +class Subquery(BaseExpression, Combinable): """ An explicit subquery. It may contain OuterRef() references to the outer query which will be resolved when it is applied to that query. @@ -1117,16 +1116,6 @@ class Subquery(Expression): self.extra = extra super().__init__(output_field) - def __getstate__(self): - state = super().__getstate__() - args, kwargs = state['_constructor_args'] - if args: - args = (self.query, *args[1:]) - else: - kwargs['queryset'] = self.query - state['_constructor_args'] = args, kwargs - return state - def get_source_expressions(self): return [self.query] @@ -1203,6 +1192,7 @@ class Exists(Subquery): return sql, params +@deconstructible class OrderBy(BaseExpression): template = '%(expression)s %(ordering)s' conditional = False diff --git a/django/db/models/query_utils.py b/django/db/models/query_utils.py index 43c93ce455..188b640850 100644 --- a/django/db/models/query_utils.py +++ b/django/db/models/query_utils.py @@ -5,6 +5,7 @@ Factored out from django.db.models.query to avoid making the main module very large and/or so that they can be used by other modules without getting into circular import difficulties. """ +import copy import functools import inspect from collections import namedtuple @@ -43,14 +44,11 @@ class Q(tree.Node): if not(isinstance(other, Q) or getattr(other, 'conditional', False) is True): raise TypeError(other) - # If the other Q() is empty, ignore it and just use `self`. - if not other: + if not self: + return other.copy() if hasattr(other, 'copy') else copy.copy(other) + elif isinstance(other, Q) and not other: _, args, kwargs = self.deconstruct() return type(self)(*args, **kwargs) - # Or if this Q is empty, ignore it and just use `other`. - elif not self: - _, args, kwargs = other.deconstruct() - return type(other)(*args, **kwargs) obj = type(self)() obj.connector = conn diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py index be08c55c7c..b505af4efa 100644 --- a/django/db/models/sql/query.py +++ b/django/db/models/sql/query.py @@ -36,7 +36,6 @@ from django.db.models.sql.where import ( AND, OR, ExtraWhere, NothingNode, WhereNode, ) from django.utils.functional import cached_property -from django.utils.hashable import make_hashable from django.utils.tree import Node __all__ = ['Query', 'RawQuery'] @@ -250,14 +249,6 @@ class Query(BaseExpression): for alias in self.alias_map: return alias - @property - def identity(self): - identity = ( - (arg, make_hashable(value)) - for arg, value in self.__dict__.items() - ) - return (self.__class__, *identity) - def __str__(self): """ Return the query as a string of SQL with the parameter values diff --git a/docs/releases/3.2.1.txt b/docs/releases/3.2.1.txt index 1c8776a0bf..545c9adce3 100644 --- a/docs/releases/3.2.1.txt +++ b/docs/releases/3.2.1.txt @@ -68,3 +68,7 @@ Bugfixes * Fixed a regression in Django 3.2 where the calling process environment would not be passed to the ``dbshell`` command on PostgreSQL (:ticket:`32687`). + +* Fixed a performance regression in Django 3.2 when building complex filters + with subqueries (:ticket:`32632`). As a side-effect the private API to check + ``django.db.sql.query.Query`` equality is removed. diff --git a/tests/queries/test_q.py b/tests/queries/test_q.py index 84ae6e90a5..bd39b2dc32 100644 --- a/tests/queries/test_q.py +++ b/tests/queries/test_q.py @@ -1,4 +1,5 @@ -from django.db.models import Exists, F, OuterRef, Q +from django.db.models import BooleanField, Exists, F, OuterRef, Q +from django.db.models.expressions import RawSQL from django.test import SimpleTestCase from .models import Tag @@ -50,6 +51,16 @@ class QTests(SimpleTestCase): with self.assertRaisesMessage(TypeError, str(obj)): q & obj + def test_combine_negated_boolean_expression(self): + tagged = Tag.objects.filter(category=OuterRef('pk')) + tests = [ + Q() & ~Exists(tagged), + Q() | ~Exists(tagged), + ] + for q in tests: + with self.subTest(q=q): + self.assertIs(q.negated, True) + def test_deconstruct(self): q = Q(price__gt=F('discounted_price')) path, args, kwargs = q.deconstruct() @@ -101,10 +112,10 @@ class QTests(SimpleTestCase): self.assertEqual(kwargs, {}) def test_deconstruct_boolean_expression(self): - tagged = Tag.objects.filter(category=OuterRef('pk')) - q = Q(Exists(tagged)) + expr = RawSQL('1 = 1', BooleanField()) + q = Q(expr) _, args, kwargs = q.deconstruct() - self.assertEqual(args, (Exists(tagged),)) + self.assertEqual(args, (expr,)) self.assertEqual(kwargs, {}) def test_reconstruct(self): diff --git a/tests/queries/test_query.py b/tests/queries/test_query.py index 5db9d96163..523fa607f0 100644 --- a/tests/queries/test_query.py +++ b/tests/queries/test_query.py @@ -150,31 +150,3 @@ class TestQuery(SimpleTestCase): msg = 'Cannot filter against a non-conditional expression.' with self.assertRaisesMessage(TypeError, msg): query.build_where(Func(output_field=CharField())) - - def test_equality(self): - self.assertNotEqual( - Author.objects.all().query, - Author.objects.filter(item__name='foo').query, - ) - self.assertEqual( - Author.objects.filter(item__name='foo').query, - Author.objects.filter(item__name='foo').query, - ) - self.assertEqual( - Author.objects.filter(item__name='foo').query, - Author.objects.filter(Q(item__name='foo')).query, - ) - - def test_hash(self): - self.assertNotEqual( - hash(Author.objects.all().query), - hash(Author.objects.filter(item__name='foo').query) - ) - self.assertEqual( - hash(Author.objects.filter(item__name='foo').query), - hash(Author.objects.filter(item__name='foo').query), - ) - self.assertEqual( - hash(Author.objects.filter(item__name='foo').query), - hash(Author.objects.filter(Q(item__name='foo')).query), - ) |