allow @filter directive to be used with expand queries. (#4404)

gql/parser.go

@@ -38,6 +38,10 @@ const (
 	countFunc = "count"
 )
 
+var (
+	errExpandType = "expand is only compatible with type filters"
+)
+
 // GraphQuery stores the parsed Query in a tree format. This gets converted to
 // pb.y used query.SubGraph before processing the query.
 type GraphQuery struct {
@@ -2276,8 +2280,14 @@ func parseDirective(it *lex.ItemIterator, curp *GraphQuery) error {
 		valid = false
 	}
 	it.Next()
-	// No directive is allowed on pb.subgraph like expand all, value variables.
-	if !valid || curp == nil || curp.IsInternal {
+
+	isExpand := false
+	if curp != nil && len(curp.Expand) > 0 {
+		isExpand = true
+	}
+	// No directive is allowed on pb.subgraph like expand all (except type filters),
+	// value variables, etc.
+	if !valid || curp == nil || (curp.IsInternal && !isExpand) {
 		return item.Errorf("Invalid use of directive.")
 	}
 
@@ -2288,6 +2298,10 @@ func parseDirective(it *lex.ItemIterator, curp *GraphQuery) error {
 		return item.Errorf("Expected directive or language list")
 	}
 
+	if isExpand && item.Val != "filter" {
+		return item.Errorf(errExpandType)
+	}
+
 	switch {
 	case item.Val == "facets": // because @facets can come w/t '()'
 		res, err := parseFacets(it)
@@ -2331,6 +2345,9 @@ func parseDirective(it *lex.ItemIterator, curp *GraphQuery) error {
 			if err != nil {
 				return err
 			}
+			if isExpand && filter != nil && filter.Func != nil && filter.Func.Name != "type" {
+				return item.Errorf(errExpandType)
+			}
 			curp.Filter = filter
 		case "groupby":
 			if curp.IsGroupby {

gql/parser_test.go

@@ -4923,3 +4923,37 @@ func TestParseVarAfterCountQry(t *testing.T) {
 	_, err := Parse(Request{Str: query})
 	require.NoError(t, err)
 }
+
+func TestParseExpandFilter(t *testing.T) {
+	query := `
+		{
+			q(func: eq(name, "Frodo")) {
+				expand(_all_) @filter(type(Person)) {
+					uid
+				}
+			}
+		}`
+
+	gq, err := Parse(Request{Str: query})
+	require.NoError(t, err)
+	require.Equal(t, 1, len(gq.Query))
+	require.Equal(t, 1, len(gq.Query[0].Children))
+	require.Equal(t, "type", gq.Query[0].Children[0].Filter.Func.Name)
+	require.Equal(t, 1, len(gq.Query[0].Children[0].Filter.Func.Args))
+	require.Equal(t, "Person", gq.Query[0].Children[0].Filter.Func.Args[0].Value)
+}
+
+func TestParseExpandFilterErr(t *testing.T) {
+	query := `
+		{
+			q(func: eq(name, "Frodo")) {
+				expand(_all_) @filter(has(Person)) {
+					uid
+				}
+			}
+		}`
+
+	_, err := Parse(Request{Str: query})
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "expand is only compatible with type filters")
+}

query/common_test.go

@@ -589,6 +589,9 @@ func populateCluster() {
 		<202> <dgraph.type> "CarModel" .
 		<202> <dgraph.type> "Object" .
 
+		<203> <owner_name> "Owner of Prius" .
+		<203> <dgraph.type> "Person" .
+
 		# data for regexp testing
 		_:luke <firstName> "Luke" .
 		_:luke <lastName> "Skywalker" .

query/query.go

@@ -1850,6 +1850,15 @@ func expandSubgraph(ctx context.Context, sg *SubGraph) ([]*SubGraph, error) {
 		}
 		preds = uniquePreds(preds)
 
+		// There's a types filter at this level so filter out any non-uid predicates
+		// since only uid nodes can have a type.
+		if len(child.Filters) > 0 {
+			preds, err = filterUidPredicates(ctx, preds)
+			if err != nil {
+				return out, err
+			}
+		}
+
 		for _, pred := range preds {
 			temp := &SubGraph{
 				ReadTs: sg.ReadTs,
@@ -1867,6 +1876,7 @@ func expandSubgraph(ctx context.Context, sg *SubGraph) ([]*SubGraph, error) {
 			temp.Params.IsInternal = false
 			temp.Params.Expand = ""
 			temp.Params.Facet = &pb.FacetParams{AllKeys: true}
+			temp.Filters = child.Filters
 
 			// Go through each child, create a copy and attach to temp.Children.
 			for _, cc := range child.Children {
@@ -2480,6 +2490,24 @@ func getPredicatesFromTypes(typeNames []string) []string {
 	return preds
 }
 
+// filterUidPredicates takes a list of predicates and returns a list of the predicates
+// that are of type uid or [uid].
+func filterUidPredicates(ctx context.Context, preds []string) ([]string, error) {
+	schs, err := worker.GetSchemaOverNetwork(ctx, &pb.SchemaRequest{Predicates: preds})
+	if err != nil {
+		return nil, err
+	}
+
+	filteredPreds := make([]string, 0)
+	for _, sch := range schs {
+		if sch.GetType() != "uid" {
+			continue
+		}
+		filteredPreds = append(filteredPreds, sch.GetPredicate())
+	}
+	return filteredPreds, nil
+}
+
 // UidsToHex converts the new UIDs to hex string.
 func UidsToHex(m map[string]uint64) map[string]string {
 	res := make(map[string]string)

query/query3_test.go

@@ -2486,7 +2486,7 @@ func TestTypeFunction(t *testing.T) {
 	`
 	js := processQueryNoErr(t, query)
 	require.JSONEq(t,
-		`{"data": {"me":[{"uid":"0x2"}, {"uid":"0x3"}, {"uid":"0x4"}]}}`,
+		`{"data": {"me":[{"uid":"0x2"}, {"uid":"0x3"}, {"uid":"0x4"}, {"uid":"0xcb"}]}}`,
 		js)
 }
 

query/query4_test.go

@@ -444,6 +444,33 @@ func TestTypeExpandMultipleExplicitTypes(t *testing.T) {
 			"owner": [{"uid": "0xcb"}]}]}}`, js)
 }
 
+func TestTypeFilterAtExpand(t *testing.T) {
+	query := `{
+		q(func: eq(make, "Toyota")) {
+			expand(_all_) @filter(type(Person)) {
+				owner_name
+				uid
+			}
+		}
+	}`
+	js := processQueryNoErr(t, query)
+	require.JSONEq(t,
+		`{"data": {"q":[{"owner": [{"owner_name": "Owner of Prius", "uid": "0xcb"}]}]}}`, js)
+}
+
+func TestTypeFilterAtExpandEmptyResults(t *testing.T) {
+	query := `{
+		q(func: eq(make, "Toyota")) {
+			expand(_all_) @filter(type(Animal)) {
+				owner_name
+				uid
+			}
+		}
+	}`
+	js := processQueryNoErr(t, query)
+	require.JSONEq(t, `{"data": {"q":[]}}`, js)
+}
+
 // Test Related to worker based pagination.
 
 func TestHasOrderDesc(t *testing.T) {