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

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

Merged
merged 6 commits into from
Dec 18, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 19 additions & 2 deletions gql/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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.")
}

Expand All @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down
34 changes: 34 additions & 0 deletions gql/parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
3 changes: 3 additions & 0 deletions query/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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" .
Expand Down
28 changes: 28 additions & 0 deletions query/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}

martinmr marked this conversation as resolved.
Show resolved Hide resolved
for _, pred := range preds {
temp := &SubGraph{
ReadTs: sg.ReadTs,
Expand All @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion query/query3_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
27 changes: 27 additions & 0 deletions query/query4_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down