From d6f94be5141e13c147794609e2fc286b9e4c3c12 Mon Sep 17 00:00:00 2001 From: kortschak Date: Wed, 30 Jul 2014 16:06:46 +0930 Subject: [PATCH] Base nexting on interface satisfaction This is done unsubtlely at the moment and there is plenty of room for optimisation of assertion location to prevent repeated reasserting as is done now. --- graph/iterator.go | 31 ++++++++++++++++--------- graph/iterator/all_iterator.go | 1 - graph/iterator/and_iterator.go | 7 +++--- graph/iterator/and_iterator_optimize.go | 6 ++--- graph/iterator/fixed_iterator.go | 1 - graph/iterator/hasa_iterator.go | 5 ++-- graph/iterator/iterator.go | 13 +---------- graph/iterator/linksto_iterator.go | 5 ++-- graph/iterator/optional_iterator.go | 12 ---------- graph/iterator/or_iterator.go | 3 +-- graph/iterator/or_iterator_test.go | 2 +- graph/iterator/query_shape.go | 2 +- graph/iterator/value_comparison_iterator.go | 3 +-- graph/leveldb/all_iterator.go | 1 - graph/leveldb/iterator.go | 1 - graph/leveldb/leveldb_test.go | 10 ++++---- graph/leveldb/triplestore_iterator_optimize.go | 2 +- graph/memstore/iterator.go | 1 - graph/memstore/triplestore_iterator_optimize.go | 2 +- graph/memstore/triplestore_test.go | 2 +- graph/mongo/iterator.go | 1 - graph/mongo/triplestore_iterator_optimize.go | 2 +- graph/result_tree_evaluator.go | 4 ++-- graph/sexp/parser_test.go | 10 ++++---- graph/sexp/session.go | 2 +- query/gremlin/finals.go | 8 +++---- query/mql/session.go | 2 +- 27 files changed, 57 insertions(+), 82 deletions(-) diff --git a/graph/iterator.go b/graph/iterator.go index bb1faba..d28f43e 100644 --- a/graph/iterator.go +++ b/graph/iterator.go @@ -14,8 +14,7 @@ package graph -// Define the general iterator interface, as well as the Base iterator which all -// iterators can "inherit" from to get default iterator functionality. +// Define the general iterator interface. import ( "strings" @@ -86,19 +85,10 @@ type Iterator interface { // All of them should set iterator.Last to be the last returned value, to // make results work. // - // Next() advances the iterator and returns the next valid result. Returns - // (, true) or (nil, false) - Next() (Value, bool) - // NextResult() advances iterators that may have more than one valid result, // from the bottom up. NextResult() bool - // Return whether this iterator is reliably nextable. Most iterators are. - // However, some iterators, like "not" are, by definition, the whole database - // except themselves. Next() on these is unproductive, if impossible. - CanNext() bool - // Check(), given a value, returns whether or not that value is within the set // held by this iterator. Check(Value) bool @@ -145,6 +135,25 @@ type Iterator interface { UID() uint64 } +type Nexter interface { + // Next() advances the iterator and returns the next valid result. Returns + // (, true) or (nil, false) + Next() (Value, bool) + + Iterator +} + +// Next is a convenience function that conditionally calls the Next method +// of an Iterator if it is a Nexter. If the Iterator is not a Nexter, Next +// return a nil Value and false. +func Next(it Iterator) (Value, bool) { + if n, ok := it.(Nexter); ok { + return n.Next() + } + glog.Errorln("Nexting an un-nextable iterator") + return nil, false +} + // FixedIterator wraps iterators that are modifiable by addition of fixed value sets. type FixedIterator interface { Iterator diff --git a/graph/iterator/all_iterator.go b/graph/iterator/all_iterator.go index 8394a0e..ff1a869 100644 --- a/graph/iterator/all_iterator.go +++ b/graph/iterator/all_iterator.go @@ -31,7 +31,6 @@ import ( // An All iterator across a range of int64 values, from `max` to `min`. type Int64 struct { - Base uid uint64 tags graph.Tagger max, min int64 diff --git a/graph/iterator/and_iterator.go b/graph/iterator/and_iterator.go index ba7b70b..902b9a1 100644 --- a/graph/iterator/and_iterator.go +++ b/graph/iterator/and_iterator.go @@ -22,10 +22,9 @@ import ( "github.com/google/cayley/graph" ) -// The And iterator. Consists of a Base and a number of subiterators, the primary of which will +// The And iterator. Consists of a number of subiterators, the primary of which will // be Next()ed if next is called. type And struct { - Base uid uint64 tags graph.Tagger internalIterators []graph.Iterator @@ -60,7 +59,7 @@ func (it *And) Tagger() *graph.Tagger { return &it.tags } -// Overrides Base TagResults, as it needs to add it's own results and +// An extended TagResults, as it needs to add it's own results and // recurse down it's subiterators. func (it *And) TagResults(dst map[string]graph.Value) { for _, tag := range it.tags.Tags() { @@ -161,7 +160,7 @@ func (it *And) Next() (graph.Value, bool) { var curr graph.Value var exists bool for { - curr, exists = it.primaryIt.Next() + curr, exists = graph.Next(it.primaryIt) if !exists { return graph.NextLogOut(it, nil, false) } diff --git a/graph/iterator/and_iterator_optimize.go b/graph/iterator/and_iterator_optimize.go index 06def9f..20a4771 100644 --- a/graph/iterator/and_iterator_optimize.go +++ b/graph/iterator/and_iterator_optimize.go @@ -145,14 +145,14 @@ func optimizeOrder(its []graph.Iterator) []graph.Iterator { // all of it's contents, and to Check() each of those against everyone // else. for _, it := range its { - if !it.CanNext() { + if _, canNext := it.(graph.Nexter); !canNext { bad = append(bad, it) continue } rootStats := it.Stats() cost := rootStats.NextCost for _, f := range its { - if !f.CanNext() { + if _, canNext := it.(graph.Nexter); !canNext { continue } if f == it { @@ -177,7 +177,7 @@ func optimizeOrder(its []graph.Iterator) []graph.Iterator { // ... push everyone else after... for _, it := range its { - if !it.CanNext() { + if _, canNext := it.(graph.Nexter); !canNext { continue } if it != best { diff --git a/graph/iterator/fixed_iterator.go b/graph/iterator/fixed_iterator.go index 2caa8e9..6bfebb5 100644 --- a/graph/iterator/fixed_iterator.go +++ b/graph/iterator/fixed_iterator.go @@ -30,7 +30,6 @@ import ( // A Fixed iterator consists of it's values, an index (where it is in the process of Next()ing) and // an equality function. type Fixed struct { - Base uid uint64 tags graph.Tagger values []graph.Value diff --git a/graph/iterator/hasa_iterator.go b/graph/iterator/hasa_iterator.go index 8d9ab46..e6060b7 100644 --- a/graph/iterator/hasa_iterator.go +++ b/graph/iterator/hasa_iterator.go @@ -46,7 +46,6 @@ import ( // a primary subiterator, a direction in which the triples for that subiterator point, // and a temporary holder for the iterator generated on Check(). type HasA struct { - Base uid uint64 tags graph.Tagger ts graph.TripleStore @@ -159,7 +158,7 @@ func (it *HasA) Check(val graph.Value) bool { // another match is made. func (it *HasA) GetCheckResult() bool { for { - linkVal, ok := it.resultIt.Next() + linkVal, ok := graph.Next(it.resultIt) if !ok { break } @@ -198,7 +197,7 @@ func (it *HasA) Next() (graph.Value, bool) { } it.resultIt = &Null{} - tID, ok := it.primaryIt.Next() + tID, ok := graph.Next(it.primaryIt) if !ok { return graph.NextLogOut(it, 0, false) } diff --git a/graph/iterator/iterator.go b/graph/iterator/iterator.go index e290dc3..e80b56c 100644 --- a/graph/iterator/iterator.go +++ b/graph/iterator/iterator.go @@ -14,8 +14,7 @@ package iterator -// Define the general iterator interface, as well as the Base which all -// iterators can "inherit" from to get default iterator functionality. +// Define the general iterator interface. import ( "strings" @@ -30,14 +29,6 @@ func NextUID() uint64 { return atomic.AddUint64(&nextIteratorID, 1) - 1 } -// The Base iterator is the iterator other iterators inherit from to get some -// default functionality. -type Base struct { -} - -// Accessor -func (it *Base) CanNext() bool { return true } - // Here we define the simplest iterator -- the Null iterator. It contains nothing. // It is the empty set. Often times, queries that contain one of these match nothing, // so it's important to give it a special iterator. @@ -88,8 +79,6 @@ func (it *Null) DebugString(indent int) string { return strings.Repeat(" ", indent) + "(null)" } -func (it *Null) CanNext() bool { return true } - func (it *Null) Next() (graph.Value, bool) { return nil, false } diff --git a/graph/iterator/linksto_iterator.go b/graph/iterator/linksto_iterator.go index fa7cf29..e5f86cc 100644 --- a/graph/iterator/linksto_iterator.go +++ b/graph/iterator/linksto_iterator.go @@ -40,7 +40,6 @@ import ( // for each node) the subiterator, and the direction the iterator comes from. // `next_it` is the tempoarary iterator held per result in `primary_it`. type LinksTo struct { - Base uid uint64 tags graph.Tagger ts graph.TripleStore @@ -155,10 +154,10 @@ func (it *LinksTo) Optimize() (graph.Iterator, bool) { // Next()ing a LinksTo operates as described above. func (it *LinksTo) Next() (graph.Value, bool) { graph.NextLogIn(it) - val, ok := it.nextIt.Next() + val, ok := graph.Next(it.nextIt) if !ok { // Subiterator is empty, get another one - candidate, ok := it.primaryIt.Next() + candidate, ok := graph.Next(it.primaryIt) if !ok { // We're out of nodes in our subiterator, so we're done as well. return graph.NextLogOut(it, 0, false) diff --git a/graph/iterator/optional_iterator.go b/graph/iterator/optional_iterator.go index 7b9435f..bc6899c 100644 --- a/graph/iterator/optional_iterator.go +++ b/graph/iterator/optional_iterator.go @@ -30,8 +30,6 @@ import ( "fmt" "strings" - "github.com/barakmich/glog" - "github.com/google/cayley/graph" ) @@ -78,16 +76,6 @@ func (it *Optional) Clone() graph.Iterator { return out } -// FIXME(kortschak) When we create a Nexter interface the -// following two methods need to go away. - -// Nexting the iterator is unsupported -- error and return an empty set. -// (As above, a reasonable alternative would be to Next() an all iterator) -func (it *Optional) Next() (graph.Value, bool) { - glog.Errorln("Nexting an un-nextable iterator") - return nil, false -} - // DEPRECATED func (it *Optional) ResultTree() *graph.ResultTree { return graph.NewResultTree(it.Result()) diff --git a/graph/iterator/or_iterator.go b/graph/iterator/or_iterator.go index db323ed..13f28b8 100644 --- a/graph/iterator/or_iterator.go +++ b/graph/iterator/or_iterator.go @@ -29,7 +29,6 @@ import ( ) type Or struct { - Base uid uint64 tags graph.Tagger isShortCircuiting bool @@ -156,7 +155,7 @@ func (it *Or) Next() (graph.Value, bool) { firstTime = true } curIt := it.internalIterators[it.currentIterator] - curr, exists = curIt.Next() + curr, exists = graph.Next(curIt) if !exists { if it.isShortCircuiting && !firstTime { return graph.NextLogOut(it, nil, false) diff --git a/graph/iterator/or_iterator_test.go b/graph/iterator/or_iterator_test.go index 3b52ccd..78742eb 100644 --- a/graph/iterator/or_iterator_test.go +++ b/graph/iterator/or_iterator_test.go @@ -24,7 +24,7 @@ import ( func iterated(it graph.Iterator) []int { var res []int for { - val, ok := it.Next() + val, ok := graph.Next(it) if !ok { break } diff --git a/graph/iterator/query_shape.go b/graph/iterator/query_shape.go index acdbfa8..8af4a09 100644 --- a/graph/iterator/query_shape.go +++ b/graph/iterator/query_shape.go @@ -129,7 +129,7 @@ func (qs *queryShape) MakeNode(it graph.Iterator) *Node { case graph.Fixed: n.IsFixed = true for { - val, more := it.Next() + val, more := graph.Next(it) if !more { break } diff --git a/graph/iterator/value_comparison_iterator.go b/graph/iterator/value_comparison_iterator.go index ab5e698..2dfae3f 100644 --- a/graph/iterator/value_comparison_iterator.go +++ b/graph/iterator/value_comparison_iterator.go @@ -46,7 +46,6 @@ const ( ) type Comparison struct { - Base uid uint64 tags graph.Tagger subIt graph.Iterator @@ -132,7 +131,7 @@ func (it *Comparison) Next() (graph.Value, bool) { var val graph.Value var ok bool for { - val, ok = it.subIt.Next() + val, ok = graph.Next(it.subIt) if !ok { return nil, false } diff --git a/graph/leveldb/all_iterator.go b/graph/leveldb/all_iterator.go index 673fbb9..b337748 100644 --- a/graph/leveldb/all_iterator.go +++ b/graph/leveldb/all_iterator.go @@ -27,7 +27,6 @@ import ( ) type AllIterator struct { - iterator.Base uid uint64 tags graph.Tagger prefix []byte diff --git a/graph/leveldb/iterator.go b/graph/leveldb/iterator.go index 9f9b0bf..0b56854 100644 --- a/graph/leveldb/iterator.go +++ b/graph/leveldb/iterator.go @@ -27,7 +27,6 @@ import ( ) type Iterator struct { - iterator.Base uid uint64 tags graph.Tagger nextPrefix []byte diff --git a/graph/leveldb/leveldb_test.go b/graph/leveldb/leveldb_test.go index e2855fb..17c7ea7 100644 --- a/graph/leveldb/leveldb_test.go +++ b/graph/leveldb/leveldb_test.go @@ -45,7 +45,7 @@ func makeTripleSet() []*graph.Triple { func iteratedTriples(ts graph.TripleStore, it graph.Iterator) []*graph.Triple { var res ordered for { - val, ok := it.Next() + val, ok := graph.Next(it) if !ok { break } @@ -85,7 +85,7 @@ func (o ordered) Swap(i, j int) { o[i], o[j] = o[j], o[i] } func iteratedNames(ts graph.TripleStore, it graph.Iterator) []string { var res []string for { - val, ok := it.Next() + val, ok := graph.Next(it) if !ok { break } @@ -265,7 +265,7 @@ func TestIterator(t *testing.T) { it.Reset() it = ts.TriplesAllIterator() - edge, _ := it.Next() + edge, _ := graph.Next(it) triple := ts.Triple(edge) set := makeTripleSet() var ok bool @@ -433,10 +433,10 @@ func TestOptimize(t *testing.T) { t.Errorf("Optimized iteration does not match original") } - oldIt.Next() + graph.Next(oldIt) oldResults := make(map[string]graph.Value) oldIt.TagResults(oldResults) - newIt.Next() + graph.Next(newIt) newResults := make(map[string]graph.Value) newIt.TagResults(newResults) if !reflect.DeepEqual(newResults, oldResults) { diff --git a/graph/leveldb/triplestore_iterator_optimize.go b/graph/leveldb/triplestore_iterator_optimize.go index f449e0e..9aab0f2 100644 --- a/graph/leveldb/triplestore_iterator_optimize.go +++ b/graph/leveldb/triplestore_iterator_optimize.go @@ -37,7 +37,7 @@ func (ts *TripleStore) optimizeLinksTo(it *iterator.LinksTo) (graph.Iterator, bo if primary.Type() == graph.Fixed { size, _ := primary.Size() if size == 1 { - val, ok := primary.Next() + val, ok := graph.Next(primary) if !ok { panic("Sizes lie") } diff --git a/graph/memstore/iterator.go b/graph/memstore/iterator.go index 4be57db..7ec913a 100644 --- a/graph/memstore/iterator.go +++ b/graph/memstore/iterator.go @@ -26,7 +26,6 @@ import ( ) type Iterator struct { - iterator.Base uid uint64 tags graph.Tagger tree *llrb.LLRB diff --git a/graph/memstore/triplestore_iterator_optimize.go b/graph/memstore/triplestore_iterator_optimize.go index d7510e3..3dc2c2c 100644 --- a/graph/memstore/triplestore_iterator_optimize.go +++ b/graph/memstore/triplestore_iterator_optimize.go @@ -37,7 +37,7 @@ func (ts *TripleStore) optimizeLinksTo(it *iterator.LinksTo) (graph.Iterator, bo if primary.Type() == graph.Fixed { size, _ := primary.Size() if size == 1 { - val, ok := primary.Next() + val, ok := graph.Next(primary) if !ok { panic("Sizes lie") } diff --git a/graph/memstore/triplestore_test.go b/graph/memstore/triplestore_test.go index aa8f93b..80d9fdb 100644 --- a/graph/memstore/triplestore_test.go +++ b/graph/memstore/triplestore_test.go @@ -189,7 +189,7 @@ func TestRemoveTriple(t *testing.T) { hasa := iterator.NewHasA(ts, innerAnd, graph.Object) newIt, _ := hasa.Optimize() - _, ok := newIt.Next() + _, ok := graph.Next(newIt) if ok { t.Error("E should not have any followers.") } diff --git a/graph/mongo/iterator.go b/graph/mongo/iterator.go index 3389e9f..f65b324 100644 --- a/graph/mongo/iterator.go +++ b/graph/mongo/iterator.go @@ -27,7 +27,6 @@ import ( ) type Iterator struct { - iterator.Base uid uint64 tags graph.Tagger ts *TripleStore diff --git a/graph/mongo/triplestore_iterator_optimize.go b/graph/mongo/triplestore_iterator_optimize.go index 76c50d8..d2d1a05 100644 --- a/graph/mongo/triplestore_iterator_optimize.go +++ b/graph/mongo/triplestore_iterator_optimize.go @@ -37,7 +37,7 @@ func (ts *TripleStore) optimizeLinksTo(it *iterator.LinksTo) (graph.Iterator, bo if primary.Type() == graph.Fixed { size, _ := primary.Size() if size == 1 { - val, ok := primary.Next() + val, ok := graph.Next(primary) if !ok { panic("Sizes lie") } diff --git a/graph/result_tree_evaluator.go b/graph/result_tree_evaluator.go index e1588d2..e2feb33 100644 --- a/graph/result_tree_evaluator.go +++ b/graph/result_tree_evaluator.go @@ -40,7 +40,7 @@ func (t *ResultTree) AddSubtree(sub *ResultTree) { t.subtrees = append(t.subtrees, sub) } -func StringResultTreeEvaluator(it Iterator) string { +func StringResultTreeEvaluator(it Nexter) string { ok := true out := "" for { @@ -59,6 +59,6 @@ func StringResultTreeEvaluator(it Iterator) string { return out } -func PrintResultTreeEvaluator(it Iterator) { +func PrintResultTreeEvaluator(it Nexter) { fmt.Print(StringResultTreeEvaluator(it)) } diff --git a/graph/sexp/parser_test.go b/graph/sexp/parser_test.go index bd9c13b..bd2b319 100644 --- a/graph/sexp/parser_test.go +++ b/graph/sexp/parser_test.go @@ -65,7 +65,7 @@ func TestMemstoreBackedSexp(t *testing.T) { if it.Type() != test.typ { t.Errorf("Incorrect type for %s, got:%q expect %q", test.message, it.Type(), test.expect) } - got, ok := it.Next() + got, ok := graph.Next(it) if !ok { t.Errorf("Failed to %s", test.message) } @@ -86,7 +86,7 @@ func TestTreeConstraintParse(t *testing.T) { if it.Type() != graph.And { t.Error("Odd iterator tree. Got: %s", it.DebugString(0)) } - out, ok := it.Next() + out, ok := graph.Next(it) if !ok { t.Error("Got no results") } @@ -103,7 +103,7 @@ func TestTreeConstraintTagParse(t *testing.T) { "(:like\n" + "($a (:is :good))))" it := BuildIteratorTreeForQuery(ts, query) - _, ok := it.Next() + _, ok := graph.Next(it) if !ok { t.Error("Got no results") } @@ -133,14 +133,14 @@ func TestMultipleConstraintParse(t *testing.T) { if it.Type() != graph.And { t.Error("Odd iterator tree. Got: %s", it.DebugString(0)) } - out, ok := it.Next() + out, ok := graph.Next(it) if !ok { t.Error("Got no results") } if out != ts.ValueOf("i") { t.Errorf("Got %d, expected %d", out, ts.ValueOf("i")) } - _, ok = it.Next() + _, ok = graph.Next(it) if ok { t.Error("Too many results") } diff --git a/graph/sexp/session.go b/graph/sexp/session.go index 8d243f0..0fb4810 100644 --- a/graph/sexp/session.go +++ b/graph/sexp/session.go @@ -77,7 +77,7 @@ func (s *Session) ExecInput(input string, out chan interface{}, limit int) { } nResults := 0 for { - _, ok := it.Next() + _, ok := graph.Next(it) if !ok { break } diff --git a/query/gremlin/finals.go b/query/gremlin/finals.go index 414d780..022a394 100644 --- a/query/gremlin/finals.go +++ b/query/gremlin/finals.go @@ -151,7 +151,7 @@ func runIteratorToArray(it graph.Iterator, ses *Session, limit int) []map[string if ses.doHalt { return nil } - _, ok := it.Next() + _, ok := graph.Next(it) if !ok { break } @@ -187,7 +187,7 @@ func runIteratorToArrayNoTags(it graph.Iterator, ses *Session, limit int) []stri if ses.doHalt { return nil } - val, ok := it.Next() + val, ok := graph.Next(it) if !ok { break } @@ -208,7 +208,7 @@ func runIteratorWithCallback(it graph.Iterator, ses *Session, callback otto.Valu if ses.doHalt { return } - _, ok := it.Next() + _, ok := graph.Next(it) if !ok { break } @@ -249,7 +249,7 @@ func runIteratorOnSession(it graph.Iterator, ses *Session) { if ses.doHalt { return } - _, ok := it.Next() + _, ok := graph.Next(it) if !ok { break } diff --git a/query/mql/session.go b/query/mql/session.go index e5944d4..882c856 100644 --- a/query/mql/session.go +++ b/query/mql/session.go @@ -88,7 +88,7 @@ func (s *Session) ExecInput(input string, c chan interface{}, limit int) { glog.V(2).Infoln(it.DebugString(0)) } for { - _, ok := it.Next() + _, ok := graph.Next(it) if !ok { break }