Graphene-django: DjangoConnectionField.resolve_connection replaces resolved queryset with clone, wiping query cache (causing N+1)

[richardpetithory]

I'm new to graphene-django so please bear with me if my issue is nonsensical.

I've been spending some time trying to figure out why, when using a resolution method in combination with a DjangoFilterConnectionField any prefetches are being unused. I've traced it down to a few things.

Within DjangoFilterConnectionField.merge_querysets an & operator is used to merge two Django QuerySets. The __and__ method on QuerySet creates a new instance of its class, thereby losing any internal caching that may have been accomplished when using prefetch_related(). There doesn't seem to be much of a way around this without accessing private attributes of the QuerySet object, which is clearly bad practice.

I then noticed that DjangoConnectionField.resolve_connection checks to see if iterable is not default_manager. If it's not (which as far as I can tell it never will be if you have a resolve_things method on your Query class) it will then follow down the aforementioned path which winds up wiping cache because it creates a new QuerySet.

I wonder, what is the purpose of the following code in resolve_connection? If I disable it, the queryset my resolve_things method returns is actually used and thus the prefetch works.

            if iterable is not default_manager:
                default_queryset = maybe_queryset(default_manager)
                iterable = cls.merge_querysets(default_queryset, iterable)

For reference, my implementation is as follows:

class PostType(DjangoObjectType):
    class Meta:
        model = Post
        filter_fields = ['id', 'title']
        interfaces = (graphene.relay.Node, )


class Query(graphene.ObjectType):
    posts = DjangoFilterConnectionField(PostType)

    def resolve_posts(self, info, **kwargs):
        return Post.objects.prefetch_related('customer_posts').all()

[richardpetithory]

@arianon Hope you don't mind me tagging you as you seem to be the one who implemented this logic.

[arianon]

I'm not, I merely lifted up that logic to a separate method to work with Promises.

I'm no longer familiar with graphene-django so I can't help you here either, apologies and godspeed.

[richardpetithory]

Gotcha. Thanks for chiming in!

[richardpetithory]

After further digging through the history I see @pchinea actually added the iterable &= maybe_queryset(default_manager) statement here https://github.com/graphql-python/graphene-django/commit/b26f914b54602a8476654a397cf2b08ebe81aa8f#diff-4693e93918b75fd2ff08fbe53fec5118

Any chance you recall what scenario exactly this fixes?

[danpalmer]

Hey, any movement on this? This seems like a fairly major bug?

[richardpetithory]

I guess I'll just submit a PR with a "fix", but I'm wary of altering behavior that may exist for a specific reason.

[maarcingebala]

Hey, any update on this? @richardpetithory Do you have any solution that works for you locally? We're building fairly large API with Graphene and this became significant optimization problem. It seems that we'll have to override yet another part of Graphene 😕

[dani0805]

We ended up overriding the method like this:

class PrefetchingConnectionField(DjangoConnectionField):

    @classmethod
    def merge_querysets(cls, default_queryset, queryset):
        return queryset

Now in your model you redefine the field and implement resolve.

So far it works, but I would love to know what am I breaking with this:

# Child has a FK to Parent and the reverse label is "children"
class ParentNode(DjangoObjectType, CommonInteractionNode):
    ...
    children = PrefetchingConnectionField(ChildNode)

    class Meta:
        model = Parent
        interfaces = (Node,)
        filter_fields = []

    def resolve_children(self, info):
        return self.children.all()

# Parent has a FK to Root and you want to prefetch in resolve_parents
class RootNode(DjangoObjectType):
    class Meta:
        model = Root
        interfaces = (Node,)
        filter_fields = []

    def resolve_parents(self, info):
        return Parents.objects.filter(root__id=self.id).select_related(
            "some_fk",
        ).prefetch_related(
            "children"
       )

[dani0805]

To my knowledge this is still an open issue. If you prefetch a query and then you go through a m2m or reverse FK resolve, the cache is ignored. The only solution I have so far is overriding ConnectionField.
I do not think this can be considered ok in the long run.

[maarcingebala]

Have you had any particular problems with the overridden ConnectionField (as in the example above)? I've also used a similar approach, but I've changed resolve_connection method instead to not use merge_querysets:

class PrefetchingConnectionField(DjangoConnectionField):

    @classmethod
    def resolve_connection(cls, connection, default_manager, args, iterable):
        if iterable is None:
            iterable = default_manager

        if isinstance(iterable, QuerySet):
            _len = iterable.count()
        else:
            _len = len(iterable)

        connection = connection_from_list_slice(
            iterable,
            args,
            slice_start=0,
            list_length=_len,
            list_slice_length=_len,
            connection_type=connection,
            edge_type=connection.Edge,
            pageinfo_type=PageInfo,
        )
        connection.iterable = iterable
        connection.length = _len
        return connection

I'm using it successfully with graphene-django-optimizer.

[gotexis]

@maarcingebala
Hey are you still using this solution which means it's not fixed as of now?

What about if I were using DjangoFilterConnectionField? Is it also OK?

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[zbyte64]

merge_queryset is also impacting #758 & #787

I don't see an easy way to fix this. Maybe instead of trying to union querysets we pass through a single queryset.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[jkimbo]

A fix for this has been released in v2.7.0