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.
hypermodeinc · Dec 10, 2020 · 67abf95 · 67abf95
1 parent 9baa3c7
commit 67abf95
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 48 deletions.
diff --git 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
@@ -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...)