strawberry-graphql / strawberry-django

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

values() and optimizer hints #574

Closed tasiotas closed 1 week ago

tasiotas commented 2 weeks ago

I am trying to make this query optimizer friendly.

object.reactions.all().values("type").annotate(authors=ArrayAgg("author__full_name", ordering="-date_created"))

As you can see I have grouped reactions by type using values(). Optimizer is accepting hints like only, select_related, prefetch_related and annotate.

I did use annotate, but because I cannot group it before with values("type"), I am getting wrong results.

Is there any way to build Query Expression that can combine value() and annotate() ?

This is my current, not working, field with annotation.

@strawberry.django.field(
        prefetch_related=["reactions", "reactions__author"],
        annotate={"authorsss": ArrayAgg("author__full_name", ordering="-date_created")},
    )
    def reactions_summary(self, info) -> JSON:
        return reactions_summary_long(self, info.context.request.user)

There is similar https://github.com/strawberry-graphql/strawberry-django/issues/549 with annotations

Thank you

Upvote & Fund

Fund with Polar

bellini666 commented 1 week ago

I have been thinking about this, but I'm not actually sure on what is the proper solution...

Allowing to do .values() in the queryset messes with it, and it can have unwanted side effect in the remaining resolution of the type itself.

2 things comes to my mind here:

1) Convert it to a prefetch related, like this:

    @strawberry.django.field(
        prefetch_related=[
            "reactions",
            "reactions__author",
            lambda info: Prefetch(
                "authors",
                queryset=Author.objects.only("full_name").order_by("-date_created")
                to_attr="authorsss",
            )
        ],
    )
    def reactions_summary(self, info) -> JSON:
        # change this function to consider that you have an `authorsss`
        # inside it, and do `authors_names = [a.full_name for author in obj.authorsss]
        return reactions_summary_long(self, info.context.request.user)

2) Use a dataloader to solve this specific situation, where you can handle more complex cases in an easier way

Let me know what you think? Does any of those 2 solves your issue? Or do you have a reason to really have to have it as an ArrayAgg?

tasiotas commented 1 week ago

Hi,

Thank you for looking into it and trying to implement values().

In fact I am already using your first solution with Prefetch. ArrayAgg is not necessary at all, I don't even remember how did I come up with that. Seems like development and prod implementation are two different things :)

I am still yet to try to use dataloaders for my custom queries, good to know we have it as an option.

Thanks!