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

annotate and other query optimizations fail in mutations #635

Open
erwinfeser opened this issue Oct 2, 2024 · 3 comments
Open

annotate and other query optimizations fail in mutations #635

erwinfeser opened this issue Oct 2, 2024 · 3 comments
Labels
bug Something isn't working

Comments

@erwinfeser
Copy link

erwinfeser commented Oct 2, 2024

Describe the Bug

Let's imagine we have two Django models: Cage and Animal where the second one has a FK to the first one. Then we have the following strawberry definition:

@strawberry_django.type(models.Cage)
class Cage:
    id: int

    @strawberry_django.field(annotate={"total_animals": Count("animals")})
    def total_animals(
        self,
        root: models.Cage,
    ) -> int:
        return root.total_animals

It works great when I ask for totalAnimals in a query but it fails if I ask for the same field in a mutation. Examples:

It works fine

query MyQuery {
  cages {
    id
    totalAnimals
  }
}

It fails with the error Error: "\'Cage\' object has no attribute \'total_animals\'"

mutation MyMutation {
  updateCage(data: {id: 10}) {
    id
    totalAnimals
  }
}

System Information

  • Operating system: GNU/Linux
  • strawberry-graphql==0.243.1
  • strawberry-graphql-django==0.48.0

Upvote & Fund

  • We're using Polar.sh so you can upvote and help fund this issue.
  • We receive the funding once the issue is completed & confirmed by you.
  • Thank you in advance for helping prioritize & fund our backlog.
Fund with Polar
@erwinfeser erwinfeser added the bug Something isn't working label Oct 2, 2024
@erwinfeser
Copy link
Author

@bellini666 a similar error raises if we use prefetch_related optimization with to_attr in the Prefetch definition, given its result is a list instead of a queryset.

@bellini666
Copy link
Member

I'm checking here, this is happening because we disable optimizations while doing CUD mutations to avoid unexpected issues inside it, but then we don't run the optimization again when returning the model.

The most straightforward fix would be to run the query to fetch the field again, which might lead to an extra query, but I'm not sure if we can do much better...

What do you think?

@philipstarkey
Copy link

Hi @bellini666, I've also run into this recently as well in strawberry-django and I agree that running a query again seems like the best (and possibly only?) way to fix this completely.

I've had a similar issue with Django REST Framework 'mutations' in the past, and I solved that similarly by refetching the Django model instance using the QuerySet that would normally be used when responding to a query. It's the only way I can see to ensure that all required annotations/prefetches are done, taking into account any changes made as part of the mutation. Annotations computed in a query prior to making the mutation leads to out-of-date values for annotations being returned in the response.

I believe this will also apply to create mutations as well as update mutations, as newly created instances won't have any annotated field on them unless they are refetched after creation.

So my vote would be to query again after performing the mutation, using the query optimiser (if enabled).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants