From 51f5a86232939aa9f3380447c5b4c7259ffc4c9f Mon Sep 17 00:00:00 2001 From: Tyler Gibbons Date: Thu, 1 Oct 2015 19:09:33 -0700 Subject: [PATCH 1/6] Refactor for readability Added more documentation, converted logical operations to (hopefully) consistent 'route' metaphor, and moved all AND(iterator, iterator) stanzas out into a single join() function that is more descriptive of what's actually happening --- graph/path/morphism_apply_functions.go | 195 +++++++++++++++++++-------------- graph/path/path.go | 6 +- 2 files changed, 116 insertions(+), 85 deletions(-) diff --git a/graph/path/morphism_apply_functions.go b/graph/path/morphism_apply_functions.go index c76c3d3..0c0c9a1 100644 --- a/graph/path/morphism_apply_functions.go +++ b/graph/path/morphism_apply_functions.go @@ -1,4 +1,4 @@ -// Copyright 2014 The Cayley Authors. All rights reserved. +// Copyright 2015 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. @@ -20,58 +20,80 @@ import ( "github.com/google/cayley/quad" ) +// join puts two iterators together by intersecting their result sets with an AND +// Since we're using an and iterator, it's a good idea to put the smallest result +// set first so that Next() produces fewer values to check Contains() +func join(qs graph.QuadStore, itL, itR graph.Iterator) graph.Iterator { + and := iterator.NewAnd(qs) + and.AddSubIterator(itL) + and.AddSubIterator(itR) + + return and +} + +// isMorphism represents all nodes passed in-- if there are none, this function +// acts as a passthrough for the previous iterator func isMorphism(nodes ...string) morphism { return morphism{ Name: "is", Reversal: func() morphism { return isMorphism(nodes...) }, - Apply: func(qs graph.QuadStore, it graph.Iterator) graph.Iterator { - var sub graph.Iterator + Apply: func(qs graph.QuadStore, in graph.Iterator) graph.Iterator { if len(nodes) == 0 { - sub = qs.NodesAllIterator() - } else { - fixed := qs.FixedIterator() - for _, n := range nodes { - fixed.Add(qs.ValueOf(n)) - } - sub = fixed + // Acting as a passthrough here is equivalent to + // building a NodesAllIterator to Next() or Contains() + // from here as in previous versions + return in } - and := iterator.NewAnd(qs) - and.AddSubIterator(sub) - and.AddSubIterator(it) - return and + + isNodes := ls.FixedIterator() + for _, n := range nodes { + isNodes.Add(qs.ValueOf(n)) + } + + // Anything with fixedIterators will usually have a much + // smaller result set, so join isNodes first here + return join(qs, isNodes, in) }, } } +// hasMorphism is func hasMorphism(via interface{}, nodes ...string) morphism { return morphism{ Name: "has", Reversal: func() morphism { return hasMorphism(via, nodes...) }, - Apply: func(qs graph.QuadStore, it graph.Iterator) graph.Iterator { - var sub graph.Iterator - if len(nodes) == 0 { - sub = qs.NodesAllIterator() - } else { + Apply: func(qs graph.QuadStore, in graph.Iterator) graph.Iterator { + viaIter := buildViaPath(qs, via). + BuildIterator() + ends := func() graph.Iterator { + if len(nodes) == 0 { + return qs.NodesAllIterator() + } + fixed := qs.FixedIterator() for _, n := range nodes { fixed.Add(qs.ValueOf(n)) } - sub = fixed + return fixed + }() + + trail := iterator.NewLinksTo(qs, viaIter, quad.Predicate) + dest := iterator.NewLinksTo(qs, ends, graph.Object) + + // If we were given nodes, intersecting with them first will + // be extremely cheap-- otherwise, it will be the most expensive + // (requiring iteration over all nodes). We have enough info to + // make this optimization now since intersections are commutative + if len(nodes) == 0 { // Where dest involves an All iterator + route := join(qs, trail, dest) + has := iterator.NewHasA(qs, route, graph.Subject) + return join(qs, in, has) } - var viaPath *Path - if via != nil { - viaPath = buildViaPath(qs, via) - } else { - viaPath = buildViaPath(qs) - } - subAnd := iterator.NewAnd(qs) - subAnd.AddSubIterator(iterator.NewLinksTo(qs, sub, quad.Object)) - subAnd.AddSubIterator(iterator.NewLinksTo(qs, viaPath.BuildIterator(), quad.Predicate)) - hasa := iterator.NewHasA(qs, subAnd, quad.Subject) - and := iterator.NewAnd(qs) - and.AddSubIterator(it) - and.AddSubIterator(hasa) - return and + + // This looks backwards. That's OK-- see the note above + route := join(qs, dest, trail) + has := iterator.NewHasA(qs, route, graph.Subject) + return join(qs, has, in) }, } } @@ -90,6 +112,7 @@ func tagMorphism(tags ...string) morphism { } } +// outMorphism iterates forward one RDF triple or via an entire path func outMorphism(via ...interface{}) morphism { return morphism{ Name: "out", @@ -101,6 +124,7 @@ func outMorphism(via ...interface{}) morphism { } } +// inMorphism iterates backwards one RDF triple or via an entire path func inMorphism(via ...interface{}) morphism { return morphism{ Name: "in", @@ -112,43 +136,42 @@ func inMorphism(via ...interface{}) morphism { } } +// iteratorMorphism simply tacks the input iterator onto the chain func iteratorMorphism(it graph.Iterator) morphism { return morphism{ Name: "iterator", Reversal: func() morphism { return iteratorMorphism(it) }, Apply: func(qs graph.QuadStore, subIt graph.Iterator) graph.Iterator { - and := iterator.NewAnd(qs) - and.AddSubIterator(it) - and.AddSubIterator(subIt) - return and + return join(qs, it, subIt) }, } } +// andMorphism sticks a path onto the current iterator chain func andMorphism(p *Path) morphism { return morphism{ Name: "and", Reversal: func() morphism { return andMorphism(p) }, - Apply: func(qs graph.QuadStore, it graph.Iterator) graph.Iterator { - subIt := p.BuildIteratorOn(qs) - and := iterator.NewAnd(qs) - and.AddSubIterator(it) - and.AddSubIterator(subIt) - return and + Apply: func(qs graph.QuadStore, itL graph.Iterator) graph.Iterator { + itR := p.BuildIteratorOn(qs) + + return join(qs, itL, itR) }, } } +// orMorphism is the union, vice intersection, of a path and the current iterator func orMorphism(p *Path) morphism { return morphism{ Name: "or", Reversal: func() morphism { return orMorphism(p) }, - Apply: func(qs graph.QuadStore, it graph.Iterator) graph.Iterator { - subIt := p.BuildIteratorOn(qs) - and := iterator.NewOr() - and.AddSubIterator(it) - and.AddSubIterator(subIt) - return and + Apply: func(qs graph.QuadStore, itL graph.Iterator) graph.Iterator { + itR := p.BuildIteratorOn(qs) + + or := iterator.NewOr() + or.AddSubIterator(itL) + or.AddSubIterator(itR) + return or }, } } @@ -163,17 +186,17 @@ func followMorphism(p *Path) morphism { } } +// exceptMorphism removes all results on p.(*Path) from the current iterators func exceptMorphism(p *Path) morphism { return morphism{ Name: "except", Reversal: func() morphism { return exceptMorphism(p) }, Apply: func(qs graph.QuadStore, base graph.Iterator) graph.Iterator { - subIt := p.BuildIteratorOn(qs) - notIt := iterator.NewNot(subIt, qs.NodesAllIterator()) - and := iterator.NewAnd(qs) - and.AddSubIterator(base) - and.AddSubIterator(notIt) - return and + in := p.BuildIteratorOn(qs) + allNodes := qs.NodesAllIterator() + notIn := iterator.NewNot(in, allNodes) + + return join(qs, base, notIn) }, } } @@ -200,40 +223,44 @@ func saveReverseMorphism(via interface{}, tag string) morphism { } } -func buildSave(qs graph.QuadStore, via interface{}, tag string, it graph.Iterator, reverse bool) graph.Iterator { - all := qs.NodesAllIterator() +func buildSave( + qs graph.QuadStore, via interface{}, + tag string, from graph.Iterator, reverse bool, +) graph.Iterator { + + allNodes := qs.NodesAllIterator() all.Tagger().Add(tag) - node, allDir := quad.Subject, quad.Object - var viaPath *Path - if via != nil { - viaPath = buildViaPath(qs, via) - } else { - viaPath = buildViaPath(qs) - } + + start, goal := graph.Subject, graph.Object if reverse { - node, allDir = allDir, node + start, goal = goal, start } - lto := iterator.NewLinksTo(qs, all, allDir) - subAnd := iterator.NewAnd(qs) - subAnd.AddSubIterator(iterator.NewLinksTo(qs, viaPath.BuildIterator(), quad.Predicate)) - subAnd.AddSubIterator(lto) - hasa := iterator.NewHasA(qs, subAnd, node) - and := iterator.NewAnd(qs) - and.AddSubIterator(hasa) - and.AddSubIterator(it) - return and + viaIter := buildViaPath(qs, via). + BuildIterator() + + dest := iterator.NewLinksTo(qs, allNodes, goal) + trail := iterator.NewLinksTo(qs, viaIter, graph.Predicate) + + route := join(qs, trail, dest) + save := iterator.NewHasA(qs, route, start) + + return join(qs, from, save) } -func inOutIterator(viaPath *Path, it graph.Iterator, reverse bool) graph.Iterator { - in, out := quad.Subject, quad.Object - if reverse { - in, out = out, in +func inOutIterator(viaPath *Path, from graph.Iterator, inIterator bool) graph.Iterator { + start, goal := quad.Subject, quad.Object + if inIterator { + start, goal = goal, start } - lto := iterator.NewLinksTo(viaPath.qs, it, in) - and := iterator.NewAnd(viaPath.qs) - and.AddSubIterator(iterator.NewLinksTo(viaPath.qs, viaPath.BuildIterator(), quad.Predicate)) - and.AddSubIterator(lto) - return iterator.NewHasA(viaPath.qs, and, out) + + viaIter := viaPath.BuildIterator() + + source := iterator.NewLinksTo(viaPath.qs, from, start) + trail := iterator.NewLinksTo(viaPath.qs, viaIter, graph.Predicate) + + route := join(viaPath.qs, source, trail) + + return iterator.NewHasA(viaPath.qs, route, goal) } func buildViaPath(qs graph.QuadStore, via ...interface{}) *Path { diff --git a/graph/path/path.go b/graph/path/path.go index 143e3d6..31baca2 100644 --- a/graph/path/path.go +++ b/graph/path/path.go @@ -1,4 +1,4 @@ -// Copyright 2014 The Cayley Authors. All rights reserved. +// Copyright 2015 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. @@ -140,11 +140,15 @@ func (p *Path) Except(path *Path) *Path { return p } +// Follow allows you to stitch two paths together. The resulting path will start +// from where the first path left off and continue iterating down the path given func (p *Path) Follow(path *Path) *Path { p.stack = append(p.stack, followMorphism(path)) return p } +// FollowReverse is the same as follow, except it will iterate backwards up the +// path given as argument func (p *Path) FollowReverse(path *Path) *Path { p.stack = append(p.stack, followMorphism(path.Reverse())) return p From 0dd93f87a9bd2b437502201a8e93e45fd98ef94f Mon Sep 17 00:00:00 2001 From: Tyler Gibbons Date: Thu, 1 Oct 2015 19:14:17 -0700 Subject: [PATCH 2/6] Fix mis-named packages, run tests --- graph/path/morphism_apply_functions.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/graph/path/morphism_apply_functions.go b/graph/path/morphism_apply_functions.go index 0c0c9a1..68e25c1 100644 --- a/graph/path/morphism_apply_functions.go +++ b/graph/path/morphism_apply_functions.go @@ -45,7 +45,7 @@ func isMorphism(nodes ...string) morphism { return in } - isNodes := ls.FixedIterator() + isNodes := qs.FixedIterator() for _, n := range nodes { isNodes.Add(qs.ValueOf(n)) } @@ -78,7 +78,7 @@ func hasMorphism(via interface{}, nodes ...string) morphism { }() trail := iterator.NewLinksTo(qs, viaIter, quad.Predicate) - dest := iterator.NewLinksTo(qs, ends, graph.Object) + dest := iterator.NewLinksTo(qs, ends, quad.Object) // If we were given nodes, intersecting with them first will // be extremely cheap-- otherwise, it will be the most expensive @@ -86,13 +86,13 @@ func hasMorphism(via interface{}, nodes ...string) morphism { // make this optimization now since intersections are commutative if len(nodes) == 0 { // Where dest involves an All iterator route := join(qs, trail, dest) - has := iterator.NewHasA(qs, route, graph.Subject) + has := iterator.NewHasA(qs, route, quad.Subject) return join(qs, in, has) } // This looks backwards. That's OK-- see the note above route := join(qs, dest, trail) - has := iterator.NewHasA(qs, route, graph.Subject) + has := iterator.NewHasA(qs, route, quad.Subject) return join(qs, has, in) }, } @@ -229,9 +229,9 @@ func buildSave( ) graph.Iterator { allNodes := qs.NodesAllIterator() - all.Tagger().Add(tag) + allNodes.Tagger().Add(tag) - start, goal := graph.Subject, graph.Object + start, goal := quad.Subject, quad.Object if reverse { start, goal = goal, start } @@ -239,7 +239,7 @@ func buildSave( BuildIterator() dest := iterator.NewLinksTo(qs, allNodes, goal) - trail := iterator.NewLinksTo(qs, viaIter, graph.Predicate) + trail := iterator.NewLinksTo(qs, viaIter, quad.Predicate) route := join(qs, trail, dest) save := iterator.NewHasA(qs, route, start) @@ -256,7 +256,7 @@ func inOutIterator(viaPath *Path, from graph.Iterator, inIterator bool) graph.It viaIter := viaPath.BuildIterator() source := iterator.NewLinksTo(viaPath.qs, from, start) - trail := iterator.NewLinksTo(viaPath.qs, viaIter, graph.Predicate) + trail := iterator.NewLinksTo(viaPath.qs, viaIter, quad.Predicate) route := join(viaPath.qs, source, trail) From 1a86cf0b13fbfb171359e960995ad54a6f297dae Mon Sep 17 00:00:00 2001 From: Tyler Gibbons Date: Thu, 1 Oct 2015 19:32:25 -0700 Subject: [PATCH 3/6] Add myself to contributors --- CONTRIBUTORS | 1 + 1 file changed, 1 insertion(+) diff --git a/CONTRIBUTORS b/CONTRIBUTORS index df4d294..23ab652 100644 --- a/CONTRIBUTORS +++ b/CONTRIBUTORS @@ -20,5 +20,6 @@ Jeremy Jay Pius Uzamere Robert Daniel Kortschak Timothy Armstrong +Tyler Gibbons Zihua Li Stefan Koshiw From 4f69eb301c9d1eec5af428940e9255ada5f7755d Mon Sep 17 00:00:00 2001 From: Tyler Gibbons Date: Thu, 1 Oct 2015 19:41:15 -0700 Subject: [PATCH 4/6] Fix copyright dates --- graph/path/morphism_apply_functions.go | 2 +- graph/path/path.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/graph/path/morphism_apply_functions.go b/graph/path/morphism_apply_functions.go index 68e25c1..838d90b 100644 --- a/graph/path/morphism_apply_functions.go +++ b/graph/path/morphism_apply_functions.go @@ -1,4 +1,4 @@ -// Copyright 2015 The Cayley Authors. All rights reserved. +// 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. diff --git a/graph/path/path.go b/graph/path/path.go index 31baca2..0712932 100644 --- a/graph/path/path.go +++ b/graph/path/path.go @@ -1,4 +1,4 @@ -// Copyright 2015 The Cayley Authors. All rights reserved. +// 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. From 893bd6ca0036d91c727afa40faa73af531baa523 Mon Sep 17 00:00:00 2001 From: Tyler Gibbons Date: Thu, 1 Oct 2015 19:46:52 -0700 Subject: [PATCH 5/6] Complete comment on line 60 --- graph/path/morphism_apply_functions.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/graph/path/morphism_apply_functions.go b/graph/path/morphism_apply_functions.go index 838d90b..846eb9f 100644 --- a/graph/path/morphism_apply_functions.go +++ b/graph/path/morphism_apply_functions.go @@ -57,7 +57,8 @@ func isMorphism(nodes ...string) morphism { } } -// hasMorphism is +// hasMorphism is the set of nodes that is reachable via either a *Path, a +// single node.(string) or a list of nodes.([]string) func hasMorphism(via interface{}, nodes ...string) morphism { return morphism{ Name: "has", From 5c41e4fe5dbee3e0d441ae485754aa2ecc7c92fd Mon Sep 17 00:00:00 2001 From: Tyler Gibbons Date: Thu, 1 Oct 2015 19:52:49 -0700 Subject: [PATCH 6/6] Add periods throughout --- graph/path/morphism_apply_functions.go | 26 +++++++++++++------------- graph/path/path.go | 6 +++--- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/graph/path/morphism_apply_functions.go b/graph/path/morphism_apply_functions.go index 846eb9f..b93c92b 100644 --- a/graph/path/morphism_apply_functions.go +++ b/graph/path/morphism_apply_functions.go @@ -22,7 +22,7 @@ import ( // join puts two iterators together by intersecting their result sets with an AND // Since we're using an and iterator, it's a good idea to put the smallest result -// set first so that Next() produces fewer values to check Contains() +// set first so that Next() produces fewer values to check Contains(). func join(qs graph.QuadStore, itL, itR graph.Iterator) graph.Iterator { and := iterator.NewAnd(qs) and.AddSubIterator(itL) @@ -32,7 +32,7 @@ func join(qs graph.QuadStore, itL, itR graph.Iterator) graph.Iterator { } // isMorphism represents all nodes passed in-- if there are none, this function -// acts as a passthrough for the previous iterator +// acts as a passthrough for the previous iterator. func isMorphism(nodes ...string) morphism { return morphism{ Name: "is", @@ -41,7 +41,7 @@ func isMorphism(nodes ...string) morphism { if len(nodes) == 0 { // Acting as a passthrough here is equivalent to // building a NodesAllIterator to Next() or Contains() - // from here as in previous versions + // from here as in previous versions. return in } @@ -51,14 +51,14 @@ func isMorphism(nodes ...string) morphism { } // Anything with fixedIterators will usually have a much - // smaller result set, so join isNodes first here + // smaller result set, so join isNodes first here. return join(qs, isNodes, in) }, } } // hasMorphism is the set of nodes that is reachable via either a *Path, a -// single node.(string) or a list of nodes.([]string) +// single node.(string) or a list of nodes.([]string). func hasMorphism(via interface{}, nodes ...string) morphism { return morphism{ Name: "has", @@ -85,13 +85,13 @@ func hasMorphism(via interface{}, nodes ...string) morphism { // be extremely cheap-- otherwise, it will be the most expensive // (requiring iteration over all nodes). We have enough info to // make this optimization now since intersections are commutative - if len(nodes) == 0 { // Where dest involves an All iterator + if len(nodes) == 0 { // Where dest involves an All iterator. route := join(qs, trail, dest) has := iterator.NewHasA(qs, route, quad.Subject) return join(qs, in, has) } - // This looks backwards. That's OK-- see the note above + // This looks backwards. That's OK-- see the note above. route := join(qs, dest, trail) has := iterator.NewHasA(qs, route, quad.Subject) return join(qs, has, in) @@ -113,7 +113,7 @@ func tagMorphism(tags ...string) morphism { } } -// outMorphism iterates forward one RDF triple or via an entire path +// outMorphism iterates forward one RDF triple or via an entire path. func outMorphism(via ...interface{}) morphism { return morphism{ Name: "out", @@ -125,7 +125,7 @@ func outMorphism(via ...interface{}) morphism { } } -// inMorphism iterates backwards one RDF triple or via an entire path +// inMorphism iterates backwards one RDF triple or via an entire path. func inMorphism(via ...interface{}) morphism { return morphism{ Name: "in", @@ -137,7 +137,7 @@ func inMorphism(via ...interface{}) morphism { } } -// iteratorMorphism simply tacks the input iterator onto the chain +// iteratorMorphism simply tacks the input iterator onto the chain. func iteratorMorphism(it graph.Iterator) morphism { return morphism{ Name: "iterator", @@ -148,7 +148,7 @@ func iteratorMorphism(it graph.Iterator) morphism { } } -// andMorphism sticks a path onto the current iterator chain +// andMorphism sticks a path onto the current iterator chain. func andMorphism(p *Path) morphism { return morphism{ Name: "and", @@ -161,7 +161,7 @@ func andMorphism(p *Path) morphism { } } -// orMorphism is the union, vice intersection, of a path and the current iterator +// orMorphism is the union, vice intersection, of a path and the current iterator. func orMorphism(p *Path) morphism { return morphism{ Name: "or", @@ -187,7 +187,7 @@ func followMorphism(p *Path) morphism { } } -// exceptMorphism removes all results on p.(*Path) from the current iterators +// exceptMorphism removes all results on p.(*Path) from the current iterators. func exceptMorphism(p *Path) morphism { return morphism{ Name: "except", diff --git a/graph/path/path.go b/graph/path/path.go index 0712932..9d7b065 100644 --- a/graph/path/path.go +++ b/graph/path/path.go @@ -141,14 +141,14 @@ func (p *Path) Except(path *Path) *Path { } // Follow allows you to stitch two paths together. The resulting path will start -// from where the first path left off and continue iterating down the path given +// from where the first path left off and continue iterating down the path given. func (p *Path) Follow(path *Path) *Path { p.stack = append(p.stack, followMorphism(path)) return p } // FollowReverse is the same as follow, except it will iterate backwards up the -// path given as argument +// path given as argument. func (p *Path) FollowReverse(path *Path) *Path { p.stack = append(p.stack, followMorphism(path.Reverse())) return p @@ -167,7 +167,7 @@ func (p *Path) Save(via interface{}, tag string) *Path { } // SaveReverse is the same as Save, only in the reverse direction -// (the subject of the linkage should be tagged, instead of the object) +// (the subject of the linkage should be tagged, instead of the object). func (p *Path) SaveReverse(via interface{}, tag string) *Path { p.stack = append(p.stack, saveReverseMorphism(via, tag)) return p