From db048fdee5142ed4db3ac8265f9f4ba53330cb58 Mon Sep 17 00:00:00 2001 From: Manish R Jain Date: Mon, 2 May 2016 12:35:47 +1000 Subject: [PATCH 1/5] Move argument parsing in a separate function --- gql/parser.go | 89 +++++++++++++++++++++++++++++++--------------- gql/parser_test.go | 30 ++++++++++++++++ 2 files changed, 91 insertions(+), 28 deletions(-) diff --git a/gql/parser.go b/gql/parser.go index 42b099fd75f..2f7cc111168 100644 --- a/gql/parser.go +++ b/gql/parser.go @@ -33,6 +33,7 @@ type GraphQuery struct { UID uint64 XID string Attr string + First int Children []*GraphQuery } @@ -41,6 +42,11 @@ type Mutation struct { Del string } +type pair struct { + Key string + Val string +} + func run(l *lex.Lexer) { for state := lexText; state != nil; { state = state(l) @@ -142,6 +148,35 @@ func parseMutationOp(l *lex.Lexer, op string, mu *Mutation) error { return errors.New("Invalid mutation formatting.") } +func parseArguments(l *lex.Lexer) (result []pair, rerr error) { + for { + var p pair + + // Get key. + item := <-l.Items + if item.Typ == itemArgName { + p.Key = item.Val + + } else if item.Typ == itemRightRound { + break + + } else { + return result, fmt.Errorf("Expecting argument name. Got: %v", item) + } + + // Get value. + item = <-l.Items + if item.Typ == itemArgVal { + p.Val = item.Val + } else { + return result, fmt.Errorf("Expecting argument value. Got: %v", item) + } + + result = append(result, p) + } + return result, nil +} + func getRoot(l *lex.Lexer) (gq *GraphQuery, rerr error) { item := <-l.Items if item.Typ != itemName { @@ -155,40 +190,24 @@ func getRoot(l *lex.Lexer) (gq *GraphQuery, rerr error) { var uid uint64 var xid string - for { - var key, val string - // Get key or close bracket - item = <-l.Items - if item.Typ == itemArgName { - key = item.Val - } else if item.Typ == itemRightRound { - break - } else { - return nil, fmt.Errorf("Expecting argument name. Got: %v", item) - } - - // Get corresponding value. - item = <-l.Items - if item.Typ == itemArgVal { - val = item.Val - } else { - return nil, fmt.Errorf("Expecting argument va Got: %v", item) - } - - if key == "_uid_" { - uid, rerr = strconv.ParseUint(val, 0, 64) + args, err := parseArguments(l) + if err != nil { + return nil, err + } + for _, p := range args { + if p.Key == "_uid_" { + uid, rerr = strconv.ParseUint(p.Val, 0, 64) if rerr != nil { return nil, rerr } - } else if key == "_xid_" { - xid = val + } else if p.Key == "_xid_" { + xid = p.Val + } else { - return nil, fmt.Errorf("Expecting _uid_ or _xid_. Got: %v", item) + return nil, fmt.Errorf("Expecting _uid_ or _xid_. Got: %+v", p) } } - if item.Typ != itemRightRound { - return nil, fmt.Errorf("Unexpected token. Got: %v", item) - } + gq = new(GraphQuery) gq.UID = uid gq.XID = xid @@ -220,7 +239,21 @@ func godeep(l *lex.Lexer, gq *GraphQuery) error { } else if item.Typ == itemLeftRound { // absorb all these, we don't use them right now. + /* + for { + var key, val string + item = <-l.Items + if item.Typ == itemArgName { + key = item.Val + } else if item.Typ == itemRightRound { + break + } else { + return nil, fmt.Errorf("Expecting argument name. Got: %v", item) + } + } + */ for ti := range l.Items { + fmt.Println(ti.String()) if ti.Typ == itemRightRound || ti.Typ == lex.ItemEOF { return nil } diff --git a/gql/parser_test.go b/gql/parser_test.go index af9fb109d84..fb5d1c81a0a 100644 --- a/gql/parser_test.go +++ b/gql/parser_test.go @@ -77,6 +77,7 @@ func TestParse(t *testing.T) { func TestParseXid(t *testing.T) { // logrus.SetLevel(logrus.DebugLevel) + // TODO: Why does the query not have _xid_ attribute? query := ` query { user(_uid_: 0x11) { @@ -100,6 +101,35 @@ func TestParseXid(t *testing.T) { } } +/* +func TestParseFirst(t *testing.T) { + // logrus.SetLevel(logrus.DebugLevel) + query := ` + query { + user(_xid_: m.abcd) { + type.object.name + friends (first: 10) { + } + } + }` + gq, _, err := Parse(query) + if err != nil { + t.Error(err) + return + } + if gq == nil { + t.Error("subgraph is nil") + return + } + if len(gq.Children) != 1 { + t.Errorf("Expected 1 children. Got: %v", len(gq.Children)) + } + if err := checkAttr(gq.Children[0], "type.object.name"); err != nil { + t.Error(err) + } +} +*/ + func TestParse_error2(t *testing.T) { query := ` query { From 3048929de4eef9d62afd003fe941574db6cf9567 Mon Sep 17 00:00:00 2001 From: Manish R Jain Date: Tue, 3 May 2016 10:11:51 +1000 Subject: [PATCH 2/5] Correctly parse argument first --- gql/parser.go | 29 ++++++++++++----------------- gql/parser_test.go | 33 ++++++++++++++++++++++++++++----- 2 files changed, 40 insertions(+), 22 deletions(-) diff --git a/gql/parser.go b/gql/parser.go index 2f7cc111168..bc63388140d 100644 --- a/gql/parser.go +++ b/gql/parser.go @@ -238,24 +238,19 @@ func godeep(l *lex.Lexer, gq *GraphQuery) error { return nil } else if item.Typ == itemLeftRound { - // absorb all these, we don't use them right now. - /* - for { - var key, val string - item = <-l.Items - if item.Typ == itemArgName { - key = item.Val - } else if item.Typ == itemRightRound { - break - } else { - return nil, fmt.Errorf("Expecting argument name. Got: %v", item) + + args, err := parseArguments(l) + if err != nil { + return err + } + // We only use argument 'first' for now. + for _, p := range args { + if p.Key == "first" { + count, err := strconv.ParseInt(p.Val, 0, 32) + if err != nil { + return err } - } - */ - for ti := range l.Items { - fmt.Println(ti.String()) - if ti.Typ == itemRightRound || ti.Typ == lex.ItemEOF { - return nil + curp.First = int(count) } } } diff --git a/gql/parser_test.go b/gql/parser_test.go index fb5d1c81a0a..e9bea44397c 100644 --- a/gql/parser_test.go +++ b/gql/parser_test.go @@ -101,9 +101,7 @@ func TestParseXid(t *testing.T) { } } -/* func TestParseFirst(t *testing.T) { - // logrus.SetLevel(logrus.DebugLevel) query := ` query { user(_xid_: m.abcd) { @@ -121,14 +119,39 @@ func TestParseFirst(t *testing.T) { t.Error("subgraph is nil") return } - if len(gq.Children) != 1 { - t.Errorf("Expected 1 children. Got: %v", len(gq.Children)) + if len(gq.Children) != 2 { + t.Errorf("Expected 2 children. Got: %v", len(gq.Children)) } if err := checkAttr(gq.Children[0], "type.object.name"); err != nil { t.Error(err) } + if gq.Children[0].First != 0 { + t.Errorf("Expected count 0. Got: %v", gq.Children[0].First) + } + if err := checkAttr(gq.Children[1], "friends"); err != nil { + t.Error(err) + } + if gq.Children[1].First != 10 { + t.Errorf("Expected count 10. Got: %v", gq.Children[1].First) + } +} + +func TestParseFirst_error(t *testing.T) { + query := ` + query { + user(_xid_: m.abcd) { + type.object.name + friends (first: ) { + } + } + }` + var err error + _, _, err = Parse(query) + t.Log(err) + if err == nil { + t.Error("Expected error") + } } -*/ func TestParse_error2(t *testing.T) { query := ` From f80cd55241a38defbbf8d9ff2982ee8103e18b63 Mon Sep 17 00:00:00 2001 From: Manish R Jain Date: Tue, 3 May 2016 11:44:40 +1000 Subject: [PATCH 3/5] Working end to end --- gql/parser.go | 5 ++--- posting/list.go | 26 +++++++++++++++++++++++--- posting/list_test.go | 26 ++++++++++++++++++++++++++ query/query.go | 17 +++++++++++++---- task.fbs | 2 ++ task/Query.go | 20 +++++++++++++++++++- worker/task.go | 2 +- 7 files changed, 86 insertions(+), 12 deletions(-) diff --git a/gql/parser.go b/gql/parser.go index bc63388140d..1e9d8643861 100644 --- a/gql/parser.go +++ b/gql/parser.go @@ -152,7 +152,7 @@ func parseArguments(l *lex.Lexer) (result []pair, rerr error) { for { var p pair - // Get key. + // Get key item := <-l.Items if item.Typ == itemArgName { p.Key = item.Val @@ -164,7 +164,7 @@ func parseArguments(l *lex.Lexer) (result []pair, rerr error) { return result, fmt.Errorf("Expecting argument name. Got: %v", item) } - // Get value. + // Get value item = <-l.Items if item.Typ == itemArgVal { p.Val = item.Val @@ -238,7 +238,6 @@ func godeep(l *lex.Lexer, gq *GraphQuery) error { return nil } else if item.Typ == itemLeftRound { - args, err := parseArguments(l) if err != nil { return err diff --git a/posting/list.go b/posting/list.go index 6f7ae0088b2..e858cf676d6 100644 --- a/posting/list.go +++ b/posting/list.go @@ -720,15 +720,35 @@ func (l *List) LastCompactionTs() time.Time { return l.lastCompact } -func (l *List) GetUids() []uint64 { +func (l *List) GetUids(offset, count int) []uint64 { l.wg.Wait() l.RLock() defer l.RUnlock() - result := make([]uint64, l.length()) + if offset < 0 { + glog.WithField("offset", offset).Fatal("Unexpected offset") + return make([]uint64, 0) + } + + if count < 0 { + count = 0 - count + offset = l.length() - count + } + + if count == 0 { + count = l.length() - offset + + } else if count > l.length()-offset { + count = l.length() - offset + } + if count < 0 { + count = 0 + } + + result := make([]uint64, count) result = result[:0] var p types.Posting - for i := 0; i < l.length(); i++ { + for i := offset; i < count+offset && i < l.length(); i++ { if ok := l.get(&p, i); !ok || p.Uid() == math.MaxUint64 { break } diff --git a/posting/list_test.go b/posting/list_test.go index 824573680a7..79cde4bcbef 100644 --- a/posting/list_test.go +++ b/posting/list_test.go @@ -44,6 +44,32 @@ func checkUids(t *testing.T, l *List, uids ...uint64) error { return fmt.Errorf("Expected: %v. Got: %v", uids[i], p.Uid()) } } + if len(uids) >= 3 { + ruids := l.GetUids(1, 2) + if len(ruids) != 2 { + return fmt.Errorf("Expected result of length: 2. Got: %v", len(ruids)) + } + + for i := 0; i < len(ruids); i++ { + if ruids[i] != uids[1+i] { + return fmt.Errorf("GetUids expected: %v. Got: %v", uids[1+i], ruids[i]) + } + } + + ruids = l.GetUids(1, -2) // offset should be ignored. + ulen := len(uids) + if ulen > 2 && len(ruids) != 2 { + return fmt.Errorf("Expected result of length: 2. Got: %v", len(ruids)) + } + + for i := 0; i < len(ruids); i++ { + if ruids[i] != uids[ulen-2+i] { + return fmt.Errorf("GetUids neg count expected: %v. Got: %v", + uids[ulen-2+i], ruids[i]) + } + } + } + return nil } diff --git a/query/query.go b/query/query.go index e06a58761e3..ee2643e0364 100644 --- a/query/query.go +++ b/query/query.go @@ -110,6 +110,8 @@ func (l *Latency) ToMap() map[string]string { // client convenient formats, like GraphQL / JSON. type SubGraph struct { Attr string + Count int + Offset int Children []*SubGraph query []byte @@ -325,9 +327,14 @@ func (g *SubGraph) PreTraverse() (gr *pb.GraphResponse, rerr error) { } func treeCopy(gq *gql.GraphQuery, sg *SubGraph) { + // Typically you act on the current node, and leave recursion to deal with + // children. But, in this case, we don't want to muck with the current + // node, because of the way we're dealing with the root node. + // So, we work on the children, and then recurse for grand children. for _, gchild := range gq.Children { dst := new(SubGraph) dst.Attr = gchild.Attr + dst.Count = gchild.First sg.Children = append(sg.Children, dst) treeCopy(gchild, dst) } @@ -394,14 +401,14 @@ func newGraph(euid uint64, exid string) (*SubGraph, error) { sg.Attr = "_root_" sg.result = b.Bytes[b.Head():] // Also add query for consistency and to allow for ToJson() later. - sg.query = createTaskQuery(sg.Attr, []uint64{euid}) + sg.query = createTaskQuery(sg, []uint64{euid}) return sg, nil } // createTaskQuery generates the query buffer. -func createTaskQuery(attr string, sorted []uint64) []byte { +func createTaskQuery(sg *SubGraph, sorted []uint64) []byte { b := flatbuffers.NewBuilder(0) - ao := b.CreateString(attr) + ao := b.CreateString(sg.Attr) task.QueryStartUidsVector(b, len(sorted)) for i := len(sorted) - 1; i >= 0; i-- { @@ -412,6 +419,8 @@ func createTaskQuery(attr string, sorted []uint64) []byte { task.QueryStart(b) task.QueryAddAttr(b, ao) task.QueryAddUids(b, vend) + task.QueryAddCount(b, int32(sg.Count)) + qend := task.QueryEnd(b) b.Finish(qend) return b.Bytes[b.Head():] @@ -531,7 +540,7 @@ func ProcessGraph(sg *SubGraph, rch chan error, td time.Duration) { childchan := make(chan error, len(sg.Children)) for i := 0; i < len(sg.Children); i++ { child := sg.Children[i] - child.query = createTaskQuery(child.Attr, sorted) + child.query = createTaskQuery(child, sorted) go ProcessGraph(child, childchan, timeleft) } diff --git a/task.fbs b/task.fbs index 2e3acbb0871..3e6f687d1a0 100644 --- a/task.fbs +++ b/task.fbs @@ -3,6 +3,8 @@ namespace task; table Query { attr:string; uids:[ulong]; + count:int; + offset:int; } table Value { diff --git a/task/Query.go b/task/Query.go index 9a2b8e25d4a..d28c04a77ca 100644 --- a/task/Query.go +++ b/task/Query.go @@ -39,9 +39,27 @@ func (rcv *Query) UidsLength() int { return 0 } -func QueryStart(builder *flatbuffers.Builder) { builder.StartObject(2) } +func (rcv *Query) Count() int32 { + o := flatbuffers.UOffsetT(rcv._tab.Offset(8)) + if o != 0 { + return rcv._tab.GetInt32(o + rcv._tab.Pos) + } + return 0 +} + +func (rcv *Query) Offset() int32 { + o := flatbuffers.UOffsetT(rcv._tab.Offset(10)) + if o != 0 { + return rcv._tab.GetInt32(o + rcv._tab.Pos) + } + return 0 +} + +func QueryStart(builder *flatbuffers.Builder) { builder.StartObject(4) } func QueryAddAttr(builder *flatbuffers.Builder, attr flatbuffers.UOffsetT) { builder.PrependUOffsetTSlot(0, flatbuffers.UOffsetT(attr), 0) } func QueryAddUids(builder *flatbuffers.Builder, uids flatbuffers.UOffsetT) { builder.PrependUOffsetTSlot(1, flatbuffers.UOffsetT(uids), 0) } func QueryStartUidsVector(builder *flatbuffers.Builder, numElems int) flatbuffers.UOffsetT { return builder.StartVector(8, numElems, 8) } +func QueryAddCount(builder *flatbuffers.Builder, count int32) { builder.PrependInt32Slot(2, count, 0) } +func QueryAddOffset(builder *flatbuffers.Builder, offset int32) { builder.PrependInt32Slot(3, offset, 0) } func QueryEnd(builder *flatbuffers.Builder) flatbuffers.UOffsetT { return builder.EndObject() } diff --git a/worker/task.go b/worker/task.go index e6e2f7eee6e..358fe322371 100644 --- a/worker/task.go +++ b/worker/task.go @@ -94,7 +94,7 @@ func processTask(query []byte) (result []byte, rerr error) { task.ValueAddVal(b, valoffset) voffsets[i] = task.ValueEnd(b) - ulist := pl.GetUids() + ulist := pl.GetUids(int(q.Offset()), int(q.Count())) uoffsets[i] = x.UidlistOffset(b, ulist) } task.ResultStartValuesVector(b, len(voffsets)) From 337b2fce06eb6a39b5cf4591232f87f5c817f0bd Mon Sep 17 00:00:00 2001 From: Manish R Jain Date: Tue, 3 May 2016 16:04:33 +1000 Subject: [PATCH 4/5] Fix a bug due to handle negative offset --- posting/list.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/posting/list.go b/posting/list.go index e858cf676d6..92a56e2ec18 100644 --- a/posting/list.go +++ b/posting/list.go @@ -734,6 +734,9 @@ func (l *List) GetUids(offset, count int) []uint64 { count = 0 - count offset = l.length() - count } + if offset < 0 { + offset = 0 + } if count == 0 { count = l.length() - offset From dc5bb7cdd5bf815eb314de7d0afdf9b8b117ec11 Mon Sep 17 00:00:00 2001 From: Manish R Jain Date: Tue, 3 May 2016 18:45:52 +1000 Subject: [PATCH 5/5] merge if else to if --- posting/list.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/posting/list.go b/posting/list.go index 92a56e2ec18..8fec9f188bd 100644 --- a/posting/list.go +++ b/posting/list.go @@ -738,10 +738,7 @@ func (l *List) GetUids(offset, count int) []uint64 { offset = 0 } - if count == 0 { - count = l.length() - offset - - } else if count > l.length()-offset { + if count == 0 || count > l.length()-offset { count = l.length() - offset } if count < 0 {