Unexpected behaviour of Prefetch with queryset filtering on a through model

[David Glenck]

Currently prefetching many-to-many objects with a queryset that filters based on fields in the through model, will return different results than applying the same filter without prefetching.

Assume the following many-to-many relationship with a through model:

class Subscriber(models.Model):
    name = models.CharField(max_length=255)
    subscriptions = models.ManyToManyField('Subscription', related_name='subscribers', through='Status')


class Subscription(models.Model):
    provider_name = models.CharField(max_length=255)


class Status(models.Model):
    subscriber = models.ForeignKey(Subscriber, related_name='status', on_delete=models.CASCADE)
    subscription = models.ForeignKey(Subscription, related_name='status', on_delete=models.CASCADE)
    is_active = models.BooleanField(default=True)

Populated with the following data:

    subscriber1 = Subscriber.objects.create(name="Sofia")
    subscriber2 = Subscriber.objects.create(name="Peter")
    subscription1 = Subscription.objects.create(provider_name="ABC")
    subscription2 = Subscription.objects.create(provider_name="123")
    Status.objects.create(subscriber=subscriber1, subscription=subscription1, is_active=True)
    Status.objects.create(subscriber=subscriber1, subscription=subscription1, is_active=True)
    Status.objects.create(subscriber=subscriber1, subscription=subscription2, is_active=False)
    Status.objects.create(subscriber=subscriber2, subscription=subscription1, is_active=True)
    Status.objects.create(subscriber=subscriber2, subscription=subscription2, is_active=True)

To get the active subscriptions of Sofia I can do this:

subscriber1.subscriptions.filter(status__is_active=True)

which gives me as expected twice the first subscription

Now, I try to prefetch the active subscriptions like this:

prefetched = Subscriber.objects.filter(pk__in=[subscriber1.pk, subscriber2.pk]).prefetch_related(
    Prefetch(
        'subscriptions',
        queryset=Subscription.objects.filter(status__is_active=True),
        to_attr='active_subscriptions'
    )
)
prefetched[0].active_subscriptions

But if I do this, I get queryset with 4 times instead of 2 times the first subscription.

Looking into the source I found, that in the first case, the internal filter of the related_descriptor is made "sticky", such that it will be combined with the next filter that is applied, as if only one filter was used. So it behaves equivalent to:

Subscription.objects.filter(subscribers=subscriber1.id, status__is_active=True)

The prefetch on the other hand doesn't do any magic to stick the filters together and it will behave like this:

Subscription.objects.filter(subscribers=subscriber1.id).filter(status__is_active=True)

which, as well documented, is behaving differently for many-to-many relationships: ​https://docs.djangoproject.com/en/5.0/topics/db/queries/#spanning-multi-valued-relationships

Ideally the first filter on the queryset would also stick to the internal filter. Or at least there should be an easy way to change the behaviour, when needed.

[Natalia Bidart]

Hello David, thank you for the detailed report and the models and reproduction steps you provided. I’m having trouble understanding the specific behavior you’re reporting that seems inconsistent with the documentation you referenced.

If the issue you’re encountering is related to the documented behavior of spanning multi-valued relationships, it might be a duplicate of issues such as #29196, #29271, #16554, or others. If the issue is different, could you please provide a brief summary or rephrase what you believe the bug in Django is?

[David Glenck]

Hi Natalia

Thank you for pointing me to some related issues.
I checked them and they refer to some behaviour I also referred to, but the issue I am pointing out is a different one.

What I am trying to do is to optimize a repeating query of this shape (but of course more complex in reality)

for subscriber in subscribers:
    subscriber.subscriptions.filter(status__is_active=True)

by prefetching the related objects like this:

prefetched = Subscriber.objects.filter(pk__in=subscribers).prefetch_related(
    Prefetch(
        'subscriptions',
        queryset=Subscription.objects.filter(status__is_active=True),
        to_attr='active_subscriptions'
    )
)
for subscriber in prefetched:
    prefetched[0].active_subscriptions

From the code above I would expect, that the prefetch gives the same results as my un-prefetched query. But it doesn't.

This is because the first case, django does some magic internally, to combine my .filter(status__is_active=True), with the internal filter to select subscriptions from this specific subscriber. Such that the result is, as if it would be a single filter (which is relevant in many-to-many relations).
But in the case of the prefetch django doesn't do that magic internally and thus it returns a different result.

So my proposal is to make the prefetched result behave the same as the non-prefetched one by modifying the internal behaviour of the prefetch query. Or at least give an option to do so. Because currently I have no clean way to instruct the prefetch to do what I want.
I'm not sure if this is a bug or a feature request, because this specific case is not mentioned in the documentation, but I would argue that it is the expected behaviour.

For illustration, this dirty workaround makes the prefetch return the result, that I would expect (but has undesired side-effects):

class StickyQueryset(models.QuerySet):
    def _chain(self):
        self._sticky_filter = True
        return self

