From 1d867dbf00f4672c49fec126aeff4d707df82620 Mon Sep 17 00:00:00 2001 From: Martin Martinez Rivera Date: Wed, 4 Sep 2019 16:36:34 -0700 Subject: [PATCH 1/4] Initial commit for expand(Type) functionality --- gql/parser.go | 35 ++++++++++++++++++++++++++++++++++- gql/parser_test.go | 28 ++++++++++++++++++++++++++++ query/common_test.go | 6 ++++++ query/query.go | 16 +++++++++++++--- query/query0_test.go | 6 +++--- query/query4_test.go | 24 +++++++++++++++++++++++- query/query_facets_test.go | 4 ++-- 7 files changed, 109 insertions(+), 10 deletions(-) diff --git a/gql/parser.go b/gql/parser.go index e6d84714c83..64ce09b18d7 100644 --- a/gql/parser.go +++ b/gql/parser.go @@ -2214,6 +2214,37 @@ 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: + 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() @@ -2838,7 +2869,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) diff --git a/gql/parser_test.go b/gql/parser_test.go index 617a11b2b6d..a6ec43265c2 100644 --- a/gql/parser_test.go +++ b/gql/parser_test.go @@ -164,6 +164,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 := ` { diff --git a/query/common_test.go b/query/common_test.go index a171d11035c..5e95b51f5eb 100644 --- a/query/common_test.go +++ b/query/common_test.go @@ -207,6 +207,10 @@ type CarModel { previous_model: CarModel } +type Object { + name: string +} + type SchoolInfo { name: string abbr: string @@ -543,11 +547,13 @@ func populateCluster() { <201> "CarModel" . <201> <200> . + <202> "Car" . <202> "Toyota" . <202> "2009" . <202> "Prius" . <202> "プリウス"@jp . <202> "CarModel" . + <202> "Object" . `) addGeoPointToCluster(1, "loc", []float64{1.1, 2.0}) diff --git a/query/query.go b/query/query.go index 5ec5b097efe..b528cf6c8c9 100644 --- a/query/query.go +++ b/query/query.go @@ -1788,9 +1788,19 @@ 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) + rpreds, err := getReversePredicates(ctx, preds) + if err != nil { + return out, err + } + preds = append(preds, rpreds...) + } } preds = uniquePreds(preds) diff --git a/query/query0_test.go b/query/query0_test.go index c238fada0fc..b6b3d2eec1e 100644 --- a/query/query0_test.go +++ b/query/query0_test.go @@ -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)}}`, @@ -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`}, } diff --git a/query/query4_test.go b/query/query4_test.go index 693b744a4a1..27c5f96217a 100644 --- a/query/query4_test.go +++ b/query/query4_test.go @@ -339,5 +339,27 @@ 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) + } + }` + 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) + } + }` + js := processQueryNoErr(t, query) + require.JSONEq(t, `{"data": {"q":[ + {"name": "Car", "make":"Toyota","model":"Prius", "year":2009}]}}`, js) } diff --git a/query/query_facets_test.go b/query/query_facets_test.go index 3be30155b31..c76f27e5d85 100644 --- a/query/query_facets_test.go +++ b/query/query_facets_test.go @@ -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) } From 4023000cfb87b5cecca9785a988001952c8351ce Mon Sep 17 00:00:00 2001 From: Martin Martinez Rivera Date: Thu, 3 Oct 2019 17:34:24 -0700 Subject: [PATCH 2/4] Fix bug in parser. --- gql/parser.go | 1 + gql/parser_test.go | 20 ++++++++++++++++++++ query/query.go | 11 +++++------ query/query4_test.go | 13 ++++++++----- 4 files changed, 34 insertions(+), 11 deletions(-) diff --git a/gql/parser.go b/gql/parser.go index 4f3c7bb19a0..a46d23ab0b1 100644 --- a/gql/parser.go +++ b/gql/parser.go @@ -2194,6 +2194,7 @@ loop: item := it.Item() switch item.Typ { case itemRightRound: + it.Prev() break loop case itemComma: if expectArg { diff --git a/gql/parser_test.go b/gql/parser_test.go index da6c7a0e160..bb763de2834 100644 --- a/gql/parser_test.go +++ b/gql/parser_test.go @@ -4832,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) +} diff --git a/query/query.go b/query/query.go index 3f5ea7711de..5ba763b6aaa 100644 --- a/query/query.go +++ b/query/query.go @@ -1864,11 +1864,6 @@ func expandSubgraph(ctx context.Context, sg *SubGraph) ([]*SubGraph, error) { } else { types := strings.Split(child.Params.Expand, ",") preds = getPredicatesFromTypes(types) - rpreds, err := getReversePredicates(ctx, preds) - if err != nil { - return out, err - } - preds = append(preds, rpreds...) } } preds = uniquePreds(preds) @@ -1879,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 @@ -2647,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 diff --git a/query/query4_test.go b/query/query4_test.go index 8f37d1a74e2..0f8527efa07 100644 --- a/query/query4_test.go +++ b/query/query4_test.go @@ -341,23 +341,26 @@ func TestTypeExpandLang(t *testing.T) { func TestTypeExpandExplicitType(t *testing.T) { query := `{ q(func: eq(make, "Toyota")) { - expand(Object) + expand(Object) { + uid + } } }` js := processQueryNoErr(t, query) - require.JSONEq(t, `{"data": {"q":[ - {"name":"Car"}]}}`, js) + require.JSONEq(t, `{"data": {"q":[{"name":"Car"}]}}`, js) } func TestTypeExpandMultipleExplicitTypes(t *testing.T) { query := `{ q(func: eq(make, "Toyota")) { - expand(CarModel, Object) + expand(CarModel, Object) { + uid + } } }` js := processQueryNoErr(t, query) require.JSONEq(t, `{"data": {"q":[ - {"name": "Car", "make":"Toyota","model":"Prius", "year":2009}]}}`, js) + {"name": "Car", "make":"Toyota","model":"Prius", "model@jp":"プリウス", "year":2009}]}}`, js) } // Test Related to worker based pagination. From 26b18f01dd2128e8993d8b2507983143ba2e7daa Mon Sep 17 00:00:00 2001 From: Martin Martinez Rivera Date: Thu, 3 Oct 2019 17:39:06 -0700 Subject: [PATCH 3/4] Remove extra line. --- query/query.go | 1 - 1 file changed, 1 deletion(-) diff --git a/query/query.go b/query/query.go index 5ba763b6aaa..95a107c1e22 100644 --- a/query/query.go +++ b/query/query.go @@ -2645,7 +2645,6 @@ 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 From ff4223030b88102863cbdc36c6e667d8fed824fc Mon Sep 17 00:00:00 2001 From: Martin Martinez Rivera Date: Thu, 17 Oct 2019 12:06:30 -0700 Subject: [PATCH 4/4] change tests to expand uids. --- query/common_test.go | 5 ++++- query/query4_test.go | 8 +++++--- query/query_facets_test.go | 2 +- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/query/common_test.go b/query/common_test.go index e772a25d973..ed1c029b28c 100644 --- a/query/common_test.go +++ b/query/common_test.go @@ -200,7 +200,8 @@ type CarModel { } type Object { - name: string + name + owner } type SchoolInfo { @@ -270,6 +271,7 @@ newname : string @index(exact, term) . newage : int . boss : uid . newfriend : [uid] . +owner : [uid] . ` func populateCluster() { @@ -554,6 +556,7 @@ func populateCluster() { <202> "2009" . <202> "Prius" . <202> "プリウス"@jp . + <202> <203> . <202> "CarModel" . <202> "Object" . diff --git a/query/query4_test.go b/query/query4_test.go index e9932e77aaf..a24006cd2c2 100644 --- a/query/query4_test.go +++ b/query/query4_test.go @@ -306,7 +306,8 @@ func TestTypeExpandLang(t *testing.T) { }` js := processQueryNoErr(t, query) require.JSONEq(t, `{"data": {"q":[ - {"name": "Car", "make":"Toyota","model":"Prius", "model@jp":"プリウス", "year":2009}]}}`, js) + {"name": "Car", "make":"Toyota","model":"Prius", "model@jp":"プリウス", "year":2009, + "owner": [{"uid": "0xcb"}]}]}}`, js) } func TestTypeExpandExplicitType(t *testing.T) { @@ -318,7 +319,7 @@ func TestTypeExpandExplicitType(t *testing.T) { } }` js := processQueryNoErr(t, query) - require.JSONEq(t, `{"data": {"q":[{"name":"Car"}]}}`, js) + require.JSONEq(t, `{"data": {"q":[{"name":"Car", "owner": [{"uid": "0xcb"}]}]}}`, js) } func TestTypeExpandMultipleExplicitTypes(t *testing.T) { @@ -331,7 +332,8 @@ func TestTypeExpandMultipleExplicitTypes(t *testing.T) { }` js := processQueryNoErr(t, query) require.JSONEq(t, `{"data": {"q":[ - {"name": "Car", "make":"Toyota","model":"Prius", "model@jp":"プリウス", "year":2009}]}}`, js) + {"name": "Car", "make":"Toyota","model":"Prius", "model@jp":"プリウス", "year":2009, + "owner": [{"uid": "0xcb"}]}]}}`, js) } // Test Related to worker based pagination. diff --git a/query/query_facets_test.go b/query/query_facets_test.go index 98cc249248d..df1af008032 100644 --- a/query/query_facets_test.go +++ b/query/query_facets_test.go @@ -999,5 +999,5 @@ func TestTypeExpandFacets(t *testing.T) { js := processQueryNoErr(t, query) require.JSONEq(t, `{"data": {"q":[ {"name": "Car", "make":"Toyota","model":"Prius", "model@jp":"プリウス", - "model|type":"Electric", "year":2009}]}}`, js) + "model|type":"Electric", "year":2009, "owner": [{"uid": "0xcb"}]}]}}`, js) }