[Django] #31667: Avoid passing NULL to prefetch_related() queries

55 views
Skip to first unread message

Django

unread,
Jun 5, 2020, 12:07:11 PM6/5/20
to django-...@googlegroups.com
#31667: Avoid passing NULL to prefetch_related() queries
-------------------------------------+-------------------------------------
Reporter: Adam | Owner: nobody
(Chainz) Johnson |
Type: | Status: new
Cleanup/optimization |
Component: Database | Version: master
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
Currently prefetch_related on a FK passes the NULL through to the database
for e.g. `author_id IN (NULL, 2)`. Passing `NULL` is always unnecessary,
since it's not allowed in FK's. There's a small risk from passing NULL
that it could lead to incorrect with complex prefetch querysets using PK
refs because of NULL's weirdness in SQL.

For example with these models:

{{{
from django.db import models


class Author(models.Model):
pass


class Book(models.Model):
author = models.ForeignKey(Author, null=True,
on_delete=models.DO_NOTHING)
}}}

Prefetching authors on Books, when at least one Book has author=None, uses
`IN (..., NULL, ...)` in the query:

{{{
In [1]: from example.core.models import Author, Book

In [2]: a1 = Author.objects.create()

In [3]: Book.objects.create(author=a1)
Out[3]: <Book: Book object (3)>

In [4]: Book.objects.create(author=None)
Out[4]: <Book: Book object (4)>

In [5]: Book.objects.prefetch_related('author')
Out[5]: <QuerySet [<Book: Book object (3)>, <Book: Book object (4)>]>

In [6]: from django.db import connection

In [7]: connection.queries
Out[7]:
[{'sql': 'INSERT INTO "core_author" ("id") VALUES (NULL)', 'time':
'0.001'},
{'sql': 'INSERT INTO "core_book" ("author_id") VALUES (2)', 'time':
'0.001'},
{'sql': 'INSERT INTO "core_book" ("author_id") VALUES (NULL)',
'time': '0.001'},
{'sql': 'SELECT "core_book"."id", "core_book"."author_id" FROM
"core_book" LIMIT 21',
'time': '0.000'},
{'sql': 'SELECT "core_author"."id" FROM "core_author" WHERE
"core_author"."id" IN (NULL, 2)',
'time': '0.000'}]
}}}

Maybe this could generally be extended to use of `__in` with non-nullable
fields?

--
Ticket URL: <https://code.djangoproject.com/ticket/31667>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Jun 5, 2020, 12:54:18 PM6/5/20
to django-...@googlegroups.com
#31667: Avoid passing NULL to prefetch_related() queries
-------------------------------------+-------------------------------------
Reporter: Adam (Chainz) | Owner: nobody

Johnson |
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Simon Charette):

* stage: Unreviewed => Accepted


Comment:

> Maybe this could generally be extended to use of `__in` with non-
nullable fields?

Since `IN` translates to `OR =` for each elements `NULL != NULL` I assume
it could be done at the `__in` lookup level even for non-nullable fields.

--
Ticket URL: <https://code.djangoproject.com/ticket/31667#comment:1>

Django

unread,
Jun 10, 2020, 4:19:17 AM6/10/20
to django-...@googlegroups.com
#31667: Avoid passing NULL to prefetch_related() queries
-------------------------------------+-------------------------------------
Reporter: Adam (Chainz) | Owner: Adam
Johnson | (Chainz) Johnson
Type: | Status: assigned

Cleanup/optimization |
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by felixxm):

* owner: nobody => Adam (Chainz) Johnson
* status: new => assigned
* has_patch: 0 => 1


Comment:

[https://github.com/django/django/pull/13030 PR]

--
Ticket URL: <https://code.djangoproject.com/ticket/31667#comment:2>

Django

unread,
Jun 10, 2020, 2:26:29 PM6/10/20
to django-...@googlegroups.com
#31667: Avoid passing NULL to prefetch_related() queries
-------------------------------------+-------------------------------------
Reporter: Adam (Chainz) | Owner: Adam
Johnson | (Chainz) Johnson
Type: | Status: assigned
Cleanup/optimization |
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Adam (Chainz) Johnson):

Apologies for not doing my paperwork.

--
Ticket URL: <https://code.djangoproject.com/ticket/31667#comment:3>

Django

unread,
Jun 11, 2020, 2:47:21 AM6/11/20
to django-...@googlegroups.com
#31667: Avoid passing NULL to the IN lookup

-------------------------------------+-------------------------------------
Reporter: Adam (Chainz) | Owner: Adam
Johnson | (Chainz) Johnson
Type: | Status: assigned
Cleanup/optimization |
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

--
Ticket URL: <https://code.djangoproject.com/ticket/31667#comment:4>

Django

unread,
Jun 11, 2020, 6:42:12 AM6/11/20
to django-...@googlegroups.com
#31667: Avoid passing NULL to the IN lookup
-------------------------------------+-------------------------------------
Reporter: Adam (Chainz) | Owner: Adam
Johnson | (Chainz) Johnson
Type: | Status: closed

Cleanup/optimization |
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak <felisiak.mariusz@…>):

* status: assigned => closed
* resolution: => fixed


Comment:

In [changeset:"5776a1660e54a95159164414829738b665c89916" 5776a166]:
{{{
#!CommitTicketReference repository=""
revision="5776a1660e54a95159164414829738b665c89916"
Fixed #31667 -- Made __in lookup ignore None values.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/31667#comment:5>

Django

unread,
Aug 15, 2020, 10:57:43 AM8/15/20
to django-...@googlegroups.com
#31667: Avoid passing NULL to the IN lookup
-------------------------------------+-------------------------------------
Reporter: Adam (Chainz) | Owner: Adam
Johnson | (Chainz) Johnson
Type: | Status: closed
Cleanup/optimization |
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Adam Sołtysik):

This has partially fixed #20024.

--
Ticket URL: <https://code.djangoproject.com/ticket/31667#comment:6>

Django

unread,
Aug 15, 2020, 12:29:03 PM8/15/20
to django-...@googlegroups.com
#31667: Avoid passing NULL to the IN lookup
-------------------------------------+-------------------------------------
Reporter: Adam (Chainz) | Owner: Adam
Johnson | (Chainz) Johnson
Type: | Status: closed
Cleanup/optimization |
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Simon Charette):

As Adam Sołtysik pointed out in #20024.this change also introduced an
inconsistency


{{{#!python
Foo.objects.filter(bar__in=list(Bar.objects.values_list('nullable_field',
flat=True)))
}}}

Won't match

{{{#!python
Foo.objects.filter(bar__in=Bar.objects.values_list('nullable_field',
flat=True))
}}}

If any `NULL` values are returned.

We should make sure that a complete corrective for #20024 lands in 3.2
otherwise 5776a1660e54a95159164414829738b665c89916 should be reverted.

--
Ticket URL: <https://code.djangoproject.com/ticket/31667#comment:7>

Django

unread,
Aug 16, 2020, 12:17:16 PM8/16/20
to django-...@googlegroups.com
#31667: Avoid passing NULL to the IN lookup
-------------------------------------+-------------------------------------
Reporter: Adam (Chainz) | Owner: Adam
Johnson | (Chainz) Johnson
Type: | Status: closed
Cleanup/optimization |
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Adam Sołtysik):

There is also an inconsistency between `In` and `RelatedIn` lookups. The
latter seems to be working consistenly with this change (details in
#20024).

--
Ticket URL: <https://code.djangoproject.com/ticket/31667#comment:8>

Django

unread,
Apr 3, 2025, 4:28:37 PM4/3/25
to django-...@googlegroups.com
#31667: Avoid passing NULL to the IN lookup
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Adam
Type: | Johnson
Cleanup/optimization | Status: closed
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):

In [changeset:"f7f38f3a0b44d8c6d14344dae66b6ce52cd77b55" f7f38f3]:
{{{#!CommitTicketReference repository=""
revision="f7f38f3a0b44d8c6d14344dae66b6ce52cd77b55"
Fixed #36290 -- Made TupleIn() lookup discard tuples containing None.

Just like the In() lookup discards of None members TupleIn() should
discard tuples containing any None as NULL != NULL in SQL and the
framework expects such queries to be elided under some circumstances.

Refs #31667, #36116.

Thanks Basptise Mispelon for bisecting the regression to 626d77e.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/31667#comment:9>

Django

unread,
Apr 3, 2025, 4:28:56 PM4/3/25
to django-...@googlegroups.com
#31667: Avoid passing NULL to the IN lookup
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Adam
Type: | Johnson
Cleanup/optimization | Status: closed
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):

In [changeset:"8ebdd37a0b1755842baae3bd34d388156ad4bf53" 8ebdd37a]:
{{{#!CommitTicketReference repository=""
revision="8ebdd37a0b1755842baae3bd34d388156ad4bf53"
[5.2.x] Fixed #36290 -- Made TupleIn() lookup discard tuples containing
None.

Just like the In() lookup discards of None members TupleIn() should
discard tuples containing any None as NULL != NULL in SQL and the
framework expects such queries to be elided under some circumstances.

Refs #31667, #36116.

Thanks Basptise Mispelon for bisecting the regression to 626d77e.

Backport of f7f38f3a0b44d8c6d14344dae66b6ce52cd77b55 from main
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/31667#comment:10>
Reply all
Reply to author
Forward
0 new messages