Opened 3 years ago

Closed 14 months ago

Last modified 14 months ago

#33482 closed Bug (fixed)

filter on exists-subquery with empty queryset removes whole WHERE block

Reported by: Tobias Bengfort Owned by: Simon Charette
Component: Database layer (models, ORM) Version: 4.0
Severity: Normal Keywords: orm, EmptyResultSet, Exists
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Tobias Bengfort)

>>> qs = MyModel.objects.filter(~models.Exists(MyModel.objects.none()), name='test')
>>> qs
<QuerySet []>
>>> print(qs.query)
EmptyResultSet

With django-debug-toolbar I can still see the query, but there WHERE block is missing completely.

This seems to be very similar to #33018.

Change History (14)

comment:1 by Tobias Bengfort, 3 years ago

Description: modified (diff)

comment:2 by Tobias Bengfort, 3 years ago

Description: modified (diff)

comment:3 by Tobias Bengfort, 3 years ago

Summary: filter on exists-subquery with emoty queryset removes whole WHERE blockfilter on exists-subquery with empty queryset removes whole WHERE block

comment:4 by Simon Charette, 3 years ago

Owner: changed from nobody to Simon Charette
Status: newassigned
Triage Stage: UnreviewedAccepted
Type: UncategorizedBug

I think that this is an issue with Exists.as_sql when self.invert is True.

Since Exists encapsulate its negation logic (see __invert__) it should catch EmptyResultSet when raised by its super() call in as_sql and return an always true predicate (e.g. 1=1).

Does the following patch address your issue?

  • django/db/models/expressions.py

    diff --git a/django/db/models/expressions.py b/django/db/models/expressions.py
    index 81f8f79c71..7ec5dad57e 100644
    a b def __invert__(self):  
    12111211
    12121212    def as_sql(self, compiler, connection, template=None, **extra_context):
    12131213        query = self.query.exists(using=connection.alias)
    1214         sql, params = super().as_sql(
    1215             compiler,
    1216             connection,
    1217             template=template,
    1218             query=query,
    1219             **extra_context,
    1220         )
     1214        try:
     1215            sql, params = super().as_sql(
     1216                compiler,
     1217                connection,
     1218                template=template,
     1219                query=query,
     1220                **extra_context,
     1221            )
     1222        except EmptyResultSet:
     1223            if self.negated:
     1224                return '%s = %s', (1, 1)
     1225            raise
    12211226        if self.negated:
    12221227            sql = 'NOT {}'.format(sql)
    12231228        return sql, params
  • tests/expressions/tests.py

    diff --git a/tests/expressions/tests.py b/tests/expressions/tests.py
    index 5cf9dd1ea5..5d902c86e8 100644
    a b def test_optimizations(self):  
    19051905        )
    19061906        self.assertNotIn('ORDER BY', captured_sql)
    19071907
     1908    def test_negated_empty_exists(self):
     1909        manager = Manager.objects.create()
     1910        qs = Manager.objects.filter(
     1911            ~Exists(Manager.objects.none()), pk=manager.pk
     1912        )
     1913        self.assertQuerysetEqual(qs, Manager.objects.filter(pk=manager.pk))
     1914
    19081915
    19091916class FieldTransformTests(TestCase):

comment:5 by Simon Charette, 3 years ago

Has patch: set

comment:6 by Mariusz Felisiak, 3 years ago

Triage Stage: AcceptedReady for checkin

comment:7 by Mariusz Felisiak <felisiak.mariusz@…>, 3 years ago

Resolution: fixed
Status: assignedclosed

In b7d1da5a:

Fixed #33482 -- Fixed QuerySet filtering againts negated Exists() with empty queryset.

Thanks Tobias Bengfort for the report.

comment:8 by GitHub <noreply@…>, 3 years ago

In 6f185a5:

Refs #33482 -- Fixed QuerySet selecting and filtering againts negated Exists() with empty queryset.

Regression in b7d1da5a62fe4141beff2bfea565f7ef0038c94c.

comment:9 by Tobias Bengfort, 14 months ago

Resolution: fixed
Status: closednew

I still get this error in 4.2 if I include the empty exists expression in the filter:

def test_filter_by_empty_exists(self):
    manager = Manager.objects.create()
    qs = Manager.objects.annotate(
        exists=Exists(Manager.objects.none())
    ).filter(pk=manager.pk, exists=False)
    self.assertSequenceEqual(qs, [manager])
    self.assertIs(qs.get().exists, False)

comment:10 by Simon Charette, 14 months ago

The fact the problem manifests itself when the queryset is not negated was missed during the initial patch.

Tobias, can you confirm the following PR addresses the issue.

It seemed worth continuing the discussion here instead of creating a separate ticket.

comment:11 by Tobias Bengfort, 14 months ago

Thanks for the quick response! It looks good. It fixed the bug in my application.

comment:12 by GitHub <noreply@…>, 14 months ago

In ea596a52:

Refs #33482 -- Fixed QuerySet selecting and filtering againts Exists() with empty queryset.

Thanks Tobias Bengfort for the report.

comment:13 by Mariusz Felisiak, 14 months ago

Resolution: fixed
Status: newclosed

comment:14 by Mariusz Felisiak <felisiak.mariusz@…>, 14 months ago

In 458bc9e:

[5.0.x] Refs #33482 -- Fixed QuerySet selecting and filtering againts Exists() with empty queryset.

Thanks Tobias Bengfort for the report.
Backport of ea596a52d9f905596cc5335930c8f2ac4511204c from main

Note: See TracTickets for help on using tickets.
Back to Top