Opened 3 months ago

Closed 3 months ago

Last modified 3 months ago

#33705 closed Bug (fixed)

IsNull() lookup cannot be used directly in filters.

Reported by: Florian Apolloner Owned by: David Wobrock
Component: Database layer (models, ORM) Version: 4.0
Severity: Release blocker Keywords:
Cc: Florian Apolloner, David Wobrock 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

The docs in https://docs.djangoproject.com/en/4.0/ref/models/lookups/#django.db.models.Lookup seem to suggest that

<lhs>__<lookup_name>=<rhs>

is somewhat equivalent to

<lookup_name>(<lhs>, <rhs>)

This does work for some lookups, but not for IsNull:

>>> from django.contrib.auth.models import User
>>> from django.db.models import F
>>> from django.db.models.lookups import GreaterThan, IsNull
>>> User.objects.filter(GreaterThan(F('pk'), 1000))
DEBUG 2022-05-13 10:45:10,907 utils 63853 (0.006) SELECT "auth_user"."id", "auth_user"."password", "auth_user"."last_login", "auth_user"."is_superuser", "auth_user"."username", "auth_user"."first_name", "auth_user"."last_name", "auth_user"."email", "auth_user"."is_staff", "auth_user"."is_active", "auth_user"."date_joined" FROM "auth_user" WHERE "auth_user"."id" > (1000) LIMIT 21; args=(1000,); alias=default
<QuerySet []>
>>> User.objects.filter(IsNull(F('pk'), True))
Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "/home/florian/dev/bap/observer/server/__pypackages__/3.10/lib/django/db/models/manager.py", line 85, in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
  File "/home/florian/dev/bap/observer/server/__pypackages__/3.10/lib/django/db/models/query.py", line 1071, in filter
    return self._filter_or_exclude(False, args, kwargs)
  File "/home/florian/dev/bap/observer/server/__pypackages__/3.10/lib/django/db/models/query.py", line 1089, in _filter_or_exclude
    clone._filter_or_exclude_inplace(negate, args, kwargs)
  File "/home/florian/dev/bap/observer/server/__pypackages__/3.10/lib/django/db/models/query.py", line 1096, in _filter_or_exclude_inplace
    self._query.add_q(Q(*args, **kwargs))
  File "/home/florian/dev/bap/observer/server/__pypackages__/3.10/lib/django/db/models/sql/query.py", line 1502, in add_q
    clause, _ = self._add_q(q_object, self.used_aliases)
  File "/home/florian/dev/bap/observer/server/__pypackages__/3.10/lib/django/db/models/sql/query.py", line 1532, in _add_q
    child_clause, needed_inner = self.build_filter(
  File "/home/florian/dev/bap/observer/server/__pypackages__/3.10/lib/django/db/models/sql/query.py", line 1370, in build_filter
    condition = filter_expr.resolve_expression(self, allow_joins=allow_joins)
  File "/home/florian/dev/bap/observer/server/__pypackages__/3.10/lib/django/db/models/lookups.py", line 174, in resolve_expression
    c.rhs = self.rhs.resolve_expression(
AttributeError: 'bool' object has no attribute 'resolve_expression'

Change History (9)

comment:1 Changed 3 months ago by Mariusz Felisiak

Summary: IsNull-Lookup not usableIsNull() lookup cannot be used directly in filters.
Triage Stage: UnreviewedAccepted

comment:2 Changed 3 months ago by David Wobrock

Cc: David Wobrock added
Has patch: set
Patch needs improvement: set

Here is a draft PR with failing tests.
Just a quick shot, trying to fix the issue with the approach of letting the IsNull handle Value objects internally instead of bools. But that might have quite a large impact for such a small change.

comment:3 Changed 3 months ago by David Wobrock

Owner: changed from nobody to David Wobrock
Patch needs improvement: unset
Status: newassigned

I used the same PR but changed the approach a bit.
It's the most straightforward approach I could think of. Let me know what you think :)

comment:4 Changed 3 months ago by Simon Charette

Shouldn't this be considered a release blocker since it's a bug in a newly released feature in Django 4.0?

comment:5 in reply to:  4 Changed 3 months ago by Mariusz Felisiak

Severity: NormalRelease blocker
Version: dev4.0

Replying to Simon Charette:

Shouldn't this be considered a release blocker since it's a bug in a newly released feature in Django 4.0?

Good catch! I missed that it's a new feature.

comment:6 Changed 3 months ago by Mariusz Felisiak

Triage Stage: AcceptedReady for checkin

comment:7 Changed 3 months ago by Mariusz Felisiak <felisiak.mariusz@…>

Resolution: fixed
Status: assignedclosed

In 9f554895:

Fixed #33705 -- Fixed crash when using IsNull() lookup in filters.

Thanks Florian Apolloner for the report.
Thanks Simon Charette for the review.

comment:8 Changed 3 months ago by Mariusz Felisiak <felisiak.mariusz@…>

In 820b4e56:

[4.1.x] Fixed #33705 -- Fixed crash when using IsNull() lookup in filters.

Thanks Florian Apolloner for the report.
Thanks Simon Charette for the review.

Backport of 9f5548952906c6ea97200c016734b4f519520a64 from main

comment:9 Changed 3 months ago by Mariusz Felisiak <felisiak.mariusz@…>

In 4a86883e:

[4.0.x] Fixed #33705 -- Fixed crash when using IsNull() lookup in filters.

Thanks Florian Apolloner for the report.
Thanks Simon Charette for the review.

Backport of 9f5548952906c6ea97200c016734b4f519520a64 from main

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