class Subscription(models.Model):
    provider_name = models.CharField(max_length=255)

    objects = StickyQueryset.as_manager()

I tried looking deeper into the QuerySet code to see if I can propose some patch. But it's rather complex code in there.

[Simon Charette]

I tried looking deeper into the QuerySet code to see if I can propose some patch. But it's rather complex code in there.

I would search for the _next_is_sticky() token around get_prefetch_querysets implementations for many-to-many fields. It seems like we are already setting this flag in ManyRelatedManager.get_prefetch_querysets (can't link as Github is down) though so I'm surprised that the filter calls are not considered to be combined.

[Simon Charette]

I suspect that the reason why subscriber.subscriptions.filter(status__is_active=True) works is that the descriptor for Subscriber.subscription calls _next_is_sticky() before any filters is applied while when it's not the case for the queryset passed to Prefetch.

From the _next_is_sticky docstring

Indicate that the next filter call and the one following that should be treated as a single filter.

In the case of subscriber.subscription.filter(status__is_active) the resulting chain is

Subcription.objects.all()._next_is_sticky().filter(subscribers=subscriber).filter(status__is_active)

while the call chain of prefetch(Prefetch("subscriptions", Subscripton.objects.filter(status__is_active)) is

Subscripton.objects.filter(status__is_active))._next_is_sticky().filter(subscribers=subscriber)

Which doesn't have the intended effect since _next_is_sticky() is not called prior to the first filter() call.

Calling _next_is_sticky() (which is effectively what you emulated with your StickyQueryset) before calling filter(status__is_active) has the same effect

prefetched = Subscriber.objects.filter(pk__in=[subscriber1.pk, subscriber2.pk]).prefetch_related(
    Prefetch(
        'subscriptions',
        queryset=Subscription.objects.all()._next_is_sticky().filter(status__is_active=True)._next_is_sticky(),
        to_attr='active_subscriptions'
    )
)

The second _next_is_sticky call is necessary because ManyRelatedManager.get_prefetch_querysets calls to using triggers _chain and clears it.

All in all this whole sticky notion is kind of hacky and simply doesn't appear appropriate in the context of ManyRelatedManager.get_prefetch_querysets (as there is no follow up filter call). It seems that we need in there is not _next_is_sticky but a way to let the ORM know that some filter calls against multi-valued relationships should reuse existing JOINs no matter what. I know we have a ticket for that but I can't find it.

[Simon Charette]

I think the following is what you are requesting. I'm re-opening Natalia as I believe this is a legitimate bug.

django/db/models/fields/related_descriptors.py

@@ -94 +94 @@
         instance.__dict__[self.field.attname] = value
 
 
-def _filter_prefetch_queryset(queryset, field_name, instances):
+def _filter_prefetch_queryset(queryset, field_name, instances, db):
     predicate = Q(**{f"{field_name}__in": instances})
-    db = queryset._db or DEFAULT_DB_ALIAS
+    db = db or DEFAULT_DB_ALIAS
     if queryset.query.is_sliced:
         if not connections[db].features.supports_over_clause:
             raise NotSupportedError(
@@ -785 +785 @@
                 )
             queryset = querysets[0] if querysets else super().get_queryset()
             queryset._add_hints(instance=instances[0])
-            queryset = queryset.using(queryset._db or self._db)
+            db = queryset._db or self._db
+            queryset = queryset.using(db)
 
             rel_obj_attr = self.field.get_local_related_value
             instance_attr = self.field.get_foreign_related_value
             instances_dict = {instance_attr(inst): inst for inst in instances}
-            queryset = _filter_prefetch_queryset(queryset, self.field.name, instances)
+            queryset = _filter_prefetch_queryset(
+                queryset, self.field.name, instances, db
+            )
 
             # Since we just bypassed this class' get_queryset(), we must manage
             # the reverse relation manually.
@@ -1165 +1168 @@
                 )
             queryset = querysets[0] if querysets else super().get_queryset()
             queryset._add_hints(instance=instances[0])
-            queryset = queryset.using(queryset._db or self._db)
+            db = queryset._db or self._db
+            # Make sure filters are applied in a "sticky" fashion to reuse
+            # multi-valued relationships like direct filter() calls against
+            # many-to-many managers do.
+            queryset.query.filter_is_sticky = True
             queryset = _filter_prefetch_queryset(
-                queryset._next_is_sticky(), self.query_field_name, instances
+                queryset, self.query_field_name, instances, db
             )
+            queryset = queryset.using(db)
 
             # M2M: need to annotate the query in order to get the primary model
             # that the secondary model was actually related to. We know that

David, if you'd up to it it seems that all this bugfix is missing are tests that can be added to the prefetch_related test app if you're interested in submitting a PR once Github is back online.

[Natalia Bidart]

Replying to Simon Charette:

All in all this whole sticky notion is kind of hacky and simply doesn't appear appropriate in the context of ManyRelatedManager.get_prefetch_querysets (as there is no follow up filter call). It seems that we need in there is not _next_is_sticky but a way to let the ORM know that some filter calls against multi-valued relationships should reuse existing JOINs no matter what. I know we have a ticket for that but I can't find it.

Simon, perhaps this is the ticket you are looking for? #27303, at first it seems like an admin specific report but reading on it feels it has some similarities with the "sticky" bits.
Also thank you for your further analysis and reopening.

[Simon Charette]

Thanks Natalia, #27303 is effectively a manifestation of this problem in the admin. We don't have a good way to denote that joins against multi-valued relationships should be reused between different QuerySet method calls. It's a problem we hacked around for filter with this sticky notion but the problem exists for annotate and any other ORM method that allows for multi-valued (many-to-many or reverse one-to-many) to be referenced.

I believe there are other tickets that discussed a generic way to alias such relationships so they are always reusable, it was brought up during the design of FilteredRelation for sure.

IIRC the design was something along the lines of

Subscriber.objects.alias(
    # Force the reuse of all JOINs up to Susbcription (including Status)
    subscriptions=Relation("subscriptions", reuse=True),
).filter(subscriber=subscriber).filter(status__is_active=True)

[David Glenck]

Hello Simon

thank you for the proposed bugfix. I prepared a test and applied your patch and it passes with your fix.
However, if I do an annotation after the filter (which I do in my real world use case). The same happens, when I add another prefetch_related after the filter.

prefetched = Subscriber.objects.filter(pk__in=subscribers).prefetch_related(
    Prefetch(
        'subscriptions',
        queryset=Subscription.objects.filter(status__is_active=True).annotate(active=F('status__is_active')),
        to_attr='active_subscriptions'
    )
)

This makes sense, because the last filter/annotation etc. is made sticky, not the first, like in the case of the many-to-many manager.
Knowing this, for some cases this can be fixed, by moving the additional annotation before the filter.

But if I do an annotation like this, is seems to fail regardless:

prefetched = Subscriber.objects.filter(pk__in=subscribers).prefetch_related(
    Prefetch(
        'subscriptions',
        queryset=Subscription.objects.annotate(active=F('status__is_active')).filter(active=True),
        to_attr='active_subscriptions'
    )
)

The many-to-many manager seems to handle this case correctly.

subscriber1.subscriptions.annotate(active=F('status__is_active')).filter(active=True)

I have started a branch with the tests here: ​https://github.com/django/django/compare/main...pascalfree:django:35677

[Simon Charette]

I think there might be a way to solve the annotate issue as well but it's not trivial.

When annotate is used sql.Query.used_aliases doesn't get populated

>>> Subscription.objects.annotate(active=F('status__is_active')).query.used_aliases
set()

This means that even if the subsequent filter() wasn't clearing it (it does since filter_is_sticky is False by then) by the time it makes its way to the prefetching logic there is nothing left in used_alias that can be reused.

The only solution I can think of is to add the filter while allowing pre-existing many-to-many aliases to be reused

django/db/models/fields/related_descriptors.py

@@ -112 +112 @@
         if high_mark is not None:
             predicate &= LessThanOrEqual(window, high_mark)
         queryset.query.clear_limits()
-    return queryset.filter(predicate)
+    queryset.query.add_q(predicate, reuse_all_aliases=True)
+    return queryset
 
 
 class ForwardManyToOneDescriptor:
@@ -1172 +1173 @@
             # Make sure filters are applied in a "sticky" fashion to reuse
             # multi-valued relationships like direct filter() calls against
             # many-to-many managers do.
-            queryset.query.filter_is_sticky = True
             queryset = _filter_prefetch_queryset(
                 queryset, self.query_field_name, instances, db
             )

django/db/models/sql/query.py

@@ -1602 +1602 @@
     def add_filter(self, filter_lhs, filter_rhs):
         self.add_q(Q((filter_lhs, filter_rhs)))
 
-    def add_q(self, q_object):
+    def add_q(self, q_object, reuse_all_aliases=False):
         """
         A preprocessor for the internal _add_q(). Responsible for doing final
         join promotion.
@@ -1616 +1616 @@
         existing_inner = {
             a for a in self.alias_map if self.alias_map[a].join_type == INNER
         }
-        clause, _ = self._add_q(q_object, self.used_aliases)
+        if reuse_all_aliases:
+            can_reuse = set(self.alias_map)
+        else:
+            can_reuse = self.used_aliases
+        clause, _ = self._add_q(q_object, can_reuse)
         if clause:
             self.where.add(clause, AND)
         self.demote_joins(existing_inner)

Ideally we wouldn't reuse all aliases though, only the ones that we know for sure we want to reuse for _filter_prefetch_queryset(field_name), which can be obtained through a combination of names_to_path and comparison to alias_map.

[Ahmed Ibrahim]

Can I help with this one? I have time

[Simon Charette]

Ahmed, thanks for offering but without a track record of contributing to the ORM I think this one might be a bit too much particularly to get the discussed names_to_path and alias_map working. We should at least ensure that the proposed solution addresses David problem.