From 9ff9edc931b14f0e41c9b3634cb484e927a97516 Mon Sep 17 00:00:00 2001 From: Connor Newton Date: Thu, 18 Oct 2018 13:51:58 +0100 Subject: [PATCH 1/2] Implement Resolver iterator Implement an iterator that takes a slice of nodes and an associated QuadStore and resolves to their respective values during iteration. Resolves #663 --- CONTRIBUTORS | 1 + graph/iterator.go | 1 + graph/iterator/resolver.go | 186 ++++++++++++++++++++++++++++++++ graph/iterator/resolver_test.go | 133 +++++++++++++++++++++++ 4 files changed, 321 insertions(+) create mode 100644 graph/iterator/resolver.go create mode 100644 graph/iterator/resolver_test.go diff --git a/CONTRIBUTORS b/CONTRIBUTORS index 6ad98d7a1..1a0c9c9a4 100644 --- a/CONTRIBUTORS +++ b/CONTRIBUTORS @@ -11,6 +11,7 @@ Alexander Peters Andrew Dunham Barak Michener Bram Leenders +Connor Newton Denys Smirnov Derek Liang Jay Graves diff --git a/graph/iterator.go b/graph/iterator.go index 3fec13ad2..f11bf7e6e 100644 --- a/graph/iterator.go +++ b/graph/iterator.go @@ -272,6 +272,7 @@ const ( Regex = Type("regexp") Count = Type("count") Recursive = Type("recursive") + Resolver = Type("resolver") ) // String returns a string representation of the Type. diff --git a/graph/iterator/resolver.go b/graph/iterator/resolver.go new file mode 100644 index 000000000..a10f6e13d --- /dev/null +++ b/graph/iterator/resolver.go @@ -0,0 +1,186 @@ +// Copyright 2018 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 iterator + +import ( + "context" + "fmt" + "github.com/cayleygraph/cayley/graph" + "github.com/cayleygraph/cayley/quad" +) + +var _ graph.Iterator = &Resolver{} + +// A Resolver iterator consists of it's order, an index (where it is in the, +// process of iterating) and a store to resolve values from. +type Resolver struct { + qs graph.QuadStore + uid uint64 + tags graph.Tagger + order []quad.Value + values map[quad.Value]graph.Value + nodes map[interface{}]quad.Value + cached bool + index int + err error + result graph.Value +} + +// Creates a new Resolver iterator. +func NewResolver(qs graph.QuadStore, nodes ...quad.Value) *Resolver { + it := &Resolver{ + uid: NextUID(), + qs: qs, + order: make([]quad.Value, len(nodes)), + // Generally there are going to be no/few duplicates given + // so allocate maps large enough to accommodate all + values: make(map[quad.Value]graph.Value, len(nodes)), + nodes: make(map[interface{}]quad.Value, len(nodes)), + } + copy(it.order, nodes) + return it +} + +func (it *Resolver) UID() uint64 { + return it.uid +} + +func (it *Resolver) Reset() { + it.index = 0 + it.err = nil + it.result = nil +} + +func (it *Resolver) Close() error { + return nil +} + +func (it *Resolver) Tagger() *graph.Tagger { + return &it.tags +} + +func (it *Resolver) TagResults(dst map[string]graph.Value) { + it.tags.TagResult(dst, it.Result()) +} + +func (it *Resolver) Clone() graph.Iterator { + out := NewResolver(it.qs, it.order...) + // Nodes and values maps should contain identical data, so + // just iterate through one + for node, value := range it.values { + out.values[node] = value + out.nodes[value.Key()] = node + } + out.tags.CopyFrom(it) + return out +} + +func (it *Resolver) String() string { + return fmt.Sprintf("Resolver(%v)", it.order) +} + +// Register this iterator as a Resolver iterator. +func (it *Resolver) Type() graph.Type { return graph.Resolver } + +// Resolve nodes to values +func (it *Resolver) resolve(ctx context.Context) error { + values, err := graph.RefsOf(ctx, it.qs, it.order) + if err != nil { + return err + } + for index, value := range values { + node := it.order[index] + it.values[node] = value + it.nodes[value.Key()] = node + } + it.cached = true + return nil +} + +// Check if the passed value is equal to one of the order stored in the iterator. +func (it *Resolver) Contains(ctx context.Context, value graph.Value) bool { + graph.ContainsLogIn(it, value) + if !it.cached { + it.resolve(ctx) + } + _, ok := it.nodes[value.Key()] + it.err = nil + return graph.ContainsLogOut(it, value, ok) +} + +// Next advances the iterator. +func (it *Resolver) Next(ctx context.Context) bool { + graph.NextLogIn(it) + if it.index == len(it.order) { + it.result = nil + return graph.NextLogOut(it, false) + } + if !it.cached { + it.resolve(ctx) + } + node := it.order[it.index] + value, ok := it.values[node] + if !ok { + it.result = nil + it.err = fmt.Errorf("not found: %v", node) + return graph.NextLogOut(it, false) + } + it.result = value + it.err = nil + it.index++ + return graph.NextLogOut(it, true) +} + +func (it *Resolver) Err() error { + return it.err +} + +func (it *Resolver) Result() graph.Value { + return it.result +} + +func (it *Resolver) NextPath(ctx context.Context) bool { + return false +} + +func (it *Resolver) SubIterators() []graph.Iterator { + return nil +} + +// Returns a Null iterator if it's empty so that upstream iterators can optimize it +// away, otherwise there is no optimization. +func (it *Resolver) Optimize() (graph.Iterator, bool) { + if len(it.order) == 0 { + return NewNull(), true + } + return it, false +} + +// Size is the number of m stored. +func (it *Resolver) Size() (int64, bool) { + return int64(len(it.order)), true +} + +func (it *Resolver) Stats() graph.IteratorStats { + s, exact := it.Size() + return graph.IteratorStats{ + // Lookup cost is size of set + ContainsCost: s, + // Next is (presumably) O(1) from store + NextCost: 1, + Size: s, + ExactSize: exact, + } +} diff --git a/graph/iterator/resolver_test.go b/graph/iterator/resolver_test.go new file mode 100644 index 000000000..bc572e24e --- /dev/null +++ b/graph/iterator/resolver_test.go @@ -0,0 +1,133 @@ +package iterator_test + +import ( + "testing" + "context" + "github.com/cayleygraph/cayley/quad" + "github.com/cayleygraph/cayley/graph" + "github.com/cayleygraph/cayley/graph/iterator" + "github.com/cayleygraph/cayley/graph/graphmock" +) + +func TestResolverIteratorIterate(t *testing.T) { + var ctx context.Context + nodes := []quad.Value{ + quad.String("1"), + quad.String("2"), + quad.String("3"), + quad.String("4"), + quad.String("5"), + quad.String("3"), // Assert iterator can handle duplicate values + } + data := make([]quad.Quad, 0, len(nodes)) + for _, node := range nodes { + data = append(data, quad.Make(quad.String("0"), "has", node, nil)) + } + qs := &graphmock.Store{ + Data: data, + } + expected := make(map[quad.Value]graph.Value) + for _, node := range nodes { + expected[node] = qs.ValueOf(node) + } + it := iterator.NewResolver(qs, nodes...) + for _, node := range nodes { + if it.Next(ctx) != true { + t.Fatal("unexpected end of iterator") + } + if err := it.Err(); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if value := it.Result(); value != expected[node] { + t.Fatalf("unexpected quad value: expected %v, got %v", expected[node], value) + } + } + if it.Next(ctx) != false { + t.Fatal("expected end of iterator") + } + if it.Result() != nil { + t.Fatal("expected nil result") + } +} + +func TestResolverIteratorNotFoundError(t *testing.T) { + var ctx context.Context + nodes := []quad.Value{ + quad.String("1"), + quad.String("2"), + quad.String("3"), + quad.String("4"), + quad.String("5"), + } + data := make([]quad.Quad, 0) + skip := 3 + for i, node := range nodes { + // Simulate a missing subject + if i == skip { + continue + } + data = append(data, quad.Make(quad.String("0"), "has", node, nil)) + } + qs := &graphmock.Store{ + Data: data, + } + count := 0 + it := iterator.NewResolver(qs, nodes...) + for it.Next(ctx) { + count++ + } + if count != 0 { + t.Fatal("expected end of iterator") + } + if it.Err() == nil { + t.Fatal("expected not found error") + } + if it.Result() != nil { + t.Fatal("expected nil result") + } +} + +func TestResolverIteratorContains(t *testing.T) { + tests := []struct { + name string + nodes []quad.Value + subject quad.Value + contains bool + }{ + { + "contains", + []quad.Value{ + quad.String("1"), + quad.String("2"), + quad.String("3"), + }, + quad.String("2"), + true, + }, + { + "not contains", + []quad.Value{ + quad.String("1"), + quad.String("3"), + }, + quad.String("2"), + false, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + var ctx context.Context + data := make([]quad.Quad, 0, len(test.nodes)) + for _, node := range test.nodes { + data = append(data, quad.Make(quad.String("0"), "has", node, nil)) + } + qs := &graphmock.Store{ + Data: data, + } + it := iterator.NewResolver(qs, test.nodes...) + if it.Contains(ctx, graph.PreFetched(test.subject)) != test.contains { + t.Fatal("unexpected result") + } + }) + } +} From 9979b4ffc52e7515033a3ae0b58178a640db9796 Mon Sep 17 00:00:00 2001 From: Denys Smirnov Date: Mon, 26 Nov 2018 17:20:26 +0200 Subject: [PATCH 2/2] iterator: do not drop an error in resolver --- graph/iterator/resolver.go | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/graph/iterator/resolver.go b/graph/iterator/resolver.go index a10f6e13d..4dae67180 100644 --- a/graph/iterator/resolver.go +++ b/graph/iterator/resolver.go @@ -17,6 +17,7 @@ package iterator import ( "context" "fmt" + "github.com/cayleygraph/cayley/graph" "github.com/cayleygraph/cayley/quad" ) @@ -41,9 +42,9 @@ type Resolver struct { // Creates a new Resolver iterator. func NewResolver(qs graph.QuadStore, nodes ...quad.Value) *Resolver { it := &Resolver{ - uid: NextUID(), - qs: qs, - order: make([]quad.Value, len(nodes)), + uid: NextUID(), + qs: qs, + order: make([]quad.Value, len(nodes)), // Generally there are going to be no/few duplicates given // so allocate maps large enough to accommodate all values: make(map[quad.Value]graph.Value, len(nodes)), @@ -113,32 +114,36 @@ func (it *Resolver) resolve(ctx context.Context) error { func (it *Resolver) Contains(ctx context.Context, value graph.Value) bool { graph.ContainsLogIn(it, value) if !it.cached { - it.resolve(ctx) + it.err = it.resolve(ctx) + if it.err != nil { + return false + } } _, ok := it.nodes[value.Key()] - it.err = nil return graph.ContainsLogOut(it, value, ok) } // Next advances the iterator. func (it *Resolver) Next(ctx context.Context) bool { graph.NextLogIn(it) - if it.index == len(it.order) { + if it.index >= len(it.order) { it.result = nil return graph.NextLogOut(it, false) } if !it.cached { - it.resolve(ctx) + it.err = it.resolve(ctx) + if it.err != nil { + return false + } } node := it.order[it.index] value, ok := it.values[node] if !ok { it.result = nil - it.err = fmt.Errorf("not found: %v", node) + it.err = fmt.Errorf("not found: %v", node) return graph.NextLogOut(it, false) } it.result = value - it.err = nil it.index++ return graph.NextLogOut(it, true) } @@ -179,8 +184,8 @@ func (it *Resolver) Stats() graph.IteratorStats { // Lookup cost is size of set ContainsCost: s, // Next is (presumably) O(1) from store - NextCost: 1, - Size: s, - ExactSize: exact, + NextCost: 1, + Size: s, + ExactSize: exact, } }