Nested Perms Results in unecessary queries (N+1?)

vecchp commented 3 months ago

Describe the Bug

Nesting permissions i.e private_details checked by HasSourcePerm yields unnecessary queries that could be optimized. Current code yields 6 queries, while the workaround below results in 2.

query = """
    {
        notes {
            id
            publicDetails
            privateDetails
        }
    }
"""

@strawberry.type
class Query:
    notes: List[NoteType] = strawberry_django.field(
        extensions=[HasRetvalPerm(NotePermissions.VIEW)],
    )

@strawberry_django.type(models.Note, pagination=True)
class NoteType:
    id: auto
    public_details: auto
    private_details: Optional[str] = strawberry_django.field(
        extensions=[HasSourcePerm(PrivateNotePermissions.VIEW)],
    )

    SELECT "accounts_user"."id", "accounts_user"."password", "accounts_user"."username", "accounts_user"."first_name", "accounts_user"."last_name", "accounts_user"."date_joined", "accounts_user"."last_login", "accounts_user"."email", "accounts_user"."is_superuser", "accounts_user"."is_staff", "accounts_user"."is_active" FROM "accounts_user" WHERE "accounts_user"."id" = 5 LIMIT 21
    SELECT "notes_note"."id", "notes_note"."public_details", "notes_note"."private_details" FROM "notes_note" WHERE (EXISTS(SELECT 1 AS "a" FROM "auth_permission" U0 INNER JOIN "accounts_user_user_permissions" U1 ON (U0."id" = U1."permission_id") WHERE (U1."user_id" = 5 AND U0."content_type_id" = 2 AND U0."codename" = 'view_note') LIMIT 1) OR EXISTS(SELECT 1 AS "a" FROM "auth_group" U0 INNER JOIN "accounts_user_groups" U1 ON (U0."id" = U1."group_id") INNER JOIN "auth_group_permissions" U3 ON (U0."id" = U3."group_id") INNER JOIN "auth_permission" U4 ON (U3."permission_id" = U4."id") WHERE (U1."user_id" = 5 AND U4."content_type_id" = 2 AND U4."codename" = 'view_note') LIMIT 1) OR "notes_note"."id" IN ((SELECT DISTINCT (U0."content_object_id")::bigint AS "cast1" FROM "notes_noteuserobjectpermission" U0 INNER JOIN "auth_permission" U2 ON (U0."permission_id" = U2."id") WHERE (U0."user_id" = 5 AND U2."content_type_id" = 2 AND U2."codename" = 'view_note')) UNION (SELECT DISTINCT (U0."content_object_id")::bigint AS "cast1" FROM "notes_notegroupobjectpermission" U0 INNER JOIN "auth_group" U1 ON (U0."group_id" = U1."id") INNER JOIN "accounts_user_groups" U2 ON (U1."id" = U2."group_id") INNER JOIN "auth_permission" U4 ON (U0."permission_id" = U4."id") WHERE (U2."user_id" = 5 AND U4."content_type_id" = 2 AND U4."codename" = 'view_note'))))
    SELECT "django_content_type"."app_label", "auth_permission"."codename" FROM "auth_permission" INNER JOIN "accounts_user_user_permissions" ON ("auth_permission"."id" = "accounts_user_user_permissions"."permission_id") INNER JOIN "django_content_type" ON ("auth_permission"."content_type_id" = "django_content_type"."id") WHERE "accounts_user_user_permissions"."user_id" = 5
    SELECT "django_content_type"."app_label", "auth_permission"."codename" FROM "auth_permission" INNER JOIN "auth_group_permissions" ON ("auth_permission"."id" = "auth_group_permissions"."permission_id") INNER JOIN "auth_group" ON ("auth_group_permissions"."group_id" = "auth_group"."id") INNER JOIN "accounts_user_groups" ON ("auth_group"."id" = "accounts_user_groups"."group_id") INNER JOIN "django_content_type" ON ("auth_permission"."content_type_id" = "django_content_type"."id") WHERE "accounts_user_groups"."user_id" = 5
    SELECT "auth_permission"."codename" FROM "auth_permission" INNER JOIN "django_content_type" ON ("auth_permission"."content_type_id" = "django_content_type"."id") INNER JOIN "notes_noteuserobjectpermission" ON ("auth_permission"."id" = "notes_noteuserobjectpermission"."permission_id") WHERE ("auth_permission"."content_type_id" = 2 AND "notes_noteuserobjectpermission"."content_object_id" = 1 AND "notes_noteuserobjectpermission"."user_id" = 5) ORDER BY "django_content_type"."app_label" ASC, "django_content_type"."model" ASC, "auth_permission"."codename" ASC
    SELECT "auth_permission"."codename" FROM "auth_permission" INNER JOIN "django_content_type" ON ("auth_permission"."content_type_id" = "django_content_type"."id") INNER JOIN "notes_notegroupobjectpermission" ON ("auth_permission"."id" = "notes_notegroupobjectpermission"."permission_id") INNER JOIN "auth_group" ON ("notes_notegroupobjectpermission"."group_id" = "auth_group"."id") INNER JOIN "accounts_user_groups" ON ("auth_group"."id" = "accounts_user_groups"."group_id") WHERE ("auth_permission"."content_type_id" = 2 AND "notes_notegroupobjectpermission"."content_object_id" = 1 AND "accounts_user_groups"."user_id" = 5) ORDER BY "django_content_type"."app_label" ASC, "django_content_type"."model" ASC, "auth_permission"."codename" ASC

