[Django] #32573: Query optimization in YearLookup breaks filtering by "__iso_year"

5 views
Skip to first unread message

Django

unread,
Mar 19, 2021, 2:22:20 PM3/19/21
to django-...@googlegroups.com
#32573: Query optimization in YearLookup breaks filtering by "__iso_year"
-------------------------------------+-------------------------------------
Reporter: Florian | Owner: nobody
Demmer |
Type: Bug | Status: new
Component: Database | Version: 2.2
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 |
-------------------------------------+-------------------------------------
The optimization to use BETWEEN instead of the EXTRACT operation in
[https://github.com/django/django/blob/main/django/db/models/lookups.py#L540
YearLookup] is also registered for the
[https://github.com/django/django/blob/main/django/db/models/functions/datetime.py#L167
"__iso_year" lookup], which breaks the functionality provided by
[https://docs.djangoproject.com/en/2.2/ref/models/database-
functions/#django.db.models.functions.ExtractIsoYear ExtractIsoYear] when
used via the lookup.

This has unfortunately been broken ever since ExtractIsoYear was
introduced in Django 2.2 via #28649 and wasn't easy to track down since
ExtractIsoYear when used by itself eg. in an annotation works perfectly
fine. Just when using the lookup in a filter, the optimization is used
(even when explicitly using an annotation):

{{{
#!python
# annotation works
>>> qs =
DTModel.objects.annotate(extracted=ExtractIsoYear('start_date')).only('id')
>>> print(qs.query)
SELECT "db_functions_dtmodel"."id", EXTRACT('isoyear' FROM
"db_functions_dtmodel"."start_date") AS "extracted" FROM
"db_functions_dtmodel"

# explicit annotation used in filter does not use "extracted" and adds
BETWEEN
>>> print(qs.filter(extracted=2020).query)
SELECT "db_functions_dtmodel"."id", EXTRACT('isoyear' FROM
"db_functions_dtmodel"."start_date") AS "extracted" FROM
"db_functions_dtmodel" WHERE "db_functions_dtmodel"."start_date" BETWEEN
2020-01-01 AND 2020-12-31

# implicit lookup uses BETWEEN
print(DTModel.objects.filter(start_date__iso_year=2020).only('id').query)
SELECT "db_functions_dtmodel"."id" FROM "db_functions_dtmodel" WHERE
"db_functions_dtmodel"."start_date" BETWEEN 2020-01-01 AND 2020-12-31
}}}

This results in the wrong data being returned by filters using iso_year.

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

Django

unread,
Mar 19, 2021, 2:25:07 PM3/19/21
to django-...@googlegroups.com
#32573: Query optimization in YearLookup breaks filtering by "__iso_year"
-------------------------------------+-------------------------------------
Reporter: Florian Demmer | Owner: Florian
| Demmer
Type: Bug | Status: assigned
Component: Database layer | Version: 2.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0

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

* owner: nobody => Florian Demmer
* status: new => assigned
* has_patch: 0 => 1


Old description:

New description:

This PR fixes the behaviour, reverts the invalid changes to the tests and
extends one test to catch this problem:
https://github.com/django/django/pull/14157

--

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

Django

unread,
Mar 19, 2021, 2:30:27 PM3/19/21
to django-...@googlegroups.com
#32573: Query optimization in YearLookup breaks filtering by "__iso_year"
-------------------------------------+-------------------------------------
Reporter: Florian Demmer | Owner: Florian
| Demmer
Type: Bug | Status: assigned
Component: Database layer | Version: 2.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Description changed by Florian Demmer:

Old description:

> This PR fixes the behaviour, reverts the invalid changes to the tests and
> extends one test to catch this problem:
> https://github.com/django/django/pull/14157

New description:

The optimization to use BETWEEN instead of the EXTRACT operation in
[https://github.com/django/django/blob/main/django/db/models/lookups.py#L540
YearLookup] is also registered for the
[https://github.com/django/django/blob/main/django/db/models/functions/datetime.py#L167
"__iso_year" lookup], which breaks the functionality provided by
[https://docs.djangoproject.com/en/2.2/ref/models/database-
functions/#django.db.models.functions.ExtractIsoYear ExtractIsoYear] when
used via the lookup.

This has unfortunately been broken ever since ExtractIsoYear was

introduced in [https://docs.djangoproject.com/en/2.2/ref/models/querysets
/#iso-year Django 2.2] via #28649 and wasn't easy to track down since


ExtractIsoYear when used by itself eg. in an annotation works perfectly
fine. Just when using the lookup in a filter, the optimization is used
(even when explicitly using an annotation):

{{{
#!python
# annotation works
>>> qs =
DTModel.objects.annotate(extracted=ExtractIsoYear('start_date')).only('id')
>>> print(qs.query)
SELECT "db_functions_dtmodel"."id", EXTRACT('isoyear' FROM
"db_functions_dtmodel"."start_date") AS "extracted" FROM
"db_functions_dtmodel"

# explicit annotation used in filter does not use "extracted" and adds
BETWEEN
>>> print(qs.filter(extracted=2020).query)
SELECT "db_functions_dtmodel"."id", EXTRACT('isoyear' FROM
"db_functions_dtmodel"."start_date") AS "extracted" FROM
"db_functions_dtmodel" WHERE "db_functions_dtmodel"."start_date" BETWEEN
2020-01-01 AND 2020-12-31

# implicit lookup uses BETWEEN
>>>
print(DTModel.objects.filter(start_date__iso_year=2020).only('id').query)
SELECT "db_functions_dtmodel"."id" FROM "db_functions_dtmodel" WHERE
"db_functions_dtmodel"."start_date" BETWEEN 2020-01-01 AND 2020-12-31
}}}

This results in the wrong data being returned by filters using iso_year.

This PR fixes the behaviour, reverts the invalid changes to the tests and


extends one test to catch this problem:
https://github.com/django/django/pull/14157

--

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

Django

unread,
Mar 22, 2021, 4:28:08 AM3/22/21
to django-...@googlegroups.com
#32573: Query optimization in YearLookup breaks filtering by "__iso_year"
-------------------------------------+-------------------------------------
Reporter: Florian Demmer | Owner: Florian
| Demmer
Type: Bug | Status: assigned
Component: Database layer | Version: 2.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak):

* needs_better_patch: 0 => 1
* stage: Unreviewed => Accepted


Comment:

Thanks for the report. I think we can preserve an optimization but for
different boundaries. We can use
[https://docs.python.org/3/library/datetime.html#datetime.date.fromisocalendar
.fromisocalendar()] (available in Python 3.8+).

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

Django

unread,
Mar 22, 2021, 4:35:54 AM3/22/21
to django-...@googlegroups.com
#32573: Query optimization in YearLookup breaks filtering by "__iso_year"
-------------------------------------+-------------------------------------
Reporter: Florian Demmer | Owner: Florian
| Demmer
Type: Bug | Status: assigned
Component: Database layer | Version: 2.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak):

* cc: Sigurd Ljødal (added)


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

Django

unread,
Mar 23, 2021, 5:47:33 AM3/23/21
to django-...@googlegroups.com
#32573: Query optimization in YearLookup breaks filtering by "__iso_year"
-------------------------------------+-------------------------------------
Reporter: Florian Demmer | Owner: Florian
| Demmer
Type: Bug | Status: assigned
Component: Database layer | Version: 2.2
(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 Mariusz Felisiak):

* needs_better_patch: 1 => 0


Comment:

[https://github.com/django/django/pull/14170 New PR]

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

Django

unread,
Mar 23, 2021, 4:29:11 PM3/23/21
to django-...@googlegroups.com
#32573: Query optimization in YearLookup breaks filtering by "__iso_year"
-------------------------------------+-------------------------------------
Reporter: Florian Demmer | Owner: Florian
| Demmer
Type: Bug | Status: assigned
Component: Database layer | Version: 2.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0

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

* stage: Accepted => Ready for checkin


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

Django

unread,
Mar 24, 2021, 2:23:55 AM3/24/21
to django-...@googlegroups.com
#32573: Query optimization in YearLookup breaks filtering by "__iso_year"
-------------------------------------+-------------------------------------
Reporter: Florian Demmer | Owner: Florian
| Demmer
Type: Bug | Status: closed

Component: Database layer | Version: 2.2
(models, ORM) |
Severity: Normal | Resolution: fixed

Keywords: | Triage Stage: Ready for
| checkin
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:"3a185cee2af919cd0e897d884eaafe421cd3ba48" 3a185cee]:
{{{
#!CommitTicketReference repository=""
revision="3a185cee2af919cd0e897d884eaafe421cd3ba48"
Fixed #32573 -- Fixed bounds in __iso_year lookup optimization.
}}}

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

Reply all
Reply to author
Forward
0 new messages