From fcf5cdb09439d05fe5dd9b5a1235e3e771bcff67 Mon Sep 17 00:00:00 2001 From: Animesh Date: Mon, 23 Sep 2019 13:51:32 +0530 Subject: [PATCH 01/25] Support @normalize directive for subqueries --- gql/parser.go | 2 ++ query/outputnode.go | 52 +++++++++++++++++++++++++++++++++++++-------- query/query.go | 2 +- 3 files changed, 46 insertions(+), 10 deletions(-) diff --git a/gql/parser.go b/gql/parser.go index 5c23e75e442..a5a7d84589b 100644 --- a/gql/parser.go +++ b/gql/parser.go @@ -2239,6 +2239,8 @@ func parseDirective(it *lex.ItemIterator, curp *GraphQuery) error { } } else if item.Val == "cascade" { curp.Cascade = true + } else if item.Val == "normalize" { + curp.Normalize = true } else if peek[0].Typ == itemLeftRound { // this is directive switch item.Val { diff --git a/query/outputnode.go b/query/outputnode.go index 88c4837621d..a69db9c73ca 100644 --- a/query/outputnode.go +++ b/query/outputnode.go @@ -61,6 +61,7 @@ type outputNode interface { AddListChild(attr string, child outputNode) New(attr string) outputNode SetUID(uid uint64, attr string) + SetFlatten(flatten bool) IsEmpty() bool addCountAtRoot(*SubGraph) @@ -84,6 +85,7 @@ type fastJsonNode struct { scalarVal []byte attrs []*fastJsonNode list bool + flatten bool } func (fj *fastJsonNode) AddValue(attr string, v types.Val) { @@ -143,6 +145,10 @@ func (fj *fastJsonNode) IsEmpty() bool { return len(fj.attrs) == 0 } +func (fj *fastJsonNode) SetFlatten(flatten bool) { + fj.flatten = flatten +} + func valToBytes(v types.Val) ([]byte, error) { switch v.Tid { case types.StringID, types.DefaultID: @@ -469,19 +475,13 @@ func processNodeUids(fj *fastJsonNode, sg *SubGraph) error { } hasChild = true - if !sg.Params.Normalize { - fj.AddListChild(sg.Params.Alias, n1) - continue - } - // Lets normalize the response now. - normalized, err := n1.(*fastJsonNode).normalize() + err := createAttrs(n1) if err != nil { return err } - for _, c := range normalized { - fj.AddListChild(sg.Params.Alias, &fastJsonNode{attrs: c}) - } + + fj.AddListChild(sg.Params.Alias, n1) } if !hasChild { @@ -613,6 +613,7 @@ func (sg *SubGraph) preTraverse(uid uint64, dst outputNode) error { sg.Params.ParentIds = append(sg.Params.ParentIds, uid) } + dst.SetFlatten(sg.Params.Normalize) var invalidUids map[uint64]bool // We go through all predicate children of the subprotos. for _, pc := range sg.Children { @@ -810,3 +811,36 @@ func (sg *SubGraph) preTraverse(uid uint64, dst outputNode) error { return nil } + +func createAttrs(oNode outputNode) error { + node := oNode.(*fastJsonNode) + + err := flattenResult(node) + if err != nil { + return err + } + + return nil +} + +func flattenResult(node *fastJsonNode) error { + if node.flatten { + attrList, err := node.normalize() + if err != nil { + return err + } + + var allAttrs []*fastJsonNode + for _, attrs := range attrList { + allAttrs = append(allAttrs, attrs...) + } + + node.attrs = allAttrs + } else { + for _, child := range node.attrs { + flattenResult(child) + } + } + + return nil +} diff --git a/query/query.go b/query/query.go index 519f237dd2b..455a5812b74 100644 --- a/query/query.go +++ b/query/query.go @@ -526,7 +526,7 @@ func treeCopy(gq *gql.GraphQuery, sg *SubGraph) error { IgnoreReflex: sg.Params.IgnoreReflex, Langs: gchild.Langs, NeedsVar: append(gchild.NeedsVar[:0:0], gchild.NeedsVar...), - Normalize: sg.Params.Normalize, + Normalize: gchild.Normalize || sg.Params.Normalize, Order: gchild.Order, Var: gchild.Var, GroupbyAttrs: gchild.GroupbyAttrs, From 5a6b831c4f7b30fe64e6d48dff72cf083f090aa1 Mon Sep 17 00:00:00 2001 From: Animesh Date: Mon, 23 Sep 2019 15:48:10 +0530 Subject: [PATCH 02/25] Address PR comments --- query/outputnode.go | 43 ++++++++++++++++--------------------------- 1 file changed, 16 insertions(+), 27 deletions(-) diff --git a/query/outputnode.go b/query/outputnode.go index a69db9c73ca..eed23036c8b 100644 --- a/query/outputnode.go +++ b/query/outputnode.go @@ -61,7 +61,7 @@ type outputNode interface { AddListChild(attr string, child outputNode) New(attr string) outputNode SetUID(uid uint64, attr string) - SetFlatten(flatten bool) + SetNormalize(isNormalized bool) IsEmpty() bool addCountAtRoot(*SubGraph) @@ -79,13 +79,13 @@ func makeScalarNode(attr string, isChild bool, val []byte, list bool) *fastJsonN } type fastJsonNode struct { - attr string - order int // relative ordering (for sorted results) - isChild bool - scalarVal []byte - attrs []*fastJsonNode - list bool - flatten bool + attr string + order int // relative ordering (for sorted results) + isChild bool + scalarVal []byte + attrs []*fastJsonNode + list bool + isNormalized bool } func (fj *fastJsonNode) AddValue(attr string, v types.Val) { @@ -145,8 +145,8 @@ func (fj *fastJsonNode) IsEmpty() bool { return len(fj.attrs) == 0 } -func (fj *fastJsonNode) SetFlatten(flatten bool) { - fj.flatten = flatten +func (fj *fastJsonNode) SetNormalize(isNormalized bool) { + fj.isNormalized = isNormalized } func valToBytes(v types.Val) ([]byte, error) { @@ -476,11 +476,9 @@ func processNodeUids(fj *fastJsonNode, sg *SubGraph) error { hasChild = true - err := createAttrs(n1) - if err != nil { + if err := flattenResult(n1.(*fastJsonNode)); err != nil { return err } - fj.AddListChild(sg.Params.Alias, n1) } @@ -613,7 +611,7 @@ func (sg *SubGraph) preTraverse(uid uint64, dst outputNode) error { sg.Params.ParentIds = append(sg.Params.ParentIds, uid) } - dst.SetFlatten(sg.Params.Normalize) + dst.SetNormalize(sg.Params.Normalize) var invalidUids map[uint64]bool // We go through all predicate children of the subprotos. for _, pc := range sg.Children { @@ -812,19 +810,8 @@ func (sg *SubGraph) preTraverse(uid uint64, dst outputNode) error { return nil } -func createAttrs(oNode outputNode) error { - node := oNode.(*fastJsonNode) - - err := flattenResult(node) - if err != nil { - return err - } - - return nil -} - func flattenResult(node *fastJsonNode) error { - if node.flatten { + if node.isNormalized { attrList, err := node.normalize() if err != nil { return err @@ -838,7 +825,9 @@ func flattenResult(node *fastJsonNode) error { node.attrs = allAttrs } else { for _, child := range node.attrs { - flattenResult(child) + if err := flattenResult(child); err != nil { + return err + } } } From e59dc9709bac7e2bda7d3d82a8e701a27a3e2e57 Mon Sep 17 00:00:00 2001 From: Animesh Date: Mon, 23 Sep 2019 17:19:01 +0530 Subject: [PATCH 03/25] Change flattenResult signature Change the signature so that it can show multiple results from a subquery. --- query/outputnode.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/query/outputnode.go b/query/outputnode.go index eed23036c8b..5f24f1adc37 100644 --- a/query/outputnode.go +++ b/query/outputnode.go @@ -476,12 +476,13 @@ func processNodeUids(fj *fastJsonNode, sg *SubGraph) error { hasChild = true - if err := flattenResult(n1.(*fastJsonNode)); err != nil { - return err - } fj.AddListChild(sg.Params.Alias, n1) } + if err := flattenResult(fj, nil, 0); err != nil { + return err + } + if !hasChild { // So that we return an empty key if the root didn't have any children. fj.AddListChild(sg.Params.Alias, &fastJsonNode{}) @@ -810,22 +811,21 @@ func (sg *SubGraph) preTraverse(uid uint64, dst outputNode) error { return nil } -func flattenResult(node *fastJsonNode) error { +func flattenResult(node, parent *fastJsonNode, childIdx int) error { if node.isNormalized { attrList, err := node.normalize() if err != nil { return err } - var allAttrs []*fastJsonNode + parent.attrs[childIdx] = &fastJsonNode{attr: node.attr, attrs: attrList[0]} + attrList = attrList[1:] for _, attrs := range attrList { - allAttrs = append(allAttrs, attrs...) + parent.attrs = append(parent.attrs, &fastJsonNode{attr: node.attr, attrs: attrs}) } - - node.attrs = allAttrs } else { - for _, child := range node.attrs { - if err := flattenResult(child); err != nil { + for idx, child := range node.attrs { + if err := flattenResult(child, node, idx); err != nil { return err } } From da985b1db77e54871474e48ac738b0bd8bf3b94e Mon Sep 17 00:00:00 2001 From: Animesh Date: Tue, 24 Sep 2019 15:00:05 +0530 Subject: [PATCH 04/25] Add tests for @normalize at subqueries --- query/query2_test.go | 73 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/query/query2_test.go b/query/query2_test.go index 18a4e423b57..1a920b2e7ce 100644 --- a/query/query2_test.go +++ b/query/query2_test.go @@ -1555,6 +1555,79 @@ func TestNormalizeDirective(t *testing.T) { js) } +func TestNormalizeDirectiveSubQueryLevel1(t *testing.T) { + + query := ` + { + me(func: uid(0x01)) { + mn: name + gender + friend @normalize { # Results of this subquery will be normalized + n: name + dob + friend { + fn : name + } + } + son { + sn: name + } + } + } + ` + + js := processQueryNoErr(t, query) + require.JSONEq(t, `{"data":{"me":[{"mn":"Michonne","gender":"female","friend":[{"fn":"Michonne","n":"Rick Grimes"},{"n":"Glenn Rhee"},{"n":"Daryl Dixon"},{"fn":"Glenn Rhee","n":"Andrea"}],"son":[{"sn":"Andre"},{"sn":"Helmut"}]}]}}`, js) +} + +func TestNormalizeDirectiveSubQueryLevel2(t *testing.T) { + query := ` + { + me(func: uid(0x01)) { + mn: name + gender + friend { + n: name + dob + friend @normalize { # Results of this subquery will be normalized + fn : name + } + } + son { + sn: name + } + } + } + ` + + js := processQueryNoErr(t, query) + require.JSONEq(t, `{"data":{"me":[{"mn":"Michonne","gender":"female","friend":[{"n":"Rick Grimes","dob":"1910-01-02T00:00:00Z","friend":{"fn":"Michonne"}},{"n":"Glenn Rhee","dob":"1909-05-05T00:00:00Z"},{"n":"Daryl Dixon","dob":"1909-01-10T00:00:00Z"},{"n":"Andrea","dob":"1901-01-15T00:00:00Z","friend":{"fn":"Glenn Rhee"}}],"son":[{"sn":"Andre"},{"sn":"Helmut"}]}]}}`, js) +} + +func TestNormalizeDirectiveRootSubQueryLevel2(t *testing.T) { + query := ` + { + me(func: uid(0x01)) @normalize { # Results of this query will be normalized + mn: name + gender + friend { + n: name + dob + friend @normalize { # It would appear as if @normalize isn't here. + fn : name + } + } + son { + sn: name + } + } + } + ` + + js := processQueryNoErr(t, query) + require.JSONEq(t, `{"data":{"me":[{"fn":"Michonne","mn":"Michonne","n":"Rick Grimes","sn":"Andre"},{"fn":"Michonne","mn":"Michonne","n":"Rick Grimes","sn":"Helmut"},{"mn":"Michonne","n":"Glenn Rhee","sn":"Andre"},{"mn":"Michonne","n":"Glenn Rhee","sn":"Helmut"},{"mn":"Michonne","n":"Daryl Dixon","sn":"Andre"},{"mn":"Michonne","n":"Daryl Dixon","sn":"Helmut"},{"fn":"Glenn Rhee","mn":"Michonne","n":"Andrea","sn":"Andre"},{"fn":"Glenn Rhee","mn":"Michonne","n":"Andrea","sn":"Helmut"}]}}`, js) +} + func TestNearPoint(t *testing.T) { query := `{ From 4e511d219b30112b44b05410c9d3eb811b7f7c7a Mon Sep 17 00:00:00 2001 From: Animesh Date: Tue, 24 Sep 2019 15:02:33 +0530 Subject: [PATCH 05/25] Fix TestReflexive3 Relative order or results has changed in the json array. --- query/query1_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/query/query1_test.go b/query/query1_test.go index 2ef9d560912..e44a7adbc81 100644 --- a/query/query1_test.go +++ b/query/query1_test.go @@ -1064,7 +1064,7 @@ func TestReflexive3(t *testing.T) { } }` js := processQueryNoErr(t, query) - require.JSONEq(t, `{"data": {"me":[{"Friend":"Rick Grimes","Me":"Michonne"},{"Friend":"Glenn Rhee","Me":"Michonne"},{"Friend":"Daryl Dixon","Me":"Michonne"},{"Cofriend":"Glenn Rhee","Friend":"Andrea","Me":"Michonne"},{"Cofriend":"Glenn Rhee","Friend":"Michonne","Me":"Rick Grimes"},{"Cofriend":"Daryl Dixon","Friend":"Michonne","Me":"Rick Grimes"},{"Cofriend":"Andrea","Friend":"Michonne","Me":"Rick Grimes"},{"Me":"Daryl Dixon"}]}}`, js) + require.JSONEq(t, `{"data":{"me":[{"Friend":"Rick Grimes","Me":"Michonne"},{"Cofriend":"Glenn Rhee","Friend":"Michonne","Me":"Rick Grimes"},{"Me":"Daryl Dixon"},{"Friend":"Glenn Rhee","Me":"Michonne"},{"Friend":"Daryl Dixon","Me":"Michonne"},{"Cofriend":"Glenn Rhee","Friend":"Andrea","Me":"Michonne"},{"Cofriend":"Daryl Dixon","Friend":"Michonne","Me":"Rick Grimes"},{"Cofriend":"Andrea","Friend":"Michonne","Me":"Rick Grimes"}]}}`, js) } func TestCascadeUid(t *testing.T) { From a924a78d76c2b92264040c1e1a48bbc531037bed Mon Sep 17 00:00:00 2001 From: Animesh Date: Tue, 24 Sep 2019 15:04:40 +0530 Subject: [PATCH 06/25] Change function name --- query/outputnode.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/query/outputnode.go b/query/outputnode.go index 5f24f1adc37..1823e8e2836 100644 --- a/query/outputnode.go +++ b/query/outputnode.go @@ -479,7 +479,7 @@ func processNodeUids(fj *fastJsonNode, sg *SubGraph) error { fj.AddListChild(sg.Params.Alias, n1) } - if err := flattenResult(fj, nil, 0); err != nil { + if err := normalizeResult(fj, nil, 0); err != nil { return err } @@ -811,7 +811,7 @@ func (sg *SubGraph) preTraverse(uid uint64, dst outputNode) error { return nil } -func flattenResult(node, parent *fastJsonNode, childIdx int) error { +func normalizeResult(node, parent *fastJsonNode, childIdx int) error { if node.isNormalized { attrList, err := node.normalize() if err != nil { @@ -825,7 +825,7 @@ func flattenResult(node, parent *fastJsonNode, childIdx int) error { } } else { for idx, child := range node.attrs { - if err := flattenResult(child, node, idx); err != nil { + if err := normalizeResult(child, node, idx); err != nil { return err } } From e42dcd5408417b7d9abde84af1934038bd264613 Mon Sep 17 00:00:00 2001 From: Animesh Date: Tue, 24 Sep 2019 17:58:55 +0530 Subject: [PATCH 07/25] Set isChild parameters while creating new nodes When creating new fastJSONNodes set isChild parameters. Also change the previous output of TestNormalizeSubqueryLevel2 --- query/outputnode.go | 2 +- query/query2_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/query/outputnode.go b/query/outputnode.go index 1823e8e2836..6646b3cc84e 100644 --- a/query/outputnode.go +++ b/query/outputnode.go @@ -818,7 +818,7 @@ func normalizeResult(node, parent *fastJsonNode, childIdx int) error { return err } - parent.attrs[childIdx] = &fastJsonNode{attr: node.attr, attrs: attrList[0]} + parent.attrs[childIdx] = &fastJsonNode{attr: node.attr, attrs: attrList[0], isChild: node.isChild} attrList = attrList[1:] for _, attrs := range attrList { parent.attrs = append(parent.attrs, &fastJsonNode{attr: node.attr, attrs: attrs}) diff --git a/query/query2_test.go b/query/query2_test.go index 1a920b2e7ce..0d722953bc5 100644 --- a/query/query2_test.go +++ b/query/query2_test.go @@ -1601,7 +1601,7 @@ func TestNormalizeDirectiveSubQueryLevel2(t *testing.T) { ` js := processQueryNoErr(t, query) - require.JSONEq(t, `{"data":{"me":[{"mn":"Michonne","gender":"female","friend":[{"n":"Rick Grimes","dob":"1910-01-02T00:00:00Z","friend":{"fn":"Michonne"}},{"n":"Glenn Rhee","dob":"1909-05-05T00:00:00Z"},{"n":"Daryl Dixon","dob":"1909-01-10T00:00:00Z"},{"n":"Andrea","dob":"1901-01-15T00:00:00Z","friend":{"fn":"Glenn Rhee"}}],"son":[{"sn":"Andre"},{"sn":"Helmut"}]}]}}`, js) + require.JSONEq(t, `{"data":{"me":[{"mn":"Michonne","gender":"female","friend":[{"n":"Rick Grimes","dob":"1910-01-02T00:00:00Z","friend":[{"fn":"Michonne"}]},{"n":"Glenn Rhee","dob":"1909-05-05T00:00:00Z"},{"n":"Daryl Dixon","dob":"1909-01-10T00:00:00Z"},{"n":"Andrea","dob":"1901-01-15T00:00:00Z","friend":[{"fn":"Glenn Rhee"}]}],"son":[{"sn":"Andre"},{"sn":"Helmut"}]}]}}`, js) } func TestNormalizeDirectiveRootSubQueryLevel2(t *testing.T) { From 3d99352d1c414c73c778a601553f4a4f4e0654ae Mon Sep 17 00:00:00 2001 From: Animesh Date: Tue, 24 Sep 2019 18:12:57 +0530 Subject: [PATCH 08/25] Minor style changes --- query/outputnode.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/query/outputnode.go b/query/outputnode.go index 6646b3cc84e..12c0a34bdac 100644 --- a/query/outputnode.go +++ b/query/outputnode.go @@ -818,10 +818,14 @@ func normalizeResult(node, parent *fastJsonNode, childIdx int) error { return err } - parent.attrs[childIdx] = &fastJsonNode{attr: node.attr, attrs: attrList[0], isChild: node.isChild} - attrList = attrList[1:] - for _, attrs := range attrList { - parent.attrs = append(parent.attrs, &fastJsonNode{attr: node.attr, attrs: attrs}) + parent.attrs[childIdx] = &fastJsonNode{ + attr: node.attr, + attrs: attrList[0], + isChild: node.isChild, + } + for _, attrs := range attrList[1:] { + parent.attrs = append(parent.attrs, + &fastJsonNode{attr: node.attr, attrs: attrs}) } } else { for idx, child := range node.attrs { From f155e41a45487c6620b7ff5ec613a280ebcef36c Mon Sep 17 00:00:00 2001 From: Animesh Date: Tue, 24 Sep 2019 18:51:27 +0530 Subject: [PATCH 09/25] Add documentation for extended normalize --- wiki/content/query-language/index.md | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/wiki/content/query-language/index.md b/wiki/content/query-language/index.md index 2b2f84df610..a3e269907d0 100644 --- a/wiki/content/query-language/index.md +++ b/wiki/content/query-language/index.md @@ -1916,6 +1916,30 @@ Query Example: Film name, country and first two actors (by UID order) of every S } {{< /runnable >}} +You can also apply `@normalize` on nested query blocks. It will work similarly but only flatten the result of nested query block where `@normalize` has been applied. +{{< runnable >}} +{ + director(func:allofterms(name@en, "steven spielberg")) { + director: name@en + director.film { + film: name@en + initial_release_date + starring(first: 2) @normalize { + performance.actor { + actor: name@en + } + performance.character { + character: name@en + } + } + country { + country: name@en + } + } + } +} +{{< /runnable >}} + ## Ignorereflex directive From 9e77e2d1a392176e6785007c5e1eb7376a1705a1 Mon Sep 17 00:00:00 2001 From: Animesh Date: Tue, 24 Sep 2019 20:16:55 +0530 Subject: [PATCH 10/25] Copy isChild attribute and change json formats --- query/outputnode.go | 16 ++-- query/query2_test.go | 206 ++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 206 insertions(+), 16 deletions(-) diff --git a/query/outputnode.go b/query/outputnode.go index 12c0a34bdac..dbf27d08f32 100644 --- a/query/outputnode.go +++ b/query/outputnode.go @@ -479,6 +479,9 @@ func processNodeUids(fj *fastJsonNode, sg *SubGraph) error { fj.AddListChild(sg.Params.Alias, n1) } + // fj is a dummy node, it doesn't have any parent. So passing nil + // for parent and 0 for childIdx. Since we are not setting isNormalized + // in dummy node, we can safely call normalizeResult for fj. if err := normalizeResult(fj, nil, 0); err != nil { return err } @@ -811,6 +814,9 @@ func (sg *SubGraph) preTraverse(uid uint64, dst outputNode) error { return nil } +// normalizeResult function recursively flattens inner nodes. If isNormalized is set +// for a node, it removes the node from it's parent and attaches it's normalized attributes. +// It doesn't do recursive call for nodes whose isNormalized is set. func normalizeResult(node, parent *fastJsonNode, childIdx int) error { if node.isNormalized { attrList, err := node.normalize() @@ -818,14 +824,14 @@ func normalizeResult(node, parent *fastJsonNode, childIdx int) error { return err } - parent.attrs[childIdx] = &fastJsonNode{ - attr: node.attr, - attrs: attrList[0], - isChild: node.isChild, + parent.attrs[childIdx] = &fastJsonNode{ // A small optimization. Instead of removing + attr: node.attr, // element from slice, we are replacing it + attrs: attrList[0], // with one of the node that we intend to + isChild: node.isChild, // attach to parent. } for _, attrs := range attrList[1:] { parent.attrs = append(parent.attrs, - &fastJsonNode{attr: node.attr, attrs: attrs}) + &fastJsonNode{attr: node.attr, attrs: attrs, isChild: node.isChild}) } } else { for idx, child := range node.attrs { diff --git a/query/query2_test.go b/query/query2_test.go index 0d722953bc5..323cf962cfc 100644 --- a/query/query2_test.go +++ b/query/query2_test.go @@ -1546,17 +1546,71 @@ func TestNormalizeDirective(t *testing.T) { sn: name } } - } - ` - - js := processQueryNoErr(t, query) - require.JSONEq(t, - `{"data": {"me":[{"d":"1910-01-02T00:00:00Z","fn":"Michonne","mn":"Michonne","n":"Rick Grimes","sn":"Andre"},{"d":"1910-01-02T00:00:00Z","fn":"Michonne","mn":"Michonne","n":"Rick Grimes","sn":"Helmut"},{"d":"1909-05-05T00:00:00Z","mn":"Michonne","n":"Glenn Rhee","sn":"Andre"},{"d":"1909-05-05T00:00:00Z","mn":"Michonne","n":"Glenn Rhee","sn":"Helmut"},{"d":"1909-01-10T00:00:00Z","mn":"Michonne","n":"Daryl Dixon","sn":"Andre"},{"d":"1909-01-10T00:00:00Z","mn":"Michonne","n":"Daryl Dixon","sn":"Helmut"},{"d":"1901-01-15T00:00:00Z","fn":"Glenn Rhee","mn":"Michonne","n":"Andrea","sn":"Andre"},{"d":"1901-01-15T00:00:00Z","fn":"Glenn Rhee","mn":"Michonne","n":"Andrea","sn":"Helmut"}]}}`, - js) + }` + + js := processQueryNoErr(t, query) + require.JSONEq(t, ` + { + "data": { + "me": [ + { + "d": "1910-01-02T00:00:00Z", + "fn": "Michonne", + "mn": "Michonne", + "n": "Rick Grimes", + "sn": "Andre" + }, + { + "d": "1910-01-02T00:00:00Z", + "fn": "Michonne", + "mn": "Michonne", + "n": "Rick Grimes", + "sn": "Helmut" + }, + { + "d": "1909-05-05T00:00:00Z", + "mn": "Michonne", + "n": "Glenn Rhee", + "sn": "Andre" + }, + { + "d": "1909-05-05T00:00:00Z", + "mn": "Michonne", + "n": "Glenn Rhee", + "sn": "Helmut" + }, + { + "d": "1909-01-10T00:00:00Z", + "mn": "Michonne", + "n": "Daryl Dixon", + "sn": "Andre" + }, + { + "d": "1909-01-10T00:00:00Z", + "mn": "Michonne", + "n": "Daryl Dixon", + "sn": "Helmut" + }, + { + "d": "1901-01-15T00:00:00Z", + "fn": "Glenn Rhee", + "mn": "Michonne", + "n": "Andrea", + "sn": "Andre" + }, + { + "d": "1901-01-15T00:00:00Z", + "fn": "Glenn Rhee", + "mn": "Michonne", + "n": "Andrea", + "sn": "Helmut" + } + ] + } + }`, js) } func TestNormalizeDirectiveSubQueryLevel1(t *testing.T) { - query := ` { me(func: uid(0x01)) { @@ -1577,7 +1631,41 @@ func TestNormalizeDirectiveSubQueryLevel1(t *testing.T) { ` js := processQueryNoErr(t, query) - require.JSONEq(t, `{"data":{"me":[{"mn":"Michonne","gender":"female","friend":[{"fn":"Michonne","n":"Rick Grimes"},{"n":"Glenn Rhee"},{"n":"Daryl Dixon"},{"fn":"Glenn Rhee","n":"Andrea"}],"son":[{"sn":"Andre"},{"sn":"Helmut"}]}]}}`, js) + require.JSONEq(t, ` + { + "data": { + "me": [ + { + "mn": "Michonne", + "gender": "female", + "friend": [ + { + "fn": "Michonne", + "n": "Rick Grimes" + }, + { + "n": "Glenn Rhee" + }, + { + "n": "Daryl Dixon" + }, + { + "fn": "Glenn Rhee", + "n": "Andrea" + } + ], + "son": [ + { + "sn": "Andre" + }, + { + "sn": "Helmut" + } + ] + } + ] + } + }`, js) } func TestNormalizeDirectiveSubQueryLevel2(t *testing.T) { @@ -1601,7 +1689,53 @@ func TestNormalizeDirectiveSubQueryLevel2(t *testing.T) { ` js := processQueryNoErr(t, query) - require.JSONEq(t, `{"data":{"me":[{"mn":"Michonne","gender":"female","friend":[{"n":"Rick Grimes","dob":"1910-01-02T00:00:00Z","friend":[{"fn":"Michonne"}]},{"n":"Glenn Rhee","dob":"1909-05-05T00:00:00Z"},{"n":"Daryl Dixon","dob":"1909-01-10T00:00:00Z"},{"n":"Andrea","dob":"1901-01-15T00:00:00Z","friend":[{"fn":"Glenn Rhee"}]}],"son":[{"sn":"Andre"},{"sn":"Helmut"}]}]}}`, js) + require.JSONEq(t, ` + { + "data": { + "me": [ + { + "mn": "Michonne", + "gender": "female", + "friend": [ + { + "n": "Rick Grimes", + "dob": "1910-01-02T00:00:00Z", + "friend": [ + { + "fn": "Michonne" + } + ] + }, + { + "n": "Glenn Rhee", + "dob": "1909-05-05T00:00:00Z" + }, + { + "n": "Daryl Dixon", + "dob": "1909-01-10T00:00:00Z" + }, + { + "n": "Andrea", + "dob": "1901-01-15T00:00:00Z", + "friend": [ + { + "fn": "Glenn Rhee" + } + ] + } + ], + "son": [ + { + "sn": "Andre" + }, + { + "sn": "Helmut" + } + ] + } + ] + } + }`, js) } func TestNormalizeDirectiveRootSubQueryLevel2(t *testing.T) { @@ -1625,7 +1759,57 @@ func TestNormalizeDirectiveRootSubQueryLevel2(t *testing.T) { ` js := processQueryNoErr(t, query) - require.JSONEq(t, `{"data":{"me":[{"fn":"Michonne","mn":"Michonne","n":"Rick Grimes","sn":"Andre"},{"fn":"Michonne","mn":"Michonne","n":"Rick Grimes","sn":"Helmut"},{"mn":"Michonne","n":"Glenn Rhee","sn":"Andre"},{"mn":"Michonne","n":"Glenn Rhee","sn":"Helmut"},{"mn":"Michonne","n":"Daryl Dixon","sn":"Andre"},{"mn":"Michonne","n":"Daryl Dixon","sn":"Helmut"},{"fn":"Glenn Rhee","mn":"Michonne","n":"Andrea","sn":"Andre"},{"fn":"Glenn Rhee","mn":"Michonne","n":"Andrea","sn":"Helmut"}]}}`, js) + require.JSONEq(t, ` + { + "data": { + "me": [ + { + "fn": "Michonne", + "mn": "Michonne", + "n": "Rick Grimes", + "sn": "Andre" + }, + { + "fn": "Michonne", + "mn": "Michonne", + "n": "Rick Grimes", + "sn": "Helmut" + }, + { + "mn": "Michonne", + "n": "Glenn Rhee", + "sn": "Andre" + }, + { + "mn": "Michonne", + "n": "Glenn Rhee", + "sn": "Helmut" + }, + { + "mn": "Michonne", + "n": "Daryl Dixon", + "sn": "Andre" + }, + { + "mn": "Michonne", + "n": "Daryl Dixon", + "sn": "Helmut" + }, + { + "fn": "Glenn Rhee", + "mn": "Michonne", + "n": "Andrea", + "sn": "Andre" + }, + { + "fn": "Glenn Rhee", + "mn": "Michonne", + "n": "Andrea", + "sn": "Helmut" + } + ] + } + }`, js) } func TestNearPoint(t *testing.T) { From a1f12502c6057190b7f98e104a88caa2a6556627 Mon Sep 17 00:00:00 2001 From: Animesh Date: Wed, 25 Sep 2019 18:50:04 +0530 Subject: [PATCH 11/25] Style changes --- query/outputnode.go | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/query/outputnode.go b/query/outputnode.go index dbf27d08f32..a68aedbe177 100644 --- a/query/outputnode.go +++ b/query/outputnode.go @@ -85,7 +85,7 @@ type fastJsonNode struct { scalarVal []byte attrs []*fastJsonNode list bool - isNormalized bool + isNormalized bool // true for the nodes with `@normalize` directive and all it's children. } func (fj *fastJsonNode) AddValue(attr string, v types.Val) { @@ -824,20 +824,24 @@ func normalizeResult(node, parent *fastJsonNode, childIdx int) error { return err } - parent.attrs[childIdx] = &fastJsonNode{ // A small optimization. Instead of removing - attr: node.attr, // element from slice, we are replacing it - attrs: attrList[0], // with one of the node that we intend to - isChild: node.isChild, // attach to parent. + // A small optimization. Instead of removing element from slice, we are replacing it + // with one of the node that we intend to attach to parent. + parent.attrs[childIdx] = &fastJsonNode{ + attr: node.attr, + attrs: attrList[0], + isChild: node.isChild, } for _, attrs := range attrList[1:] { parent.attrs = append(parent.attrs, &fastJsonNode{attr: node.attr, attrs: attrs, isChild: node.isChild}) } - } else { - for idx, child := range node.attrs { - if err := normalizeResult(child, node, idx); err != nil { - return err - } + + return nil + } + + for idx, child := range node.attrs { + if err := normalizeResult(child, node, idx); err != nil { + return err } } From a4a5ab39218b5ca26af8f6c9fcfcb1ee32bbcc56 Mon Sep 17 00:00:00 2001 From: Animesh Date: Wed, 25 Sep 2019 20:19:50 +0530 Subject: [PATCH 12/25] More tests --- query/query2_test.go | 379 ++++++++++++++++++++++++++++++++++++++----- 1 file changed, 339 insertions(+), 40 deletions(-) diff --git a/query/query2_test.go b/query/query2_test.go index 323cf962cfc..006b51176b5 100644 --- a/query/query2_test.go +++ b/query/query2_test.go @@ -1679,6 +1679,10 @@ func TestNormalizeDirectiveSubQueryLevel2(t *testing.T) { dob friend @normalize { # Results of this subquery will be normalized fn : name + dob + friend { + ffn: name + } } } son { @@ -1692,48 +1696,61 @@ func TestNormalizeDirectiveSubQueryLevel2(t *testing.T) { require.JSONEq(t, ` { "data": { - "me": [ - { - "mn": "Michonne", - "gender": "female", - "friend": [ - { - "n": "Rick Grimes", - "dob": "1910-01-02T00:00:00Z", - "friend": [ - { - "fn": "Michonne" - } - ] - }, + "me": [ { - "n": "Glenn Rhee", - "dob": "1909-05-05T00:00:00Z" - }, - { - "n": "Daryl Dixon", - "dob": "1909-01-10T00:00:00Z" - }, - { - "n": "Andrea", - "dob": "1901-01-15T00:00:00Z", - "friend": [ - { - "fn": "Glenn Rhee" - } - ] + "friend": [ + { + "dob": "1910-01-02T00:00:00Z", + "friend": [ + { + "ffn": "Rick Grimes", + "fn": "Michonne" + }, + { + "ffn": "Glenn Rhee", + "fn": "Michonne" + }, + { + "ffn": "Daryl Dixon", + "fn": "Michonne" + }, + { + "ffn": "Andrea", + "fn": "Michonne" + } + ], + "n": "Rick Grimes" + }, + { + "dob": "1909-05-05T00:00:00Z", + "n": "Glenn Rhee" + }, + { + "dob": "1909-01-10T00:00:00Z", + "n": "Daryl Dixon" + }, + { + "dob": "1901-01-15T00:00:00Z", + "friend": [ + { + "fn": "Glenn Rhee" + } + ], + "n": "Andrea" + } + ], + "gender": "female", + "mn": "Michonne", + "son": [ + { + "sn": "Andre" + }, + { + "sn": "Helmut" + } + ] } - ], - "son": [ - { - "sn": "Andre" - }, - { - "sn": "Helmut" - } - ] - } - ] + ] } }`, js) } @@ -1812,6 +1829,288 @@ func TestNormalizeDirectiveRootSubQueryLevel2(t *testing.T) { }`, js) } +func TestNormalizeDirectiveSubQueryLevel1MultipleUIDs(t *testing.T) { + query := ` + { + me(func: uid(1, 23)) { + mn: name + gender + friend @normalize { # Results of this subquery will be normalized + n: name + dob + friend { + fn : name + } + } + son { + sn: name + } + } + } + ` + + js := processQueryNoErr(t, query) + require.JSONEq(t, ` + { + "data": { + "me": [ + { + "friend": [ + { + "fn": "Michonne", + "n": "Rick Grimes" + }, + { + "n": "Glenn Rhee" + }, + { + "n": "Daryl Dixon" + }, + { + "fn": "Glenn Rhee", + "n": "Andrea" + } + ], + "gender": "female", + "mn": "Michonne", + "son": [ + { + "sn": "Andre" + }, + { + "sn": "Helmut" + } + ] + }, + { + "friend": [ + { + "fn": "Rick Grimes", + "n": "Michonne" + }, + { + "fn": "Glenn Rhee", + "n": "Michonne" + }, + { + "fn": "Daryl Dixon", + "n": "Michonne" + }, + { + "fn": "Andrea", + "n": "Michonne" + } + ], + "gender": "male", + "mn": "Rick Grimes" + } + ] + } + }`, js) +} + +func TestNormalizeDirectiveMultipleSubQueryLevel1(t *testing.T) { + query := ` + { + me(func: uid(1, 23)) { + mn: name + gender + friend @normalize { + fn: name + dob + friend { + ffn : name + } + } + follow @normalize { + foln: name + friend { + fofn: name + } + } + } + } + ` + + js := processQueryNoErr(t, query) + require.JSONEq(t, ` + { + "data": { + "me": [ + { + "follow": [ + { + "foln": "Glenn Rhee" + }, + { + "fofn": "Glenn Rhee", + "foln": "Andrea" + } + ], + "friend": [ + { + "ffn": "Michonne", + "fn": "Rick Grimes" + }, + { + "fn": "Glenn Rhee" + }, + { + "fn": "Daryl Dixon" + }, + { + "ffn": "Glenn Rhee", + "fn": "Andrea" + } + ], + "gender": "female", + "mn": "Michonne" + }, + { + "friend": [ + { + "ffn": "Rick Grimes", + "fn": "Michonne" + }, + { + "ffn": "Glenn Rhee", + "fn": "Michonne" + }, + { + "ffn": "Daryl Dixon", + "fn": "Michonne" + }, + { + "ffn": "Andrea", + "fn": "Michonne" + } + ], + "gender": "male", + "mn": "Rick Grimes" + } + ] + } + }`, js) +} + +func TestNormalizeDirectiveMultipleQuery(t *testing.T) { + query := ` + { + me(func: uid(1)) @normalize { + mn: name + gender + friend { # Results of this subquery will be normalized + n: name + dob + friend { + fn : name + } + } + son { + sn: name + } + } + me2(func: uid(1)) { + mn: name + gender + friend @normalize { # Results of this subquery will be normalized + n: name + dob + friend { + fn : name + } + } + son { + sn: name + } + } + } + ` + + js := processQueryNoErr(t, query) + require.JSONEq(t, ` + { + "data": { + "me": [ + { + "fn": "Michonne", + "mn": "Michonne", + "n": "Rick Grimes", + "sn": "Andre" + }, + { + "fn": "Michonne", + "mn": "Michonne", + "n": "Rick Grimes", + "sn": "Helmut" + }, + { + "mn": "Michonne", + "n": "Glenn Rhee", + "sn": "Andre" + }, + { + "mn": "Michonne", + "n": "Glenn Rhee", + "sn": "Helmut" + }, + { + "mn": "Michonne", + "n": "Daryl Dixon", + "sn": "Andre" + }, + { + "mn": "Michonne", + "n": "Daryl Dixon", + "sn": "Helmut" + }, + { + "fn": "Glenn Rhee", + "mn": "Michonne", + "n": "Andrea", + "sn": "Andre" + }, + { + "fn": "Glenn Rhee", + "mn": "Michonne", + "n": "Andrea", + "sn": "Helmut" + } + ], + "me2": [ + { + "friend": [ + { + "fn": "Michonne", + "n": "Rick Grimes" + }, + { + "n": "Glenn Rhee" + }, + { + "n": "Daryl Dixon" + }, + { + "fn": "Glenn Rhee", + "n": "Andrea" + } + ], + "gender": "female", + "mn": "Michonne", + "son": [ + { + "sn": "Andre" + }, + { + "sn": "Helmut" + } + ] + } + ] + } + }`, js) +} + func TestNearPoint(t *testing.T) { query := `{ From bc5211115d0aab74d7ed842ea8a07df8a27d5185 Mon Sep 17 00:00:00 2001 From: animesh Date: Fri, 27 Sep 2019 18:11:46 +0530 Subject: [PATCH 13/25] Add benchmark for normalizeResult --- query/outputnode_test.go | 50 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/query/outputnode_test.go b/query/outputnode_test.go index 2a6836b326c..2570e864afe 100644 --- a/query/outputnode_test.go +++ b/query/outputnode_test.go @@ -95,6 +95,56 @@ func TestNormalizeJSONLimit(t *testing.T) { require.Error(t, err, "Couldn't evaluate @normalize directive - too many results") } +func BenchmarkNormalizePerformance(b *testing.B) { + nodeHeight := 3 + treeDepth := 3 + + b.Run("Old Normalize", func(b *testing.B) { + for i := 0; i < b.N; i++ { + root := (&fastJsonNode{}).New("_root_") + fillNode(root.(*fastJsonNode), nodeHeight, treeDepth) + + b.StartTimer() + _, _ = root.(*fastJsonNode).normalize() + b.StopTimer() + } + }) + + b.Run("New Normalize", func(b *testing.B) { + for i := 0; i < b.N; i++ { + root := (&fastJsonNode{}).New("_root_") + rootChild := root.New("_rootchild") + root.AddListChild("_rootchild_", rootChild) + fillNode(rootChild.(*fastJsonNode), nodeHeight, treeDepth) + rootChild.(*fastJsonNode).isNormalized = true + + b.StartTimer() + _ = normalizeResult(rootChild.(*fastJsonNode), root.(*fastJsonNode), 0) + b.StopTimer() + } + }) + + fmt.Println("done") +} + +func fillNode(node *fastJsonNode, nodeHeight, treeDepth int) { + if treeDepth <= 0 { + return + } + + i := 0 + for ; i < nodeHeight/2; i++ { + node.AddValue(fmt.Sprintf("Attribute %d", i), types.ValueForType(types.StringID)) + } + + for ; i < nodeHeight; i++ { + child := node.New(fmt.Sprintf("Child%d", i)) + node.AddListChild(fmt.Sprintf("Child%d", i), child) + node.AddValue(fmt.Sprintf("Attribute %d", i), types.ValueForType(types.StringID)) + fillNode(child.(*fastJsonNode), nodeHeight, treeDepth-1) + } +} + func TestNormalizeJSONUid1(t *testing.T) { // Set default normalize limit. x.Config.NormalizeNodeLimit = 1e4 From d6bbe1ca4e45df074d4b7850f8d98c7f4e89feed Mon Sep 17 00:00:00 2001 From: animesh Date: Fri, 27 Sep 2019 18:16:10 +0530 Subject: [PATCH 14/25] Minor changes --- query/query2_test.go | 2 +- wiki/content/query-language/index.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/query/query2_test.go b/query/query2_test.go index 006b51176b5..8d17dcaae32 100644 --- a/query/query2_test.go +++ b/query/query2_test.go @@ -1764,7 +1764,7 @@ func TestNormalizeDirectiveRootSubQueryLevel2(t *testing.T) { friend { n: name dob - friend @normalize { # It would appear as if @normalize isn't here. + friend @normalize { # This would be ignored. fn : name } } diff --git a/wiki/content/query-language/index.md b/wiki/content/query-language/index.md index a3e269907d0..faf062ff268 100644 --- a/wiki/content/query-language/index.md +++ b/wiki/content/query-language/index.md @@ -1916,7 +1916,7 @@ Query Example: Film name, country and first two actors (by UID order) of every S } {{< /runnable >}} -You can also apply `@normalize` on nested query blocks. It will work similarly but only flatten the result of nested query block where `@normalize` has been applied. +You can also apply `@normalize` on nested query blocks. It will work similarly but only flatten the result of the nested query block where `@normalize` has been applied. {{< runnable >}} { director(func:allofterms(name@en, "steven spielberg")) { From 089bdd8cb734661f264939c92e5bd962e59c2837 Mon Sep 17 00:00:00 2001 From: Animesh Date: Mon, 30 Sep 2019 20:21:07 +0530 Subject: [PATCH 15/25] Normalize while preTraverse --- query/outputnode.go | 94 ++++++++++++-------------------- query/outputnode_test.go | 115 --------------------------------------- query/query1_test.go | 3 +- 3 files changed, 36 insertions(+), 176 deletions(-) diff --git a/query/outputnode.go b/query/outputnode.go index a68aedbe177..488ebc3658c 100644 --- a/query/outputnode.go +++ b/query/outputnode.go @@ -61,7 +61,6 @@ type outputNode interface { AddListChild(attr string, child outputNode) New(attr string) outputNode SetUID(uid uint64, attr string) - SetNormalize(isNormalized bool) IsEmpty() bool addCountAtRoot(*SubGraph) @@ -79,13 +78,12 @@ func makeScalarNode(attr string, isChild bool, val []byte, list bool) *fastJsonN } type fastJsonNode struct { - attr string - order int // relative ordering (for sorted results) - isChild bool - scalarVal []byte - attrs []*fastJsonNode - list bool - isNormalized bool // true for the nodes with `@normalize` directive and all it's children. + attr string + order int // relative ordering (for sorted results) + isChild bool + scalarVal []byte + attrs []*fastJsonNode + list bool } func (fj *fastJsonNode) AddValue(attr string, v types.Val) { @@ -145,10 +143,6 @@ func (fj *fastJsonNode) IsEmpty() bool { return len(fj.attrs) == 0 } -func (fj *fastJsonNode) SetNormalize(isNormalized bool) { - fj.isNormalized = isNormalized -} - func valToBytes(v types.Val) ([]byte, error) { switch v.Tid { case types.StringID, types.DefaultID: @@ -334,11 +328,8 @@ func (fj *fastJsonNode) normalize() ([][]*fastJsonNode, error) { } childSlice := make([][]*fastJsonNode, 0, 5) for ci < len(fj.attrs) && childNode.attr == fj.attrs[ci].attr { - normalized, err := fj.attrs[ci].normalize() - if err != nil { - return nil, err - } - childSlice = append(childSlice, normalized...) + normalized := fj.attrs[ci].attrs + childSlice = append(childSlice, normalized) ci++ } // Merging with parent. @@ -475,15 +466,21 @@ func processNodeUids(fj *fastJsonNode, sg *SubGraph) error { } hasChild = true + if !sg.Params.Normalize { + fj.AddListChild(sg.Params.Alias, n1) + continue + } - fj.AddListChild(sg.Params.Alias, n1) - } + // Lets normalize the response now. + normalized, err := n1.(*fastJsonNode).normalize() + if err != nil { + return err + } + + for _, c := range normalized { + fj.AddListChild(sg.Params.Alias, &fastJsonNode{attrs: c}) + } - // fj is a dummy node, it doesn't have any parent. So passing nil - // for parent and 0 for childIdx. Since we are not setting isNormalized - // in dummy node, we can safely call normalizeResult for fj. - if err := normalizeResult(fj, nil, 0); err != nil { - return err } if !hasChild { @@ -615,7 +612,6 @@ func (sg *SubGraph) preTraverse(uid uint64, dst outputNode) error { sg.Params.ParentIds = append(sg.Params.ParentIds, uid) } - dst.SetNormalize(sg.Params.Normalize) var invalidUids map[uint64]bool // We go through all predicate children of the subprotos. for _, pc := range sg.Children { @@ -712,7 +708,19 @@ func (sg *SubGraph) preTraverse(uid uint64, dst outputNode) error { uc.SetUID(childUID, "uid") } if pc.List { - dst.AddListChild(fieldName, uc) + if pc.Params.Normalize { + normalized, err := uc.(*fastJsonNode).normalize() + if err != nil { + return err + } + + for _, c := range normalized { + dst.AddListChild(fieldName, + &fastJsonNode{attrs: c}) + } + } else { + dst.AddListChild(fieldName, uc) + } } else { dst.AddMapChild(fieldName, uc, false) } @@ -813,37 +821,3 @@ func (sg *SubGraph) preTraverse(uid uint64, dst outputNode) error { return nil } - -// normalizeResult function recursively flattens inner nodes. If isNormalized is set -// for a node, it removes the node from it's parent and attaches it's normalized attributes. -// It doesn't do recursive call for nodes whose isNormalized is set. -func normalizeResult(node, parent *fastJsonNode, childIdx int) error { - if node.isNormalized { - attrList, err := node.normalize() - if err != nil { - return err - } - - // A small optimization. Instead of removing element from slice, we are replacing it - // with one of the node that we intend to attach to parent. - parent.attrs[childIdx] = &fastJsonNode{ - attr: node.attr, - attrs: attrList[0], - isChild: node.isChild, - } - for _, attrs := range attrList[1:] { - parent.attrs = append(parent.attrs, - &fastJsonNode{attr: node.attr, attrs: attrs, isChild: node.isChild}) - } - - return nil - } - - for idx, child := range node.attrs { - if err := normalizeResult(child, node, idx); err != nil { - return err - } - } - - return nil -} diff --git a/query/outputnode_test.go b/query/outputnode_test.go index 2570e864afe..8bb1248daa8 100644 --- a/query/outputnode_test.go +++ b/query/outputnode_test.go @@ -94,118 +94,3 @@ func TestNormalizeJSONLimit(t *testing.T) { _, err := n.(*fastJsonNode).normalize() require.Error(t, err, "Couldn't evaluate @normalize directive - too many results") } - -func BenchmarkNormalizePerformance(b *testing.B) { - nodeHeight := 3 - treeDepth := 3 - - b.Run("Old Normalize", func(b *testing.B) { - for i := 0; i < b.N; i++ { - root := (&fastJsonNode{}).New("_root_") - fillNode(root.(*fastJsonNode), nodeHeight, treeDepth) - - b.StartTimer() - _, _ = root.(*fastJsonNode).normalize() - b.StopTimer() - } - }) - - b.Run("New Normalize", func(b *testing.B) { - for i := 0; i < b.N; i++ { - root := (&fastJsonNode{}).New("_root_") - rootChild := root.New("_rootchild") - root.AddListChild("_rootchild_", rootChild) - fillNode(rootChild.(*fastJsonNode), nodeHeight, treeDepth) - rootChild.(*fastJsonNode).isNormalized = true - - b.StartTimer() - _ = normalizeResult(rootChild.(*fastJsonNode), root.(*fastJsonNode), 0) - b.StopTimer() - } - }) - - fmt.Println("done") -} - -func fillNode(node *fastJsonNode, nodeHeight, treeDepth int) { - if treeDepth <= 0 { - return - } - - i := 0 - for ; i < nodeHeight/2; i++ { - node.AddValue(fmt.Sprintf("Attribute %d", i), types.ValueForType(types.StringID)) - } - - for ; i < nodeHeight; i++ { - child := node.New(fmt.Sprintf("Child%d", i)) - node.AddListChild(fmt.Sprintf("Child%d", i), child) - node.AddValue(fmt.Sprintf("Attribute %d", i), types.ValueForType(types.StringID)) - fillNode(child.(*fastJsonNode), nodeHeight, treeDepth-1) - } -} - -func TestNormalizeJSONUid1(t *testing.T) { - // Set default normalize limit. - x.Config.NormalizeNodeLimit = 1e4 - - n := (&fastJsonNode{}).New("root") - require.NotNil(t, n) - child1 := n.New("child1") - child1.SetUID(uint64(1), "uid") - child1.AddValue("attr1", types.ValueForType(types.StringID)) - n.AddListChild("child1", child1) - - child2 := n.New("child2") - child2.SetUID(uint64(2), "uid") - child2.AddValue("attr2", types.ValueForType(types.StringID)) - child1.AddListChild("child2", child2) - - child3 := n.New("child3") - child3.SetUID(uint64(3), "uid") - child3.AddValue("attr3", types.ValueForType(types.StringID)) - child2.AddListChild("child3", child3) - - normalized, err := n.(*fastJsonNode).normalize() - require.NoError(t, err) - require.NotNil(t, normalized) - nn := (&fastJsonNode{}).New("root") - for _, c := range normalized { - nn.AddListChild("alias", &fastJsonNode{attrs: c}) - } - - var b bytes.Buffer - nn.(*fastJsonNode).encode(&b) - require.JSONEq(t, `{"alias":[{"uid":"0x3","attr1":"","attr2":"","attr3":""}]}`, b.String()) -} - -func TestNormalizeJSONUid2(t *testing.T) { - n := (&fastJsonNode{}).New("root") - require.NotNil(t, n) - child1 := n.New("child1") - child1.SetUID(uint64(1), "uid") - child1.AddValue("___attr1", types.ValueForType(types.StringID)) - n.AddListChild("child1", child1) - - child2 := n.New("child2") - child2.SetUID(uint64(2), "uid") - child2.AddValue("___attr2", types.ValueForType(types.StringID)) - child1.AddListChild("child2", child2) - - child3 := n.New("child3") - child3.SetUID(uint64(3), "uid") - child3.AddValue(fmt.Sprintf("attr3"), types.ValueForType(types.StringID)) - child2.AddListChild("child3", child3) - - normalized, err := n.(*fastJsonNode).normalize() - require.NoError(t, err) - require.NotNil(t, normalized) - nn := (&fastJsonNode{}).New("root") - for _, c := range normalized { - nn.AddListChild("alias", &fastJsonNode{attrs: c}) - } - - var b bytes.Buffer - nn.(*fastJsonNode).encode(&b) - require.JSONEq(t, `{"alias":[{"___attr1":"","___attr2":"","uid":"0x3","attr3":""}]}`, b.String()) -} diff --git a/query/query1_test.go b/query/query1_test.go index e44a7adbc81..d6ac2970c0f 100644 --- a/query/query1_test.go +++ b/query/query1_test.go @@ -1064,7 +1064,8 @@ func TestReflexive3(t *testing.T) { } }` js := processQueryNoErr(t, query) - require.JSONEq(t, `{"data":{"me":[{"Friend":"Rick Grimes","Me":"Michonne"},{"Cofriend":"Glenn Rhee","Friend":"Michonne","Me":"Rick Grimes"},{"Me":"Daryl Dixon"},{"Friend":"Glenn Rhee","Me":"Michonne"},{"Friend":"Daryl Dixon","Me":"Michonne"},{"Cofriend":"Glenn Rhee","Friend":"Andrea","Me":"Michonne"},{"Cofriend":"Daryl Dixon","Friend":"Michonne","Me":"Rick Grimes"},{"Cofriend":"Andrea","Friend":"Michonne","Me":"Rick Grimes"}]}}`, js) + require.JSONEq(t, `{"data": {"me":[{"Friend":"Rick Grimes","Me":"Michonne"},{"Friend":"Glenn Rhee","Me":"Michonne"},{"Friend":"Daryl Dixon","Me":"Michonne"},{"Cofriend":"Glenn Rhee","Friend":"Andrea","Me":"Michonne"},{"Cofriend":"Glenn Rhee","Friend":"Michonne","Me":"Rick Grimes"},{"Cofriend":"Daryl Dixon","Friend":"Michonne","Me":"Rick Grimes"},{"Cofriend":"Andrea","Friend":"Michonne","Me":"Rick Grimes"},{"Me":"Daryl Dixon"}]}}`, js) + } func TestCascadeUid(t *testing.T) { From 5a8f16b225e886e46efa89819d8220804191d26d Mon Sep 17 00:00:00 2001 From: Animesh Date: Mon, 30 Sep 2019 20:30:23 +0530 Subject: [PATCH 16/25] Add comment --- query/outputnode.go | 7 +++++-- query/query1_test.go | 1 - 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/query/outputnode.go b/query/outputnode.go index 488ebc3658c..54509fede00 100644 --- a/query/outputnode.go +++ b/query/outputnode.go @@ -476,11 +476,9 @@ func processNodeUids(fj *fastJsonNode, sg *SubGraph) error { if err != nil { return err } - for _, c := range normalized { fj.AddListChild(sg.Params.Alias, &fastJsonNode{attrs: c}) } - } if !hasChild { @@ -708,6 +706,11 @@ func (sg *SubGraph) preTraverse(uid uint64, dst outputNode) error { uc.SetUID(childUID, "uid") } if pc.List { + // We will normalize at each level instead of + // calling normalize after pretraverse. + // Now normalize() only flattens one level, + // the expectation is that it's children have + // already been normalized. if pc.Params.Normalize { normalized, err := uc.(*fastJsonNode).normalize() if err != nil { diff --git a/query/query1_test.go b/query/query1_test.go index d6ac2970c0f..2ef9d560912 100644 --- a/query/query1_test.go +++ b/query/query1_test.go @@ -1065,7 +1065,6 @@ func TestReflexive3(t *testing.T) { }` js := processQueryNoErr(t, query) require.JSONEq(t, `{"data": {"me":[{"Friend":"Rick Grimes","Me":"Michonne"},{"Friend":"Glenn Rhee","Me":"Michonne"},{"Friend":"Daryl Dixon","Me":"Michonne"},{"Cofriend":"Glenn Rhee","Friend":"Andrea","Me":"Michonne"},{"Cofriend":"Glenn Rhee","Friend":"Michonne","Me":"Rick Grimes"},{"Cofriend":"Daryl Dixon","Friend":"Michonne","Me":"Rick Grimes"},{"Cofriend":"Andrea","Friend":"Michonne","Me":"Rick Grimes"},{"Me":"Daryl Dixon"}]}}`, js) - } func TestCascadeUid(t *testing.T) { From c503fd83969d062b004521da6cdde6224ec805f9 Mon Sep 17 00:00:00 2001 From: Animesh Date: Thu, 3 Oct 2019 15:34:15 +0530 Subject: [PATCH 17/25] Fix output for non list uid types --- query/outputnode.go | 39 ++++++++++++++++++++++++++------------- 1 file changed, 26 insertions(+), 13 deletions(-) diff --git a/query/outputnode.go b/query/outputnode.go index 54509fede00..03cf4a904c0 100644 --- a/query/outputnode.go +++ b/query/outputnode.go @@ -298,7 +298,11 @@ func merge(parent [][]*fastJsonNode, child [][]*fastJsonNode) ([][]*fastJsonNode func (fj *fastJsonNode) normalize() ([][]*fastJsonNode, error) { cnt := 0 for _, a := range fj.attrs { - if a.isChild { + // When we call addMapChild it tries to find whether there is already a node with + // attribute same as attribute argument of addMapChild. If it doesn't find any such + // node, it creates a node with isChild = false. In those cases normalize fails. + // So we are using len(a.attrs) > 0 instead of a.isChild + if len(a.attrs) > 0 { cnt++ } } @@ -314,7 +318,8 @@ func (fj *fastJsonNode) normalize() ([][]*fastJsonNode, error) { // merged with children later. attrs := make([]*fastJsonNode, 0, len(fj.attrs)-cnt) for _, a := range fj.attrs { - if !a.isChild { + // Check comment at previous occurrence of len(a.attrs) > 0 + if len(a.attrs) == 0 { attrs = append(attrs, a) } } @@ -322,7 +327,8 @@ func (fj *fastJsonNode) normalize() ([][]*fastJsonNode, error) { for ci := 0; ci < len(fj.attrs); { childNode := fj.attrs[ci] - if !childNode.isChild { + // Check comment at previous occurrence of len(a.attrs) > 0 + if len(childNode.attrs) == 0 { ci++ continue } @@ -705,30 +711,37 @@ func (sg *SubGraph) preTraverse(uid uint64, dst outputNode) error { if sg.Params.GetUid { uc.SetUID(childUID, "uid") } - if pc.List { + if pc.Params.Normalize { // We will normalize at each level instead of // calling normalize after pretraverse. // Now normalize() only flattens one level, // the expectation is that it's children have // already been normalized. - if pc.Params.Normalize { - normalized, err := uc.(*fastJsonNode).normalize() - if err != nil { - return err - } + normalized, err := uc.(*fastJsonNode).normalize() + if err != nil { + return err + } + if pc.List { for _, c := range normalized { - dst.AddListChild(fieldName, - &fastJsonNode{attrs: c}) + dst.AddListChild(fieldName, &fastJsonNode{attrs: c}) } } else { - dst.AddListChild(fieldName, uc) + for _, c := range normalized { + dst.AddMapChild(fieldName, &fastJsonNode{attrs: c}, false) + } } + } else { - dst.AddMapChild(fieldName, uc, false) + if pc.List { + dst.AddListChild(fieldName, uc) + } else { + dst.AddMapChild(fieldName, uc, false) + } } } } + if pc.Params.UidCount && !(pc.Params.UidCountAlias == "" && pc.Params.Normalize) { uc := dst.New(fieldName) c := types.ValueForType(types.IntID) From 7929bd829282f8dfbcf278944fbff254371ad7ec Mon Sep 17 00:00:00 2001 From: Animesh Date: Thu, 3 Oct 2019 18:24:17 +0530 Subject: [PATCH 18/25] Address PR comments --- query/outputnode.go | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/query/outputnode.go b/query/outputnode.go index 03cf4a904c0..e29523e7722 100644 --- a/query/outputnode.go +++ b/query/outputnode.go @@ -295,12 +295,17 @@ func merge(parent [][]*fastJsonNode, child [][]*fastJsonNode) ([][]*fastJsonNode return mergedList, nil } +// normalize returns all attributes of fj and the attributes of all attributes of fj (if any). func (fj *fastJsonNode) normalize() ([][]*fastJsonNode, error) { cnt := 0 for _, a := range fj.attrs { - // When we call addMapChild it tries to find whether there is already a node with - // attribute same as attribute argument of addMapChild. If it doesn't find any such - // node, it creates a node with isChild = false. In those cases normalize fails. + // Here we are counting all non scalar attributes of fj. If there are any such + // attributes, we will flatten it, otherwise we will return all attributes. + + // When we call addMapChild it tries to find whether there is already a attribute + // with attribute same as attribute argument of addMapChild. If it doesn't find any + // such attribute, it creates an attribute with isChild = false. In those cases + // sometimes cnt remains zero and normalize returns attributes without flattening. // So we are using len(a.attrs) > 0 instead of a.isChild if len(a.attrs) > 0 { cnt++ @@ -334,8 +339,7 @@ func (fj *fastJsonNode) normalize() ([][]*fastJsonNode, error) { } childSlice := make([][]*fastJsonNode, 0, 5) for ci < len(fj.attrs) && childNode.attr == fj.attrs[ci].attr { - normalized := fj.attrs[ci].attrs - childSlice = append(childSlice, normalized) + childSlice = append(childSlice, fj.attrs[ci].attrs) ci++ } // Merging with parent. @@ -717,17 +721,17 @@ func (sg *SubGraph) preTraverse(uid uint64, dst outputNode) error { // Now normalize() only flattens one level, // the expectation is that it's children have // already been normalized. - normalized, err := uc.(*fastJsonNode).normalize() + normAttrs, err := uc.(*fastJsonNode).normalize() if err != nil { return err } if pc.List { - for _, c := range normalized { + for _, c := range normAttrs { dst.AddListChild(fieldName, &fastJsonNode{attrs: c}) } } else { - for _, c := range normalized { + for _, c := range normAttrs { dst.AddMapChild(fieldName, &fastJsonNode{attrs: c}, false) } } From dbf7191850fe821e8a6678b4d638f30c9f0c2b42 Mon Sep 17 00:00:00 2001 From: Animesh Date: Mon, 7 Oct 2019 14:33:37 +0530 Subject: [PATCH 19/25] Add test case for non list uid type --- query/common_test.go | 44 ++++++++++++++++++++++++++++++++++++++++ query/query2_test.go | 48 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 92 insertions(+) diff --git a/query/common_test.go b/query/common_test.go index 12598a3898c..c1e9e25f479 100644 --- a/query/common_test.go +++ b/query/common_test.go @@ -270,6 +270,10 @@ state : [uid] . county : [uid] . firstName : string . lastName : string . +newname : string @index(exact, term) . +newage : int . +boss : uid . +newfriend : [uid] . ` func populateCluster() { @@ -566,6 +570,46 @@ func populateCluster() { _:har "Ford" . _:ss "Steven" . _:ss "Spielberg" . + + <501> "P1" . + <502> "P2" . + <503> "P3" . + <504> "P4" . + <505> "P5" . + <506> "P6" . + <507> "P7" . + <508> "P8" . + <509> "P9" . + <510> "P10" . + <511> "P11" . + <512> "P12" . + + <501> "21" . + <502> "22" . + <503> "23" . + <504> "24" . + <505> "25" . + <506> "26" . + <507> "27" . + <508> "28" . + <509> "29" . + <510> "30" . + <511> "31" . + <512> "32" . + + <501> <502> . + <501> <503> . + <501> <504> . + <502> <505> . + <502> <506> . + <503> <507> . + <503> <508> . + <504> <509> . + <504> <510> . + <502> <510> . + <510> <511> . + <510> <512> . +>>>>>>> Add test case for non list uid type `) addGeoPointToCluster(1, "loc", []float64{1.1, 2.0}) diff --git a/query/query2_test.go b/query/query2_test.go index 8d17dcaae32..6d4d5a8d538 100644 --- a/query/query2_test.go +++ b/query/query2_test.go @@ -18,6 +18,7 @@ package query import ( "context" + "fmt" "testing" "time" @@ -2111,6 +2112,53 @@ func TestNormalizeDirectiveMultipleQuery(t *testing.T) { }`, js) } +func TestNormalizeDirectiveListAndNonListChild1(t *testing.T) { + query := ` + { + me(func: uid(501, 502)) { + mn: newname + newfriend @normalize { # Results of this subquery will be normalized + fn: newname + newfriend @normalize { + ffn: newname + } + } + boss @normalize { + bn: newname + newfriend { + bfn: newname + } + } + } + } + ` + + js := processQueryNoErr(t, query) + fmt.Println(js) +} + +func TestNormalizeDirectiveListAndNonListChild2(t *testing.T) { + query := ` + { + me(func: uid(501, 502)) { + mn: newname + newfriend @normalize { # Results of this subquery will be normalized + fn: newname + boss @normalize { + bn: newname + newfriend { + bfn: newname + } + } + } + } + } + ` + + js := processQueryNoErr(t, query) + fmt.Println(js) +} + func TestNearPoint(t *testing.T) { query := `{ From f43f29ba858d003802db0936850a23055b49aeff Mon Sep 17 00:00:00 2001 From: Animesh Date: Mon, 7 Oct 2019 18:55:14 +0530 Subject: [PATCH 20/25] Normalize non list type with list type child --- query/outputnode.go | 54 +++++++++++++++++++++++- query/query2_test.go | 98 ++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 148 insertions(+), 4 deletions(-) diff --git a/query/outputnode.go b/query/outputnode.go index e29523e7722..8810092c071 100644 --- a/query/outputnode.go +++ b/query/outputnode.go @@ -677,6 +677,12 @@ func (sg *SubGraph) preTraverse(uid uint64, dst outputNode) error { if sg.Params.IgnoreReflex { pc.Params.ParentIds = sg.Params.ParentIds } + + var addAsList bool + for _, ch := range pc.Children { + addAsList = addAsList || ch.List + } + // We create as many predicate entity children as the length of uids for // this predicate. ul := pc.uidMatrix[idx] @@ -726,7 +732,53 @@ func (sg *SubGraph) preTraverse(uid uint64, dst outputNode) error { return err } - if pc.List { + // addList means atleat one of the children are of + // list type.Example below explains need of addList. + // Schema: + // ``` + // name: string @index(exact) . + // friend: [uid] . + // boss: uid . + // ``` + // Data + // ``` + // <3> "3" . + // <4> "4" . + // <1> <2> + // <2> <3> + // <2> <4> + // ``` + // Query + // ``` + // { + // me(func: uid(0x01)) { + // boss { + // friend { + // n: name + // } + // } + // } + // } + // ``` + // Expected output + // ``` + // "me": [ + // { + // "boss": [ + // { + // "n": "P9" + // }, + // { + // "n": "P10" + // } + // ] + // } + // ] + // ``` + // boss is of type uid, but since friend is its + // child which is of type [uid], all the results + // are returned as list. + if pc.List || addAsList { for _, c := range normAttrs { dst.AddListChild(fieldName, &fastJsonNode{attrs: c}) } diff --git a/query/query2_test.go b/query/query2_test.go index 6d4d5a8d538..4fdc9793a87 100644 --- a/query/query2_test.go +++ b/query/query2_test.go @@ -18,7 +18,6 @@ package query import ( "context" - "fmt" "testing" "time" @@ -2134,7 +2133,65 @@ func TestNormalizeDirectiveListAndNonListChild1(t *testing.T) { ` js := processQueryNoErr(t, query) - fmt.Println(js) + require.JSONEq(t, ` + { + "data": { + "me": [ + { + "mn": "P1", + "newfriend": [ + { + "ffn": "P5", + "fn": "P2" + }, + { + "ffn": "P6", + "fn": "P2" + }, + { + "ffn": "P7", + "fn": "P3" + }, + { + "ffn": "P8", + "fn": "P3" + } + ], + "boss": [ + { + "bfn": "P9", + "bn": "P4" + }, + { + "bfn": "P10", + "bn": "P4" + } + ] + }, + { + "mn": "P2", + "newfriend": [ + { + "fn": "P5" + }, + { + "fn": "P6" + } + ], + "boss": [ + { + "bfn": "P11", + "bn": "P10" + }, + { + "bfn": "P12", + "bn": "P10" + } + ] + } + ] + } + }`, js) } func TestNormalizeDirectiveListAndNonListChild2(t *testing.T) { @@ -2156,7 +2213,42 @@ func TestNormalizeDirectiveListAndNonListChild2(t *testing.T) { ` js := processQueryNoErr(t, query) - fmt.Println(js) + require.JSONEq(t, ` + { + "data": { + "me": [ + { + "mn": "P1", + "newfriend": [ + { + "bfn": "P11", + "bn": "P10", + "fn": "P2" + }, + { + "bfn": "P12", + "bn": "P10", + "fn": "P2" + }, + { + "fn": "P3" + } + ] + }, + { + "mn": "P2", + "newfriend": [ + { + "fn": "P5" + }, + { + "fn": "P6" + } + ] + } + ] + } + }`) } func TestNearPoint(t *testing.T) { From 40352019849e5c4f67e463038ee3f5396517750a Mon Sep 17 00:00:00 2001 From: Animesh Date: Mon, 7 Oct 2019 19:14:46 +0530 Subject: [PATCH 21/25] Style changes --- query/outputnode.go | 15 +++++++-------- query/query2_test.go | 2 +- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/query/outputnode.go b/query/outputnode.go index 8810092c071..4ed904bd6d0 100644 --- a/query/outputnode.go +++ b/query/outputnode.go @@ -295,7 +295,7 @@ func merge(parent [][]*fastJsonNode, child [][]*fastJsonNode) ([][]*fastJsonNode return mergedList, nil } -// normalize returns all attributes of fj and the attributes of all attributes of fj (if any). +// normalize returns all attributes of fj and its children (if any). func (fj *fastJsonNode) normalize() ([][]*fastJsonNode, error) { cnt := 0 for _, a := range fj.attrs { @@ -303,7 +303,7 @@ func (fj *fastJsonNode) normalize() ([][]*fastJsonNode, error) { // attributes, we will flatten it, otherwise we will return all attributes. // When we call addMapChild it tries to find whether there is already a attribute - // with attribute same as attribute argument of addMapChild. If it doesn't find any + // with attr field same as attribute argument of addMapChild. If it doesn't find any // such attribute, it creates an attribute with isChild = false. In those cases // sometimes cnt remains zero and normalize returns attributes without flattening. // So we are using len(a.attrs) > 0 instead of a.isChild @@ -787,13 +787,12 @@ func (sg *SubGraph) preTraverse(uid uint64, dst outputNode) error { dst.AddMapChild(fieldName, &fastJsonNode{attrs: c}, false) } } - + continue + } + if pc.List { + dst.AddListChild(fieldName, uc) } else { - if pc.List { - dst.AddListChild(fieldName, uc) - } else { - dst.AddMapChild(fieldName, uc, false) - } + dst.AddMapChild(fieldName, uc, false) } } } diff --git a/query/query2_test.go b/query/query2_test.go index 4fdc9793a87..0ed0d006973 100644 --- a/query/query2_test.go +++ b/query/query2_test.go @@ -2248,7 +2248,7 @@ func TestNormalizeDirectiveListAndNonListChild2(t *testing.T) { } ] } - }`) + }`, js) } func TestNearPoint(t *testing.T) { From e9ce5c7a17340a809173551f7734527b66de3f0e Mon Sep 17 00:00:00 2001 From: Animesh Date: Wed, 9 Oct 2019 13:05:53 +0530 Subject: [PATCH 22/25] Add everything as list for normalize --- query/outputnode.go | 87 ++++++++++++++------------------------------ query/query2_test.go | 60 ++++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+), 59 deletions(-) diff --git a/query/outputnode.go b/query/outputnode.go index 4ed904bd6d0..31558962667 100644 --- a/query/outputnode.go +++ b/query/outputnode.go @@ -678,11 +678,6 @@ func (sg *SubGraph) preTraverse(uid uint64, dst outputNode) error { pc.Params.ParentIds = sg.Params.ParentIds } - var addAsList bool - for _, ch := range pc.Children { - addAsList = addAsList || ch.List - } - // We create as many predicate entity children as the length of uids for // this predicate. ul := pc.uidMatrix[idx] @@ -732,60 +727,34 @@ func (sg *SubGraph) preTraverse(uid uint64, dst outputNode) error { return err } - // addList means atleat one of the children are of - // list type.Example below explains need of addList. - // Schema: - // ``` - // name: string @index(exact) . - // friend: [uid] . - // boss: uid . - // ``` - // Data - // ``` - // <3> "3" . - // <4> "4" . - // <1> <2> - // <2> <3> - // <2> <4> - // ``` - // Query - // ``` - // { - // me(func: uid(0x01)) { - // boss { - // friend { - // n: name - // } - // } - // } - // } - // ``` - // Expected output - // ``` - // "me": [ - // { - // "boss": [ - // { - // "n": "P9" - // }, - // { - // "n": "P10" - // } - // ] - // } - // ] - // ``` - // boss is of type uid, but since friend is its - // child which is of type [uid], all the results - // are returned as list. - if pc.List || addAsList { - for _, c := range normAttrs { - dst.AddListChild(fieldName, &fastJsonNode{attrs: c}) - } - } else { - for _, c := range normAttrs { - dst.AddMapChild(fieldName, &fastJsonNode{attrs: c}, false) - } + for _, c := range normAttrs { + // Adding as list child irrespective of the type of pc + // (list or non-list), otherwise result might be inconsistent or might + // depend on children and grandchildren of pc. Consider the case: + // boss: uid . + // friend: [uid] . + // name: string . + // For query like: + // { + // me(func: uid(0x1)) { + // boss @normalize { + // name + // } + // } + // } + // boss will be non list type in response, but for query like: + // { + // me(func: uid(0x1)) { + // boss @normalize { + // friend { + // name + // } + // } + // } + // } + // boss should be of list type because there can be mutliple friends of + // boss. + dst.AddListChild(fieldName, &fastJsonNode{attrs: c}) } continue } diff --git a/query/query2_test.go b/query/query2_test.go index 0ed0d006973..d9c09911d86 100644 --- a/query/query2_test.go +++ b/query/query2_test.go @@ -2251,6 +2251,66 @@ func TestNormalizeDirectiveListAndNonListChild2(t *testing.T) { }`, js) } +func TestNormalizeDirectiveListAndNonListChild3(t *testing.T) { + query := ` + { + me(func: uid(501, 502)) { + mn: newname + boss @normalize { # Results of this subquery will be normalized + bn: newname + newfriend @normalize { + bfn: newname + newfriend { + bffn: newname + } + } + } + } + } + ` + + js := processQueryNoErr(t, query) + require.JSONEq(t, ` + { + "data": { + "me": [ + { + "mn": "P1", + "boss": [ + { + "bfn": "P9", + "bn": "P4" + }, + { + "bffn": "P11", + "bfn": "P10", + "bn": "P4" + }, + { + "bffn": "P12", + "bfn": "P10", + "bn": "P4" + } + ] + }, + { + "mn": "P2", + "boss": [ + { + "bfn": "P11", + "bn": "P10" + }, + { + "bfn": "P12", + "bn": "P10" + } + ] + } + ] + } + }`, js) +} + func TestNearPoint(t *testing.T) { query := `{ From 7b623d819e756e117815b52e436d019f5c9c8fb1 Mon Sep 17 00:00:00 2001 From: Animesh Date: Wed, 9 Oct 2019 23:56:13 +0530 Subject: [PATCH 23/25] Add documentation about behavior --- wiki/content/query-language/index.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wiki/content/query-language/index.md b/wiki/content/query-language/index.md index faf062ff268..68642594034 100644 --- a/wiki/content/query-language/index.md +++ b/wiki/content/query-language/index.md @@ -1916,7 +1916,7 @@ Query Example: Film name, country and first two actors (by UID order) of every S } {{< /runnable >}} -You can also apply `@normalize` on nested query blocks. It will work similarly but only flatten the result of the nested query block where `@normalize` has been applied. +You can also apply `@normalize` on nested query blocks. It will work similarly but only flatten the result of the nested query block where `@normalize` has been applied. `@normalize` will return a list irrespective of the type of attribute on which it is applied. {{< runnable >}} { director(func:allofterms(name@en, "steven spielberg")) { From bbf6b69b9b875ee2d689e167ea9d1aef42b60c43 Mon Sep 17 00:00:00 2001 From: Animesh Date: Thu, 10 Oct 2019 00:01:21 +0530 Subject: [PATCH 24/25] Remove left over conflict characters --- query/common_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/query/common_test.go b/query/common_test.go index c1e9e25f479..44de0568dd3 100644 --- a/query/common_test.go +++ b/query/common_test.go @@ -609,7 +609,6 @@ func populateCluster() { <502> <510> . <510> <511> . <510> <512> . ->>>>>>> Add test case for non list uid type `) addGeoPointToCluster(1, "loc", []float64{1.1, 2.0}) From 3d3a9899843ef51851e845f23c765f56387df8fb Mon Sep 17 00:00:00 2001 From: Animesh Date: Thu, 10 Oct 2019 19:11:12 +0530 Subject: [PATCH 25/25] Address PR comments --- query/outputnode.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/query/outputnode.go b/query/outputnode.go index 31558962667..a05ddcea717 100644 --- a/query/outputnode.go +++ b/query/outputnode.go @@ -299,10 +299,10 @@ func merge(parent [][]*fastJsonNode, child [][]*fastJsonNode) ([][]*fastJsonNode func (fj *fastJsonNode) normalize() ([][]*fastJsonNode, error) { cnt := 0 for _, a := range fj.attrs { - // Here we are counting all non scalar attributes of fj. If there are any such + // Here we are counting all non-scalar attributes of fj. If there are any such // attributes, we will flatten it, otherwise we will return all attributes. - // When we call addMapChild it tries to find whether there is already a attribute + // When we call addMapChild it tries to find whether there is already an attribute // with attr field same as attribute argument of addMapChild. If it doesn't find any // such attribute, it creates an attribute with isChild = false. In those cases // sometimes cnt remains zero and normalize returns attributes without flattening. @@ -720,7 +720,7 @@ func (sg *SubGraph) preTraverse(uid uint64, dst outputNode) error { // We will normalize at each level instead of // calling normalize after pretraverse. // Now normalize() only flattens one level, - // the expectation is that it's children have + // the expectation is that its children have // already been normalized. normAttrs, err := uc.(*fastJsonNode).normalize() if err != nil {