From 67abf955fd6aeeb0097eabc89244df35ee41ed00 Mon Sep 17 00:00:00 2001 From: Abhimanyu Singh Gaur <12651351+abhimanyusinghgaur@users.noreply.github.com> Date: Thu, 10 Dec 2020 18:11:41 +0530 Subject: [PATCH] perf(GraphQL): Part-3: Filter child auth queries as early as possible (GRAPHQL-854) (#7107) This PR moves the filter for user child queries to the first child auth variable. This will reduce the number of uids that need to be processed by the child auth rules. --- graphql/resolve/auth_query_test.yaml | 32 ++++++++++----------- graphql/resolve/query_rewriter.go | 42 +++++++--------------------- 2 files changed, 26 insertions(+), 48 deletions(-) diff --git a/graphql/resolve/auth_query_test.yaml b/graphql/resolve/auth_query_test.yaml index 502eecd8f4c..015c78fa290 100644 --- a/graphql/resolve/auth_query_test.yaml +++ b/graphql/resolve/auth_query_test.yaml @@ -795,9 +795,9 @@ UserRoot as var(func: uid(User4)) User4 as var(func: type(User)) var(func: uid(UserRoot)) { - Ticket1 as User.tickets + Ticket1 as User.tickets @filter(anyofterms(Ticket.title, "graphql")) } - Ticket3 as var(func: uid(Ticket1)) @filter((anyofterms(Ticket.title, "graphql") AND uid(TicketAuth2))) + Ticket3 as var(func: uid(Ticket1)) @filter(uid(TicketAuth2)) TicketAuth2 as var(func: uid(Ticket1)) @cascade { onColumn : Ticket.onColumn { inProject : Column.inProject { @@ -873,9 +873,9 @@ regionsAvailable : Movie.regionsAvailable @filter(eq(Region.global, true)) } var(func: uid(MovieRoot)) { - Region1 as Movie.regionsAvailable + Region1 as Movie.regionsAvailable @filter(eq(Region.name, "Region123")) } - Region2 as var(func: uid(Region1)) @filter(eq(Region.name, "Region123")) + Region2 as var(func: uid(Region1)) } - name: "Auth deep query - 3 level" @@ -934,17 +934,17 @@ regionsAvailable : Movie.regionsAvailable @filter(eq(Region.global, true)) } var(func: uid(MovieRoot)) { - Region1 as Movie.regionsAvailable + Region1 as Movie.regionsAvailable @filter(eq(Region.name, "Region123")) } - Region7 as var(func: uid(Region1)) @filter(eq(Region.name, "Region123")) + Region7 as var(func: uid(Region1)) var(func: uid(Region1)) { - User2 as Region.users + User2 as Region.users @filter(eq(User.username, "User321")) } - User6 as var(func: uid(User2)) @filter(eq(User.username, "User321")) + User6 as var(func: uid(User2)) var(func: uid(User2)) { - UserSecret3 as User.secrets + UserSecret3 as User.secrets @filter(allofterms(UserSecret.aSecret, "Secret132")) } - UserSecret5 as var(func: uid(UserSecret3)) @filter((allofterms(UserSecret.aSecret, "Secret132") AND uid(UserSecretAuth4))) + UserSecret5 as var(func: uid(UserSecret3)) @filter(uid(UserSecretAuth4)) UserSecretAuth4 as var(func: uid(UserSecret3)) @filter(eq(UserSecret.ownedBy, "user1")) @cascade } @@ -1154,9 +1154,9 @@ UserRoot as var(func: uid(User4)) User4 as var(func: type(User)) var(func: uid(UserRoot)) { - TicketAggregateResult1 as User.tickets + TicketAggregateResult1 as User.tickets @filter(anyofterms(Ticket.title, "graphql")) } - Ticket3 as var(func: uid(TicketAggregateResult1)) @filter((anyofterms(Ticket.title, "graphql") AND uid(TicketAuth2))) + Ticket3 as var(func: uid(TicketAggregateResult1)) @filter(uid(TicketAuth2)) TicketAuth2 as var(func: uid(TicketAggregateResult1)) @cascade { onColumn : Ticket.onColumn { inProject : Column.inProject { @@ -1210,9 +1210,9 @@ UserRoot as var(func: uid(User10)) User10 as var(func: type(User)) var(func: uid(UserRoot)) { - TicketAggregateResult1 as User.tickets + TicketAggregateResult1 as User.tickets @filter(anyofterms(Ticket.title, "graphql")) } - Ticket3 as var(func: uid(TicketAggregateResult1)) @filter((anyofterms(Ticket.title, "graphql") AND uid(TicketAuth2))) + Ticket3 as var(func: uid(TicketAggregateResult1)) @filter(uid(TicketAuth2)) TicketAuth2 as var(func: uid(TicketAggregateResult1)) @cascade { onColumn : Ticket.onColumn { inProject : Column.inProject { @@ -1230,9 +1230,9 @@ owner : Issue.owner @filter(eq(User.username, "user1")) } var(func: uid(UserRoot)) { - Ticket7 as User.tickets + Ticket7 as User.tickets @filter(anyofterms(Ticket.title, "graphql2")) } - Ticket9 as var(func: uid(Ticket7)) @filter((anyofterms(Ticket.title, "graphql2") AND uid(TicketAuth8))) + Ticket9 as var(func: uid(Ticket7)) @filter(uid(TicketAuth8)) TicketAuth8 as var(func: uid(Ticket7)) @cascade { onColumn : Ticket.onColumn { inProject : Column.inProject { diff --git a/graphql/resolve/query_rewriter.go b/graphql/resolve/query_rewriter.go index d4c271171c1..02b2a637006 100644 --- a/graphql/resolve/query_rewriter.go +++ b/graphql/resolve/query_rewriter.go @@ -415,11 +415,6 @@ func addArgumentsToField(dgQuery *gql.GraphQuery, field schema.Field) { addPagination(dgQuery, field) } -func addFilterToField(dgQuery *gql.GraphQuery, field schema.Field) { - filter, _ := field.ArgValue("filter").(map[string]interface{}) - _ = addFilter(dgQuery, field.Type(), filter) -} - func addTopLevelTypeFilter(query *gql.GraphQuery, field schema.Field) { addTypeFilter(query, field.Type()) } @@ -977,7 +972,6 @@ func buildCommonAuthQueries( }, } - addFilterToField(selectionQry, f) return commonAuthQueryVars{ parentQry: parentQry, selectionQry: selectionQry, @@ -1114,27 +1108,19 @@ func buildAggregateFields( // possibly mainField. Auth filters are added to count aggregation fields and // mainField. Adding filters only for mainField is sufficient for other aggregate // functions as the aggregation functions use var from mainField. - for _, aggregateChild := range aggregateChildren { - if authFilter != nil { - if aggregateChild.Filter == nil { - aggregateChild.Filter = authFilter - } else { - aggregateChild.Filter = &gql.FilterTree{ - Op: "and", - Child: []*gql.FilterTree{aggregateChild.Filter, authFilter}, - } - } - } - } + // Adds auth queries. The variable authQueriesAppended ensures that auth queries are // appended only once. This also merges auth filters and any other filters of count // aggregation fields / mainField. if len(f.SelectionSet()) > 0 && !auth.isWritingAuth && auth.hasAuthRules { commonAuthQueryVars := buildCommonAuthQueries(f, auth, parentQryName) + // add child filter to parent query, auth filters to selection query and + // selection query as a filter to child + commonAuthQueryVars.selectionQry.Filter = authFilter var authQueriesAppended = false for _, aggregateChild := range aggregateChildren { - commonAuthQueryVars.selectionQry.Filter = aggregateChild.Filter if !authQueriesAppended { + commonAuthQueryVars.parentQry.Children[0].Filter = aggregateChild.Filter retAuthQueries = append(retAuthQueries, commonAuthQueryVars.parentQry, commonAuthQueryVars.selectionQry) authQueriesAppended = true } @@ -1319,27 +1305,19 @@ func addSelectionSetFrom( fieldAuth, authFilter = auth.rewriteAuthQueries(f.Type()) } - if authFilter != nil { - if child.Filter == nil { - child.Filter = authFilter - } else { - child.Filter = &gql.FilterTree{ - Op: "and", - Child: []*gql.FilterTree{child.Filter, authFilter}, - } - } - } - if len(f.SelectionSet()) > 0 && !auth.isWritingAuth && auth.hasAuthRules { commonAuthQueryVars := buildCommonAuthQueries(f, auth, parentQryName) - commonAuthQueryVars.selectionQry.Filter = child.Filter - authQueries = append(authQueries, commonAuthQueryVars.parentQry, commonAuthQueryVars.selectionQry) + // add child filter to parent query, auth filters to selection query and + // selection query as a filter to child + commonAuthQueryVars.parentQry.Children[0].Filter = child.Filter + commonAuthQueryVars.selectionQry.Filter = authFilter child.Filter = &gql.FilterTree{ Func: &gql.Function{ Name: "uid", Args: []gql.Arg{{Value: commonAuthQueryVars.filterVarName}}, }, } + authQueries = append(authQueries, commonAuthQueryVars.parentQry, commonAuthQueryVars.selectionQry) } authQueries = append(authQueries, selectionAuth...) authQueries = append(authQueries, fieldAuth...)