Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nested prefetch_related resolver optimizations are seemingly ignored #697

Open
jamie-cndr opened this issue Jan 24, 2025 · 3 comments
Open
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@jamie-cndr
Copy link

jamie-cndr commented Jan 24, 2025

Describe the Bug

The schema that I'm working with often has nested field types with prefetch_related optimizer hints. I noticed that if one field resolver has a prefetch_related defined, nested field resolvers would not get optimized, which resulted in N+1s.

Example schema:

@strawberry_django.type(Project, filters=ProjectFilter, pagination=True)
class ProjectType(relay.Node, Named):
    @strawberry_django.field(
        pagination=True,
        prefetch_related=["milestones"],
    )
    def milestones(self) -> list["MilestoneType"]:
        # This resolver is contrived for this example. In actuality more would
        # be happening other than returning .all(), such as extra filtering, etc.
        return self.milestones.all()

@strawberry_django.type(
    Milestone, filters=MilestoneFilter, order=MilestoneOrder, pagination=True
)
class MilestoneType(relay.Node, Named):
    issues: list["IssueType"] = strawberry_django.field(
        filters=IssueFilter,
        order=IssueOrder,
        pagination=True,
    )

@strawberry_django.type(Issue)
class IssueType(relay.Node, Named):
    priority: strawberry.auto

Test query:

query TestQuery ($node_id: GlobalID!) {
  project (id: $node_id) {
    milestones {
      issues {
        priority
      }
    }
  }
}

Resulting DB queries from test:

AssertionError: 8 queries executed, 3 expected
Captured queries were:
1. SELECT "projects_project"."id" FROM "projects_project" WHERE "projects_project"."id" = 1 ORDER BY "projects_project"."id" ASC LIMIT 1
2. SELECT "projects_milestone"."name", "projects_milestone"."id", "projects_milestone"."due_date", "projects_milestone"."project_id" FROM "projects_milestone" WHERE "projects_milestone"."project_id" IN (1)
3. SELECT "projects_issue"."id", "projects_issue"."priority" FROM "projects_issue" WHERE "projects_issue"."milestone_id" = 1 ORDER BY "projects_issue"."id" ASC
4. SELECT "projects_issue"."id", "projects_issue"."milestone_id" FROM "projects_issue" WHERE "projects_issue"."id" = 1 LIMIT 21
5. SELECT "projects_issue"."id", "projects_issue"."milestone_id" FROM "projects_issue" WHERE "projects_issue"."id" = 2 LIMIT 21
6. SELECT "projects_issue"."id", "projects_issue"."priority" FROM "projects_issue" WHERE "projects_issue"."milestone_id" = 2 ORDER BY "projects_issue"."id" ASC
7. SELECT "projects_issue"."id", "projects_issue"."milestone_id" FROM "projects_issue" WHERE "projects_issue"."id" = 3 LIMIT 21
8. SELECT "projects_issue"."id", "projects_issue"."milestone_id" FROM "projects_issue" WHERE "projects_issue"."id" = 4 LIMIT 21

milestones is prefetched appropriately, but then the optimizer doesn't continue with optimizing the issues queries. Changing the milestones field resolver back to milestones: list["MilestoneType"] = strawberry_django.field(pagination=True) resulted in these expected db queries with issues prefetched:

1. SELECT "projects_project"."id" FROM "projects_project" WHERE "projects_project"."id" = 1 ORDER BY "projects_project"."id" ASC LIMIT 1
2. SELECT "projects_milestone"."id", "projects_milestone"."project_id", 1 AS "_strawberry_nested_prefetch_optimized" FROM "projects_milestone" WHERE "projects_milestone"."project_id" IN (1)
3. SELECT "projects_issue"."id", "projects_issue"."priority", "projects_issue"."milestone_id", 1 AS "_strawberry_nested_prefetch_optimized" FROM "projects_issue" WHERE "projects_issue"."milestone_id" IN (1, 2) ORDER BY "projects_issue"."id" ASC

Additional Context

I'm unsure if this is an actual bug or expected behavior based on the schema setup. I know there were two different changes that skip optimizations early:

  1. https://github.com/strawberry-graphql/strawberry-django/pull/583/files - skipping optimizations if a field has a base resolver
  2. https://github.com/strawberry-graphql/strawberry-django/pull/582/files - skipping optimizations if prefetch_related is in the field's store
    • This check targets prefetches with filters/annotations, but I think it's also having the side effect of skipping nested optimizations if prefetches are querysets without filters/annotations

So I believe nested fields aren't reaching _get_hints_from_django_relation / _get_model_hints due to those early returns. I tried removing these early return blocks locally out of curiosity, and that reduced the number of issue queries back to one (though I don't believe that's the right fix, it was purely for testing).

If this is an issue due to schema setup, is there general guidance on how to set up resolvers to ensure that all prefetch_related optimizer hints are successfully handled?

Thanks in advance!

System Information

  • Operating system: macOS
  • Strawberry version (if applicable): 0.55.0
@jamie-cndr jamie-cndr added the bug Something isn't working label Jan 24, 2025
@bellini666
Copy link
Member

That's a case where it is very hard to properly workaround...

Before those PRs that you mentioned, the optimizer would try to get hints based on the name of the resolver, but that resulted in a lot of issues (like #559)

The idea of those fixes are: If you are defining a resolver, you are responsible for how it will be resolved, and the maximum that the optimizer will do is to use the hints you can pass to strawberry_django.field.

I'm not really sure how we can workaround this one... will think about it, but open to suggestions!

@bellini666 bellini666 added enhancement New feature or request help wanted Extra attention is needed and removed bug Something isn't working labels Jan 25, 2025
@jamie-cndr
Copy link
Author

@bellini666 Appreciate the quick response! Are these the main past issues that were caused by recursive optimization?:

If there are others let me know, I'll look over them and also think on potential workarounds.

@bellini666
Copy link
Member

@jamie-cndr yep, I think those are the ones

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants