Skip to content
This repository has been archived by the owner on Jul 8, 2023. It is now read-only.

DjangoOptimizerExtension not optimizing when fragments are used and field is queried twice #144

Closed
edomora97 opened this issue Nov 19, 2022 · 3 comments
Labels
bug Something isn't working

Comments

@edomora97
Copy link
Contributor

Given this schema (simplified from the demo):

schema.py
from typing import List, cast

from strawberry_django_plus import gql
from strawberry_django_plus.directives import SchemaDirectiveExtension
from strawberry_django_plus.optimizer import DjangoOptimizerExtension

from .models import Issue, Tag


@gql.django.type(Issue)
class IssueType:
    name: gql.auto
    tags: List["TagType"]


@gql.django.type(Tag)
class TagType:
    name: gql.auto
    issues: List[IssueType]


@gql.type
class Query:
    issue_list: List[IssueType] = gql.django.field()


schema = gql.Schema(
    query=Query,
    extensions=[
        SchemaDirectiveExtension,
        DjangoOptimizerExtension,
    ],
)

And given these two equivalent queries:

slow.graphql
query Slow {
  issueList {
    name
    tags {
      ...A
    }
    tags {
      name
    }
  }
}

fragment A on TagType {
  issues {
    ...B
  }
}

fragment B on IssueType {
  name
}
fast.graphql (obtained by clicking on "Merge fragments into query" in GraphiQL)
query Fast {
  issueList {
    name
    tags {
      issues {
        name
      }
      name
    }
  }
}

Initializing the database with this queries:

Details
INSERT INTO demo_issue (id, name, priority)
VALUES (1, 'issue1', 1), (2, 'issue2', 2), (3, 'issue3', 3);

INSERT INTO demo_tag (id, name)
VALUES (1, 'tag1'), (2, 'tag2'), (3, 'tag3');

INSERT INTO demo_issue_tags (issue_id, tag_id)
VALUES (1, 1), (1, 2), (1, 3), (2, 1), (2, 2), (2, 3), (3, 1), (3, 2), (3, 3);

Slow is experiencing the N+1 queries problem, while Fast is not.

Using GraphiQL and the Django toolbar I can see that Fast is making 3 queries:

image

While Slow is making 11:

image

Note that moving the field name from issueList into the fragment A doesn't trigger the issue.

@blueyed
Copy link
Contributor

blueyed commented Nov 21, 2022

@edomora97
btw/OT:

Using GraphiQL and the Django toolbar

How does this work for you?
I'm using https://github.com/przemub/django-graphiql-strawberry-debug-toolbar, but despite the fix from przemub/django-graphiql-strawberry-debug-toolbar#1 it appears to not be working for me anymore currently.

@bellini666 bellini666 added the bug Something isn't working label Nov 21, 2022
@bellini666
Copy link
Member

Hey @edomora97 , thanks for reporting this.

The optimizer does take fragments into consideration, but your example might be hitting a corner case that it is not considering. The code that inspects the query is

, with the fragment handling being done in this elif clause.

I'll try to take a look at that by the end of the week, but if you have time to take a look at this, feel free to open a PR that fixes it and I'll glagly review it :)

@blueyed this lib provides a fix to the toolbar to make it work, even with async views. Try using it: https://blb-ventures.github.io/strawberry-django-plus/debug-toolbar/

frleb pushed a commit to frleb/strawberry-django-plus that referenced this issue Feb 28, 2023
feat: link JSONField to strawberry.scalars.JSON
@LucasPickering
Copy link
Contributor

@bellini666 Did you ever get a chance to look at this one? I'm running into the same issue.

LucasPickering added a commit to LucasPickering/beta-spray that referenced this issue May 15, 2023
Still seeing issues with *nested* fragments. Probably an extension of blb-ventures/strawberry-django-plus#144

Try to make a minified repro and file a ticket?
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants