merge_querysets breaks annotations with DjangoFilterConnectionField

[jarcoal]

We recently updated to v2.5 and have found that many of our connections that use DjangoFilterConnectionField no longer work as expected (aka the filters aren't applied).

After some digging it looks like any filter that uses annotations are having them stripped off by DjangoConnectionField.merge_querysets here.

Specifically it looks like PR #693 started this issue because before that merge_querysets wouldn't be called in many situations.

Thoughts? Is there a better way to go handling annotations or this is just a bug?

[gabelimon]

We also found that there are scenarios where queryset and default_queryset were the same and were subsequently self-joined. Some protection against self-joining by mistake would be appreciated

[aminland]

+1, this doubled the number of joins happening in our queries, causing everything to slow down.

[jarcoal]

If one of the maintainers could chime in here on the purpose of merge_querysets that might help us open up a PR to resolve this issue. That area of the codebase is very poorly commented.

Right now my guess is that it's to try and consolidate the queryset returned from Node.get_queryset and what a specified resolver on the parent.

[zbyte64]

@jarcoal I believe you are correct, merge_querysets is to allow merging of queries from the Node and Connection.

[jarcoal]

I thought I would paste our hacked together solution for this so that it could be discussed as an alternative to the current implementation. The goal here was to pass a single queryset through the entire code path so that all interested parties can tack on their filters/annotations/prefetches/etc.

from graphene_django.filter import DjangoFilterConnectionField


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

    @classmethod
    def connection_resolver(
        cls,
        resolver,
        connection,
        default_manager,
        max_limit,
        enforce_first_or_last,
        filterset_class,
        filtering_args,
        root,
        info,
        **args
    ):
        first = args.get("first")
        last = args.get("last")

        if enforce_first_or_last:
            assert first or last, (
                "You must provide a `first` or `last` value to properly paginate the `{}` connection." # noqa
            ).format(info.field_name)

        if max_limit:
            if first:
                assert first <= max_limit, (
                    "Requesting {} records on the `{}` connection exceeds the `first` limit of {} records." # noqa
                ).format(first, info.field_name, max_limit)
                args["first"] = min(first, max_limit)

            if last:
                assert last <= max_limit, (
                    "Requesting {} records on the `{}` connection exceeds the `last` limit of {} records." # noqa
                ).format(last, info.field_name, max_limit)
                args["last"] = min(last, max_limit)

        iterable = resolver(root, info, **args)

        filter_kwargs = {k: v for k, v in args.items() if k in filtering_args}

        # pass the queryset through the filterset class
        queryset = filterset_class(
            data=filter_kwargs,
            queryset=iterable,
            request=info.context,
        ).qs

        # pass the queryset through any subclass that wants to post-process it
        queryset = cls.resolve_queryset(connection, queryset, info, args)

        return cls.resolve_connection(connection, queryset, args, None)

[jkimbo]

Apologies for the slow response on this @jarcoal . I think @zbyte64 's PR: #796 should resolve the issue. Could you try it out?

[jkimbo]

This should be fixed in v2.7.0