From b5c96f69510a897d34c313a227396c3989f730cc Mon Sep 17 00:00:00 2001 From: minhaj-shakeel Date: Tue, 16 Feb 2021 14:46:30 +0530 Subject: [PATCH 01/18] modify parser --- gql/parser.go | 20 +++++++++++++++--- gql/parser_test.go | 34 ++++++++++++++++++++++++++++--- graphql/dgraph/graphquery.go | 6 +++--- graphql/resolve/query_rewriter.go | 14 +++++++++++-- query/query.go | 33 ++++++++++++++++++++++-------- query/query0_test.go | 15 ++++++++++++++ 6 files changed, 103 insertions(+), 19 deletions(-) diff --git a/gql/parser.go b/gql/parser.go index 9574886fa82..fad49b4959f 100644 --- a/gql/parser.go +++ b/gql/parser.go @@ -68,7 +68,7 @@ type GraphQuery struct { Recurse bool RecurseArgs RecurseArgs ShortestPathArgs ShortestPathArgs - Cascade []string + Cascade *CascadeArgs IgnoreReflex bool Facets *pb.FacetParams FacetsFilter *FilterTree @@ -88,6 +88,14 @@ type GraphQuery struct { IsEmpty bool } +// CascadeArgs stores the arguments needed to process @cascade directive. +// It is introduced to ensure correct behaviour for cascade with pagination. +type CascadeArgs struct { + Fields []string + First int + Offset int +} + // RecurseArgs stores the arguments needed to process the @recurse directive. type RecurseArgs struct { Depth uint64 @@ -2127,10 +2135,16 @@ func parseCascade(it *lex.ItemIterator, gq *GraphQuery) error { // 2. @cascade } // 3. @cascade @ // 4. @cascade\n someOtherPred + gq.Cascade = &CascadeArgs{} + gq.Cascade.First, _ = strconv.Atoi(gq.Args["first"]) + delete(gq.Args, "first") + gq.Cascade.Offset, _ = strconv.Atoi(gq.Args["offset"]) + delete(gq.Args, "offset") + if items[0].Typ == itemLeftCurl || items[0].Typ == itemRightCurl || items[0]. Typ == itemAt || items[0].Typ == itemName { // __all__ implies @cascade i.e. implies values for all the children are mandatory. - gq.Cascade = append(gq.Cascade, "__all__") + gq.Cascade.Fields = append(gq.Cascade.Fields, "__all__") return nil } @@ -2157,7 +2171,7 @@ loop: if !expectArg { return item.Errorf("Expected a comma or right round but got: %v", item.Val) } - gq.Cascade = append(gq.Cascade, collectName(it, item.Val)) + gq.Cascade.Fields = append(gq.Cascade.Fields, collectName(it, item.Val)) count++ expectArg = false default: diff --git a/gql/parser_test.go b/gql/parser_test.go index 8384628d9ed..c3d88dcc614 100644 --- a/gql/parser_test.go +++ b/gql/parser_test.go @@ -5371,7 +5371,7 @@ func TestCascade(t *testing.T) { Str: query, }) require.NoError(t, err) - require.Equal(t, gq.Query[0].Cascade[0], "__all__") + require.Equal(t, gq.Query[0].Cascade.Fields[0], "__all__") } func TestCascadeParameterized(t *testing.T) { @@ -5386,8 +5386,8 @@ func TestCascadeParameterized(t *testing.T) { Str: query, }) require.NoError(t, err) - require.Equal(t, gq.Query[0].Cascade[0], "name") - require.Equal(t, gq.Query[0].Cascade[1], "age") + require.Equal(t, gq.Query[0].Cascade.Fields[0], "name") + require.Equal(t, gq.Query[0].Cascade.Fields[1], "age") } func TestBadCascadeParameterized(t *testing.T) { @@ -5444,6 +5444,34 @@ func TestBadCascadeParameterized(t *testing.T) { } } +// __expand_all_ +// what if first = 0 and we do sort +// add test of no top level cascade and cascade on children +func TestParseCascadeWithPagination(t *testing.T) { + query := ` +{ + query(func: type(Student), first: 2) @cascade{ + name + age + classes ( first: 3, offset: 10) { + standard + subjects + teacher + } + + } +} + ` + gq, err := Parse(Request{Str: query}) + require.NoError(t, err) + require.Equal(t, len(gq.Query[0].Args), 0) + require.Equal(t, gq.Query[0].Cascade.First, 2) + require.Equal(t, gq.Query[0].Cascade.Offset, 0) + require.Equal(t, len(gq.Query[0].Children[2].Args), 0) + require.Equal(t, gq.Query[0].Children[2].Cascade.First, 3) + require.Equal(t, gq.Query[0].Children[2].Cascade.Offset, 10) +} + func TestEmptyId(t *testing.T) { q := "query me($a: string) { q(func: uid($a)) { name }}" r := Request{ diff --git a/graphql/dgraph/graphquery.go b/graphql/dgraph/graphquery.go index 94aaac698b6..d193f471ded 100644 --- a/graphql/dgraph/graphquery.go +++ b/graphql/dgraph/graphquery.go @@ -83,12 +83,12 @@ func writeQuery(b *strings.Builder, query *gql.GraphQuery, prefix string) { x.Check2(b.WriteRune(')')) } - if len(query.Cascade) != 0 { - if query.Cascade[0] == "__all__" { + if query.Cascade != nil && len(query.Cascade.Fields) != 0 { + if query.Cascade.Fields[0] == "__all__" { x.Check2(b.WriteString(" @cascade")) } else { x.Check2(b.WriteString(" @cascade(")) - x.Check2(b.WriteString(strings.Join(query.Cascade, ", "))) + x.Check2(b.WriteString(strings.Join(query.Cascade.Fields, ", "))) x.Check2(b.WriteRune(')')) } } diff --git a/graphql/resolve/query_rewriter.go b/graphql/resolve/query_rewriter.go index e6db6fb1eb1..5d39e257ded 100644 --- a/graphql/resolve/query_rewriter.go +++ b/graphql/resolve/query_rewriter.go @@ -1029,7 +1029,7 @@ func (authRw *authRewriter) rewriteRuleNode( r1 := rewriteAsQuery(qry, authRw) r1[0].Var = varName r1[0].Attr = "var" - r1[0].Cascade = append(r1[0].Cascade, "__all__") + r1[0].Cascade.Fields = append(r1[0].Cascade.Fields, "__all__") return []*gql.GraphQuery{r1[0]}, &gql.FilterTree{ Func: &gql.Function{ @@ -1536,7 +1536,17 @@ func addPagination(q *gql.GraphQuery, field schema.Field) { } func addCascadeDirective(q *gql.GraphQuery, field schema.Field) { - q.Cascade = field.Cascade() + + if len(field.Cascade()) > 0 { + first, _ := strconv.Atoi(q.Args["first"]) + delete(q.Args, "first") + offset, _ := strconv.Atoi(q.Args["offset"]) + delete(q.Args, "offset") + q.Cascade = &gql.CascadeArgs{Fields: field.Cascade(), + First: first, + Offset: offset, + } + } } func convertIDs(idsSlice []interface{}) []uint64 { diff --git a/query/query.go b/query/query.go index bc5cca4ce7c..ab89a48ee88 100644 --- a/query/query.go +++ b/query/query.go @@ -159,7 +159,7 @@ type params struct { // Cascade is the list of predicates to apply @cascade to. // __all__ is special to mean @cascade i.e. all the children of this subgraph are mandatory // and should have values otherwise the node will be excluded. - Cascade []string + Cascade *gql.CascadeArgs // IgnoreReflex is true if the @ignorereflex directive is specified. IgnoreReflex bool @@ -559,15 +559,16 @@ func treeCopy(gq *gql.GraphQuery, sg *SubGraph) error { GroupbyAttrs: gchild.GroupbyAttrs, IsGroupBy: gchild.IsGroupby, IsInternal: gchild.IsInternal, + Cascade: &gql.CascadeArgs{}, } // Inherit from the parent. - if len(sg.Params.Cascade) > 0 { - args.Cascade = append(args.Cascade, sg.Params.Cascade...) + if sg.Params.Cascade != nil && (len(sg.Params.Cascade.Fields) > 0) { + args.Cascade.Fields = append(args.Cascade.Fields, sg.Params.Cascade.Fields...) } // Allow over-riding at this level. - if len(gchild.Cascade) > 0 { - args.Cascade = gchild.Cascade + if gchild.Cascade != nil && (len(gchild.Cascade.Fields) > 0) { + args.Cascade.Fields = gchild.Cascade.Fields } if gchild.IsCount { @@ -795,6 +796,10 @@ func newGraph(ctx context.Context, gq *gql.GraphQuery) (*SubGraph, error) { AllowedPreds: gq.AllowedPreds, } + if args.Cascade == nil { + args.Cascade = &gql.CascadeArgs{} + } + for argk := range gq.Args { if !isValidArg(argk) { return nil, errors.Errorf("Invalid argument: %s", argk) @@ -1320,7 +1325,7 @@ func (sg *SubGraph) populateVarMap(doneVars map[string]varValue, sgPath []*SubGr } cascadeArgMap := make(map[string]bool) - for _, pred := range sg.Params.Cascade { + for _, pred := range sg.Params.Cascade.Fields { cascadeArgMap[pred] = true } cascadeAllPreds := cascadeArgMap["__all__"] @@ -1340,16 +1345,21 @@ func (sg *SubGraph) populateVarMap(doneVars map[string]varValue, sgPath []*SubGr return err } sgPath = sgPath[:len(sgPath)-1] // Backtrack - if len(child.Params.Cascade) == 0 { + if len(child.Params.Cascade.Fields) == 0 { continue } // Intersect the UidMatrix with the DestUids as some UIDs might have been removed // by other operations. So we need to apply it on the UidMatrix. child.updateUidMatrix() + + // for i := 0; i < len(child.uidMatrix); i++ { + // start, end := x.PageRange(child.Params.Cascade.First, child.Params.Cascade.Offset, len(child.uidMatrix[i].Uids)) + // child.uidMatrix[i].Uids = child.uidMatrix[i].Uids[start:end] + // } } - if len(sg.Params.Cascade) == 0 { + if len(sg.Params.Cascade.Fields) == 0 { goto AssignStep } @@ -2803,6 +2813,13 @@ func (req *Request) ProcessQuery(ctx context.Context) (err error) { if err := sg.populateVarMap(req.Vars, sgPath); err != nil { return err } + // first time at the root here. + + for i := 0; i < len(sg.uidMatrix); i++ { + start, end := x.PageRange(sg.Params.Cascade.First, sg.Params.Cascade.Offset, len(sg.uidMatrix[i].Uids)) + sg.uidMatrix[i].Uids = sg.uidMatrix[i].Uids[start:end] + } + if err := sg.populatePostAggregation(req.Vars, []*SubGraph{}, nil); err != nil { return err } diff --git a/query/query0_test.go b/query/query0_test.go index 0a427098a20..48c12100f31 100644 --- a/query/query0_test.go +++ b/query/query0_test.go @@ -1764,6 +1764,21 @@ func TestUseVarsMultiCascade(t *testing.T) { js) } +func TestCascadeWithPagination(t *testing.T) { + query := ` + { + query(func: type(Student), first: 2) @cascade { + name + age + class(first: 3, offset: 10) { + standard + subjects + } + } + } + ` + _ = processQueryNoErr(t, query) +} func TestUseVarsMultiOrder(t *testing.T) { query := ` From da4c94e5bc737b19303c91c507320b3b804d0e96 Mon Sep 17 00:00:00 2001 From: minhaj-shakeel Date: Tue, 16 Feb 2021 15:22:03 +0530 Subject: [PATCH 02/18] add test --- gql/parser_test.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/gql/parser_test.go b/gql/parser_test.go index c3d88dcc614..4add7262628 100644 --- a/gql/parser_test.go +++ b/gql/parser_test.go @@ -5456,7 +5456,10 @@ func TestParseCascadeWithPagination(t *testing.T) { classes ( first: 3, offset: 10) { standard subjects - teacher + teachers ( first: 4, offset: 1) { + name + age + } } } @@ -5470,6 +5473,9 @@ func TestParseCascadeWithPagination(t *testing.T) { require.Equal(t, len(gq.Query[0].Children[2].Args), 0) require.Equal(t, gq.Query[0].Children[2].Cascade.First, 3) require.Equal(t, gq.Query[0].Children[2].Cascade.Offset, 10) + require.Equal(t, len(gq.Query[0].Children[2].Children[2].Args), 0) + require.Equal(t, gq.Query[0].Children[2].Children[2].Cascade.First, 4) + require.Equal(t, gq.Query[0].Children[2].Children[2].Cascade.Offset, 1) } func TestEmptyId(t *testing.T) { From ae63e9b5324b1e1f822416424f9377b31eb578a7 Mon Sep 17 00:00:00 2001 From: minhaj-shakeel Date: Tue, 16 Feb 2021 16:38:01 +0530 Subject: [PATCH 03/18] pagination working fine at deeper level --- query/query.go | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/query/query.go b/query/query.go index ab89a48ee88..d9f77bef2e3 100644 --- a/query/query.go +++ b/query/query.go @@ -571,6 +571,13 @@ func treeCopy(gq *gql.GraphQuery, sg *SubGraph) error { args.Cascade.Fields = gchild.Cascade.Fields } + if len(args.Cascade.Fields) > 0 { + args.Cascade.First, _ = strconv.Atoi(gchild.Args["first"]) + args.Cascade.Offset, _ = strconv.Atoi(gchild.Args["offset"]) + delete(gchild.Args, "first") + delete(gchild.Args, "offset") + } + if gchild.IsCount { if len(gchild.Children) != 0 { return errors.New("Node with count cannot have child attributes") @@ -1353,10 +1360,10 @@ func (sg *SubGraph) populateVarMap(doneVars map[string]varValue, sgPath []*SubGr // by other operations. So we need to apply it on the UidMatrix. child.updateUidMatrix() - // for i := 0; i < len(child.uidMatrix); i++ { - // start, end := x.PageRange(child.Params.Cascade.First, child.Params.Cascade.Offset, len(child.uidMatrix[i].Uids)) - // child.uidMatrix[i].Uids = child.uidMatrix[i].Uids[start:end] - // } + for i := 0; i < len(child.uidMatrix); i++ { + start, end := x.PageRange(child.Params.Cascade.First, child.Params.Cascade.Offset, len(child.uidMatrix[i].Uids)) + child.uidMatrix[i].Uids = child.uidMatrix[i].Uids[start:end] + } } if len(sg.Params.Cascade.Fields) == 0 { @@ -2815,10 +2822,10 @@ func (req *Request) ProcessQuery(ctx context.Context) (err error) { } // first time at the root here. - for i := 0; i < len(sg.uidMatrix); i++ { - start, end := x.PageRange(sg.Params.Cascade.First, sg.Params.Cascade.Offset, len(sg.uidMatrix[i].Uids)) - sg.uidMatrix[i].Uids = sg.uidMatrix[i].Uids[start:end] - } + // for i := 0; i < len(sg.uidMatrix); i++ { + // start, end := x.PageRange(sg.Params.Cascade.First, sg.Params.Cascade.Offset, len(sg.uidMatrix[i].Uids)) + // sg.uidMatrix[i].Uids = sg.uidMatrix[i].Uids[start:end] + // } if err := sg.populatePostAggregation(req.Vars, []*SubGraph{}, nil); err != nil { return err From f8bd881c966a6270f7b8b438c1bb89591d97469c Mon Sep 17 00:00:00 2001 From: minhaj-shakeel Date: Wed, 17 Feb 2021 10:35:59 +0530 Subject: [PATCH 04/18] fix pagination at root --- query/query.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/query/query.go b/query/query.go index d9f77bef2e3..567c359b0e3 100644 --- a/query/query.go +++ b/query/query.go @@ -2822,10 +2822,11 @@ func (req *Request) ProcessQuery(ctx context.Context) (err error) { } // first time at the root here. - // for i := 0; i < len(sg.uidMatrix); i++ { - // start, end := x.PageRange(sg.Params.Cascade.First, sg.Params.Cascade.Offset, len(sg.uidMatrix[i].Uids)) - // sg.uidMatrix[i].Uids = sg.uidMatrix[i].Uids[start:end] - // } + sg.updateUidMatrix() + for i := 0; i < len(sg.uidMatrix); i++ { + start, end := x.PageRange(sg.Params.Cascade.First, sg.Params.Cascade.Offset, len(sg.uidMatrix[i].Uids)) + sg.uidMatrix[i].Uids = sg.uidMatrix[i].Uids[start:end] + } if err := sg.populatePostAggregation(req.Vars, []*SubGraph{}, nil); err != nil { return err From f2be50b8d4b452311a85d8c8ee14a1bcf8a49c33 Mon Sep 17 00:00:00 2001 From: minhaj-shakeel Date: Wed, 17 Feb 2021 11:05:06 +0530 Subject: [PATCH 05/18] restore old method of cascade parsing --- gql/parser.go | 20 +++------------- gql/parser_test.go | 40 +++---------------------------- graphql/dgraph/graphquery.go | 6 ++--- graphql/resolve/query_rewriter.go | 14 ++--------- query/query.go | 27 ++++++++++++++------- query/query0_test.go | 15 ------------ 6 files changed, 30 insertions(+), 92 deletions(-) diff --git a/gql/parser.go b/gql/parser.go index fad49b4959f..9574886fa82 100644 --- a/gql/parser.go +++ b/gql/parser.go @@ -68,7 +68,7 @@ type GraphQuery struct { Recurse bool RecurseArgs RecurseArgs ShortestPathArgs ShortestPathArgs - Cascade *CascadeArgs + Cascade []string IgnoreReflex bool Facets *pb.FacetParams FacetsFilter *FilterTree @@ -88,14 +88,6 @@ type GraphQuery struct { IsEmpty bool } -// CascadeArgs stores the arguments needed to process @cascade directive. -// It is introduced to ensure correct behaviour for cascade with pagination. -type CascadeArgs struct { - Fields []string - First int - Offset int -} - // RecurseArgs stores the arguments needed to process the @recurse directive. type RecurseArgs struct { Depth uint64 @@ -2135,16 +2127,10 @@ func parseCascade(it *lex.ItemIterator, gq *GraphQuery) error { // 2. @cascade } // 3. @cascade @ // 4. @cascade\n someOtherPred - gq.Cascade = &CascadeArgs{} - gq.Cascade.First, _ = strconv.Atoi(gq.Args["first"]) - delete(gq.Args, "first") - gq.Cascade.Offset, _ = strconv.Atoi(gq.Args["offset"]) - delete(gq.Args, "offset") - if items[0].Typ == itemLeftCurl || items[0].Typ == itemRightCurl || items[0]. Typ == itemAt || items[0].Typ == itemName { // __all__ implies @cascade i.e. implies values for all the children are mandatory. - gq.Cascade.Fields = append(gq.Cascade.Fields, "__all__") + gq.Cascade = append(gq.Cascade, "__all__") return nil } @@ -2171,7 +2157,7 @@ loop: if !expectArg { return item.Errorf("Expected a comma or right round but got: %v", item.Val) } - gq.Cascade.Fields = append(gq.Cascade.Fields, collectName(it, item.Val)) + gq.Cascade = append(gq.Cascade, collectName(it, item.Val)) count++ expectArg = false default: diff --git a/gql/parser_test.go b/gql/parser_test.go index 4add7262628..8384628d9ed 100644 --- a/gql/parser_test.go +++ b/gql/parser_test.go @@ -5371,7 +5371,7 @@ func TestCascade(t *testing.T) { Str: query, }) require.NoError(t, err) - require.Equal(t, gq.Query[0].Cascade.Fields[0], "__all__") + require.Equal(t, gq.Query[0].Cascade[0], "__all__") } func TestCascadeParameterized(t *testing.T) { @@ -5386,8 +5386,8 @@ func TestCascadeParameterized(t *testing.T) { Str: query, }) require.NoError(t, err) - require.Equal(t, gq.Query[0].Cascade.Fields[0], "name") - require.Equal(t, gq.Query[0].Cascade.Fields[1], "age") + require.Equal(t, gq.Query[0].Cascade[0], "name") + require.Equal(t, gq.Query[0].Cascade[1], "age") } func TestBadCascadeParameterized(t *testing.T) { @@ -5444,40 +5444,6 @@ func TestBadCascadeParameterized(t *testing.T) { } } -// __expand_all_ -// what if first = 0 and we do sort -// add test of no top level cascade and cascade on children -func TestParseCascadeWithPagination(t *testing.T) { - query := ` -{ - query(func: type(Student), first: 2) @cascade{ - name - age - classes ( first: 3, offset: 10) { - standard - subjects - teachers ( first: 4, offset: 1) { - name - age - } - } - - } -} - ` - gq, err := Parse(Request{Str: query}) - require.NoError(t, err) - require.Equal(t, len(gq.Query[0].Args), 0) - require.Equal(t, gq.Query[0].Cascade.First, 2) - require.Equal(t, gq.Query[0].Cascade.Offset, 0) - require.Equal(t, len(gq.Query[0].Children[2].Args), 0) - require.Equal(t, gq.Query[0].Children[2].Cascade.First, 3) - require.Equal(t, gq.Query[0].Children[2].Cascade.Offset, 10) - require.Equal(t, len(gq.Query[0].Children[2].Children[2].Args), 0) - require.Equal(t, gq.Query[0].Children[2].Children[2].Cascade.First, 4) - require.Equal(t, gq.Query[0].Children[2].Children[2].Cascade.Offset, 1) -} - func TestEmptyId(t *testing.T) { q := "query me($a: string) { q(func: uid($a)) { name }}" r := Request{ diff --git a/graphql/dgraph/graphquery.go b/graphql/dgraph/graphquery.go index d193f471ded..94aaac698b6 100644 --- a/graphql/dgraph/graphquery.go +++ b/graphql/dgraph/graphquery.go @@ -83,12 +83,12 @@ func writeQuery(b *strings.Builder, query *gql.GraphQuery, prefix string) { x.Check2(b.WriteRune(')')) } - if query.Cascade != nil && len(query.Cascade.Fields) != 0 { - if query.Cascade.Fields[0] == "__all__" { + if len(query.Cascade) != 0 { + if query.Cascade[0] == "__all__" { x.Check2(b.WriteString(" @cascade")) } else { x.Check2(b.WriteString(" @cascade(")) - x.Check2(b.WriteString(strings.Join(query.Cascade.Fields, ", "))) + x.Check2(b.WriteString(strings.Join(query.Cascade, ", "))) x.Check2(b.WriteRune(')')) } } diff --git a/graphql/resolve/query_rewriter.go b/graphql/resolve/query_rewriter.go index 5d39e257ded..e6db6fb1eb1 100644 --- a/graphql/resolve/query_rewriter.go +++ b/graphql/resolve/query_rewriter.go @@ -1029,7 +1029,7 @@ func (authRw *authRewriter) rewriteRuleNode( r1 := rewriteAsQuery(qry, authRw) r1[0].Var = varName r1[0].Attr = "var" - r1[0].Cascade.Fields = append(r1[0].Cascade.Fields, "__all__") + r1[0].Cascade = append(r1[0].Cascade, "__all__") return []*gql.GraphQuery{r1[0]}, &gql.FilterTree{ Func: &gql.Function{ @@ -1536,17 +1536,7 @@ func addPagination(q *gql.GraphQuery, field schema.Field) { } func addCascadeDirective(q *gql.GraphQuery, field schema.Field) { - - if len(field.Cascade()) > 0 { - first, _ := strconv.Atoi(q.Args["first"]) - delete(q.Args, "first") - offset, _ := strconv.Atoi(q.Args["offset"]) - delete(q.Args, "offset") - q.Cascade = &gql.CascadeArgs{Fields: field.Cascade(), - First: first, - Offset: offset, - } - } + q.Cascade = field.Cascade() } func convertIDs(idsSlice []interface{}) []uint64 { diff --git a/query/query.go b/query/query.go index 567c359b0e3..11268a115e0 100644 --- a/query/query.go +++ b/query/query.go @@ -159,7 +159,7 @@ type params struct { // Cascade is the list of predicates to apply @cascade to. // __all__ is special to mean @cascade i.e. all the children of this subgraph are mandatory // and should have values otherwise the node will be excluded. - Cascade *gql.CascadeArgs + Cascade *CascadeArgs // IgnoreReflex is true if the @ignorereflex directive is specified. IgnoreReflex bool @@ -206,6 +206,14 @@ type params struct { AllowedPreds []string } +// CascadeArgs stores the arguments needed to process @cascade directive. +// It is introduced to ensure correct behaviour for cascade with pagination. +type CascadeArgs struct { + Fields []string + First int + Offset int +} + type pathMetadata struct { weight float64 // Total weight of the path. } @@ -559,16 +567,16 @@ func treeCopy(gq *gql.GraphQuery, sg *SubGraph) error { GroupbyAttrs: gchild.GroupbyAttrs, IsGroupBy: gchild.IsGroupby, IsInternal: gchild.IsInternal, - Cascade: &gql.CascadeArgs{}, + Cascade: &CascadeArgs{}, } // Inherit from the parent. - if sg.Params.Cascade != nil && (len(sg.Params.Cascade.Fields) > 0) { + if len(sg.Params.Cascade.Fields) > 0 { args.Cascade.Fields = append(args.Cascade.Fields, sg.Params.Cascade.Fields...) } // Allow over-riding at this level. - if gchild.Cascade != nil && (len(gchild.Cascade.Fields) > 0) { - args.Cascade.Fields = gchild.Cascade.Fields + if len(gchild.Cascade) > 0 { + args.Cascade.Fields = gchild.Cascade } if len(args.Cascade.Fields) > 0 { @@ -785,7 +793,7 @@ func newGraph(ctx context.Context, gq *gql.GraphQuery) (*SubGraph, error) { // The attr at root (if present) would stand for the source functions attr. args := params{ Alias: gq.Alias, - Cascade: gq.Cascade, + Cascade: &CascadeArgs{Fields: gq.Cascade}, GetUid: isDebug(ctx), IgnoreReflex: gq.IgnoreReflex, IsEmpty: gq.IsEmpty, @@ -803,8 +811,11 @@ func newGraph(ctx context.Context, gq *gql.GraphQuery) (*SubGraph, error) { AllowedPreds: gq.AllowedPreds, } - if args.Cascade == nil { - args.Cascade = &gql.CascadeArgs{} + if len(args.Cascade.Fields) != 0 { + args.Cascade.First, _ = strconv.Atoi(gq.Args["first"]) + delete(gq.Args, "first") + args.Cascade.Offset, _ = strconv.Atoi(gq.Args["offset"]) + delete(gq.Args, "offset") } for argk := range gq.Args { diff --git a/query/query0_test.go b/query/query0_test.go index 48c12100f31..0a427098a20 100644 --- a/query/query0_test.go +++ b/query/query0_test.go @@ -1764,21 +1764,6 @@ func TestUseVarsMultiCascade(t *testing.T) { js) } -func TestCascadeWithPagination(t *testing.T) { - query := ` - { - query(func: type(Student), first: 2) @cascade { - name - age - class(first: 3, offset: 10) { - standard - subjects - } - } - } - ` - _ = processQueryNoErr(t, query) -} func TestUseVarsMultiOrder(t *testing.T) { query := ` From ef84edb41d0dd24823c1b41b953d2de6e7fd7879 Mon Sep 17 00:00:00 2001 From: minhaj-shakeel Date: Wed, 17 Feb 2021 11:37:32 +0530 Subject: [PATCH 06/18] minor change --- query/query.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/query/query.go b/query/query.go index 11268a115e0..f05da0ec076 100644 --- a/query/query.go +++ b/query/query.go @@ -811,7 +811,7 @@ func newGraph(ctx context.Context, gq *gql.GraphQuery) (*SubGraph, error) { AllowedPreds: gq.AllowedPreds, } - if len(args.Cascade.Fields) != 0 { + if len(args.Cascade.Fields) >= 0 { args.Cascade.First, _ = strconv.Atoi(gq.Args["first"]) delete(gq.Args, "first") args.Cascade.Offset, _ = strconv.Atoi(gq.Args["offset"]) From 4cbe38da0ef081703265e50d35e91280f54e67cc Mon Sep 17 00:00:00 2001 From: minhaj-shakeel Date: Fri, 19 Feb 2021 02:02:18 +0530 Subject: [PATCH 07/18] fix pagination at root --- query/query.go | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/query/query.go b/query/query.go index f05da0ec076..53045e90fbe 100644 --- a/query/query.go +++ b/query/query.go @@ -1371,9 +1371,11 @@ func (sg *SubGraph) populateVarMap(doneVars map[string]varValue, sgPath []*SubGr // by other operations. So we need to apply it on the UidMatrix. child.updateUidMatrix() - for i := 0; i < len(child.uidMatrix); i++ { - start, end := x.PageRange(child.Params.Cascade.First, child.Params.Cascade.Offset, len(child.uidMatrix[i].Uids)) - child.uidMatrix[i].Uids = child.uidMatrix[i].Uids[start:end] + if child.Params.Cascade.First != 0 && child.Params.Cascade.Offset != 0 { + for i := 0; i < len(child.uidMatrix); i++ { + start, end := x.PageRange(child.Params.Cascade.First, child.Params.Cascade.Offset, len(child.uidMatrix[i].Uids)) + child.uidMatrix[i].Uids = child.uidMatrix[i].Uids[start:end] + } } } @@ -2160,7 +2162,11 @@ func ProcessGraph(ctx context.Context, sg, parent *SubGraph, rch chan error) { if parent == nil { // I'm root. We reach here if root had a function. - sg.uidMatrix = []*pb.List{sg.DestUIDs} + newDestUidList := &pb.List{Uids: make([]uint64, 0, len(sg.DestUIDs.Uids))} + for _, uid := range sg.DestUIDs.GetUids() { + newDestUidList.Uids = append(newDestUidList.Uids, uid) + } + sg.uidMatrix = []*pb.List{newDestUidList} } } } @@ -2833,10 +2839,12 @@ func (req *Request) ProcessQuery(ctx context.Context) (err error) { } // first time at the root here. - sg.updateUidMatrix() - for i := 0; i < len(sg.uidMatrix); i++ { - start, end := x.PageRange(sg.Params.Cascade.First, sg.Params.Cascade.Offset, len(sg.uidMatrix[i].Uids)) - sg.uidMatrix[i].Uids = sg.uidMatrix[i].Uids[start:end] + if len(sg.Params.Cascade.Fields) > 0 { + sg.updateUidMatrix() + for i := 0; i < len(sg.uidMatrix); i++ { + start, end := x.PageRange(sg.Params.Cascade.First, sg.Params.Cascade.Offset, len(sg.uidMatrix[i].Uids)) + sg.uidMatrix[i].Uids = sg.uidMatrix[i].Uids[start:end] + } } if err := sg.populatePostAggregation(req.Vars, []*SubGraph{}, nil); err != nil { From 0a1e27a1391d0f9acc8439a6d563e81519020f7c Mon Sep 17 00:00:00 2001 From: minhaj-shakeel Date: Fri, 19 Feb 2021 02:26:35 +0530 Subject: [PATCH 08/18] minor fix --- query/query.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/query/query.go b/query/query.go index 53045e90fbe..4b554e9d6de 100644 --- a/query/query.go +++ b/query/query.go @@ -811,7 +811,7 @@ func newGraph(ctx context.Context, gq *gql.GraphQuery) (*SubGraph, error) { AllowedPreds: gq.AllowedPreds, } - if len(args.Cascade.Fields) >= 0 { + if len(args.Cascade.Fields) > 0 { args.Cascade.First, _ = strconv.Atoi(gq.Args["first"]) delete(gq.Args, "first") args.Cascade.Offset, _ = strconv.Atoi(gq.Args["offset"]) From ce93ba6e9a032e4c8aadaebe48f465a5707a8eb6 Mon Sep 17 00:00:00 2001 From: minhaj-shakeel Date: Fri, 19 Feb 2021 10:35:24 +0530 Subject: [PATCH 09/18] add comments --- query/query.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/query/query.go b/query/query.go index 4b554e9d6de..c14a5502d06 100644 --- a/query/query.go +++ b/query/query.go @@ -579,6 +579,8 @@ func treeCopy(gq *gql.GraphQuery, sg *SubGraph) error { args.Cascade.Fields = gchild.Cascade } + // Remove pagination arguments from the query if @cascade is mentioned since + // pagination will be applied post processing the data. if len(args.Cascade.Fields) > 0 { args.Cascade.First, _ = strconv.Atoi(gchild.Args["first"]) args.Cascade.Offset, _ = strconv.Atoi(gchild.Args["offset"]) @@ -811,6 +813,8 @@ func newGraph(ctx context.Context, gq *gql.GraphQuery) (*SubGraph, error) { AllowedPreds: gq.AllowedPreds, } + // Remove pagination arguments from the query if @cascade is mentioned since + // pagination will be applied post processing the data. if len(args.Cascade.Fields) > 0 { args.Cascade.First, _ = strconv.Atoi(gq.Args["first"]) delete(gq.Args, "first") @@ -1371,7 +1375,8 @@ func (sg *SubGraph) populateVarMap(doneVars map[string]varValue, sgPath []*SubGr // by other operations. So we need to apply it on the UidMatrix. child.updateUidMatrix() - if child.Params.Cascade.First != 0 && child.Params.Cascade.Offset != 0 { + // Apply pagination after the @cascade. + if len(child.Params.Cascade.Fields) > 0 && child.Params.Cascade.First != 0 && child.Params.Cascade.Offset != 0 { for i := 0; i < len(child.uidMatrix); i++ { start, end := x.PageRange(child.Params.Cascade.First, child.Params.Cascade.Offset, len(child.uidMatrix[i].Uids)) child.uidMatrix[i].Uids = child.uidMatrix[i].Uids[start:end] @@ -2839,8 +2844,9 @@ func (req *Request) ProcessQuery(ctx context.Context) (err error) { } // first time at the root here. - if len(sg.Params.Cascade.Fields) > 0 { - sg.updateUidMatrix() + sg.updateUidMatrix() + //Apply pagination at the root after @cascade. + if len(sg.Params.Cascade.Fields) > 0 && sg.Params.Cascade.First != 0 && sg.Params.Cascade.First != 0 { for i := 0; i < len(sg.uidMatrix); i++ { start, end := x.PageRange(sg.Params.Cascade.First, sg.Params.Cascade.Offset, len(sg.uidMatrix[i].Uids)) sg.uidMatrix[i].Uids = sg.uidMatrix[i].Uids[start:end] From 88c1f4ee203b0d09b5d17a8f9632b0a518e3f0b7 Mon Sep 17 00:00:00 2001 From: minhaj-shakeel Date: Fri, 19 Feb 2021 11:18:51 +0530 Subject: [PATCH 10/18] add test cases --- query/query0_test.go | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/query/query0_test.go b/query/query0_test.go index 0a427098a20..27d5e68a5a8 100644 --- a/query/query0_test.go +++ b/query/query0_test.go @@ -487,6 +487,41 @@ func TestCascadeDirective(t *testing.T) { js) } +func TestCascadeWithPagination(t *testing.T) { + query := ` + { + me(func: uid(0x01)) @cascade { + name + gender + friend(first: 2, offset: 1) { + name + friend{ + name + dob + age + } + } + } + } + ` + + js := processQueryNoErr(t, query) + require.JSONEq(t, `{"data": {"me":[{"friend":[{"friend":[{"age":15,"dob":"1909-05-05T00:00:00Z","name":"Glenn Rhee"}],"name":"Andrea"}],"gender":"female","name":"Michonne"}]}}`, + js) +} + +func TestCascadeWithPaginationAtRoot(t *testing.T) { + query := ` + { + me(func: type(Person), first: 2, offset: 2) @cascade{ + name + alive + } + } + ` + js := processQueryNoErr(t, query) + require.JSONEq(t, `{"data":{"me":[{"name":"Andrea","alive":false}]}}`) +} func TestLevelBasedFacetVarAggSum(t *testing.T) { query := ` { From 9e494752ab96ee0c54a44dd3ec47b41119233ddd Mon Sep 17 00:00:00 2001 From: minhaj-shakeel Date: Fri, 19 Feb 2021 11:53:01 +0530 Subject: [PATCH 11/18] fix test cases --- query/query0_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/query/query0_test.go b/query/query0_test.go index 27d5e68a5a8..fb691004c6f 100644 --- a/query/query0_test.go +++ b/query/query0_test.go @@ -520,7 +520,7 @@ func TestCascadeWithPaginationAtRoot(t *testing.T) { } ` js := processQueryNoErr(t, query) - require.JSONEq(t, `{"data":{"me":[{"name":"Andrea","alive":false}]}}`) + require.JSONEq(t, `{"data":{"me":[{"name":"Andrea","alive":false}]}}`, js) } func TestLevelBasedFacetVarAggSum(t *testing.T) { query := ` From 1faa558d64461a8b41c7b9686b18ecb0b3a5cc12 Mon Sep 17 00:00:00 2001 From: minhaj-shakeel Date: Fri, 19 Feb 2021 12:28:57 +0530 Subject: [PATCH 12/18] fix build --- query/query.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/query/query.go b/query/query.go index c14a5502d06..09c2ef9c158 100644 --- a/query/query.go +++ b/query/query.go @@ -2846,7 +2846,7 @@ func (req *Request) ProcessQuery(ctx context.Context) (err error) { sg.updateUidMatrix() //Apply pagination at the root after @cascade. - if len(sg.Params.Cascade.Fields) > 0 && sg.Params.Cascade.First != 0 && sg.Params.Cascade.First != 0 { + if len(sg.Params.Cascade.Fields) > 0 && sg.Params.Cascade.First != 0 && sg.Params.Cascade.Offset != 0 { for i := 0; i < len(sg.uidMatrix); i++ { start, end := x.PageRange(sg.Params.Cascade.First, sg.Params.Cascade.Offset, len(sg.uidMatrix[i].Uids)) sg.uidMatrix[i].Uids = sg.uidMatrix[i].Uids[start:end] From 1003ff781d66c905ed42293be2613b16133f2c05 Mon Sep 17 00:00:00 2001 From: minhaj-shakeel Date: Fri, 19 Feb 2021 12:42:51 +0530 Subject: [PATCH 13/18] make updating uid matrix conditional --- query/query.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/query/query.go b/query/query.go index 09c2ef9c158..6f0ec6e40a4 100644 --- a/query/query.go +++ b/query/query.go @@ -2844,9 +2844,9 @@ func (req *Request) ProcessQuery(ctx context.Context) (err error) { } // first time at the root here. - sg.updateUidMatrix() //Apply pagination at the root after @cascade. if len(sg.Params.Cascade.Fields) > 0 && sg.Params.Cascade.First != 0 && sg.Params.Cascade.Offset != 0 { + sg.updateUidMatrix() for i := 0; i < len(sg.uidMatrix); i++ { start, end := x.PageRange(sg.Params.Cascade.First, sg.Params.Cascade.Offset, len(sg.uidMatrix[i].Uids)) sg.uidMatrix[i].Uids = sg.uidMatrix[i].Uids[start:end] From 4cf2c9e70f046a6b709ddf27db626e9c4799c282 Mon Sep 17 00:00:00 2001 From: minhaj-shakeel Date: Fri, 19 Feb 2021 12:44:53 +0530 Subject: [PATCH 14/18] add newline --- query/query0_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/query/query0_test.go b/query/query0_test.go index fb691004c6f..4ec829405a0 100644 --- a/query/query0_test.go +++ b/query/query0_test.go @@ -522,6 +522,7 @@ func TestCascadeWithPaginationAtRoot(t *testing.T) { js := processQueryNoErr(t, query) require.JSONEq(t, `{"data":{"me":[{"name":"Andrea","alive":false}]}}`, js) } + func TestLevelBasedFacetVarAggSum(t *testing.T) { query := ` { From 701e10cabcd453fed1821984007ea64679d845e3 Mon Sep 17 00:00:00 2001 From: minhaj-shakeel Date: Fri, 19 Feb 2021 14:15:26 +0530 Subject: [PATCH 15/18] address Pawan's comments --- query/query.go | 35 ++++++++++++++++++++++------------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/query/query.go b/query/query.go index 6f0ec6e40a4..71eb6379b1c 100644 --- a/query/query.go +++ b/query/query.go @@ -582,10 +582,7 @@ func treeCopy(gq *gql.GraphQuery, sg *SubGraph) error { // Remove pagination arguments from the query if @cascade is mentioned since // pagination will be applied post processing the data. if len(args.Cascade.Fields) > 0 { - args.Cascade.First, _ = strconv.Atoi(gchild.Args["first"]) - args.Cascade.Offset, _ = strconv.Atoi(gchild.Args["offset"]) - delete(gchild.Args, "first") - delete(gchild.Args, "offset") + args.addCascadePaginationArguments(gchild) } if gchild.IsCount { @@ -664,6 +661,13 @@ func treeCopy(gq *gql.GraphQuery, sg *SubGraph) error { return nil } +func (args *params) addCascadePaginationArguments(gq *gql.GraphQuery) { + args.Cascade.First, _ = strconv.Atoi(gq.Args["first"]) + delete(gq.Args, "first") + args.Cascade.Offset, _ = strconv.Atoi(gq.Args["offset"]) + delete(gq.Args, "offset") +} + func (args *params) fill(gq *gql.GraphQuery) error { if v, ok := gq.Args["offset"]; ok { offset, err := strconv.ParseInt(v, 0, 32) @@ -816,10 +820,7 @@ func newGraph(ctx context.Context, gq *gql.GraphQuery) (*SubGraph, error) { // Remove pagination arguments from the query if @cascade is mentioned since // pagination will be applied post processing the data. if len(args.Cascade.Fields) > 0 { - args.Cascade.First, _ = strconv.Atoi(gq.Args["first"]) - delete(gq.Args, "first") - args.Cascade.Offset, _ = strconv.Atoi(gq.Args["offset"]) - delete(gq.Args, "offset") + args.addCascadePaginationArguments(gq) } for argk := range gq.Args { @@ -2167,11 +2168,19 @@ func ProcessGraph(ctx context.Context, sg, parent *SubGraph, rch chan error) { if parent == nil { // I'm root. We reach here if root had a function. - newDestUidList := &pb.List{Uids: make([]uint64, 0, len(sg.DestUIDs.Uids))} - for _, uid := range sg.DestUIDs.GetUids() { - newDestUidList.Uids = append(newDestUidList.Uids, uid) + + if len(sg.Params.Cascade.Fields) >= 0 { + // DesitUIDs for this level becomes the sourceUIDs for the next level. In updateUidMatrix with cascade, + // we end up modifying the first list from the uidMatrix which ends up modifying the srcUids of the next level. + // So to avoid that we make a copy. + newDestUIDList := &pb.List{Uids: make([]uint64, 0, len(sg.DestUIDs.Uids))} + for _, uid := range sg.DestUIDs.GetUids() { + newDestUidList.Uids = append(newDestUIDList.Uids, uid) + } + sg.uidMatrix = []*pb.List{newDestUidList} + } else { + sg.uidMatrix = []*pb.List{sg.DestUIDs} } - sg.uidMatrix = []*pb.List{newDestUidList} } } } @@ -2844,7 +2853,7 @@ func (req *Request) ProcessQuery(ctx context.Context) (err error) { } // first time at the root here. - //Apply pagination at the root after @cascade. + // Apply pagination at the root after @cascade. if len(sg.Params.Cascade.Fields) > 0 && sg.Params.Cascade.First != 0 && sg.Params.Cascade.Offset != 0 { sg.updateUidMatrix() for i := 0; i < len(sg.uidMatrix); i++ { From 927f87221775ec4b30f871a498b8e8d64e21d6e9 Mon Sep 17 00:00:00 2001 From: minhaj-shakeel Date: Fri, 19 Feb 2021 14:35:53 +0530 Subject: [PATCH 16/18] add deep test --- query/query0_test.go | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/query/query0_test.go b/query/query0_test.go index 4ec829405a0..ba2acf7d411 100644 --- a/query/query0_test.go +++ b/query/query0_test.go @@ -487,27 +487,24 @@ func TestCascadeDirective(t *testing.T) { js) } -func TestCascadeWithPagination(t *testing.T) { +func TestCascadeWithPaginationDeep(t *testing.T) { query := ` { - me(func: uid(0x01)) @cascade { + me(func: type("Person")) @cascade{ + name + friend { name - gender friend(first: 2, offset: 1) { - name - friend{ - name - dob - age - } + name + alive } + } } - } + } ` js := processQueryNoErr(t, query) - require.JSONEq(t, `{"data": {"me":[{"friend":[{"friend":[{"age":15,"dob":"1909-05-05T00:00:00Z","name":"Glenn Rhee"}],"name":"Andrea"}],"gender":"female","name":"Michonne"}]}}`, - js) + require,JSONEq(t,`{"data":{"me":[{"name":"Rick Grimes","friend":[{"name": "Michonne","friend":[{"name":"Daryl Dixon","alive":false},{"name": "Andrea","alive": false}]}]}]}}`, js) } func TestCascadeWithPaginationAtRoot(t *testing.T) { From 976ec3463aa5cbd7235b35da75a5e33d0a8e0a73 Mon Sep 17 00:00:00 2001 From: minhaj-shakeel Date: Fri, 19 Feb 2021 14:50:16 +0530 Subject: [PATCH 17/18] change typo --- query/query.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/query/query.go b/query/query.go index 71eb6379b1c..a2b788142b1 100644 --- a/query/query.go +++ b/query/query.go @@ -2175,9 +2175,9 @@ func ProcessGraph(ctx context.Context, sg, parent *SubGraph, rch chan error) { // So to avoid that we make a copy. newDestUIDList := &pb.List{Uids: make([]uint64, 0, len(sg.DestUIDs.Uids))} for _, uid := range sg.DestUIDs.GetUids() { - newDestUidList.Uids = append(newDestUIDList.Uids, uid) + newDestUIDList.Uids = append(newDestUIDList.Uids, uid) } - sg.uidMatrix = []*pb.List{newDestUidList} + sg.uidMatrix = []*pb.List{newDestUIDList} } else { sg.uidMatrix = []*pb.List{sg.DestUIDs} } From 4088f05e972b3b3423a1a0816337d3d65dd2da9d Mon Sep 17 00:00:00 2001 From: minhaj-shakeel Date: Fri, 19 Feb 2021 15:07:10 +0530 Subject: [PATCH 18/18] fix test --- query/query0_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/query/query0_test.go b/query/query0_test.go index ba2acf7d411..4b6469d25ab 100644 --- a/query/query0_test.go +++ b/query/query0_test.go @@ -504,7 +504,7 @@ func TestCascadeWithPaginationDeep(t *testing.T) { ` js := processQueryNoErr(t, query) - require,JSONEq(t,`{"data":{"me":[{"name":"Rick Grimes","friend":[{"name": "Michonne","friend":[{"name":"Daryl Dixon","alive":false},{"name": "Andrea","alive": false}]}]}]}}`, js) + require.JSONEq(t, `{"data":{"me":[{"name":"Rick Grimes","friend":[{"name": "Michonne","friend":[{"name":"Daryl Dixon","alive":false},{"name": "Andrea","alive": false}]}]}]}}`, js) } func TestCascadeWithPaginationAtRoot(t *testing.T) {