Skip to content

Prefetch_related in root resolver not preserved in children resolvers #327

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

Closed
hgylfason opened this issue Nov 17, 2017 · 11 comments · Fixed by #693
Closed

Prefetch_related in root resolver not preserved in children resolvers #327

hgylfason opened this issue Nov 17, 2017 · 11 comments · Fixed by #693
Assignees

Comments

@hgylfason
Copy link

It seems that prefetch_related is not preserved in children resolvers. (select_related is no problem).

Example

This example is a simplified version of the code I am working with.

Models

class Family(models.Model):
    name = models.CharField(max_length=255)


class Person(models.Model):
    name = models.CharField(max_length=256)
    family = models.ForeignKey('Family', related_name='members')

Nodes

class PersonNode(DjangoObjectType):
    class Meta:
        model = Person
        interfaces = (graphene.relay.Node,)


class FamilyNode(DjangoObjectType):
    members = graphene.ConnectionField(PersonNode)

    class Meta:
        model = Family
        interfaces = (graphene.relay.Node,)

"Endpoint"

class Query(graphene.ObjectType):
    families = graphene.ConnectionField(Family)

    def resolve_families(self, args, context, info):
        qs = Family.objects.prefetch_related('members')
        return qs

Query

{
  families {
    edges {
      node {
        members {
          edges {
            node {
              name
            }
          }
        }
      }
    }
  }
}

Problem

The family members were prefetched in the root resolver but that is definitely not used in children resolvers as a db query is made for person data each time a family is resolved.

Question

Does anyone have an idea how to get prefetch_related used in children resolvers?

@spockNinja
Copy link
Contributor

It does seem odd that the prefetch_related seems to be clearing out.

Do you happen to know if the prefetch query is being made, and then just being ignored? I have seen cases where if the queryset for the child relationship isn't a .all(), or something else triggers django to think that the prefetch wouldn't match the new query, it doesn't use the prefetched results.

If you can find that out, it could help narrow down the problem.

Also, simple performance operations like this will hopefully be "built-in" soon. See #220

@japrogramer
Copy link

qs = Family.objects.prefetch_related('members') should be done in the resolve for members In the FamilyNode


 16 class WishReasonType(TypeMixin, DjangoObjectType):
 15     class Meta:
 14         model = Reason
 13         only_fields = ('reason', 'qty')
 12
 11
 10 class WishPQConnection(relay.Connection):
  9     page_info = graphene.Field(PaginatorInfo, required=True)
  8
  7     class Meta:
  6         node = ProductType
  5
  4     class Edge:
  3         through = graphene.Field(WishReasonType, description='Connection reason')
  2
  1         def resolve_through(self, info, **args):
47              return self.node.preason[0]
  1
  2
  3 class WishType(TypeMixin, DjangoObjectType):
  4     """Query API def for Wish"""
  5     attr = 'pk'
  6
  7     products = PQCField(WishPQConnection)
  8
  9     def resolve_products(self, info, **args):
 10         perpage = min(args.get('perpage', 30), 30)
 11         page_width = slice(args.get('first', 0), args.get('last', None))
 12         qs = WishReasonType.get_queryset().filter(wish=self)
 13         try:
 14             return Paginator(self.products.prefetch_related(
 15                 Prefetch('WishReason_set', queryset=qs, to_attr='preason')).
 16                 all()[page_width], perpage)
 17         except Exception as e:
 18             print(e)
 19             return None
 20
 21     class Meta:
 22         model = Wish
 23         only_fields = (
 24             'private', 'created', 'updated',
 25             'title', 'owner', 'description')
 26
 27         interfaces = (relay.Node, )

@gotexis
Copy link

gotexis commented Apr 11, 2019

What's the progress on this 2 year old issue? it seem to still affect me, as I cannot seem to put prefetch_related on children objects to work at all.

@japrogramer
I didn't quite understand your solution :)

@hgylfason Did you get this issue resolved?

@japrogramer
Copy link

japrogramer commented Apr 11, 2019

@gotexis im adding an attribute to each object in my queryset that is actually the prefetched models.
than in the connections edge
i add a field and a resolver to get that prefetched object into the field in the graph.

the issue is in the relay.ConnectionField s

    @classmethod
    def resolve_connection(cls, connection_type, args, resolved):

it seems that resolved is falsed, and that leads to some logic that causes the db to be hit again. Or maybe a shallow copy of the queryset is made somewhere .. not preserving the prefetched attributes.

@japrogramer
Copy link

I think this line in the django connection resolver that combines the queryset provided
with a default_manager for the model is too blame.

https://github.com/graphql-python/graphene-django/blob/master/graphene_django/fields.py#L90

@jonbesga
Copy link

jonbesga commented May 9, 2019

Is there already any solution for this?

@japrogramer
Copy link

nope, no solution. big flaw, starting to see some big db hits.

@hgylfason
Copy link
Author

Sorry for the late reply @gotexis but we never found any solution to this problem.

@jkimbo
Copy link
Member

jkimbo commented Jul 1, 2019

@japrogramer @hgylfason @jonbesga I think I've found the issue that is meaning that prefetch_related is not preserved. It's a bug and this PR should fix it: #693

@japrogramer
Copy link

@jkimbo I thought that line looked like it was responsible, I didn't tested because i didn't understand the reasoning for merging the querysets.

Thanks for the work, hopefully it gets merged soon.

@jkimbo
Copy link
Member

jkimbo commented Jul 1, 2019

And thanks @japrogramer for helping figure out the problem. Made fixing it very simple 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants