Opened 13 months ago

Closed 13 months ago

Last modified 13 months ago

#34889 closed Bug (fixed)

Broken fallback for prefetchers that only implement get_prefetch_queryset

Reported by: Matt Westcott Owned by: Mariusz Felisiak
Component: Database layer (models, ORM) Version: 5.0
Severity: Release blocker Keywords:
Cc: Clément Escolano Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

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:

https://github.com/django/django/blob/4e790271e3e65c9ad037b347a34fa95e11982228/django/db/models/query.py#L2535-L2562

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

Change History (5)

comment:1 by Mariusz Felisiak, 13 months ago

Cc: Clément Escolano added
Triage Stage: UnreviewedAccepted

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.

comment:2 by Mariusz Felisiak, 13 months ago

Owner: changed from nobody to Mariusz Felisiak
Status: newassigned

comment:3 by Mariusz Felisiak, 13 months ago

Has patch: set

comment:4 by GitHub <noreply@…>, 13 months ago

Resolution: fixed
Status: assignedclosed

In 296b75a:

Fixed #34889 -- Fixed get_prefetch_queryset() fallback in prefetch_one_level().

Thanks Matt Westcott for the report.

Regression in cac94dd8aa2fb49cd2e06b5b37cf039257284bb0.

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

In 9f8bf7ae:

[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

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