Additional Context

Workaround

@strawberry_django.type(models.Note, pagination=True)
class NoteType:
    id: auto
    public_details: auto

    @strawberry_django.field(
        annotate={
            "_private_details": lambda info: Coalesce(
                Subquery(
                    # Needs Fix In #498 
                    filter_for_user(
                        models.Note.objects.all(),
                        info.context.request.user,
                        [PrivateNotePermissions.VIEW],
                    )
                    .filter(id=OuterRef("pk"))
                    .values("private_details")[:1],
                    output_field=CharField(),
                ),
                Value(None),
            ),
        },
    )
    def private_details(self, root: models.Note) -> Optional[str]:
        return root._private_details  # type: ignore

    SELECT "accounts_user"."id", "accounts_user"."password", "accounts_user"."username", "accounts_user"."first_name", "accounts_user"."last_name", "accounts_user"."date_joined", "accounts_user"."last_login", "accounts_user"."email", "accounts_user"."is_superuser", "accounts_user"."is_staff", "accounts_user"."is_active" FROM "accounts_user" WHERE "accounts_user"."id" = 5 LIMIT 21
    SELECT "notes_note"."id", "notes_note"."public_details", "notes_note"."private_details", COALESCE((SELECT W0."private_details" FROM "notes_note" W0 WHERE ((EXISTS(SELECT 1 AS "a" FROM "auth_permission" U0 INNER JOIN "accounts_user_user_permissions" U1 ON (U0."id" = U1."permission_id") WHERE (U1."user_id" = 5 AND U0."content_type_id" = 2 AND U0."codename" = 'view_note_private_details') LIMIT 1) OR EXISTS(SELECT 1 AS "a" FROM "auth_group" U0 INNER JOIN "accounts_user_groups" U1 ON (U0."id" = U1."group_id") INNER JOIN "auth_group_permissions" U3 ON (U0."id" = U3."group_id") INNER JOIN "auth_permission" U4 ON (U3."permission_id" = U4."id") WHERE (U1."user_id" = 5 AND U4."content_type_id" = 2 AND U4."codename" = 'view_note_private_details') LIMIT 1) OR W0."id" IN ((SELECT DISTINCT (U0."content_object_id")::bigint AS "cast1" FROM "notes_noteuserobjectpermission" U0 INNER JOIN "auth_permission" U2 ON (U0."permission_id" = U2."id") WHERE (U0."user_id" = 5 AND U2."content_type_id" = 2 AND U2."codename" = 'view_note_private_details')) UNION (SELECT DISTINCT (U0."content_object_id")::bigint AS "cast1" FROM "notes_notegroupobjectpermission" U0 INNER JOIN "auth_group" U1 ON (U0."group_id" = U1."id") INNER JOIN "accounts_user_groups" U2 ON (U1."id" = U2."group_id") INNER JOIN "auth_permission" U4 ON (U0."permission_id" = U4."id") WHERE (U2."user_id" = 5 AND U4."content_type_id" = 2 AND U4."codename" = 'view_note_private_details')))) AND W0."id" = ("notes_note"."id")) LIMIT 1), NULL) AS "_private_details" FROM "notes_note" WHERE (EXISTS(SELECT 1 AS "a" FROM "auth_permission" U0 INNER JOIN "accounts_user_user_permissions" U1 ON (U0."id" = U1."permission_id") WHERE (U1."user_id" = 5 AND U0."content_type_id" = 2 AND U0."codename" = 'view_note') LIMIT 1) OR EXISTS(SELECT 1 AS "a" FROM "auth_group" U0 INNER JOIN "accounts_user_groups" U1 ON (U0."id" = U1."group_id") INNER JOIN "auth_group_permissions" U3 ON (U0."id" = U3."group_id") INNER JOIN "auth_permission" U4 ON (U3."permission_id" = U4."id") WHERE (U1."user_id" = 5 AND U4."content_type_id" = 2 AND U4."codename" = 'view_note') LIMIT 1) OR "notes_note"."id" IN ((SELECT DISTINCT (U0."content_object_id")::bigint AS "cast1" FROM "notes_noteuserobjectpermission" U0 INNER JOIN "auth_permission" U2 ON (U0."permission_id" = U2."id") WHERE (U0."user_id" = 5 AND U2."content_type_id" = 2 AND U2."codename" = 'view_note')) UNION (SELECT DISTINCT (U0."content_object_id")::bigint AS "cast1" FROM "notes_notegroupobjectpermission" U0 INNER JOIN "auth_group" U1 ON (U0."group_id" = U1."id") INNER JOIN "accounts_user_groups" U2 ON (U1."id" = U2."group_id") INNER JOIN "auth_permission" U4 ON (U0."permission_id" = U4."id") WHERE (U2."user_id" = 5 AND U4."content_type_id" = 2 AND U4."codename" = 'view_note'))))

Upvote & Fund

We're using Polar.sh so you can upvote and help fund this issue.
We receive the funding once the issue is completed & confirmed by you.
Thank you in advance for helping prioritize & fund our backlog.

vecchp commented 3 months ago

this may be a more performant workaround

    @strawberry_django.field(
        annotate={
            "_private_details": lambda info: Case(
                When(
                    Exists(
                        filter_for_user(
                            models.Note.objects.all(),
                            info.context.request.user,
                            [PrivateNotePermissions.VIEW],
                        )
                    ),
                    then=F("private_details"),
                ),
                default=Value(None),
            ),
        }
    )
    def private_details(self, root: models.Note) -> Optional[str]:
        return root._private_details

bellini666 commented 3 months ago

Hey @vecchp ,

The main difference between the 2 codes is that in the first one you are checking for HasSourcePerm, which will check permissions on top of the root object itself when resolving private_details, and in the second one you are doing the prefetch with the permissions.

This is a similar issue to https://github.com/strawberry-graphql/strawberry-django/issues/337 in which the optimizer is not currently able to optimize some more complex nested lists/connections, specially when they are using pagination, filtering, permissioning (like this), etc

Thanks for reporting this! I'll soon try to take a look at that nested issue and will try to solve this issue together :)

vecchp commented 3 months ago

Seems like a similar library for graphene has worked around this issue. At least with nested queries maybe unrelated to perms.

https://github.com/MrThearMan/graphene-django-query-optimizer/issues/62 https://github.com/MrThearMan/graphene-django-query-optimizer/commit/355b50783cc239c76ca1dbeb11ab515d87197c2c

bellini666 commented 3 months ago

Seems like a similar library for graphene has worked around this issue. At least with nested queries maybe unrelated to perms.

MrThearMan/graphene-django-query-optimizer#62 MrThearMan/graphene-django-query-optimizer@355b507

Hrmm thanks for linking those :), I'll take a look at that for inspiration!

strawberry-graphql / strawberry-django