Opened 6 weeks ago

Closed 6 weeks ago

Last modified 6 weeks ago

#36432 closed Bug (fixed)

Regression in Prefetch and multi-table inherited models since 5.2.0

Reported by: Cornelis Poppema Owned by: Simon Charette
Component: Database layer (models, ORM) Version: 5.2
Severity: Release blocker Keywords: prefetch inheritance
Cc: 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 (last modified by Cornelis Poppema)

We've started working our way through the next LTS upgrade to 5.2 coming from 4.2, and I've run into the following problem.

We have certain tenants with different attributes and spread these tenant models across tables. These tenants exist in a parent-children tree. In certain views we know what kind of data to expect, so we use Prefetch to enable querying a specific inherited model to basically "promote" an "owner" ForeignKey from its base model to a sub model to get immediate access to its fields (and custom properties) instead of having to traverse the subclass structure.

I've created an MRE here: https://github.com/cpoppema/django52-prefetch-related
This repository runs the testcase in django 4.2 through 5.2 and only fails in 5.2.

models.py:

from django.db import models


class Tenant(models.Model):
    name = models.CharField(max_length=30)
    owner = models.ForeignKey(
        to="self",
        null=True,
        blank=True,
        on_delete=models.CASCADE,
    )


class Partner(Tenant):
    partner_field = models.CharField(max_length=30)


class Client(Tenant):
    client_field = models.CharField(max_length=30)

the query that fails - demonstrated in the tests, which you can run after installing django with python manage.py test or if you install tox, just run tox:

qs = Client.objects.all()

prefetch_owner_as_partners = Prefetch("owner", Partner.objects.all())
qs = qs.prefetch_related(prefetch_owner_as_partners)

len(qs)

In Django 4.2 (or pre-5.2) this yields a sql statement containing: WHERE "tenant_partner"."tenant_ptr_id" IN (?); where in Django 5.2 it attempts to use id instead of tenant_ptr_id, resulting in the following stacktrace:

Traceback (most recent call last):
  File "/local/sandbox/django52-prefetch-related/myproject/tenant/tests.py", line 83, in test_client_owner_with_prefetch
    for item in qs:
  File "/local/sandbox/django52-prefetch-related/.tox/py312-django52/lib/python3.12/site-packages/django/db/models/query.py", line 384, in __iter__
    self._fetch_all()
  File "/local/sandbox/django52-prefetch-related/.tox/py312-django52/lib/python3.12/site-packages/django/db/models/query.py", line 1947, in _fetch_all
    self._prefetch_related_objects()
  File "/local/sandbox/django52-prefetch-related/.tox/py312-django52/lib/python3.12/site-packages/django/db/models/query.py", line 1326, in _prefetch_related_objects
    prefetch_related_objects(self._result_cache, *self._prefetch_related_lookups)
  File "/local/sandbox/django52-prefetch-related/.tox/py312-django52/lib/python3.12/site-packages/django/db/models/query.py", line 2393, in prefetch_related_objects
    obj_list, additional_lookups = prefetch_one_level(
                                   ^^^^^^^^^^^^^^^^^^^
  File "/local/sandbox/django52-prefetch-related/.tox/py312-django52/lib/python3.12/site-packages/django/db/models/query.py", line 2594, in prefetch_one_level
    all_related_objects = list(rel_qs)
                          ^^^^^^^^^^^^
  File "/local/sandbox/django52-prefetch-related/.tox/py312-django52/lib/python3.12/site-packages/django/db/models/query.py", line 384, in __iter__
    self._fetch_all()
  File "/local/sandbox/django52-prefetch-related/.tox/py312-django52/lib/python3.12/site-packages/django/db/models/query.py", line 1945, in _fetch_all
    self._result_cache = list(self._iterable_class(self))
                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/local/sandbox/django52-prefetch-related/.tox/py312-django52/lib/python3.12/site-packages/django/db/models/query.py", line 91, in __iter__
    results = compiler.execute_sql(
              ^^^^^^^^^^^^^^^^^^^^^
  File "/local/sandbox/django52-prefetch-related/.tox/py312-django52/lib/python3.12/site-packages/django/db/models/sql/compiler.py", line 1623, in execute_sql
    cursor.execute(sql, params)
  File "/local/sandbox/django52-prefetch-related/.tox/py312-django52/lib/python3.12/site-packages/django/db/backends/utils.py", line 122, in execute
    return super().execute(sql, params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/local/sandbox/django52-prefetch-related/.tox/py312-django52/lib/python3.12/site-packages/django/db/backends/utils.py", line 79, in execute
    return self._execute_with_wrappers(
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/local/sandbox/django52-prefetch-related/.tox/py312-django52/lib/python3.12/site-packages/django/db/backends/utils.py", line 92, in _execute_with_wrappers
    return executor(sql, params, many, context)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/local/sandbox/django52-prefetch-related/.tox/py312-django52/lib/python3.12/site-packages/django/db/backends/utils.py", line 100, in _execute
    with self.db.wrap_database_errors:
  File "/local/sandbox/django52-prefetch-related/.tox/py312-django52/lib/python3.12/site-packages/django/db/utils.py", line 91, in __exit__
    raise dj_exc_value.with_traceback(traceback) from exc_value
  File "/local/sandbox/django52-prefetch-related/.tox/py312-django52/lib/python3.12/site-packages/django/db/backends/utils.py", line 105, in _execute
    return self.cursor.execute(sql, params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/local/sandbox/django52-prefetch-related/.tox/py312-django52/lib/python3.12/site-packages/django/db/backends/sqlite3/base.py", line 360, in execute
    return super().execute(query, params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
django.db.utils.OperationalError: no such column: tenant_partner.id

I found the (only recent) change inside of ForwardManyToOneDescriptor.get_prefetch_queryset affecting this logic, to be the patch for #36116

Going back to Django 4.2 reveals at this point the query variable is not so different from what Django generates after #36116.

Django 4.2 query variable contains:

  {'id__in': {2}}

In Django 5.2 we can peek at the SQL to be rendered (I believe) with a breakpoint before queryset.filter

(Pdb) query = TupleIn(ColPairs(queryset.model._meta.db_table, related_fields, related_fields, self.field), list(instances_dict))
(Pdb) from django.db import connection
(Pdb) compiler = queryset.query.get_compiler(using='default')
(Pdb) query.as_sql(compiler, connection)
('("tenant_partner"."id") IN ((%s))', [2])
(Pdb) query.as_sqlite(compiler, connection)
('"tenant_partner"."id" = %s', [2])

Essentially the same, but somehow it does break after this. So the "breaking change" for my usecase is confirmed to be #36116.

This is last working version. Run with tox -e py312-django52-works.

This is the first broken commit. Run with tox -e py312-django52-fails.

I've also confirmed that this last commit in stable/5.2.x still fails.

If this is not a bug, I apologize and am obviously interested in what I should do here to work around this issue :)

Change History (8)

comment:1 by Cornelis Poppema, 6 weeks ago

Summary: Regression in Prefetch and multi-table inherited models in 5.2.0Regression in Prefetch and multi-table inherited models since 5.2.0

comment:2 by Cornelis Poppema, 6 weeks ago

Description: modified (diff)

comment:3 by Simon Charette, 6 weeks ago

Keywords: prefetch inheritance added; Prefetch removed
Owner: set to Simon Charette
Severity: NormalRelease blocker
Status: newassigned
Triage Stage: UnreviewedAccepted

Thanks for the excellent report, we effectively are creating a ColsPair without explicitly resolving the foreign related fields like we should.

comment:4 by Simon Charette, 6 weeks ago

Has patch: set

I submitted a PR in hope that it might make the cut for Django 5.2.2 which is meant to be released tomorrow but it's possible you might have to wait longer to get a release that addresses the issue. I'd appreciate if you could validate the proposed changes address your reported issue, thanks!

in reply to:  4 comment:5 by Cornelis Poppema, 6 weeks ago

Replying to Simon Charette:

I submitted a PR in hope that it might make the cut for Django 5.2.2 which is meant to be released tomorrow but it's possible you might have to wait longer to get a release that addresses the issue. I'd appreciate if you could validate the proposed changes address your reported issue, thanks!

That would be amazing! Thank you for the quick patch.

I can confirm this patch works for me 😁

comment:6 by Sarah Boyce, 6 weeks ago

Triage Stage: AcceptedReady for checkin

comment:7 by Sarah Boyce <42296566+sarahboyce@…>, 6 weeks ago

Resolution: fixed
Status: assignedclosed

In 08187c94:

Fixed #36432 -- Fixed a prefetch_related crash on related target subclass queryset.

Regression in 626d77e52a3f247358514bcf51c761283968099c.

Refs #36116.

Thanks Cornelis Poppema for the excellent report.

comment:8 by Sarah Boyce <42296566+sarahboyce@…>, 6 weeks ago

In 3340d414:

[5.2.x] Fixed #36432 -- Fixed a prefetch_related crash on related target subclass queryset.

Regression in 626d77e52a3f247358514bcf51c761283968099c.

Refs #36116.

Thanks Cornelis Poppema for the excellent report.

Backport of 08187c94ed02c45ad40a32244dedeaa7ac71ca87 from main.

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