Refactor code

graphql/e2e/common/common.go

@@ -364,7 +364,7 @@ func RunAll(t *testing.T) {
 	t.Run("persisted query", persistedQuery)
 	t.Run("query aggregate without filter", queryAggregateWithoutFilter)
 	t.Run("query aggregate with filter", queryAggregateWithFilter)
-	t.Run("query count with alias", queryAggregateWithAlias)
+	t.Run("query aggregate with alias", queryAggregateWithAlias)
 	t.Run("query count at child level", queryCountAtChildLevel)
 	t.Run("query count at child level with filter", queryCountAtChildLevelWithFilter)
 	t.Run("query count at child level with multiple alias", queryCountAtChildLevelWithMultipleAlias)

graphql/resolve/query_rewriter.go

@@ -195,93 +195,59 @@ func aggregateQuery(query schema.Query, authRw *authRewriter) *gql.GraphQuery {
 	// Add selection set to mainQuery and finalMainQuery.
 	isAggregateFieldVisited := make(map[string]bool)
 	for _, f := range query.SelectionSet() {
-		if f.Name() == "count" {
+		fldName := f.Name()
+		if fldName == "count" {
 			child := &gql.GraphQuery{
 				Var:  "countVar",
 				Attr: "count(uid)",
 			}
 			finalQueryChild := &gql.GraphQuery{
-				Alias: f.Name(),
+				Alias: fldName,
 				Attr:  "max(val(countVar))",
 			}
 			mainQuery.Children = append(mainQuery.Children, child)
 			finalMainQuery.Children = append(finalMainQuery.Children, finalQueryChild)
+			continue
 		}
-		if strings.HasSuffix(f.Name(), "Max") {
-			// constructedForDgraphPredicate stores the field for which Max has been queried.
-			constructedForDgraphPredicate := f.DgraphPredicateForAggregateField()
-			fldName := f.Name()
-			constructedForField := fldName[:len(fldName)-3]
-			if !isAggregateFieldVisited[constructedForField] {
-				child := &gql.GraphQuery{
-					Var:  constructedForField + "Var",
-					Attr: constructedForDgraphPredicate,
-				}
-				mainQuery.Children = append(mainQuery.Children, child)
-				isAggregateFieldVisited[constructedForField] = true
-			}
-			finalQueryChild := &gql.GraphQuery{
-				Alias: f.Name(),
-				Attr:  "max(val(" + constructedForField + "Var))",
-			}
-			finalMainQuery.Children = append(finalMainQuery.Children, finalQueryChild)
-		}
-		if strings.HasSuffix(f.Name(), "Min") {
-			// constructedForDgraphPredicate stores the field for which Max has been queried.
-			constructedForDgraphPredicate := f.DgraphPredicateForAggregateField()
-			fldName := f.Name()
-			constructedForField := fldName[:len(fldName)-3]
-			if !isAggregateFieldVisited[constructedForField] {
-				child := &gql.GraphQuery{
-					Var:  constructedForField + "Var",
-					Attr: constructedForDgraphPredicate,
-				}
-				mainQuery.Children = append(mainQuery.Children, child)
-				isAggregateFieldVisited[constructedForField] = true
-			}
-			finalQueryChild := &gql.GraphQuery{
-				Alias: f.Name(),
-				Attr:  "min(val(" + constructedForField + "Var))",
-			}
-			finalMainQuery.Children = append(finalMainQuery.Children, finalQueryChild)
-		}
-		if strings.HasSuffix(f.Name(), "Sum") {
-			// constructedForDgraphPredicate stores the field for which Max has been queried.
-			constructedForDgraphPredicate := f.DgraphPredicateForAggregateField()
-			fldName := f.Name()
-			constructedForField := fldName[:len(fldName)-3]
-			if !isAggregateFieldVisited[constructedForField] {
-				child := &gql.GraphQuery{
-					Var:  constructedForField + "Var",
-					Attr: constructedForDgraphPredicate,
+
+		// Handle other aggregate functions than count
+		aggregateFunctions := []string{"Max", "Min", "Sum", "Avg"}
+
+		for _, function := range aggregateFunctions {
+			// A field can have at maximum one of the aggregation functions as suffix
+			if strings.HasSuffix(fldName, function) {
+				// constructedForDgraphPredicate stores the Dgraph predicate for which aggregate function has been queried.
+				constructedForDgraphPredicate := f.DgraphPredicateForAggregateField()
+				// constructedForField contains the field for which aggregate function has been queried.
+				// As all aggregate functions have length 3, removing last 3 characters from fldName.
+				constructedForField := fldName[:len(fldName)-3]
+				// isAggregateFieldVisited ensures that a field is added to Var query at maximum once.
+				// If a field has already been added to the var query, don't add it again.
+				// Eg. Even if scoreMax and scoreMin are queried, the query will contain only one expression
+				// of the from, "scoreVar as Tweets.score"
+				if !isAggregateFieldVisited[constructedForField] {
+					child := &gql.GraphQuery{
+						Var:  constructedForField + "Var",
+						Attr: constructedForDgraphPredicate,
+					}
+					// The var field is added to mainQuery. This adds the following DQL query.
+					// var(func: type(Tweets)) {
+					//        scoreVar as Tweets.score
+					// }
+					mainQuery.Children = append(mainQuery.Children, child)
+					isAggregateFieldVisited[constructedForField] = true
 				}
-				mainQuery.Children = append(mainQuery.Children, child)
-				isAggregateFieldVisited[constructedForField] = true
-			}
-			finalQueryChild := &gql.GraphQuery{
-				Alias: f.Name(),
-				Attr:  "sum(val(" + constructedForField + "Var))",
-			}
-			finalMainQuery.Children = append(finalMainQuery.Children, finalQueryChild)
-		}
-		if strings.HasSuffix(f.Name(), "Avg") {
-			// constructedForDgraphPredicate stores the field for which Max has been queried.
-			constructedForDgraphPredicate := f.DgraphPredicateForAggregateField()
-			fldName := f.Name()
-			constructedForField := fldName[:len(fldName)-3]
-			if !isAggregateFieldVisited[constructedForField] {
-				child := &gql.GraphQuery{
-					Var:  constructedForField + "Var",
-					Attr: constructedForDgraphPredicate,
+				finalQueryChild := &gql.GraphQuery{
+					Alias: fldName,
+					Attr:  strings.ToLower(function) + "(val(" + constructedForField + "Var))",
 				}
-				mainQuery.Children = append(mainQuery.Children, child)
-				isAggregateFieldVisited[constructedForField] = true
-			}
-			finalQueryChild := &gql.GraphQuery{
-				Alias: f.Name(),
-				Attr:  "avg(val(" + constructedForField + "Var))",
+				// This adds the following DQL query
+				// aggregateTweets() {
+				//        scoreMin : min(val(scoreVar))
+				// }
+				finalMainQuery.Children = append(finalMainQuery.Children, finalQueryChild)
+				break
 			}
-			finalMainQuery.Children = append(finalMainQuery.Children, finalQueryChild)
 		}
 	}
 

graphql/schema/gqlschema.go

@@ -1649,6 +1649,7 @@ func addAggregationResultType(schema *ast.Schema, defn *ast.Definition) {
 			// as input type and the fields may not be always needed.
 		}
 
+		// Adds titleMax, titleMin fields for a field of name title.
 		if isOrderable(fld) {
 			minField := &ast.FieldDefinition{
 				Name: fld.Name + "Min",
@@ -1662,6 +1663,8 @@ func addAggregationResultType(schema *ast.Schema, defn *ast.Definition) {
 			aggregateFields = append(aggregateFields, maxField)
 		}
 
+		// Adds scoreSum and scoreAvg field for a field of name score.
+		// The type of scoreAvg is Float irrespective of the type of score.
 		if isSummable(fld) {
 			sumField := &ast.FieldDefinition{
 				Name: fld.Name + "Sum",

graphql/schema/wrappers.go

@@ -1524,9 +1524,16 @@ func (f *field) DgraphPredicateForAggregateField() string {
 	// it will be of the form <Type>AggregateResult
 	// we need to obtain the type name from <Type> from <Type>AggregateResult
 	aggregateFieldName := f.field.ObjectDefinition.Name
+
+	// If aggregateFieldName is found to not end with AggregateResult, just return DgraphPredicate()
+	if !strings.HasSuffix(aggregateFieldName, "AggregateResult") {
+		// This is an extra precaution and ideally, the code should not reach over here.
+		return f.DgraphPredicate()
+	}
 	mainTypeName := aggregateFieldName[:len(aggregateFieldName)-15]
 	// Remove last 3 characters of the field name.
 	// Eg. to get "FieldName" from "FieldNameMax"
+	// As all Aggregate functions are of length 3, removing last 3 characters from fldName
 	return f.op.inSchema.dgraphPredicate[mainTypeName][fldName[:len(fldName)-3]]
 }