Skip to content
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

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

Open
vecchp opened this issue Mar 10, 2024 · 4 comments
Open

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

vecchp opened this issue Mar 10, 2024 · 4 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@vecchp
Copy link
Contributor

vecchp commented Mar 10, 2024

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.
Fund with Polar
@vecchp vecchp added the bug Something isn't working label Mar 10, 2024
@vecchp
Copy link
Contributor Author

vecchp commented Mar 10, 2024

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
Copy link
Member

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 #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
Copy link
Contributor Author

vecchp commented Mar 16, 2024

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

@bellini666
Copy link
Member

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!

@bellini666 bellini666 added enhancement New feature or request help wanted Extra attention is needed and removed bug Something isn't working labels Jun 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants