FETCH_PEERS can overwrite deferred-field changes and raise KeyError for deleted peers

[Patel Namraa]

When using FETCH_PEERS with deferred fields, DeferredAttribute.fetch_many() does not correctly handle peer instances whose state has changed since the queryset was evaluated.

There are two related problems.

1. Locally assigned values can be overwritten

Consider two objects loaded with a deferred field using FETCH_PEERS:

class Author(models.Model):
name = models.CharField(max_length=100)

authors = list(
Author.objects.defer("name").fetch_mode(FETCH_PEERS)
)

authors[0].name = "locally changed"

# Fetch the deferred field for another peer.

_ = authors[1].name

Fetching authors[1].name causes fetch_many() to populate the field on all peers. This also replaces authors[0].name with the value currently stored in the database.

The locally assigned value is therefore silently lost.

This does not happen with FETCH_ONE, where the locally assigned value is preserved.

2. A deleted peer can cause KeyError

A second problem occurs when one of the peer rows is deleted after the queryset has been evaluated.

fetch_many() uses in_bulk() to retrieve the deferred values and then indexes the returned dictionary for every peer:

value_by_pk = (
self.field.model._base_manager.using(db)
.values_list(attname, flat=True)
.in_bulk({instance.pk for instance in instances})
)

for instance in instances:
    setattr(instance, attname, value_by_pk[instance.pk])

in_bulk() does not include primary keys for rows that no longer exist. Consequently, if one peer has been deleted, accessing the deferred field on another existing peer can raise KeyError.

The missing peer should not cause an unrelated peer's deferred-field access to fail.

If the instance being accessed itself has been deleted, its behavior should remain consistent with FETCH_ONE rather than producing a KeyError caused by another peer.

Expected behavior:

FETCH_PEERS should not overwrite a peer's field if that value has already been assigned locally.

A peer whose database row no longer exists should not cause deferred-field access on other existing peers to fail.

The behavior for the instance whose own row has been deleted should remain consistent with FETCH_ONE.

I reproduced both behaviors against Django 6.1. The corresponding FETCH_ONE cases behave correctly.

This appears to be a missing state check in DeferredAttribute.fetch_many(): it unconditionally assigns the fetched value to every peer and assumes that every requested primary key is present in the result returned by in_bulk().

[Vishy]

Both issues were reviewed with the following tests,

tests/defer/tests.py

@@ -194 +194 @@
         with self.assertNumQueries(0):
             p2.value
 
+    def test_defer_fetch_mode_fetch_peers_preserves_locally_assigned_value(self):
+        p1, p2 = Primary.objects.fetch_mode(FETCH_PEERS).defer("value")
+        p1.value = "unsaved assignment"
+
+        # Accessing deferred field on p2 should not overwrite p1's local changes.
+        self.assertEqual(p2.value, "yy")
+        self.assertEqual(p1.value, "unsaved assignment")
+
+    def test_defer_fetch_mode_fetch_peers_deleted_peer(self):
+        p1, p2 = Primary.objects.fetch_mode(FETCH_PEERS).defer("value")
+
+        # Delete p1 from the database while its instance remains in memory.
+        Primary.objects.filter(pk=p1.pk).delete()
+
+        # Fetching deferred attribute on p2 should succeed and not raise
+        # KeyError for the missing p1.pk in in_bulk().
+        self.assertEqual(p2.value, "yy")
+
     def test_defer_fetch_mode_fetch_peers_single(self):
         p1 = Primary.objects.fetch_mode(FETCH_PEERS).defer("value").get(name="p1")
         with self.assertNumQueries(1):

[Jacob Walls]

Thanks for the reports. Next time, please present separate reports for separate bugs. Also, for the second issue, it would have been helpful to include the reproducer instead of just reasoning about the code.

I feel more sure about accepting the issue for the KeyError in in_bulk().

I suppose we can also accept the issue for the "don't fetch over an already set value", but I'll CC Adam to see if they have any opinions on that. The proposal is coherent with how .defer() works today -- accessing doesn't overwrite.