Opened 3 years ago
Closed 3 years ago
#33596 closed Bug (wontfix)
Subsequent calls to prefetch_related_objects() don't work for model instances that have changed.
Reported by: | Dennis Kliban | Owned by: | nobody |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | 3.2 |
Severity: | Normal | Keywords: | |
Cc: | AlexeyNigin, François Freitag | Triage Stage: | Unreviewed |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Our project processes instances in a stream. In some cases the instances are repeated. In these cases, we discovered that prefetch_related_objects() does not work correctly if the list of related objects has changed since the first time prefetch_related_objects() on the instance. I've written a simple reproducer here[0].
In the reproducer, if I call delattr(book, "the_authors")
after adding the author, everything works.
[0] https://github.com/django/django/commit/73a138665ba20915f51680816120727a5e2ba771
Change History (3)
comment:1 by , 3 years ago
Cc: | added |
---|---|
Summary: | subsequent calls to prefetch_related_objects() don't work for model instances that have changed → Subsequent calls to prefetch_related_objects() don't work for model instances that have changed. |
Triage Stage: | Unreviewed → Accepted |
comment:2 by , 3 years ago
I wonder if this is actually a bug or we should wontfix this one.
The definition of is_fetched
for a prefetched relationship via to_attr
is the presence of the attribute itself. The many-to-many manager already takes care of clearing the prefetched objects that Django knows about when add is called (_prefetched_objects_cache
is under Django's control) but unless we introduce a way to do some bookeeping of the form _prefetched_to_attrs: dict[prefetch_cache_name: str, to_attrs: list[str]]
when to_attr
is used I don't see how we can fix this issue.
Then comes the question of what to do if the property doesn't support del
and documenting that this change now breaks patterns of the form
bookings = Booking.objects.prefetch_related( 'seats', Prefetch('seats', to_attr='existing_seats'), ) sync_bookings(bookings) # function that alters .seats one way or the other added_seats = [] removed_seats = [] for booking in bookings: added_seats.extend(set(booking.seats.all()) - set(booking.existing_seats)) removed_seats.extend(set(booking.existing_seats) - set(booking.seats.all())
Since it's not obvious if to_attr
should be cleared or not on a prefetched relationship change and we've defaulting to not doing so since the introduction of the feature I'd be inclined to wontfix this one and declare that it's the user responsibility to clear to_attr
fetched relationships prior to attempting a second round of prefetch_related_objects
and do the book keeping themselves:
-
tests/prefetch_related/test_prefetch_related_objects.py
diff --git a/tests/prefetch_related/test_prefetch_related_objects.py b/tests/prefetch_related/test_prefetch_related_objects.py index 8be643e6dc..edd0bffe2f 100644
a b def test_prefetch_object_to_attr_twice_with_change(self): 151 151 ) 152 152 self.assertCountEqual(book.the_authors, []) 153 153 book.authors.add(author) 154 del book.the_authors 154 155 with self.assertNumQueries(1): 155 156 prefetch_related_objects( 156 157 [book],
comment:3 by , 3 years ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
Triage Stage: | Accepted → Unreviewed |
Agreed with Simon, I was torn too.
Thanks for the report and a regression test. It's similar to the #32089, but it's not the same issue.