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

Add ability to expand only on certain types. #3920

Merged
merged 6 commits into from
Oct 17, 2019
Merged
Show file tree
Hide file tree
Changes from 3 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
36 changes: 35 additions & 1 deletion gql/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -2186,6 +2186,38 @@ func parseVarList(it *lex.ItemIterator, gq *GraphQuery) (int, error) {
return count, nil
}

func parseTypeList(it *lex.ItemIterator, gq *GraphQuery) error {
typeList := it.Item().Val
expectArg := false
loop:
for it.Next() {
item := it.Item()
switch item.Typ {
case itemRightRound:
it.Prev()
break loop
case itemComma:
if expectArg {
return item.Errorf("Expected a variable but got comma")
}
expectArg = true
case itemName:
if !expectArg {
return item.Errorf("Expected a variable but got comma")
}
typeList = fmt.Sprintf("%s,%s", typeList, item.Val)
expectArg = false
default:
return item.Errorf("Unexpected token %s when reading a type list", item.Val)
}
}
if expectArg {
return it.Item().Errorf("Unnecessary comma in val()")
}
gq.Expand = typeList
return nil
}

func parseDirective(it *lex.ItemIterator, curp *GraphQuery) error {
valid := true
it.Prev()
Expand Down Expand Up @@ -2807,7 +2839,9 @@ func godeep(it *lex.ItemIterator, gq *GraphQuery) error {
case "_reverse_":
child.Expand = "_reverse_"
default:
return item.Errorf("Invalid argument %v in expand()", item.Val)
if err := parseTypeList(it, child); err != nil {
return err
}
}
it.Next() // Consume ')'
gq.Children = append(gq.Children, child)
Expand Down
48 changes: 48 additions & 0 deletions gql/parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,34 @@ func TestParseQueryExpandReverse(t *testing.T) {
require.NoError(t, err)
}

func TestParseQueryExpandType(t *testing.T) {
query := `
{
var(func: uid( 0x0a)) {
friends {
expand(Person)
}
}
}
`
_, err := Parse(Request{Str: query})
require.NoError(t, err)
}

func TestParseQueryExpandMultipleTypes(t *testing.T) {
query := `
{
var(func: uid( 0x0a)) {
friends {
expand(Person, Relative)
}
}
}
`
_, err := Parse(Request{Str: query})
require.NoError(t, err)
}

func TestParseQueryAliasListPred(t *testing.T) {
query := `
{
Expand Down Expand Up @@ -4804,3 +4832,23 @@ func TestTypeFilterInPredicate(t *testing.T) {
require.Equal(t, 1, len(gq.Query[0].Children[0].Children))
require.Equal(t, "name", gq.Query[0].Children[0].Children[0].Attr)
}

func TestParseExpandType(t *testing.T) {
query := `
{
var(func: has(name)) {
expand(Person,Animal) {
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, "expand", gq.Query[0].Children[0].Attr)
require.Equal(t, "Person,Animal", gq.Query[0].Children[0].Expand)
require.Equal(t, 1, len(gq.Query[0].Children[0].Children))
require.Equal(t, "uid", gq.Query[0].Children[0].Children[0].Attr)
}
6 changes: 6 additions & 0 deletions query/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,10 @@ type CarModel {
previous_model
}

type Object {
name: string
}

type SchoolInfo {
name
abbr
Expand Down Expand Up @@ -545,11 +549,13 @@ func populateCluster() {
<201> <dgraph.type> "CarModel" .
<201> <previous_model> <200> .

<202> <name> "Car" .
<202> <make> "Toyota" .
<202> <year> "2009" .
<202> <model> "Prius" .
<202> <model> "プリウス"@jp .
<202> <dgraph.type> "CarModel" .
<202> <dgraph.type> "Object" .

# data for regexp testing
_:luke <firstName> "Luke" .
Expand Down
17 changes: 13 additions & 4 deletions query/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -1857,9 +1857,14 @@ func expandSubgraph(ctx context.Context, sg *SubGraph) ([]*SubGraph, error) {
}
preds = append(preds, rpreds...)
default:
span.Annotate(nil, "expand default")
// We already have the predicates populated from the var.
preds = getPredsFromVals(child.ExpandPreds)
if len(child.ExpandPreds) > 0 {
span.Annotate(nil, "expand default")
// We already have the predicates populated from the var.
preds = getPredsFromVals(child.ExpandPreds)
} else {
types := strings.Split(child.Params.Expand, ",")
preds = getPredicatesFromTypes(types)
}
}
preds = uniquePreds(preds)

Expand All @@ -1869,7 +1874,10 @@ func expandSubgraph(ctx context.Context, sg *SubGraph) ([]*SubGraph, error) {
Attr: pred,
}
temp.Params = child.Params
temp.Params.ExpandAll = child.Params.Expand == "_all_"
// TODO(martinmr): simplify this condition once _reverse_ and _forward_
// are removed
temp.Params.ExpandAll = child.Params.Expand != "_reverse_" &&
child.Params.Expand != "_forward_"
temp.Params.ParentVars = make(map[string]varValue)
for k, v := range child.Params.ParentVars {
temp.Params.ParentVars[k] = v
Expand Down Expand Up @@ -2637,6 +2645,7 @@ func (req *Request) ProcessQuery(ctx context.Context) (err error) {
continue
}
sg := req.Subgraphs[idx]

// Check the list for the requires variables.
if !canExecute(idx) {
continue
Expand Down
6 changes: 3 additions & 3 deletions query/query0_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,9 +152,9 @@ func TestQueryCountEmptyNames(t *testing.T) {
{in: `{q(func: has(name)) @filter(eq(name, "")) {count(uid)}}`,
out: `{"data":{"q": [{"count":2}]}}`},
{in: `{q(func: has(name)) @filter(gt(name, "")) {count(uid)}}`,
out: `{"data":{"q": [{"count":46}]}}`},
out: `{"data":{"q": [{"count":47}]}}`},
{in: `{q(func: has(name)) @filter(ge(name, "")) {count(uid)}}`,
out: `{"data":{"q": [{"count":48}]}}`},
out: `{"data":{"q": [{"count":49}]}}`},
{in: `{q(func: has(name)) @filter(lt(name, "")) {count(uid)}}`,
out: `{"data":{"q": [{"count":0}]}}`},
{in: `{q(func: has(name)) @filter(le(name, "")) {count(uid)}}`,
Expand All @@ -165,7 +165,7 @@ func TestQueryCountEmptyNames(t *testing.T) {
out: `{"data":{"q": [{"count":2}]}}`},
// NOTE: match with empty string filters values greater than the max distance.
{in: `{q(func: has(name)) @filter(match(name, "", 8)) {count(uid)}}`,
out: `{"data":{"q": [{"count":28}]}}`},
out: `{"data":{"q": [{"count":29}]}}`},
{in: `{q(func: has(name)) @filter(uid_in(name, "")) {count(uid)}}`,
failure: `Value "" in uid_in is not a number`},
}
Expand Down
27 changes: 26 additions & 1 deletion query/query4_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,32 @@ func TestTypeExpandLang(t *testing.T) {
}`
js := processQueryNoErr(t, query)
require.JSONEq(t, `{"data": {"q":[
{"make":"Toyota","model":"Prius", "model@jp":"プリウス", "year":2009}]}}`, js)
{"name": "Car", "make":"Toyota","model":"Prius", "model@jp":"プリウス", "year":2009}]}}`, js)
}

func TestTypeExpandExplicitType(t *testing.T) {
query := `{
q(func: eq(make, "Toyota")) {
expand(Object) {
uid
}
}
}`
js := processQueryNoErr(t, query)
require.JSONEq(t, `{"data": {"q":[{"name":"Car"}]}}`, js)
}

func TestTypeExpandMultipleExplicitTypes(t *testing.T) {
query := `{
q(func: eq(make, "Toyota")) {
expand(CarModel, Object) {
uid
}
}
}`
js := processQueryNoErr(t, query)
require.JSONEq(t, `{"data": {"q":[
{"name": "Car", "make":"Toyota","model":"Prius", "model@jp":"プリウス", "year":2009}]}}`, js)
}

// Test Related to worker based pagination.
Expand Down
4 changes: 2 additions & 2 deletions query/query_facets_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -915,6 +915,6 @@ func TestTypeExpandFacets(t *testing.T) {
}`
js := processQueryNoErr(t, query)
require.JSONEq(t, `{"data": {"q":[
{"make":"Toyota","model":"Prius", "model@jp":"プリウス", "model|type":"Electric",
"year":2009}]}}`, js)
{"name": "Car", "make":"Toyota","model":"Prius", "model@jp":"プリウス",
"model|type":"Electric", "year":2009}]}}`, js)
}