strawberry-graphql / strawberry-django

Strawberry GraphQL Django extension
MIT License
391 stars 115 forks source link

fix(optimizer): handle existing select_related in querysets #515

Closed taobojlen closed 1 month ago

taobojlen commented 2 months ago

Description

The strawberry-django optimizer can automatically add select_related and only clauses to a queryset. However, it currently fails if you enable the only optimization while using querysets that already have select_relateds on them.

We use strawberry at work, and have a model like:

class MyManager(models.Manager):
  def get_queryset(self):
    return super().get_queryset().select_related("foo")

class MyModel(models.Model):
  objects = MyManager()
  foo = models.ForeignKey(Foo, on_delete=models.CASCADE)

This causes issues with the optimizer if you try to execute a graphQL query that doesn't include foo: you get errors like Field cannot be both deferred and traversed using select_related at the same time. We've actually been running without the only optimization for a long time for this reason.

This PR adds some code to the optimizer that inspects the queryset for any existing select_relateds, and includes them in the .only() call.

Types of Changes

Issues Fixed or Closed by This PR

N/A, there's no issue for this, though it related to this old message in the Strawberry Discord.

Checklist

bellini666 commented 2 months ago

Note: this is a draft PR! I am testing this out on our code, but I figured I'd open this draft so you're aware :)

Let me know once it is ready to be reviewed :)

codecov-commenter commented 2 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 88.63%. Comparing base (ff50d0a) to head (973c687).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #515 +/- ## ========================================== + Coverage 88.59% 88.63% +0.03% ========================================== Files 40 40 Lines 3402 3413 +11 ========================================== + Hits 3014 3025 +11 Misses 388 388 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

taobojlen commented 1 month ago

apologies for the delay here @bellini666 ! this is now ready for review :)