#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 )
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 , 6 weeks ago
Summary: | Regression in Prefetch and multi-table inherited models in 5.2.0 → Regression in Prefetch and multi-table inherited models since 5.2.0 |
---|
comment:2 by , 6 weeks ago
Description: | modified (diff) |
---|
comment:3 by , 6 weeks ago
Keywords: | prefetch inheritance added; Prefetch removed |
---|---|
Owner: | set to |
Severity: | Normal → Release blocker |
Status: | new → assigned |
Triage Stage: | Unreviewed → Accepted |
follow-up: 5 comment:4 by , 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!
comment:5 by , 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 , 6 weeks ago
Triage Stage: | Accepted → Ready for checkin |
---|
Thanks for the excellent report, we effectively are creating a
ColsPair
without explicitly resolving the foreign related fields like we should.