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

Query optimizer doesn't work with relay connection #337

Closed
jlgonzalez-martinez opened this issue Aug 16, 2023 · 7 comments · Fixed by #540
Closed

Query optimizer doesn't work with relay connection #337

jlgonzalez-martinez opened this issue Aug 16, 2023 · 7 comments · Fixed by #540
Assignees
Labels
bug Something isn't working

Comments

@jlgonzalez-martinez
Copy link

jlgonzalez-martinez commented Aug 16, 2023

Describe the Bug

Example project could be found here.

The problem is that uncomment the students that use ListConnectionWithTotalCount and comment the other one seems to not optimize automatically. I try to put a specific prefetch in the connection but then don't optimize inner fields.

System Information

  • Operating system: Macos
  • Strawberry version (if applicable): strawberry-graphql 0.204.0 strawberry-graphql-django 0.16.0

Additional Context

types.py

from typing import List

import strawberry_django as gql
from strawberry import relay, auto
from strawberry_django.relay import ListConnectionWithTotalCount

from school.models import Career, Student, School


@gql.type(Career)
class CareerType(relay.Node):
    title: auto


@gql.type(Student)
class StudentType(relay.Node):
    name: auto
    nia: auto
    career: "CareerType"

    @gql.field
    def upper_name(self) -> str:
        """Return the name in uppercase."""
        return self.name.upper()


@gql.type(School)
class SchoolType(relay.Node):
    name: auto
    # students: List[StudentType]
    students: ListConnectionWithTotalCount[StudentType] = gql.connection(
         prefetch_related="students"
    )

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
@jlgonzalez-martinez jlgonzalez-martinez added the bug Something isn't working label Aug 16, 2023
@jlgonzalez-martinez jlgonzalez-martinez changed the title Query optimizer doesn't work with custom resolvers Query optimizer doesn't work with relay connection Aug 17, 2023
@bellini666
Copy link
Member

This issue is the same as #340 .

This is happening because the optimizer currently is skipping any nested connections: https://github.com/strawberry-graphql/strawberry-graphql-django/blob/main/strawberry_django/optimizer.py#L320C29-L320C29

I'll try to create a meta issue about this to discuss how we can properly optimize nested connections. Because right now, since the nested query will be sliced, trying to optimize it will do the opposite of what we want the optimizer to do, which is do an extra prefetch without limit/offset.

@bellini666
Copy link
Member

@jlgonzalez-martinez as I mentioned in this comment, I just noticed that you are also doing a prefetch_related in the connection.

For the reasons I mentioned there, can you test without it to see if it does work correctly?

@patrick91
Copy link
Member

@bellini666 suggested this while on a call :D

WITH numbered_rows AS (
  SELECT 
     "_FilmToSpecies"."A" AS "_prefetch_related_val_a_id",
     "Species"."id",
     "Species"."name",
     ROW_NUMBER() OVER (
       PARTITION BY "_FilmToSpecies"."A"
       ORDER BY "Species"."id"
     ) row_num
   FROM "Species"
   INNER JOIN "_FilmToSpecies"
     ON ("Species"."id" = "_FilmToSpecies"."B")
   WHERE "_FilmToSpecies"."A" IN ('1', '2', '3')
)
SELECT "_prefetch_related_val_a_id", "id", "name" FROM numbered_rows WHERE row_num <= 10;

@bellini666 bellini666 self-assigned this Sep 28, 2023
@bellini666
Copy link
Member

@bellini666 suggested this while on a call :D

WITH numbered_rows AS (
  SELECT 
     "_FilmToSpecies"."A" AS "_prefetch_related_val_a_id",
     "Species"."id",
     "Species"."name",
     ROW_NUMBER() OVER (
       PARTITION BY "_FilmToSpecies"."A"
       ORDER BY "Species"."id"
     ) row_num
   FROM "Species"
   INNER JOIN "_FilmToSpecies"
     ON ("Species"."id" = "_FilmToSpecies"."B")
   WHERE "_FilmToSpecies"."A" IN ('1', '2', '3')
)
SELECT "_prefetch_related_val_a_id", "id", "name" FROM numbered_rows WHERE row_num <= 10;

@patrick91 wow, was testing here and just noticed that Django 4.2+ actually does this window function filtering when you slice a prefetch. e.g.

SomeModel.objects.prefetch_related(
    Prefetch(
        "some_relation",
        queryset=SomeRelation.objects.all()[:10],
    )
)

It will use the ROW_NUMBER() window function and filter by it

@catgirlinspace
Copy link

hiya! am running into this same issue :( the 11 similar queries are always queries like SELECT "battles_battle"."id", "battles_battle"."uploader_id" FROM "battles_battle" WHERE "battles_battle"."id" = (some int) LIMIT 21
image
image

@stygmate
Copy link
Contributor

Hi,
any progress on this one ?

@patrick91
Copy link
Member

@stygmate Thiago is working on adding support for this, hopefully we get it soon 😊

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

Successfully merging a pull request may close this issue.

5 participants