N+1 when using a prefetch_related without explicit ordering

[taobojlen]

Describe the Bug

When using prefetch_related on a resolver, unless you explicitly define an ordering, there's an N+1.

@strawberry_django.type(MyModel)
class MyModelType(strawberry.relay.Node):
    # this causes an N+1 on MyModel.foo_set when it's resolved
    @strawberry.django.field(prefetch_related=[Prefetch("foo_set", queryset=Foo.objects.filter(bar="baz")])
    def my_resolver(self) -> list[Foo]:
        # do something

    # this causes an N+1 on MyModel.bar_set
    @strawberry.django.field(prefetch_related=["bar_set"])
    def other_resolver(self) -> list[Bar]:
        # do something

    # no N+1 here
    @strawberry.django.field(prefetch_related=[Prefetch("foo_set", queryset=Foo.objects.filter(bar="baz").order_by("pk")])
    def third_resolver(self) -> list[Foo]:
        # do something

We have a number of resolvers with prefetch_related. When I updated from strawberry-django 0.54.0 to 0.62.0, I started getting N+1 errors (via django-zeal, a library to detect N+1s). I also saw some of my tests failing because the DB query count was higher than expected.

Using git bisect, I tracked this down to a specific change in strawberry-django: #715

Since then, if using custom querysets in your prefetches, it seems you have to add an explicit .order_by() clause to avoid N+1s.

System Information

• Operating system: MacOS 15.5
• Python version: 3.12.8
• Strawberry version (if applicable): 0.275.5

[bellini666]

Hrm, I've seen other reports like this and am wondering if we should revert #175

@SupImDos as you added it, do you know what might be going on? And what side-effects we need to workaround, or inform as a "breaking change", in case we need to revert it?

Or maybe we can fix it differently? I'll also try to take a look at this

[SupImDos]

@SupImDos as you added it, do you know what might be going on? And what side-effects we need to workaround, or inform as a "breaking change", in case we need to revert it?

#715 was added so that we never return multiple records from an unordered QuerySet, as it would be non-deterministic - which still seems a sensible assumption to me. If we were to revert this, the results from querying models with no default Meta.ordering would become non-deterministic again, and end users would either need to:

a) Add a default Meta.ordering to their model
b) Add a get_queryset method to their Strawberry type that applies .order_by(...)

I would need a more complete test case / reproducible example to understand the cause of the N+1s though. Maybe the .order_by("pk") is being applied too late in the process - and the ordering is somehow breaking the prefetch cache causing an N+1?

I can look into this too 🙂

---

@taobojlen Can you provide a more concrete reproduction? In your example, do any of MyModel, Foo or Bar have default Meta.ordering? This may also depend on what you're doing in # do something.

[bellini666]

Hrm, let's see if we can fix this without removing the default order_by fallback

but, I just remembered that I sometimes see a warning about the lack of an orderby when paginating on django admin. If we remove it, we might want to add a similar warning, which might be enough

Just checked, this is the code: https://github.com/django/django/blob/8499fba0e18826a77fe32cbc13a3d951d9ca8924/django/core/paginator.py#L71

[SupImDos]

@bellini666 For reference, I haven't been able to reproduce this N+1, so I'll need an MRP before I can look at proper solutions 😄