mirror of
https://github.com/django/django.git
synced 2024-12-22 17:16:24 +00:00
Subquery deconstruction support required implementing complex and
expensive equality rules for sql.Query objects for little benefit as
the latter cannot themselves be made deconstructible to their reference
to model classes.
Making Expression @deconstructible and not BaseExpression allows
interested parties to conform to the "expression" API even if they are
not deconstructible as it's only a requirement for expressions allowed
in Model fields and meta options (e.g. constraints, indexes).
Thanks Phillip Cutter for the report.
This also fixes a performance regression in bbf141bcdc
.
This commit is contained in:
parent
4f600673d7
commit
c8b6594305
@ -147,7 +147,6 @@ class Combinable:
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
@deconstructible
|
|
||||||
class BaseExpression:
|
class BaseExpression:
|
||||||
"""Base class for all query expressions."""
|
"""Base class for all query expressions."""
|
||||||
|
|
||||||
@ -389,6 +388,11 @@ class BaseExpression:
|
|||||||
return self.output_field.select_format(compiler, sql, params)
|
return self.output_field.select_format(compiler, sql, params)
|
||||||
return sql, params
|
return sql, params
|
||||||
|
|
||||||
|
|
||||||
|
@deconstructible
|
||||||
|
class Expression(BaseExpression, Combinable):
|
||||||
|
"""An expression that can be combined with other expressions."""
|
||||||
|
|
||||||
@cached_property
|
@cached_property
|
||||||
def identity(self):
|
def identity(self):
|
||||||
constructor_signature = inspect.signature(self.__init__)
|
constructor_signature = inspect.signature(self.__init__)
|
||||||
@ -409,7 +413,7 @@ class BaseExpression:
|
|||||||
return tuple(identity)
|
return tuple(identity)
|
||||||
|
|
||||||
def __eq__(self, other):
|
def __eq__(self, other):
|
||||||
if not isinstance(other, BaseExpression):
|
if not isinstance(other, Expression):
|
||||||
return NotImplemented
|
return NotImplemented
|
||||||
return other.identity == self.identity
|
return other.identity == self.identity
|
||||||
|
|
||||||
@ -417,11 +421,6 @@ class BaseExpression:
|
|||||||
return hash(self.identity)
|
return hash(self.identity)
|
||||||
|
|
||||||
|
|
||||||
class Expression(BaseExpression, Combinable):
|
|
||||||
"""An expression that can be combined with other expressions."""
|
|
||||||
pass
|
|
||||||
|
|
||||||
|
|
||||||
_connector_combinators = {
|
_connector_combinators = {
|
||||||
connector: [
|
connector: [
|
||||||
(fields.IntegerField, fields.IntegerField, fields.IntegerField),
|
(fields.IntegerField, fields.IntegerField, fields.IntegerField),
|
||||||
@ -1103,7 +1102,7 @@ class Case(Expression):
|
|||||||
return super().get_group_by_cols(alias)
|
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
|
An explicit subquery. It may contain OuterRef() references to the outer
|
||||||
query which will be resolved when it is applied to that query.
|
query which will be resolved when it is applied to that query.
|
||||||
@ -1117,16 +1116,6 @@ class Subquery(Expression):
|
|||||||
self.extra = extra
|
self.extra = extra
|
||||||
super().__init__(output_field)
|
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):
|
def get_source_expressions(self):
|
||||||
return [self.query]
|
return [self.query]
|
||||||
|
|
||||||
@ -1203,6 +1192,7 @@ class Exists(Subquery):
|
|||||||
return sql, params
|
return sql, params
|
||||||
|
|
||||||
|
|
||||||
|
@deconstructible
|
||||||
class OrderBy(BaseExpression):
|
class OrderBy(BaseExpression):
|
||||||
template = '%(expression)s %(ordering)s'
|
template = '%(expression)s %(ordering)s'
|
||||||
conditional = False
|
conditional = False
|
||||||
|
@ -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
|
large and/or so that they can be used by other modules without getting into
|
||||||
circular import difficulties.
|
circular import difficulties.
|
||||||
"""
|
"""
|
||||||
|
import copy
|
||||||
import functools
|
import functools
|
||||||
import inspect
|
import inspect
|
||||||
from collections import namedtuple
|
from collections import namedtuple
|
||||||
@ -43,14 +44,11 @@ class Q(tree.Node):
|
|||||||
if not(isinstance(other, Q) or getattr(other, 'conditional', False) is True):
|
if not(isinstance(other, Q) or getattr(other, 'conditional', False) is True):
|
||||||
raise TypeError(other)
|
raise TypeError(other)
|
||||||
|
|
||||||
# If the other Q() is empty, ignore it and just use `self`.
|
if not self:
|
||||||
if not other:
|
return other.copy() if hasattr(other, 'copy') else copy.copy(other)
|
||||||
|
elif isinstance(other, Q) and not other:
|
||||||
_, args, kwargs = self.deconstruct()
|
_, args, kwargs = self.deconstruct()
|
||||||
return type(self)(*args, **kwargs)
|
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 = type(self)()
|
||||||
obj.connector = conn
|
obj.connector = conn
|
||||||
|
@ -36,7 +36,6 @@ from django.db.models.sql.where import (
|
|||||||
AND, OR, ExtraWhere, NothingNode, WhereNode,
|
AND, OR, ExtraWhere, NothingNode, WhereNode,
|
||||||
)
|
)
|
||||||
from django.utils.functional import cached_property
|
from django.utils.functional import cached_property
|
||||||
from django.utils.hashable import make_hashable
|
|
||||||
from django.utils.tree import Node
|
from django.utils.tree import Node
|
||||||
|
|
||||||
__all__ = ['Query', 'RawQuery']
|
__all__ = ['Query', 'RawQuery']
|
||||||
@ -250,14 +249,6 @@ class Query(BaseExpression):
|
|||||||
for alias in self.alias_map:
|
for alias in self.alias_map:
|
||||||
return alias
|
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):
|
def __str__(self):
|
||||||
"""
|
"""
|
||||||
Return the query as a string of SQL with the parameter values
|
Return the query as a string of SQL with the parameter values
|
||||||
|
@ -68,3 +68,7 @@ Bugfixes
|
|||||||
|
|
||||||
* Fixed a regression in Django 3.2 where the calling process environment would
|
* Fixed a regression in Django 3.2 where the calling process environment would
|
||||||
not be passed to the ``dbshell`` command on PostgreSQL (:ticket:`32687`).
|
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.
|
||||||
|
@ -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 django.test import SimpleTestCase
|
||||||
|
|
||||||
from .models import Tag
|
from .models import Tag
|
||||||
@ -50,6 +51,16 @@ class QTests(SimpleTestCase):
|
|||||||
with self.assertRaisesMessage(TypeError, str(obj)):
|
with self.assertRaisesMessage(TypeError, str(obj)):
|
||||||
q & 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):
|
def test_deconstruct(self):
|
||||||
q = Q(price__gt=F('discounted_price'))
|
q = Q(price__gt=F('discounted_price'))
|
||||||
path, args, kwargs = q.deconstruct()
|
path, args, kwargs = q.deconstruct()
|
||||||
@ -101,10 +112,10 @@ class QTests(SimpleTestCase):
|
|||||||
self.assertEqual(kwargs, {})
|
self.assertEqual(kwargs, {})
|
||||||
|
|
||||||
def test_deconstruct_boolean_expression(self):
|
def test_deconstruct_boolean_expression(self):
|
||||||
tagged = Tag.objects.filter(category=OuterRef('pk'))
|
expr = RawSQL('1 = 1', BooleanField())
|
||||||
q = Q(Exists(tagged))
|
q = Q(expr)
|
||||||
_, args, kwargs = q.deconstruct()
|
_, args, kwargs = q.deconstruct()
|
||||||
self.assertEqual(args, (Exists(tagged),))
|
self.assertEqual(args, (expr,))
|
||||||
self.assertEqual(kwargs, {})
|
self.assertEqual(kwargs, {})
|
||||||
|
|
||||||
def test_reconstruct(self):
|
def test_reconstruct(self):
|
||||||
|
@ -150,31 +150,3 @@ class TestQuery(SimpleTestCase):
|
|||||||
msg = 'Cannot filter against a non-conditional expression.'
|
msg = 'Cannot filter against a non-conditional expression.'
|
||||||
with self.assertRaisesMessage(TypeError, msg):
|
with self.assertRaisesMessage(TypeError, msg):
|
||||||
query.build_where(Func(output_field=CharField()))
|
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),
|
|
||||||
)
|
|
||||||
|
Loading…
Reference in New Issue
Block a user