[Django] #34889: Broken fallback for prefetchers that only implement get_prefetch_queryset
Django
-------------------------------------+-------------------------------------
Reporter: Matt | Owner: nobody
Westcott |
Type: Bug | Status: new
Component: Database | Version: 5.0
layer (models, ORM) |
Severity: Release | Keywords:
blocker |
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
In #33651, the `get_prefetch_queryset()` method of related managers and
descriptors is deprecated in favour of `get_prefetch_querysets()`. Based
on the deprecation warnings, it's clear that subclasses implementing only
`get_prefetch_queryset()` are intended to continue working until this is
removed outright in 6.0. However, the fallback code in
`prefetch_one_level` calls it with an invalid signature:
Here the `get_prefetch_queryset` method is passed
`lookup.get_current_querysets(level)` - which returns a list of querysets
- as the `queryset` argument, which expects a single queryset. Changing
this to `lookup.get_current_queryset(level)` fixes the problem (although
it also doubles up the deprecation warnings, which may not be ideal).
Unfortunately I'm slightly stumped over how I might create a regression
test for this - presumably this would involve writing a minimal related
manager class that implements `get_current_queryset` but not
`get_current_querysets`, but since this is an internal API it's hard to
know what's considered minimal while still being a legal implementation.
If it's any help, I encountered this as a regression to the `django-
modelcluster` package: https://github.com/wagtail/django-
modelcluster/actions/runs/6407673311/job/17394971115
--
Ticket URL: <https://code.djangoproject.com/ticket/34889>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 5.0
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: Clément Escolano (added)
* stage: Unreviewed => Accepted
Comment:
Great catch! I think we should still call `get_current_querysets()`, but
pass only the first one when a return value is not `None`.
Regression in cac94dd8aa2fb49cd2e06b5b37cf039257284bb0.
--
Ticket URL: <https://code.djangoproject.com/ticket/34889#comment:1>
Django
-------------------------------------+-------------------------------------
| Felisiak
Type: Bug | Status: assigned
Component: Database layer | Version: 5.0
(models, ORM) |
Severity: Release blocker | 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 Mariusz Felisiak):
* owner: nobody => Mariusz Felisiak
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/34889#comment:2>
Django
-------------------------------------+-------------------------------------
Reporter: Matt Westcott | Owner: Mariusz
| Felisiak
Type: Bug | Status: assigned
Component: Database layer | Version: 5.0
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak):
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/17343 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/34889#comment:3>
Django
-------------------------------------+-------------------------------------
Reporter: Matt Westcott | Owner: Mariusz
| Felisiak
Component: Database layer | Version: 5.0
(models, ORM) |
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"296b75a3c0309a936a6c07d8f711f722e3b96e63" 296b75a]:
{{{
#!CommitTicketReference repository=""
revision="296b75a3c0309a936a6c07d8f711f722e3b96e63"
Fixed #34889 -- Fixed get_prefetch_queryset() fallback in
prefetch_one_level().
Thanks Matt Westcott for the report.
Regression in cac94dd8aa2fb49cd2e06b5b37cf039257284bb0.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34889#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: Matt Westcott | Owner: Mariusz
| Felisiak
Type: Bug | Status: closed
Component: Database layer | Version: 5.0
(models, ORM) |
Severity: Release blocker | 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:"9f8bf7aebe1d389aa06f41bcaccf5c4418c71026" 9f8bf7ae]:
{{{
#!CommitTicketReference repository=""
revision="9f8bf7aebe1d389aa06f41bcaccf5c4418c71026"
[5.0.x] Fixed #34889 -- Fixed get_prefetch_queryset() fallback in
prefetch_one_level().
Thanks Matt Westcott for the report.
Regression in cac94dd8aa2fb49cd2e06b5b37cf039257284bb0.
Backport of 296b75a3c0309a936a6c07d8f711f722e3b96e63 from main
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34889#comment:5>