Opened 6 months ago
Closed 6 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 , 6 months ago
| Cc: | added |
|---|
comment:2 by , 6 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 , 6 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 , 6 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
fieldscan 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