tfoxy / graphene-django-optimizer

Optimize database access inside graphene queries
MIT License
428 stars 84 forks source link

Prefetching for generic relations does not seem to work #75

Open crazyscientist opened 3 years ago

crazyscientist commented 3 years ago

Hi there,

first of all, thanks for the project, and I'd like to point out that prefetching works for ForeignKey and ManyToMany relations. Somehow only GenericForeignKey is not recognized as optimizable.

Versions

Package Version
Python 3.8
Django 2.2
graphene 2.1.9
graphene-django 2.15.0
graphene-django-optimizer 0.8.0
graphql-core 2.3.2
graphql-relay 2.0.1

Models

class Comment(models.Model):
    text = models.TextField(null=True, blank=True)
    when = models.DateTimeField(null=True)
    who = models.ForeignKey(User, null=True, on_delete=models.SET_NULL)
    remote_id = models.PositiveIntegerField(null=True)
    content_type = models.ForeignKey(ContentType, on_delete=models.CASCADE)
    object_id = models.PositiveIntegerField()
    content_object = GenericForeignKey('content_type', 'object_id')

class CommentMixin(models.Model):
    comments = GenericRelation(Comment)

    class Meta:
        abstract = True

class Incident(CommentMixin, models.Model):
    incident_id = models.IntegerField(unique=True)

Schema

class CommentType(OptimizedDjangoObjectType):
    class Meta:
        model = models.Comment
        interfaces = (graphene.relay.Node,)

class IncidentType(OptimizedDjangoObjectType):
    comments = DjangoConnectionField(CommentType)

    @resolver_hints(model_field=models.Comment)
    def resolve_comments(self, info, **kwargs):
        return self.comments.all()

Query

{
  incidents(first: 20) {
    edges {
      node {
        id
        comments {
          edges {
            node {
              text
              when
              who {
                username
              }
            }
          }
        }
      }
    }
  }
}

Problem

For non-generic relations the optimization works as expected, but with a generic relation (in the above example: comments) it does not. Based on the raw SQL queries executed, it looks like for every incident the comments are queried and for each comment the related User model is queried.

Expected

Generic relations are tough. Django allows prefetching of the model that is a member of the generic relation, but nothing further, e.g.:

Incident.objects.all().prefetch_related("comments")  # works
Incident.objects.all().prefetch_related("comments", "comments__who")  # does not work

It would be unfair to expect that the optimizer overcomes this limitation of Django. It would be really great, if the optimizer could prefetch the comments in this example.

crazyscientist commented 3 years ago

Maybe this is related to #45?

tfoxy commented 3 years ago

Hi @crazyscientist ! The detection of foreign key id is done manually by using isinstance (https://github.com/tfoxy/graphene-django-optimizer/blob/e5c57fc8e15691d37efde2af60f70e9e787e4344/graphene_django_optimizer/query.py#L321-L326). You can try to add GenericForeignKey to the detection. If that works, feel free to create a PR together with a test for the GenericForeignKey case.

tfoxy commented 3 years ago

Looking into this a little bit deeper, I think it would require some changes to the code to support this properly. At the time I made this, I only had a few generic relations in the codebase I was applying this library. But if a codebase has many fields like that, I see the value that solving this issue would provide.

If you have the time, feel free to create a PR. Will gladly review it.