Re: [Django] #36432: Regression in Prefetch and multi-table inherited models since 5.2.0

[Django]

#36432: Regression in Prefetch and multi-table inherited models since 5.2.0
-------------------------------------+-------------------------------------
Reporter: Cornelis Poppema | Owner: (none)
Type: Bug | Status: new
Component: Database layer | Version: 5.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: Prefetch | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Description changed by Cornelis Poppema:

Old description:

> 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 if
> you 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
> [https://github.com/django/django/commit/626d77e52a3f247358514bcf51c761283968099c
> #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 peak 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
> [https://github.com/django/django/commit/337c641abb36b3c2501b14e1290b800831bb20ad
> last working version]. Run with `tox -e py312-django52-works`.
>
> This is the
> [https://github.com/django/django/commit/626d77e52a3f247358514bcf51c761283968099c
> first broken commit]. Run with `tox -e py312-django52-fails`.
>
> I've also confirmed that this
> [https://github.com/django/django/tree/37e5cc6d89b4653f05a1a00af2eb2187c907c935
> 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 :)

New description:

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
[https://github.com/django/django/commit/626d77e52a3f247358514bcf51c761283968099c
#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
[https://github.com/django/django/commit/337c641abb36b3c2501b14e1290b800831bb20ad
last working version]. Run with `tox -e py312-django52-works`.

This is the
[https://github.com/django/django/commit/626d77e52a3f247358514bcf51c761283968099c
first broken commit]. Run with `tox -e py312-django52-fails`.

I've also confirmed that this
[https://github.com/django/django/tree/37e5cc6d89b4653f05a1a00af2eb2187c907c935
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 :)

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

[Django]

#36432: Regression in Prefetch and multi-table inherited models since 5.2.0
-------------------------------------+-------------------------------------

Reporter: Cornelis Poppema | Owner: Simon
| Charette
Type: Bug | Status: assigned

Component: Database layer | Version: 5.2
(models, ORM) |

Severity: Release blocker | Resolution:
Keywords: prefetch | Triage Stage: Accepted
inheritance |

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

Changes (by Simon Charette):

* keywords: Prefetch => prefetch inheritance
* owner: (none) => Simon Charette
* severity: Normal => Release blocker
* stage: Unreviewed => Accepted
* status: new => assigned

Comment:

Thanks for the excellent report, we effectively are creating a `ColsPair`
without explicitly resolving the foreign related fields like we should.
--
Ticket URL: <https://code.djangoproject.com/ticket/36432#comment:3>

[Django]

#36432: Regression in Prefetch and multi-table inherited models since 5.2.0
-------------------------------------+-------------------------------------
Reporter: Cornelis Poppema | Owner: Simon
| Charette
Type: Bug | Status: assigned
Component: Database layer | Version: 5.2
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: prefetch | Triage Stage: Accepted
inheritance |

Has patch: 1 | Needs documentation: 0

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

* has_patch: 0 => 1

Comment:

I submitted [https://github.com/django/django/pull/19523 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!
--
Ticket URL: <https://code.djangoproject.com/ticket/36432#comment:4>

[Django]

#36432: Regression in Prefetch and multi-table inherited models since 5.2.0
-------------------------------------+-------------------------------------
Reporter: Cornelis Poppema | Owner: Simon
| Charette
Type: Bug | Status: assigned
Component: Database layer | Version: 5.2
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: prefetch | Triage Stage: Accepted
inheritance |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Cornelis Poppema):

Replying to [comment:4 Simon Charette]:

> I submitted [https://github.com/django/django/pull/19523 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 😁
--
Ticket URL: <https://code.djangoproject.com/ticket/36432#comment:5>

[Django]

#36432: Regression in Prefetch and multi-table inherited models since 5.2.0
-------------------------------------+-------------------------------------
Reporter: Cornelis Poppema | Owner: Simon
| Charette
Type: Bug | Status: assigned
Component: Database layer | Version: 5.2
(models, ORM) |
Severity: Release blocker | Resolution:

Keywords: prefetch | Triage Stage: Ready for
inheritance | checkin

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Sarah Boyce):

* stage: Accepted => Ready for checkin

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

[Django]

#36432: Regression in Prefetch and multi-table inherited models since 5.2.0
-------------------------------------+-------------------------------------
Reporter: Cornelis Poppema | Owner: Simon
| Charette

Type: Bug | Status: closed

Component: Database layer | Version: 5.2
(models, ORM) |

Severity: Release blocker | Resolution: fixed

Keywords: prefetch | Triage Stage: Ready for
inheritance | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Sarah Boyce <42296566+sarahboyce@…>):

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

Comment:

In [changeset:"08187c94ed02c45ad40a32244dedeaa7ac71ca87" 08187c94]:
{{{#!CommitTicketReference repository=""
revision="08187c94ed02c45ad40a32244dedeaa7ac71ca87"
Fixed #36432 -- Fixed a prefetch_related crash on related target subclass
queryset.

Regression in 626d77e52a3f247358514bcf51c761283968099c.

Refs #36116.

Thanks Cornelis Poppema for the excellent report.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36432#comment:7>

[Django]

#36432: Regression in Prefetch and multi-table inherited models since 5.2.0
-------------------------------------+-------------------------------------
Reporter: Cornelis Poppema | Owner: Simon
| Charette
Type: Bug | Status: closed
Component: Database layer | Version: 5.2
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: prefetch | Triage Stage: Ready for
inheritance | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Sarah Boyce <42296566+sarahboyce@…>):

In [changeset:"3340d4144605bd150906d070ae3e910941fa83c9" 3340d414]:
{{{#!CommitTicketReference repository=""
revision="3340d4144605bd150906d070ae3e910941fa83c9"
[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.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36432#comment:8>