From 62013d3dfc74a27bf00d6d2d7791ea78867b2a1f Mon Sep 17 00:00:00 2001 From: kortschak Date: Fri, 5 Sep 2014 09:05:02 +0930 Subject: [PATCH 1/4] Replace DebugString with Describe This change makes tree description completely open to mechanical analysis and ensures consistency between description formats for each of the iterator types. Renamed StatsContainer.(Kind -> Type) for consistency. --- graph/bolt/all_iterator.go | 11 +++++++--- graph/bolt/iterator.go | 20 ++++++++--------- graph/iterator.go | 22 ++++++++++++++----- graph/iterator/all_iterator.go | 12 +++++----- graph/iterator/and_iterator.go | 34 +++++++++-------------------- graph/iterator/fixed_iterator.go | 27 +++++++++++++---------- graph/iterator/hasa_iterator.go | 17 +++++++-------- graph/iterator/iterator.go | 9 ++++---- graph/iterator/linksto_iterator.go | 16 +++++++------- graph/iterator/materialize_iterator.go | 21 ++++++++---------- graph/iterator/optional_iterator.go | 18 +++++++-------- graph/iterator/or_iterator.go | 28 +++++++----------------- graph/iterator/value_comparison_iterator.go | 14 ++++++------ graph/leveldb/all_iterator.go | 12 ++++++---- graph/leveldb/iterator.go | 21 ++++++++---------- graph/memstore/iterator.go | 12 ++++++---- graph/memstore/quadstore_test.go | 7 ++++-- graph/mongo/iterator.go | 10 ++++++--- query/gremlin/finals.go | 18 +++++++++++++-- query/mql/session.go | 7 +++++- query/sexp/parser_test.go | 4 ++-- query/sexp/session.go | 8 ++++++- 22 files changed, 186 insertions(+), 162 deletions(-) diff --git a/graph/bolt/all_iterator.go b/graph/bolt/all_iterator.go index 0047666..5d8bf89 100644 --- a/graph/bolt/all_iterator.go +++ b/graph/bolt/all_iterator.go @@ -17,7 +17,6 @@ package bolt import ( "bytes" "fmt" - "strings" "github.com/barakmich/glog" "github.com/boltdb/bolt" @@ -179,9 +178,15 @@ func (it *AllIterator) Size() (int64, bool) { return it.qs.size, true } -func (it *AllIterator) DebugString(indent int) string { +func (it *AllIterator) Describe() graph.Description { size, _ := it.Size() - return fmt.Sprintf("%s(%s tags: %v bolt size:%d %s %p)", strings.Repeat(" ", indent), it.Type(), it.tags.Tags(), size, it.dir, it) + return graph.Description{ + UID: it.UID(), + Type: it.Type().String(), + Tags: it.tags.Tags(), + Size: size, + Direction: it.dir, + } } func (it *AllIterator) Type() graph.Type { return graph.All } diff --git a/graph/bolt/iterator.go b/graph/bolt/iterator.go index ad56a47..8709ebe 100644 --- a/graph/bolt/iterator.go +++ b/graph/bolt/iterator.go @@ -19,7 +19,6 @@ import ( "encoding/json" "errors" "fmt" - "strings" "github.com/barakmich/glog" "github.com/boltdb/bolt" @@ -291,16 +290,15 @@ func (it *Iterator) Size() (int64, bool) { return it.size, true } -func (it *Iterator) DebugString(indent int) string { - return fmt.Sprintf("%s(%s %d tags: %v dir: %s size:%d %s)", - strings.Repeat(" ", indent), - it.Type(), - it.UID(), - it.tags.Tags(), - it.dir, - it.size, - it.qs.NameOf(&Token{it.bucket, it.checkID}), - ) +func (it *Iterator) Describe() graph.Description { + return graph.Description{ + UID: it.UID(), + Name: it.qs.NameOf(&Token{it.bucket, it.checkID}), + Type: it.Type().String(), + Tags: it.tags.Tags(), + Size: it.size, + Direction: it.dir, + } } func (it *Iterator) Type() graph.Type { return boltType } diff --git a/graph/iterator.go b/graph/iterator.go index 9fe2d12..3a74dc6 100644 --- a/graph/iterator.go +++ b/graph/iterator.go @@ -17,6 +17,7 @@ package graph // Define the general iterator interface. import ( + "github.com/google/cayley/quad" "strings" "sync" @@ -130,8 +131,8 @@ type Iterator interface { // Return a slice of the subiterators for this iterator. SubIterators() []Iterator - // Return a string representation of the iterator, indented by the given amount. - DebugString(int) string + // Return a string representation of the iterator. + Describe() Description // Close the iterator and do internal cleanup. Close() @@ -140,6 +141,17 @@ type Iterator interface { UID() uint64 } +type Description struct { + UID uint64 `json:",omitempty"` + Name string `json:",omitempty"` + Type string `json:",omitempty"` + Tags []string `json:",omitempty"` + Size int64 `json:",omitempty"` + Direction quad.Direction `json:",omitempty"` + Iterator *Description `json:",omitempty"` + Iterators []Description `json:",omitempty"` +} + type Nexter interface { // Next advances the iterator to the next value, which will then be available through // the Result method. It returns false if no further advancement is possible. @@ -256,16 +268,16 @@ func (t Type) String() string { } type StatsContainer struct { + UID uint64 + Type string IteratorStats - Kind string - UID uint64 SubIts []StatsContainer } func DumpStats(it Iterator) StatsContainer { var out StatsContainer out.IteratorStats = it.Stats() - out.Kind = it.Type().String() + out.Type = it.Type().String() out.UID = it.UID() for _, sub := range it.SubIterators() { out.SubIts = append(out.SubIts, DumpStats(sub)) diff --git a/graph/iterator/all_iterator.go b/graph/iterator/all_iterator.go index 0106143..c89c44c 100644 --- a/graph/iterator/all_iterator.go +++ b/graph/iterator/all_iterator.go @@ -23,9 +23,6 @@ package iterator // the base iterators, and it helps just to see it here. import ( - "fmt" - "strings" - "github.com/google/cayley/graph" ) @@ -81,9 +78,12 @@ func (it *Int64) TagResults(dst map[string]graph.Value) { } } -// Prints the All iterator as just an "all". -func (it *Int64) DebugString(indent int) string { - return fmt.Sprintf("%s(%s tags: %v)", strings.Repeat(" ", indent), it.Type(), it.tags.Tags()) +func (it *Int64) Describe() graph.Description { + return graph.Description{ + UID: it.UID(), + Type: it.Type().String(), + Tags: it.tags.Tags(), + } } // Next() on an Int64 all iterator is a simple incrementing counter. diff --git a/graph/iterator/and_iterator.go b/graph/iterator/and_iterator.go index bc1859a..64c68f0 100644 --- a/graph/iterator/and_iterator.go +++ b/graph/iterator/and_iterator.go @@ -16,9 +16,6 @@ package iterator import ( - "fmt" - "strings" - "github.com/google/cayley/graph" ) @@ -111,30 +108,19 @@ func (it *And) ResultTree() *graph.ResultTree { return tree } -// Prints information about this iterator. -func (it *And) DebugString(indent int) string { - var total string +func (it *And) Describe() graph.Description { + subIts := make([]graph.Description, len(it.internalIterators)) for i, sub := range it.internalIterators { - total += strings.Repeat(" ", indent+2) - total += fmt.Sprintf("%d:\n%s\n", i, sub.DebugString(indent+4)) + subIts[i] = sub.Describe() } - var tags string - for _, k := range it.tags.Tags() { - tags += fmt.Sprintf("%s;", k) + primary := it.primaryIt.Describe() + return graph.Description{ + UID: it.UID(), + Type: it.Type().String(), + Tags: it.tags.Tags(), + Iterator: &primary, + Iterators: subIts, } - spaces := strings.Repeat(" ", indent+2) - - return fmt.Sprintf("%s(%s %d\n%stags:%s\n%sprimary_it:\n%s\n%sother_its:\n%s)", - strings.Repeat(" ", indent), - it.Type(), - it.UID(), - spaces, - tags, - spaces, - it.primaryIt.DebugString(indent+4), - spaces, - total, - ) } // Add a subiterator to this And iterator. diff --git a/graph/iterator/fixed_iterator.go b/graph/iterator/fixed_iterator.go index a0298fc..93d31b4 100644 --- a/graph/iterator/fixed_iterator.go +++ b/graph/iterator/fixed_iterator.go @@ -22,7 +22,7 @@ package iterator import ( "fmt" - "strings" + "sort" "github.com/google/cayley/graph" ) @@ -94,20 +94,23 @@ func (it *Fixed) Add(v graph.Value) { it.values = append(it.values, v) } -// Print some information about the iterator. -func (it *Fixed) DebugString(indent int) string { - value := "" +func (it *Fixed) Describe() graph.Description { + var value string if len(it.values) > 0 { value = fmt.Sprint(it.values[0]) } - return fmt.Sprintf("%s(%s %d tags: %s Size: %d id0: %s)", - strings.Repeat(" ", indent), - it.Type(), - it.UID(), - it.tags.Fixed(), - len(it.values), - value, - ) + fixed := make([]string, 0, len(it.tags.Fixed())) + for k := range it.tags.Fixed() { + fixed = append(fixed, k) + } + sort.Strings(fixed) + return graph.Description{ + UID: it.UID(), + Name: value, + Type: it.Type().String(), + Tags: fixed, + Size: int64(len(it.values)), + } } // Register this iterator as a Fixed iterator. diff --git a/graph/iterator/hasa_iterator.go b/graph/iterator/hasa_iterator.go index a6440a4..fed6469 100644 --- a/graph/iterator/hasa_iterator.go +++ b/graph/iterator/hasa_iterator.go @@ -34,9 +34,6 @@ package iterator // Alternatively, can be seen as the dual of the LinksTo iterator. import ( - "fmt" - "strings" - "github.com/barakmich/glog" "github.com/google/cayley/graph" @@ -130,13 +127,15 @@ func (it *HasA) ResultTree() *graph.ResultTree { return tree } -// Print some information about this iterator. -func (it *HasA) DebugString(indent int) string { - var tags string - for _, k := range it.tags.Tags() { - tags += fmt.Sprintf("%s;", k) +func (it *HasA) Describe() graph.Description { + primary := it.primaryIt.Describe() + return graph.Description{ + UID: it.UID(), + Type: it.Type().String(), + Tags: it.tags.Tags(), + Direction: it.dir, + Iterator: &primary, } - return fmt.Sprintf("%s(%s %d tags:%s direction:%s\n%s)", strings.Repeat(" ", indent), it.Type(), it.UID(), tags, it.dir, it.primaryIt.DebugString(indent+4)) } // Check a value against our internal iterator. In order to do this, we must first open a new diff --git a/graph/iterator/iterator.go b/graph/iterator/iterator.go index ab36b7b..e1c98c9 100644 --- a/graph/iterator/iterator.go +++ b/graph/iterator/iterator.go @@ -17,7 +17,6 @@ package iterator // Define the general iterator interface. import ( - "strings" "sync/atomic" "github.com/google/cayley/graph" @@ -78,9 +77,11 @@ func (it *Null) Type() graph.Type { return graph.Null } // Null has nothing it needs to do. func (it *Null) Optimize() (graph.Iterator, bool) { return it, false } -// Print the null iterator. -func (it *Null) DebugString(indent int) string { - return strings.Repeat(" ", indent) + "(null)" +func (it *Null) Describe() graph.Description { + return graph.Description{ + UID: it.UID(), + Type: it.Type().String(), + } } func (it *Null) Next() bool { diff --git a/graph/iterator/linksto_iterator.go b/graph/iterator/linksto_iterator.go index f0f985c..5952cd1 100644 --- a/graph/iterator/linksto_iterator.go +++ b/graph/iterator/linksto_iterator.go @@ -30,9 +30,6 @@ package iterator // Can be seen as the dual of the HasA iterator. import ( - "fmt" - "strings" - "github.com/google/cayley/graph" "github.com/google/cayley/quad" ) @@ -108,11 +105,14 @@ func (it *LinksTo) ResultTree() *graph.ResultTree { return tree } -// Print the iterator. -func (it *LinksTo) DebugString(indent int) string { - return fmt.Sprintf("%s(%s %d direction:%s\n%s)", - strings.Repeat(" ", indent), - it.Type(), it.UID(), it.dir, it.primaryIt.DebugString(indent+4)) +func (it *LinksTo) Describe() graph.Description { + primary := it.primaryIt.Describe() + return graph.Description{ + UID: it.UID(), + Type: it.Type().String(), + Direction: it.dir, + Iterator: &primary, + } } // If it checks in the right direction for the subiterator, it is a valid link diff --git a/graph/iterator/materialize_iterator.go b/graph/iterator/materialize_iterator.go index 46f2d57..7adb515 100644 --- a/graph/iterator/materialize_iterator.go +++ b/graph/iterator/materialize_iterator.go @@ -17,9 +17,6 @@ package iterator // A simple iterator that, when first called Contains() or Next() upon, materializes the whole subiterator, stores it locally, and responds. Essentially a cache. import ( - "fmt" - "strings" - "github.com/barakmich/glog" "github.com/google/cayley/graph" @@ -118,15 +115,15 @@ func (it *Materialize) Clone() graph.Iterator { return out } -// Print some information about the iterator. -func (it *Materialize) DebugString(indent int) string { - return fmt.Sprintf("%s(%s tags: %s Size: %d\n%s)", - strings.Repeat(" ", indent), - it.Type(), - it.tags.Tags(), - len(it.values), - it.subIt.DebugString(indent+4), - ) +func (it *Materialize) Describe() graph.Description { + primary := it.subIt.Describe() + return graph.Description{ + UID: it.UID(), + Type: it.Type().String(), + Tags: it.tags.Tags(), + Size: int64(len(it.values)), + Iterator: &primary, + } } // Register this iterator as a Materialize iterator. diff --git a/graph/iterator/optional_iterator.go b/graph/iterator/optional_iterator.go index 2432390..5622f59 100644 --- a/graph/iterator/optional_iterator.go +++ b/graph/iterator/optional_iterator.go @@ -27,9 +27,6 @@ package iterator // -- all things in the graph. It matches everything (as does the regex "(a)?") import ( - "fmt" - "strings" - "github.com/google/cayley/graph" ) @@ -120,13 +117,14 @@ func (it *Optional) TagResults(dst map[string]graph.Value) { // Registers the optional iterator. func (it *Optional) Type() graph.Type { return graph.Optional } -// Prints the optional and it's subiterator. -func (it *Optional) DebugString(indent int) string { - return fmt.Sprintf("%s(%s tags:%s\n%s)", - strings.Repeat(" ", indent), - it.Type(), - it.tags.Tags(), - it.subIt.DebugString(indent+4)) +func (it *Optional) Describe() graph.Description { + primary := it.subIt.Describe() + return graph.Description{ + UID: it.UID(), + Type: it.Type().String(), + Tags: it.tags.Tags(), + Iterator: &primary, + } } // There's nothing to optimize for an optional. Optimize the subiterator and diff --git a/graph/iterator/or_iterator.go b/graph/iterator/or_iterator.go index ddff768..805ede0 100644 --- a/graph/iterator/or_iterator.go +++ b/graph/iterator/or_iterator.go @@ -22,9 +22,6 @@ package iterator // May return the same value twice -- once for each branch. import ( - "fmt" - "strings" - "github.com/google/cayley/graph" ) @@ -113,26 +110,17 @@ func (it *Or) ResultTree() *graph.ResultTree { return tree } -// Prints information about this graph.iterator. -func (it *Or) DebugString(indent int) string { - var total string +func (it *Or) Describe() graph.Description { + var subIts []graph.Description for i, sub := range it.internalIterators { - total += strings.Repeat(" ", indent+2) - total += fmt.Sprintf("%d:\n%s\n", i, sub.DebugString(indent+4)) + subIts[i] = sub.Describe() } - var tags string - for _, k := range it.tags.Tags() { - tags += fmt.Sprintf("%s;", k) + return graph.Description{ + UID: it.UID(), + Type: it.Type().String(), + Tags: it.tags.Tags(), + Iterators: subIts, } - spaces := strings.Repeat(" ", indent+2) - - return fmt.Sprintf("%s(%s\n%stags:%s\n%sits:\n%s)", - strings.Repeat(" ", indent), - it.Type(), - spaces, - tags, - spaces, - total) } // Add a subiterator to this Or graph.iterator. Order matters. diff --git a/graph/iterator/value_comparison_iterator.go b/graph/iterator/value_comparison_iterator.go index a0beeb3..dae9956 100644 --- a/graph/iterator/value_comparison_iterator.go +++ b/graph/iterator/value_comparison_iterator.go @@ -27,10 +27,8 @@ package iterator // In MQL terms, this is the [{"age>=": 21}] concept. import ( - "fmt" "log" "strconv" - "strings" "github.com/google/cayley/graph" ) @@ -190,11 +188,13 @@ func (it *Comparison) TagResults(dst map[string]graph.Value) { // Registers the value-comparison iterator. func (it *Comparison) Type() graph.Type { return graph.Comparison } -// Prints the value-comparison and its subiterator. -func (it *Comparison) DebugString(indent int) string { - return fmt.Sprintf("%s(%s\n%s)", - strings.Repeat(" ", indent), - it.Type(), it.subIt.DebugString(indent+4)) +func (it *Comparison) Describe() graph.Description { + primary := it.subIt.Describe() + return graph.Description{ + UID: it.UID(), + Type: it.Type().String(), + Iterator: &primary, + } } // There's nothing to optimize, locally, for a value-comparison iterator. diff --git a/graph/leveldb/all_iterator.go b/graph/leveldb/all_iterator.go index b5b879c..346a973 100644 --- a/graph/leveldb/all_iterator.go +++ b/graph/leveldb/all_iterator.go @@ -16,8 +16,6 @@ package leveldb import ( "bytes" - "fmt" - "strings" ldbit "github.com/syndtr/goleveldb/leveldb/iterator" "github.com/syndtr/goleveldb/leveldb/opt" @@ -159,9 +157,15 @@ func (it *AllIterator) Size() (int64, bool) { return int64(^uint64(0) >> 1), false } -func (it *AllIterator) DebugString(indent int) string { +func (it *AllIterator) Describe() graph.Description { size, _ := it.Size() - return fmt.Sprintf("%s(%s tags: %v leveldb size:%d %s %p)", strings.Repeat(" ", indent), it.Type(), it.tags.Tags(), size, it.dir, it) + return graph.Description{ + UID: it.UID(), + Type: it.Type().String(), + Tags: it.tags.Tags(), + Size: size, + Direction: it.dir, + } } func (it *AllIterator) Type() graph.Type { return graph.All } diff --git a/graph/leveldb/iterator.go b/graph/leveldb/iterator.go index b35b904..8bdd86d 100644 --- a/graph/leveldb/iterator.go +++ b/graph/leveldb/iterator.go @@ -17,8 +17,6 @@ package leveldb import ( "bytes" "encoding/json" - "fmt" - "strings" "github.com/barakmich/glog" ldbit "github.com/syndtr/goleveldb/leveldb/iterator" @@ -250,17 +248,16 @@ func (it *Iterator) Size() (int64, bool) { return it.qs.SizeOf(Token(it.checkID)), true } -func (it *Iterator) DebugString(indent int) string { +func (it *Iterator) Describe() graph.Description { size, _ := it.Size() - return fmt.Sprintf("%s(%s %d tags: %v dir: %s size:%d %s)", - strings.Repeat(" ", indent), - it.Type(), - it.UID(), - it.tags.Tags(), - it.dir, - size, - it.qs.NameOf(Token(it.checkID)), - ) + return graph.Description{ + UID: it.UID(), + Name: it.qs.NameOf(Token(it.checkID)), + Type: it.Type().String(), + Tags: it.tags.Tags(), + Size: size, + Direction: it.dir, + } } var levelDBType graph.Type diff --git a/graph/memstore/iterator.go b/graph/memstore/iterator.go index ac69a37..fb1047e 100644 --- a/graph/memstore/iterator.go +++ b/graph/memstore/iterator.go @@ -15,9 +15,7 @@ package memstore import ( - "fmt" "math" - "strings" "github.com/google/cayley/graph" "github.com/google/cayley/graph/iterator" @@ -159,9 +157,15 @@ func (it *Iterator) Contains(v graph.Value) bool { return graph.ContainsLogOut(it, v, false) } -func (it *Iterator) DebugString(indent int) string { +func (it *Iterator) Describe() graph.Description { size, _ := it.Size() - return fmt.Sprintf("%s(%s tags:%s size:%d %s)", strings.Repeat(" ", indent), it.Type(), it.tags.Tags(), size, it.data) + return graph.Description{ + UID: it.UID(), + Name: it.data, + Type: it.Type().String(), + Tags: it.tags.Tags(), + Size: size, + } } var memType graph.Type diff --git a/graph/memstore/quadstore_test.go b/graph/memstore/quadstore_test.go index 57e54f4..020ca5d 100644 --- a/graph/memstore/quadstore_test.go +++ b/graph/memstore/quadstore_test.go @@ -165,8 +165,11 @@ func TestLinksToOptimization(t *testing.T) { v := newIt.(*Iterator) vClone := v.Clone() - if vClone.DebugString(0) != v.DebugString(0) { - t.Fatal("Wrong iterator. Got ", vClone.DebugString(0)) + origDesc := v.Describe() + cloneDesc := vClone.Describe() + origDesc.UID, cloneDesc.UID = 0, 0 // We are more strict now, so fake UID equality. + if !reflect.DeepEqual(cloneDesc, origDesc) { + t.Fatalf("Unexpected iterator description.\ngot: %#v\nexpect: %#v", cloneDesc, origDesc) } vt := vClone.Tagger() if len(vt.Tags()) < 1 || vt.Tags()[0] != "foo" { diff --git a/graph/mongo/iterator.go b/graph/mongo/iterator.go index e9b6f51..40694fb 100644 --- a/graph/mongo/iterator.go +++ b/graph/mongo/iterator.go @@ -16,7 +16,6 @@ package mongo import ( "fmt" - "strings" "github.com/barakmich/glog" "gopkg.in/mgo.v2" @@ -213,9 +212,14 @@ func (it *Iterator) Type() graph.Type { func (it *Iterator) Sorted() bool { return true } func (it *Iterator) Optimize() (graph.Iterator, bool) { return it, false } -func (it *Iterator) DebugString(indent int) string { +func (it *Iterator) Describe() graph.Description { size, _ := it.Size() - return fmt.Sprintf("%s(%s size:%d %s %s)", strings.Repeat(" ", indent), it.Type(), size, it.hash, it.name) + return graph.Description{ + UID: it.UID(), + Name: fmt.Sprintf("%s/%s", it.name, it.hash), + Type: it.Type().String(), + Size: size, + } } func (it *Iterator) Stats() graph.IteratorStats { diff --git a/query/gremlin/finals.go b/query/gremlin/finals.go index 37caad7..583b8d0 100644 --- a/query/gremlin/finals.go +++ b/query/gremlin/finals.go @@ -208,7 +208,14 @@ func (wk *worker) runIteratorToArrayNoTags(it graph.Iterator, limit int) []strin func (wk *worker) runIteratorWithCallback(it graph.Iterator, callback otto.Value, this otto.FunctionCall, limit int) { n := 0 it, _ = it.Optimize() - glog.V(2).Infoln(it.DebugString(0)) + if glog.V(2) { + b, err := json.MarshalIndent(it.Describe(), "", " ") + if err != nil { + glog.V(2).Infof("failed to format description: %v", err) + } else { + glog.V(2).Infof("%s", b) + } + } for { select { case <-wk.kill: @@ -271,7 +278,14 @@ func (wk *worker) runIterator(it graph.Iterator) { return } it, _ = it.Optimize() - glog.V(2).Infoln(it.DebugString(0)) + if glog.V(2) { + b, err := json.MarshalIndent(it.Describe(), "", " ") + if err != nil { + glog.Infof("failed to format description: %v", err) + } else { + glog.Infof("%s", b) + } + } for { select { case <-wk.kill: diff --git a/query/mql/session.go b/query/mql/session.go index c48686e..5e54ca9 100644 --- a/query/mql/session.go +++ b/query/mql/session.go @@ -85,7 +85,12 @@ func (s *Session) ExecInput(input string, c chan interface{}, _ int) { } it, _ := s.currentQuery.it.Optimize() if glog.V(2) { - glog.V(2).Infoln(it.DebugString(0)) + b, err := json.MarshalIndent(it.Describe(), "", " ") + if err != nil { + glog.Infof("failed to format description: %v", err) + } else { + glog.Infof("%s", b) + } } for graph.Next(it) { tags := make(map[string]graph.Value) diff --git a/query/sexp/parser_test.go b/query/sexp/parser_test.go index d00823d..77f16cb 100644 --- a/query/sexp/parser_test.go +++ b/query/sexp/parser_test.go @@ -89,7 +89,7 @@ func TestTreeConstraintParse(t *testing.T) { "($a (:is :good))))" it := BuildIteratorTreeForQuery(qs, query) if it.Type() != graph.And { - t.Errorf("Odd iterator tree. Got: %s", it.DebugString(0)) + t.Errorf("Odd iterator tree. Got: %#v", it.Describe()) } if !graph.Next(it) { t.Error("Got no results") @@ -137,7 +137,7 @@ func TestMultipleConstraintParse(t *testing.T) { )` it := BuildIteratorTreeForQuery(qs, query) if it.Type() != graph.And { - t.Errorf("Odd iterator tree. Got: %s", it.DebugString(0)) + t.Errorf("Odd iterator tree. Got: %#v", it.Describe()) } if !graph.Next(it) { t.Error("Got no results") diff --git a/query/sexp/session.go b/query/sexp/session.go index 6684303..172db8d 100644 --- a/query/sexp/session.go +++ b/query/sexp/session.go @@ -17,6 +17,7 @@ package sexp // Defines a running session of the sexp query language. import ( + "encoding/json" "errors" "fmt" "sort" @@ -74,7 +75,12 @@ func (s *Session) ExecInput(input string, out chan interface{}, limit int) { } if s.debug { - fmt.Println(it.DebugString(0)) + b, err := json.MarshalIndent(it.Describe(), "", " ") + if err != nil { + fmt.Printf("failed to format description: %v", err) + } else { + fmt.Printf("%s", b) + } } nResults := 0 for graph.Next(it) { From e2eea6c283d1b8b49eb1c7ae5678ddccf9f5d039 Mon Sep 17 00:00:00 2001 From: kortschak Date: Fri, 5 Sep 2014 09:49:15 +0930 Subject: [PATCH 2/4] Convert Type fields to use graph.Type Add text encoding methods to replace string storage. --- graph/bolt/all_iterator.go | 2 +- graph/bolt/iterator.go | 2 +- graph/iterator.go | 27 ++++++++++++++--- graph/iterator/all_iterator.go | 2 +- graph/iterator/and_iterator.go | 2 +- graph/iterator/fixed_iterator.go | 2 +- graph/iterator/hasa_iterator.go | 2 +- graph/iterator/iterator.go | 2 +- graph/iterator/linksto_iterator.go | 2 +- graph/iterator/materialize_iterator.go | 2 +- graph/iterator/optional_iterator.go | 2 +- graph/iterator/or_iterator.go | 2 +- graph/iterator/value_comparison_iterator.go | 2 +- graph/iterator_test.go | 45 +++++++++++++++++++++++++++++ graph/leveldb/all_iterator.go | 2 +- graph/leveldb/iterator.go | 2 +- graph/memstore/iterator.go | 2 +- graph/mongo/iterator.go | 2 +- 18 files changed, 84 insertions(+), 20 deletions(-) create mode 100644 graph/iterator_test.go diff --git a/graph/bolt/all_iterator.go b/graph/bolt/all_iterator.go index 5d8bf89..4cd017e 100644 --- a/graph/bolt/all_iterator.go +++ b/graph/bolt/all_iterator.go @@ -182,7 +182,7 @@ func (it *AllIterator) Describe() graph.Description { size, _ := it.Size() return graph.Description{ UID: it.UID(), - Type: it.Type().String(), + Type: it.Type(), Tags: it.tags.Tags(), Size: size, Direction: it.dir, diff --git a/graph/bolt/iterator.go b/graph/bolt/iterator.go index 8709ebe..ec298de 100644 --- a/graph/bolt/iterator.go +++ b/graph/bolt/iterator.go @@ -294,7 +294,7 @@ func (it *Iterator) Describe() graph.Description { return graph.Description{ UID: it.UID(), Name: it.qs.NameOf(&Token{it.bucket, it.checkID}), - Type: it.Type().String(), + Type: it.Type(), Tags: it.tags.Tags(), Size: it.size, Direction: it.dir, diff --git a/graph/iterator.go b/graph/iterator.go index 3a74dc6..4442ea5 100644 --- a/graph/iterator.go +++ b/graph/iterator.go @@ -17,11 +17,12 @@ package graph // Define the general iterator interface. import ( - "github.com/google/cayley/quad" + "fmt" "strings" "sync" "github.com/barakmich/glog" + "github.com/google/cayley/quad" ) type Tagger struct { @@ -144,7 +145,7 @@ type Iterator interface { type Description struct { UID uint64 `json:",omitempty"` Name string `json:",omitempty"` - Type string `json:",omitempty"` + Type Type `json:",omitempty"` Tags []string `json:",omitempty"` Size int64 `json:",omitempty"` Direction quad.Direction `json:",omitempty"` @@ -267,9 +268,27 @@ func (t Type) String() string { return types[t] } +func (t *Type) MarshalText() (text []byte, err error) { + if *t < 0 || int(*t) >= len(types) { + return nil, fmt.Errorf("graph: illegal iterator type: %d", *t) + } + return []byte(types[*t]), nil +} + +func (t *Type) UnmarshalText(text []byte) error { + s := string(text) + for i, c := range types[1:] { + if c == s { + *t = Type(i + 1) + return nil + } + } + return fmt.Errorf("graph: unknown iterator label: %q", text) +} + type StatsContainer struct { UID uint64 - Type string + Type Type IteratorStats SubIts []StatsContainer } @@ -277,7 +296,7 @@ type StatsContainer struct { func DumpStats(it Iterator) StatsContainer { var out StatsContainer out.IteratorStats = it.Stats() - out.Type = it.Type().String() + out.Type = it.Type() out.UID = it.UID() for _, sub := range it.SubIterators() { out.SubIts = append(out.SubIts, DumpStats(sub)) diff --git a/graph/iterator/all_iterator.go b/graph/iterator/all_iterator.go index c89c44c..d937ce4 100644 --- a/graph/iterator/all_iterator.go +++ b/graph/iterator/all_iterator.go @@ -81,7 +81,7 @@ func (it *Int64) TagResults(dst map[string]graph.Value) { func (it *Int64) Describe() graph.Description { return graph.Description{ UID: it.UID(), - Type: it.Type().String(), + Type: it.Type(), Tags: it.tags.Tags(), } } diff --git a/graph/iterator/and_iterator.go b/graph/iterator/and_iterator.go index 64c68f0..3764600 100644 --- a/graph/iterator/and_iterator.go +++ b/graph/iterator/and_iterator.go @@ -116,7 +116,7 @@ func (it *And) Describe() graph.Description { primary := it.primaryIt.Describe() return graph.Description{ UID: it.UID(), - Type: it.Type().String(), + Type: it.Type(), Tags: it.tags.Tags(), Iterator: &primary, Iterators: subIts, diff --git a/graph/iterator/fixed_iterator.go b/graph/iterator/fixed_iterator.go index 93d31b4..4a8afb9 100644 --- a/graph/iterator/fixed_iterator.go +++ b/graph/iterator/fixed_iterator.go @@ -107,7 +107,7 @@ func (it *Fixed) Describe() graph.Description { return graph.Description{ UID: it.UID(), Name: value, - Type: it.Type().String(), + Type: it.Type(), Tags: fixed, Size: int64(len(it.values)), } diff --git a/graph/iterator/hasa_iterator.go b/graph/iterator/hasa_iterator.go index fed6469..3788b40 100644 --- a/graph/iterator/hasa_iterator.go +++ b/graph/iterator/hasa_iterator.go @@ -131,7 +131,7 @@ func (it *HasA) Describe() graph.Description { primary := it.primaryIt.Describe() return graph.Description{ UID: it.UID(), - Type: it.Type().String(), + Type: it.Type(), Tags: it.tags.Tags(), Direction: it.dir, Iterator: &primary, diff --git a/graph/iterator/iterator.go b/graph/iterator/iterator.go index e1c98c9..954ddba 100644 --- a/graph/iterator/iterator.go +++ b/graph/iterator/iterator.go @@ -80,7 +80,7 @@ func (it *Null) Optimize() (graph.Iterator, bool) { return it, false } func (it *Null) Describe() graph.Description { return graph.Description{ UID: it.UID(), - Type: it.Type().String(), + Type: it.Type(), } } diff --git a/graph/iterator/linksto_iterator.go b/graph/iterator/linksto_iterator.go index 5952cd1..99a8df5 100644 --- a/graph/iterator/linksto_iterator.go +++ b/graph/iterator/linksto_iterator.go @@ -109,7 +109,7 @@ func (it *LinksTo) Describe() graph.Description { primary := it.primaryIt.Describe() return graph.Description{ UID: it.UID(), - Type: it.Type().String(), + Type: it.Type(), Direction: it.dir, Iterator: &primary, } diff --git a/graph/iterator/materialize_iterator.go b/graph/iterator/materialize_iterator.go index 7adb515..00b2102 100644 --- a/graph/iterator/materialize_iterator.go +++ b/graph/iterator/materialize_iterator.go @@ -119,7 +119,7 @@ func (it *Materialize) Describe() graph.Description { primary := it.subIt.Describe() return graph.Description{ UID: it.UID(), - Type: it.Type().String(), + Type: it.Type(), Tags: it.tags.Tags(), Size: int64(len(it.values)), Iterator: &primary, diff --git a/graph/iterator/optional_iterator.go b/graph/iterator/optional_iterator.go index 5622f59..bc65acc 100644 --- a/graph/iterator/optional_iterator.go +++ b/graph/iterator/optional_iterator.go @@ -121,7 +121,7 @@ func (it *Optional) Describe() graph.Description { primary := it.subIt.Describe() return graph.Description{ UID: it.UID(), - Type: it.Type().String(), + Type: it.Type(), Tags: it.tags.Tags(), Iterator: &primary, } diff --git a/graph/iterator/or_iterator.go b/graph/iterator/or_iterator.go index 805ede0..6c58238 100644 --- a/graph/iterator/or_iterator.go +++ b/graph/iterator/or_iterator.go @@ -117,7 +117,7 @@ func (it *Or) Describe() graph.Description { } return graph.Description{ UID: it.UID(), - Type: it.Type().String(), + Type: it.Type(), Tags: it.tags.Tags(), Iterators: subIts, } diff --git a/graph/iterator/value_comparison_iterator.go b/graph/iterator/value_comparison_iterator.go index dae9956..627b853 100644 --- a/graph/iterator/value_comparison_iterator.go +++ b/graph/iterator/value_comparison_iterator.go @@ -192,7 +192,7 @@ func (it *Comparison) Describe() graph.Description { primary := it.subIt.Describe() return graph.Description{ UID: it.UID(), - Type: it.Type().String(), + Type: it.Type(), Iterator: &primary, } } diff --git a/graph/iterator_test.go b/graph/iterator_test.go new file mode 100644 index 0000000..3d6f14e --- /dev/null +++ b/graph/iterator_test.go @@ -0,0 +1,45 @@ +// Copyright 2014 The Cayley Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package graph + +import ( + "testing" +) + +func TestTypeEncodingRoundtrip(t *testing.T) { + for i := Invalid; i <= Materialize; i++ { + text, err := i.MarshalText() + if err != nil { + t.Errorf("Unexpected error when marshaling %s: %v", i, err) + } + if string(text) != i.String() { + t.Errorf("Unexpected MarshalText result, got:%q expect:%q", i, text) + } + var m Type + err = m.UnmarshalText(text) + if i == Invalid { + if err == nil || err.Error() != `graph: unknown iterator label: "invalid"` { + t.Errorf("Unexpected error when unmarshaling %q: %v", text, err) + } + } else { + if err != nil { + t.Errorf("Unexpected error when unmarshaling %q: %v", text, err) + } + } + if m != i { + t.Errorf("Unexpected UnmarshalText result, got:Type(%d) expect:Type(%d)", m, i) + } + } +} diff --git a/graph/leveldb/all_iterator.go b/graph/leveldb/all_iterator.go index 346a973..515133f 100644 --- a/graph/leveldb/all_iterator.go +++ b/graph/leveldb/all_iterator.go @@ -161,7 +161,7 @@ func (it *AllIterator) Describe() graph.Description { size, _ := it.Size() return graph.Description{ UID: it.UID(), - Type: it.Type().String(), + Type: it.Type(), Tags: it.tags.Tags(), Size: size, Direction: it.dir, diff --git a/graph/leveldb/iterator.go b/graph/leveldb/iterator.go index 8bdd86d..6ccd3fe 100644 --- a/graph/leveldb/iterator.go +++ b/graph/leveldb/iterator.go @@ -253,7 +253,7 @@ func (it *Iterator) Describe() graph.Description { return graph.Description{ UID: it.UID(), Name: it.qs.NameOf(Token(it.checkID)), - Type: it.Type().String(), + Type: it.Type(), Tags: it.tags.Tags(), Size: size, Direction: it.dir, diff --git a/graph/memstore/iterator.go b/graph/memstore/iterator.go index fb1047e..d79e6c8 100644 --- a/graph/memstore/iterator.go +++ b/graph/memstore/iterator.go @@ -162,7 +162,7 @@ func (it *Iterator) Describe() graph.Description { return graph.Description{ UID: it.UID(), Name: it.data, - Type: it.Type().String(), + Type: it.Type(), Tags: it.tags.Tags(), Size: size, } diff --git a/graph/mongo/iterator.go b/graph/mongo/iterator.go index 40694fb..32bc472 100644 --- a/graph/mongo/iterator.go +++ b/graph/mongo/iterator.go @@ -217,7 +217,7 @@ func (it *Iterator) Describe() graph.Description { return graph.Description{ UID: it.UID(), Name: fmt.Sprintf("%s/%s", it.name, it.hash), - Type: it.Type().String(), + Type: it.Type(), Size: size, } } From 887c23e640784cc153c61ce21236577e5108637a Mon Sep 17 00:00:00 2001 From: kortschak Date: Wed, 24 Sep 2014 08:52:00 +0930 Subject: [PATCH 3/4] Ensure we don't examine empty token Fixes issue #163. --- graph/bolt/iterator.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/graph/bolt/iterator.go b/graph/bolt/iterator.go index ec298de..efb11d6 100644 --- a/graph/bolt/iterator.go +++ b/graph/bolt/iterator.go @@ -271,7 +271,7 @@ func (it *Iterator) Contains(v graph.Value) bool { return false } offset := PositionOf(val, it.dir, it.qs) - if bytes.HasPrefix(val.key[offset:], it.checkID) { + if len(val.key) != 0 && bytes.HasPrefix(val.key[offset:], it.checkID) { // You may ask, why don't we check to see if it's a valid (not deleted) quad // again? // From e71d19c851b9ffa61cf45547c7d75b32b2de33f4 Mon Sep 17 00:00:00 2001 From: kortschak Date: Wed, 24 Sep 2014 09:14:59 +0930 Subject: [PATCH 4/4] Don't retain results where the value is empty Empty quad terms are not valid, so we should be able to safely drop any results where the value is "". --- query/gremlin/session.go | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/query/gremlin/session.go b/query/gremlin/session.go index 887027e..bf9f7c3 100644 --- a/query/gremlin/session.go +++ b/query/gremlin/session.go @@ -210,17 +210,22 @@ func (s *Session) BuildJSON(result interface{}) { if data.val == nil { obj := make(map[string]string) tags := data.actualResults - tagKeys := make([]string, len(tags)) - i := 0 + var tagKeys []string for k := range tags { - tagKeys[i] = k - i++ + tagKeys = append(tagKeys, k) } sort.Strings(tagKeys) for _, k := range tagKeys { - obj[k] = s.qs.NameOf(tags[k]) + name := s.qs.NameOf(tags[k]) + if name != "" { + obj[k] = name + } else { + delete(obj, k) + } + } + if len(obj) != 0 { + s.dataOutput = append(s.dataOutput, obj) } - s.dataOutput = append(s.dataOutput, obj) } else { if data.val.IsObject() { export, _ := data.val.Export()