Fixed #32143 -- Used EXISTS to exclude multi-valued relationships.

As mentioned in the pre-existing split_exclude() docstring EXISTS is
easier to optimize for query planers and circumvents the IN (NULL)
handling issue.
This commit is contained in:
Simon Charette 2020-10-25 16:04:21 -04:00 committed by Mariusz Felisiak
parent bbf141bcdc
commit 8593e162c9
3 changed files with 28 additions and 25 deletions

View File

@ -1083,19 +1083,18 @@ class Subquery(Expression):
contains_aggregate = False contains_aggregate = False
def __init__(self, queryset, output_field=None, **extra): def __init__(self, queryset, output_field=None, **extra):
self.query = queryset.query # Allow the usage of both QuerySet and sql.Query objects.
self.query = getattr(queryset, 'query', queryset)
self.extra = extra self.extra = extra
# Prevent the QuerySet from being evaluated.
self.queryset = queryset._chain(_result_cache=[], prefetch_done=True)
super().__init__(output_field) super().__init__(output_field)
def __getstate__(self): def __getstate__(self):
state = super().__getstate__() state = super().__getstate__()
args, kwargs = state['_constructor_args'] args, kwargs = state['_constructor_args']
if args: if args:
args = (self.queryset, *args[1:]) args = (self.query, *args[1:])
else: else:
kwargs['queryset'] = self.queryset kwargs['queryset'] = self.query
state['_constructor_args'] = args, kwargs state['_constructor_args'] = args, kwargs
return state return state

View File

@ -23,7 +23,9 @@ from django.core.exceptions import (
from django.db import DEFAULT_DB_ALIAS, NotSupportedError, connections from django.db import DEFAULT_DB_ALIAS, NotSupportedError, connections
from django.db.models.aggregates import Count from django.db.models.aggregates import Count
from django.db.models.constants import LOOKUP_SEP from django.db.models.constants import LOOKUP_SEP
from django.db.models.expressions import BaseExpression, Col, F, OuterRef, Ref from django.db.models.expressions import (
BaseExpression, Col, Exists, F, OuterRef, Ref, ResolvedOuterRef,
)
from django.db.models.fields import Field from django.db.models.fields import Field
from django.db.models.fields.related_lookups import MultiColSource from django.db.models.fields.related_lookups import MultiColSource
from django.db.models.lookups import Lookup from django.db.models.lookups import Lookup
@ -1765,12 +1767,12 @@ class Query(BaseExpression):
filters in the original query. filters in the original query.
We will turn this into equivalent of: We will turn this into equivalent of:
WHERE NOT (pk IN (SELECT parent_id FROM thetable WHERE NOT EXISTS(
WHERE name = 'foo' AND parent_id IS NOT NULL)) SELECT 1
FROM child
It might be worth it to consider using WHERE NOT EXISTS as that has WHERE name = 'foo' AND child.parent_id = parent.id
saner null handling, and is easier for the backend's optimizer to LIMIT 1
handle. )
""" """
filter_lhs, filter_rhs = filter_expr filter_lhs, filter_rhs = filter_expr
if isinstance(filter_rhs, OuterRef): if isinstance(filter_rhs, OuterRef):
@ -1786,17 +1788,9 @@ class Query(BaseExpression):
# the subquery. # the subquery.
trimmed_prefix, contains_louter = query.trim_start(names_with_path) trimmed_prefix, contains_louter = query.trim_start(names_with_path)
# Add extra check to make sure the selected field will not be null
# since we are adding an IN <subquery> clause. This prevents the
# database from tripping over IN (...,NULL,...) selects and returning
# nothing
col = query.select[0] col = query.select[0]
select_field = col.target select_field = col.target
alias = col.alias alias = col.alias
if self.is_nullable(select_field):
lookup_class = select_field.get_lookup('isnull')
lookup = lookup_class(select_field.get_col(alias), False)
query.where.add(lookup, AND)
if alias in can_reuse: if alias in can_reuse:
pk = select_field.model._meta.pk pk = select_field.model._meta.pk
# Need to add a restriction so that outer query's filters are in effect for # Need to add a restriction so that outer query's filters are in effect for
@ -1810,9 +1804,11 @@ class Query(BaseExpression):
query.where.add(lookup, AND) query.where.add(lookup, AND)
query.external_aliases[alias] = True query.external_aliases[alias] = True
condition, needed_inner = self.build_filter( lookup_class = select_field.get_lookup('exact')
('%s__in' % trimmed_prefix, query), lookup = lookup_class(col, ResolvedOuterRef(trimmed_prefix))
current_negated=True, branch_negated=True, can_reuse=can_reuse) query.where.add(lookup, AND)
condition, needed_inner = self.build_filter(Exists(query))
if contains_louter: if contains_louter:
or_null_condition, _ = self.build_filter( or_null_condition, _ = self.build_filter(
('%s__isnull' % trimmed_prefix, True), ('%s__isnull' % trimmed_prefix, True),

View File

@ -2807,11 +2807,11 @@ class ExcludeTests(TestCase):
f1 = Food.objects.create(name='apples') f1 = Food.objects.create(name='apples')
Food.objects.create(name='oranges') Food.objects.create(name='oranges')
Eaten.objects.create(food=f1, meal='dinner') Eaten.objects.create(food=f1, meal='dinner')
j1 = Job.objects.create(name='Manager') cls.j1 = Job.objects.create(name='Manager')
cls.r1 = Responsibility.objects.create(description='Playing golf') cls.r1 = Responsibility.objects.create(description='Playing golf')
j2 = Job.objects.create(name='Programmer') j2 = Job.objects.create(name='Programmer')
r2 = Responsibility.objects.create(description='Programming') r2 = Responsibility.objects.create(description='Programming')
JobResponsibilities.objects.create(job=j1, responsibility=cls.r1) JobResponsibilities.objects.create(job=cls.j1, responsibility=cls.r1)
JobResponsibilities.objects.create(job=j2, responsibility=r2) JobResponsibilities.objects.create(job=j2, responsibility=r2)
def test_to_field(self): def test_to_field(self):
@ -2884,6 +2884,14 @@ class ExcludeTests(TestCase):
[number], [number],
) )
def test_exclude_multivalued_exists(self):
with CaptureQueriesContext(connection) as captured_queries:
self.assertSequenceEqual(
Job.objects.exclude(responsibilities__description='Programming'),
[self.j1],
)
self.assertIn('exists', captured_queries[0]['sql'].lower())
class ExcludeTest17600(TestCase): class ExcludeTest17600(TestCase):
""" """