Opened 4 months ago
Closed 4 months ago
#36372 closed Bug (wontfix)
refresh_from_db with explicit fields can't clear a relation by it's query name unless it's prefetched
Reported by: | Roman Donchenko | Owned by: | |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | 5.2 |
Severity: | Normal | Keywords: | |
Cc: | Mariusz Felisiak, Simon Charette | Triage Stage: | Unreviewed |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Consider the following project:
testapp/__init__.py
: empty
testapp/models.py
:
from django.db import models class Container(models.Model): pass class Item(models.Model): container = models.ForeignKey(Container, on_delete=models.CASCADE)
testapp/migrations/__init__.py
: empty
testapp/migrations/0001_initial.py
:
# Generated by Django 5.2 on 2025-05-06 17:03 import django.db.models.deletion from django.db import migrations, models class Migration(migrations.Migration): initial = True dependencies = [ ] operations = [ migrations.CreateModel( name='Container', fields=[ ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), ], ), migrations.CreateModel( name='Item', fields=[ ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), ('container', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='testapp.container')), ], ), ]
test.py
:
import django from django.conf import settings from django.core import management settings.configure( INSTALLED_APPS=["testapp"], DATABASES={ 'default': { 'ENGINE': 'django.db.backends.sqlite3', 'NAME': ':memory:', }, }, ) django.setup() management.call_command("migrate") from testapp.models import Container Container.objects.create() c = Container.objects.first() # c = Container.objects.prefetch_related("item_set").first() c.refresh_from_db(fields=["item_set"])
Running test.py
results in the following output:
Operations to perform: Apply all migrations: testapp Running migrations: Applying testapp.0001_initial... OK Traceback (most recent call last): File "[...]/venv/lib/python3.12/site-packages/django/db/models/options.py", line 683, in get_field return self.fields_map[field_name] ~~~~~~~~~~~~~~~^^^^^^^^^^^^ KeyError: 'item_set' During handling of the above exception, another exception occurred: Traceback (most recent call last): File "[...]/test.py", line 24, in <module> c.refresh_from_db(fields=["item_set"]) File "[...]/venv/lib/python3.12/site-packages/django/db/models/base.py", line 737, in refresh_from_db db_instance = db_instance_qs.get() ^^^^^^^^^^^^^^^^^^^^ File "[...]/venv/lib/python3.12/site-packages/django/db/models/query.py", line 629, in get num = len(clone) ^^^^^^^^^^ File "[...]/venv/lib/python3.12/site-packages/django/db/models/query.py", line 366, in __len__ self._fetch_all() File "[...]/venv/lib/python3.12/site-packages/django/db/models/query.py", line 1935, in _fetch_all self._result_cache = list(self._iterable_class(self)) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "[...]/venv/lib/python3.12/site-packages/django/db/models/query.py", line 91, in __iter__ results = compiler.execute_sql( ^^^^^^^^^^^^^^^^^^^^^ File "[...]/venv/lib/python3.12/site-packages/django/db/models/sql/compiler.py", line 1609, in execute_sql sql, params = self.as_sql() ^^^^^^^^^^^^^ File "[...]/venv/lib/python3.12/site-packages/django/db/models/sql/compiler.py", line 765, in as_sql extra_select, order_by, group_by = self.pre_sql_setup( ^^^^^^^^^^^^^^^^^^^ File "[...]/venv/lib/python3.12/site-packages/django/db/models/sql/compiler.py", line 85, in pre_sql_setup self.setup_query(with_col_aliases=with_col_aliases) File "[...]/venv/lib/python3.12/site-packages/django/db/models/sql/compiler.py", line 74, in setup_query self.select, self.klass_info, self.annotation_col_map = self.get_select( ^^^^^^^^^^^^^^^^ File "[...]/venv/lib/python3.12/site-packages/django/db/models/sql/compiler.py", line 252, in get_select select_mask = self.query.get_select_mask() ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "[...]/venv/lib/python3.12/site-packages/django/db/models/sql/query.py", line 885, in get_select_mask return self._get_only_select_mask(opts, mask) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "[...]/venv/lib/python3.12/site-packages/django/db/models/sql/query.py", line 854, in _get_only_select_mask field = opts.get_field(field_name) ^^^^^^^^^^^^^^^^^^^^^^^^^^ File "[...]/venv/lib/python3.12/site-packages/django/db/models/options.py", line 685, in get_field raise FieldDoesNotExist( django.core.exceptions.FieldDoesNotExist: Container has no field named 'item_set'
OTOH, if you uncomment the second-to-last line in test.py
, there is no exception.
I would expect the refresh_from_db
call to simply do nothing if the relation is not prefetched. This behavior is making it impossible to use this functionality unless you know for sure whether the relation is prefetched.
Change History (4)
comment:1 by , 4 months ago
Cc: | added |
---|
comment:2 by , 4 months ago
IIUC, if the relation is prefetched, then refresh_from_db
will un-prefetch it. So it stands to reason that if it's not prefetched, there's nothing to do.
comment:3 by , 4 months ago
There's an important aspect of this problem that wasn't discussed here, the fact that when a related_name
is not specified for a ForeignKey
the reverse field name and the query name will differ.
In in case the reverse field name on Article
is named item
and its query name and descriptor name is item_set
. The changes implemented in #29625 failed to account for the fact that the related query name, which is used for the prefetched key namespace, might differ from the field name for reverse relationships.
If an explicit related_name
is defined (e.g. items
) then both match and calling refresh_from_db(fields=["items"])
doesn't crash with and without the prior prefetch_related("items")
call (albeit it performs an unnecessary query of the form SELECT id FROM container WHERE container = ?
is performed when not prefefetched).
Changing ForeignObjectRel.name
to default to .accessor_name
is tempting here but that would likely break tons of things.
comment:4 by , 4 months ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
Summary: | refresh_from_db with explicit fields can't clear a relation unless it's prefetched → refresh_from_db with explicit fields can't clear a relation by it's query name unless it's prefetched |
Thank you Simon
Confirmed that adding related_name="items"
and calling refresh_from_db(fields=["items"])
doesn't crash
Changing ForeignObjectRel.name to default to .accessor_name is tempting here but that would likely break tons of things.
As this can be resolved on the model level, I will close this as wontfix for now.
If we receive a non-invasive fix to support this, I think we would be open to accepting a patch
For context, clearing the prefetch cache was added as part of #29625
Given that
fields
can include relations that exist in the cache, I think the docs could be more specific here: https://docs.djangoproject.com/en/5.2/ref/models/instances/#django.db.models.Model.refresh_from_dbIt's not clear to me that this should do nothing (rather than error) if the relation is not prefetched given this has been explicitly specified as a field to "refresh"
Hoping to get the opinion of a few others here