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 Sarah Boyce, 4 months ago

Cc: Mariusz Felisiak Simon Charette added

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_db

It is possible to force the set of fields to be loaded by using the fields argument.

It'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

comment:2 by Roman Donchenko, 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.

Last edited 4 months ago by Roman Donchenko (previous) (diff)

comment:3 by Simon Charette, 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.

Last edited 4 months ago by Simon Charette (previous) (diff)

comment:4 by Sarah Boyce, 4 months ago

Resolution: wontfix
Status: newclosed
Summary: refresh_from_db with explicit fields can't clear a relation unless it's prefetchedrefresh_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

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