From d8db590abbae06b55aec887d1f10cd5d06f616ee Mon Sep 17 00:00:00 2001 From: Iddan Aaronsohn Date: Tue, 9 Jul 2019 00:52:17 +0300 Subject: [PATCH 01/11] iterator: implement Sort --- graph/iterator/sort.go | 224 +++++++++++++++++++++++++ graph/path/morphism_apply_functions.go | 9 + graph/path/path.go | 5 + graph/shape/path.go | 2 +- graph/shape/shape.go | 27 +++ query/gizmo/gizmo_test.go | 22 +++ query/gizmo/traversals.go | 5 + 7 files changed, 293 insertions(+), 1 deletion(-) create mode 100644 graph/iterator/sort.go diff --git a/graph/iterator/sort.go b/graph/iterator/sort.go new file mode 100644 index 000000000..6220d3100 --- /dev/null +++ b/graph/iterator/sort.go @@ -0,0 +1,224 @@ +package iterator + +import ( + "context" + "sort" + + "github.com/cayleygraph/cayley/graph" + "github.com/cayleygraph/cayley/quad" +) + +var _ graph.IteratorFuture = &Sort{} + +// Sort iterator orders values from it's subiterator. +type Sort struct { + it *sortIt + graph.Iterator +} + +// NewSort creates a new Sort iterator +func NewSort(namer graph.Namer, it graph.Iterator) *Sort { + return &Sort{ + it: newSort(namer, graph.AsShape(it)), + } +} + +// AsShape returns Sort's underlying iterator shape +func (it *Sort) AsShape() graph.IteratorShape { + return it.it +} + +type sortIt struct { + namer graph.Namer + subIt graph.IteratorShape +} + +var _ graph.IteratorShapeCompat = (*sortIt)(nil) + +func newSort(namer graph.Namer, subIt graph.IteratorShape) *sortIt { + return &sortIt{namer, subIt} +} + +func (it *sortIt) Iterate() graph.Scanner { + return newSortNext(it.namer, it.subIt.Iterate()) +} + +func (it *sortIt) AsLegacy() graph.Iterator { + it2 := &Sort{it: it} + it2.Iterator = graph.NewLegacy(it, it2) + return it2 +} + +func (it *sortIt) Lookup() graph.Index { + return newSortContains(it.subIt.Lookup()) +} + +func (it *sortIt) Optimize(ctx context.Context) (graph.IteratorShape, bool) { + newIt, optimized := it.subIt.Optimize(ctx) + if optimized { + it.subIt = newIt + } + return it, false +} + +func (it *sortIt) Stats(ctx context.Context) (graph.IteratorCosts, error) { + subStats, err := it.subIt.Stats(ctx) + return graph.IteratorCosts{ + NextCost: subStats.NextCost, + ContainsCost: subStats.ContainsCost, + Size: graph.Size{ + Size: subStats.Size.Size, + Exact: true, + }, + }, err +} + +func (it *sortIt) String() string { + return "Sort" +} + +// SubIterators returns a slice of the sub iterators. +func (it *sortIt) SubIterators() []graph.IteratorShape { + return []graph.IteratorShape{it.subIt} +} + +type value struct { + result result + name quad.Value + str string +} +type values []value + +func (v values) Len() int { return len(v) } +func (v values) Less(i, j int) bool { + return v[i].str < v[j].str +} +func (v values) Swap(i, j int) { v[i], v[j] = v[j], v[i] } + +type sortNext struct { + namer graph.Namer + subIt graph.Scanner + ordered values + result graph.Ref + err error + index int +} + +func newSortNext(namer graph.Namer, subIt graph.Scanner) *sortNext { + return &sortNext{ + namer: namer, + subIt: subIt, + } +} + +func (it *sortNext) TagResults(dst map[string]graph.Value) { + if it.subIt != nil { + it.subIt.TagResults(dst) + } +} + +func (it *sortNext) Err() error { + return it.err +} + +func (it *sortNext) Result() graph.Value { + return it.result +} + +func (it *sortNext) Next(ctx context.Context) bool { + if it.err != nil { + return false + } + if it.ordered == nil { + v, err := getSortedValues(ctx, it.namer, it.subIt) + it.ordered = v + it.err = err + } + ordered := it.ordered + if it.index < len(ordered) { + it.result = ordered[it.index].result.id + it.index++ + return true + } + return false +} + +func (it *sortNext) NextPath(ctx context.Context) bool { + return false +} + +func (it *sortNext) Close() error { + it.ordered = nil + return it.subIt.Close() +} + +func (it *sortNext) String() string { + return "SortNext" +} + +func getSortedValues(ctx context.Context, namer graph.Namer, it graph.Scanner) (values, error) { + var v values + + for it.Next(ctx) { + var id = it.Result() + var name = namer.NameOf(id) + var str = name.String() + var tags = make(map[string]graph.Value) + it.TagResults(tags) + result := result{id, tags} + value := value{result, name, str} + v = append(v, value) + err := it.Err() + if err != nil { + return v, err + } + } + + sort.Sort(v) + + return v, nil +} + +type sortContains struct { + subIt graph.Index +} + +func newSortContains(subIt graph.Index) *sortContains { + return &sortContains{subIt} +} + +func (it *sortContains) TagResults(dst map[string]graph.Value) { + if it.subIt != nil { + it.subIt.TagResults(dst) + } +} + +func (it *sortContains) Err() error { + return it.subIt.Err() +} + +func (it *sortContains) Result() graph.Value { + return it.subIt.Result() +} + +// Contains checks whether the passed value is part of the primary iterator, +// which is irrelevant for sorted. +func (it *sortContains) Contains(ctx context.Context, val graph.Value) bool { + return it.subIt.Contains(ctx, val) +} + +// NextPath for Sort always returns false. If we were to return multiple +// paths, we'd no longer be a Sort result, so we have to choose only the first +// path that got us here. Sort is serious on this point. +func (it *sortContains) NextPath(ctx context.Context) bool { + return false +} + +// Close closes the primary iterators. +func (it *sortContains) Close() error { + return it.subIt.Close() +} + +func (it *sortContains) String() string { + return "SortContains" +} diff --git a/graph/path/morphism_apply_functions.go b/graph/path/morphism_apply_functions.go index dcafc3cde..47460dc52 100644 --- a/graph/path/morphism_apply_functions.go +++ b/graph/path/morphism_apply_functions.go @@ -412,6 +412,15 @@ func skipMorphism(v int64) morphism { } } +func orderMorphism() morphism { + return morphism{ + Reversal: func(ctx *pathContext) (morphism, *pathContext) { return orderMorphism(), ctx }, + Apply: func(in shape.Shape, ctx *pathContext) (shape.Shape, *pathContext) { + return shape.Sort{From: in}, ctx + }, + } +} + // limitMorphism will limit a number of values-- if number is negative or zero, this function // acts as a passthrough for the previous iterator. func limitMorphism(v int64) morphism { diff --git a/graph/path/path.go b/graph/path/path.go index 0957e6474..e60958c66 100644 --- a/graph/path/path.go +++ b/graph/path/path.go @@ -536,6 +536,11 @@ func (p *Path) Skip(v int64) *Path { return p } +func (p *Path) Order() *Path { + p.stack = append(p.stack, orderMorphism()) + return p +} + // Limit will limit a number of values in result set. func (p *Path) Limit(v int64) *Path { p.stack = append(p.stack, limitMorphism(v)) diff --git a/graph/shape/path.go b/graph/shape/path.go index 57112677e..9688001b2 100644 --- a/graph/shape/path.go +++ b/graph/shape/path.go @@ -240,4 +240,4 @@ func Compare(nodes Shape, op iterator.Operator, v quad.Value) Shape { func Iterate(ctx context.Context, qs graph.QuadStore, s Shape) *graph.IterateChain { it := BuildIterator(qs, s) return graph.Iterate(ctx, it).On(qs) -} +} \ No newline at end of file diff --git a/graph/shape/shape.go b/graph/shape/shape.go index 055710ab1..d756af53d 100644 --- a/graph/shape/shape.go +++ b/graph/shape/shape.go @@ -1433,3 +1433,30 @@ func FilterQuads(subject, predicate, object, label []quad.Value) Shape { } return q } + +type Sort struct { + From Shape +} + +func (s Sort) BuildIterator(qs graph.QuadStore) graph.Iterator { + if IsNull(s.From) { + return iterator.NewNull() + } + it := s.From.BuildIterator(qs) + return iterator.NewSort(qs, it) +} +func (s Sort) Optimize(r Optimizer) (Shape, bool) { + if IsNull(s.From) { + return nil, true + } + var opt bool + s.From, opt = s.From.Optimize(r) + if IsNull(s.From) { + return nil, true + } + if r != nil { + ns, nopt := r.OptimizeShape(s) + return ns, opt || nopt + } + return s, opt +} diff --git a/query/gizmo/gizmo_test.go b/query/gizmo/gizmo_test.go index da192ff63..0ce0de5d3 100644 --- a/query/gizmo/gizmo_test.go +++ b/query/gizmo/gizmo_test.go @@ -639,6 +639,28 @@ var testQueries = []struct { file: multiGraphTestFile, expect: []string{""}, }, + { + message: "Use order", + query: ` + g.V().order().all() + `, + expect: []string{ + "", + "", + "", + "", + "", + "", + "", + "", + "", + "", + "", + "", + "cool_person", + "smart_person", + }, + }, } func runQueryGetTag(rec func(), g []quad.Quad, qu string, tag string, limit int) ([]string, error) { diff --git a/query/gizmo/traversals.go b/query/gizmo/traversals.go index e56e75b5a..4035ddf10 100644 --- a/query/gizmo/traversals.go +++ b/query/gizmo/traversals.go @@ -684,6 +684,11 @@ func (p *pathObject) Skip(offset int) *pathObject { return p.new(np) } +func (p *pathObject) Order() *pathObject { + np := p.clonePath().Order() + return p.new(np) +} + // Backwards compatibility func (p *pathObject) CapitalizedIs(call goja.FunctionCall) goja.Value { return p.Is(call) From f9439d779e4b81dca6b2727b720323f2ec2ba393 Mon Sep 17 00:00:00 2001 From: Iddan Aaronsohn Date: Sat, 5 Oct 2019 18:07:44 +0300 Subject: [PATCH 02/11] docs: document order in GizmoAPI --- docs/gizmoapi.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/gizmoapi.md b/docs/gizmoapi.md index 811785d78..b0a7fa407 100644 --- a/docs/gizmoapi.md +++ b/docs/gizmoapi.md @@ -677,3 +677,6 @@ cFollows.union(dFollows).all(); Unique removes duplicate values from the path. +### `path.order()` + +Order returns values from the path in ascending order. From 1a7147df23ce2365d9e25188c399d9171c23b87c Mon Sep 17 00:00:00 2001 From: Iddan Aaronsohn Date: Sun, 6 Oct 2019 10:24:05 +0300 Subject: [PATCH 03/11] iterator: use underlying iterator instead of sortContains --- graph/iterator/sort.go | 46 +----------------------------------------- 1 file changed, 1 insertion(+), 45 deletions(-) diff --git a/graph/iterator/sort.go b/graph/iterator/sort.go index 6220d3100..6b3c50f69 100644 --- a/graph/iterator/sort.go +++ b/graph/iterator/sort.go @@ -50,7 +50,7 @@ func (it *sortIt) AsLegacy() graph.Iterator { } func (it *sortIt) Lookup() graph.Index { - return newSortContains(it.subIt.Lookup()) + return it.subIt.Lookup() } func (it *sortIt) Optimize(ctx context.Context) (graph.IteratorShape, bool) { @@ -178,47 +178,3 @@ func getSortedValues(ctx context.Context, namer graph.Namer, it graph.Scanner) ( return v, nil } - -type sortContains struct { - subIt graph.Index -} - -func newSortContains(subIt graph.Index) *sortContains { - return &sortContains{subIt} -} - -func (it *sortContains) TagResults(dst map[string]graph.Value) { - if it.subIt != nil { - it.subIt.TagResults(dst) - } -} - -func (it *sortContains) Err() error { - return it.subIt.Err() -} - -func (it *sortContains) Result() graph.Value { - return it.subIt.Result() -} - -// Contains checks whether the passed value is part of the primary iterator, -// which is irrelevant for sorted. -func (it *sortContains) Contains(ctx context.Context, val graph.Value) bool { - return it.subIt.Contains(ctx, val) -} - -// NextPath for Sort always returns false. If we were to return multiple -// paths, we'd no longer be a Sort result, so we have to choose only the first -// path that got us here. Sort is serious on this point. -func (it *sortContains) NextPath(ctx context.Context) bool { - return false -} - -// Close closes the primary iterators. -func (it *sortContains) Close() error { - return it.subIt.Close() -} - -func (it *sortContains) String() string { - return "SortContains" -} From d34992ac18a1530a19c04f6b2c0e376b46291c27 Mon Sep 17 00:00:00 2001 From: Iddan Aaronsohn Date: Sun, 6 Oct 2019 10:24:24 +0300 Subject: [PATCH 04/11] iterator: bigger NextCost in Sort --- graph/iterator/sort.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/graph/iterator/sort.go b/graph/iterator/sort.go index 6b3c50f69..5ebdec0ed 100644 --- a/graph/iterator/sort.go +++ b/graph/iterator/sort.go @@ -64,7 +64,7 @@ func (it *sortIt) Optimize(ctx context.Context) (graph.IteratorShape, bool) { func (it *sortIt) Stats(ctx context.Context) (graph.IteratorCosts, error) { subStats, err := it.subIt.Stats(ctx) return graph.IteratorCosts{ - NextCost: subStats.NextCost, + NextCost: subStats.NextCost*2, // TODO(dennwc): better cost calculation, ContainsCost: subStats.ContainsCost, Size: graph.Size{ Size: subStats.Size.Size, From a14ced22dcb2ff2e76a0c2fd1903d6c6be57f8d7 Mon Sep 17 00:00:00 2001 From: Iddan Aaronsohn Date: Sun, 6 Oct 2019 10:25:11 +0300 Subject: [PATCH 05/11] iterator: add TODO comments for NewSort --- graph/iterator/sort.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/graph/iterator/sort.go b/graph/iterator/sort.go index 5ebdec0ed..bcae0ebc2 100644 --- a/graph/iterator/sort.go +++ b/graph/iterator/sort.go @@ -16,7 +16,8 @@ type Sort struct { graph.Iterator } -// NewSort creates a new Sort iterator +// NewSort creates a new Sort iterator. +// TODO(dennwc): This iterator must not be used inside And: in this cases it may be moved to a Contains branch and won't do anything. We should make And account for this. func NewSort(namer graph.Namer, it graph.Iterator) *Sort { return &Sort{ it: newSort(namer, graph.AsShape(it)), @@ -64,7 +65,7 @@ func (it *sortIt) Optimize(ctx context.Context) (graph.IteratorShape, bool) { func (it *sortIt) Stats(ctx context.Context) (graph.IteratorCosts, error) { subStats, err := it.subIt.Stats(ctx) return graph.IteratorCosts{ - NextCost: subStats.NextCost*2, // TODO(dennwc): better cost calculation, + NextCost: subStats.NextCost * 2, // TODO(dennwc): better cost calculation, ContainsCost: subStats.ContainsCost, Size: graph.Size{ Size: subStats.Size.Size, From bb92911a9d81b5c29b78bc9f8c7c65ddb7af7995 Mon Sep 17 00:00:00 2001 From: Iddan Aaronsohn Date: Sun, 6 Oct 2019 10:31:57 +0300 Subject: [PATCH 06/11] iterator: implement TagResults for Sort iterator --- graph/iterator/sort.go | 10 +++++----- query/gizmo/gizmo_test.go | 23 +++++++++++++++++++++++ 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/graph/iterator/sort.go b/graph/iterator/sort.go index bcae0ebc2..9a2737209 100644 --- a/graph/iterator/sort.go +++ b/graph/iterator/sort.go @@ -100,7 +100,7 @@ type sortNext struct { namer graph.Namer subIt graph.Scanner ordered values - result graph.Ref + result result err error index int } @@ -113,8 +113,8 @@ func newSortNext(namer graph.Namer, subIt graph.Scanner) *sortNext { } func (it *sortNext) TagResults(dst map[string]graph.Value) { - if it.subIt != nil { - it.subIt.TagResults(dst) + for tag, value := range it.result.tags { + dst[tag] = value } } @@ -123,7 +123,7 @@ func (it *sortNext) Err() error { } func (it *sortNext) Result() graph.Value { - return it.result + return it.result.id } func (it *sortNext) Next(ctx context.Context) bool { @@ -137,7 +137,7 @@ func (it *sortNext) Next(ctx context.Context) bool { } ordered := it.ordered if it.index < len(ordered) { - it.result = ordered[it.index].result.id + it.result = ordered[it.index].result it.index++ return true } diff --git a/query/gizmo/gizmo_test.go b/query/gizmo/gizmo_test.go index 0ce0de5d3..4500b8f37 100644 --- a/query/gizmo/gizmo_test.go +++ b/query/gizmo/gizmo_test.go @@ -661,6 +661,29 @@ var testQueries = []struct { "smart_person", }, }, + { + message: "Use order tags", + query: ` + g.V().Tag("target").order().all() + `, + tag: "target", + expect: []string{ + "", + "", + "", + "", + "", + "", + "", + "", + "", + "", + "", + "", + "cool_person", + "smart_person", + }, + }, } func runQueryGetTag(rec func(), g []quad.Quad, qu string, tag string, limit int) ([]string, error) { From 6854fd03d3a6e580d6bc17db19f6466d8882f7a4 Mon Sep 17 00:00:00 2001 From: Denys Smirnov Date: Mon, 7 Oct 2019 21:15:48 +0300 Subject: [PATCH 07/11] iterator: add comments for Sort iterator --- graph/iterator/sort.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/graph/iterator/sort.go b/graph/iterator/sort.go index 9a2737209..f6f9cc007 100644 --- a/graph/iterator/sort.go +++ b/graph/iterator/sort.go @@ -17,7 +17,8 @@ type Sort struct { } // NewSort creates a new Sort iterator. -// TODO(dennwc): This iterator must not be used inside And: in this cases it may be moved to a Contains branch and won't do anything. We should make And account for this. +// TODO(dennwc): This iterator must not be used inside And: it may be moved to a Contains branch and won't do anything. +// We should make And/Intersect account for this. func NewSort(namer graph.Namer, it graph.Iterator) *Sort { return &Sort{ it: newSort(namer, graph.AsShape(it)), @@ -51,6 +52,9 @@ func (it *sortIt) AsLegacy() graph.Iterator { } func (it *sortIt) Lookup() graph.Index { + // TODO(dennwc): Lookup doesn't need any sorting. Using it this way is a bug in the optimizer. + // But instead of failing here, let still allow the query to execute. It won't be sorted, + // but it will work at least. Later consider changing returning an error here. return it.subIt.Lookup() } @@ -65,7 +69,8 @@ func (it *sortIt) Optimize(ctx context.Context) (graph.IteratorShape, bool) { func (it *sortIt) Stats(ctx context.Context) (graph.IteratorCosts, error) { subStats, err := it.subIt.Stats(ctx) return graph.IteratorCosts{ - NextCost: subStats.NextCost * 2, // TODO(dennwc): better cost calculation, + // TODO(dennwc): better cost calculation; we probably need an InitCost defined in graph.IteratorCosts + NextCost: subStats.NextCost * 2, ContainsCost: subStats.ContainsCost, Size: graph.Size{ Size: subStats.Size.Size, From e6259266016130c04c84daa2606fd24dea0a6a49 Mon Sep 17 00:00:00 2001 From: Denys Smirnov Date: Mon, 7 Oct 2019 21:19:51 +0300 Subject: [PATCH 08/11] path: add tests for sorting --- graph/path/pathtest/pathtest.go | 47 +++++++++++++++++++++++++++++++++ query/gizmo/gizmo_test.go | 4 +-- 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/graph/path/pathtest/pathtest.go b/graph/path/pathtest/pathtest.go index 14ac270b0..4a5b99a8b 100644 --- a/graph/path/pathtest/pathtest.go +++ b/graph/path/pathtest/pathtest.go @@ -433,6 +433,53 @@ func testSet(qs graph.QuadStore) []test { path: StartPath(qs, quad.IRI("")), expect: nil, }, + { + message: "use order", + path: StartPath(qs).Order(), + expect: []quad.Value{ + vAlice, + vAre, + vBob, + vCharlie, + vDani, + vEmily, + vFollows, + vFred, + vGreg, + vPredicate, + vSmartGraph, + vStatus, + vCool, + vSmart, + }, + }, + { + message: "use order tags", + path: StartPath(qs).Tag("target").Order(), + tag: "target", + expect: []quad.Value{ + vAlice, + vAre, + vBob, + vCharlie, + vDani, + vEmily, + vFollows, + vFred, + vGreg, + vPredicate, + vSmartGraph, + vStatus, + vCool, + vSmart, + }, + }, + { + message: "order with a next path", + path: StartPath(qs, vDani, vBob).Save(vFollows, "target").Order(), + tag: "target", + expect: []quad.Value{vBob, vFred, vGreg}, + }, } } diff --git a/query/gizmo/gizmo_test.go b/query/gizmo/gizmo_test.go index 4500b8f37..5d6ddaf1f 100644 --- a/query/gizmo/gizmo_test.go +++ b/query/gizmo/gizmo_test.go @@ -640,7 +640,7 @@ var testQueries = []struct { expect: []string{""}, }, { - message: "Use order", + message: "use order", query: ` g.V().order().all() `, @@ -662,7 +662,7 @@ var testQueries = []struct { }, }, { - message: "Use order tags", + message: "use order tags", query: ` g.V().Tag("target").order().all() `, From a4c8e9fa9f1358cb85ba356f890b41c3d32d44f1 Mon Sep 17 00:00:00 2001 From: Denys Smirnov Date: Mon, 7 Oct 2019 22:47:58 +0300 Subject: [PATCH 09/11] iterator: implement NextPath for Sort --- graph/iterator/sort.go | 95 +++++++++++++++++++++++++----------------- 1 file changed, 56 insertions(+), 39 deletions(-) diff --git a/graph/iterator/sort.go b/graph/iterator/sort.go index f6f9cc007..e643c33d3 100644 --- a/graph/iterator/sort.go +++ b/graph/iterator/sort.go @@ -5,7 +5,6 @@ import ( "sort" "github.com/cayleygraph/cayley/graph" - "github.com/cayleygraph/cayley/quad" ) var _ graph.IteratorFuture = &Sort{} @@ -88,32 +87,34 @@ func (it *sortIt) SubIterators() []graph.IteratorShape { return []graph.IteratorShape{it.subIt} } -type value struct { - result result - name quad.Value - str string +type sortValue struct { + result + str string + paths []result } -type values []value +type sortByString []sortValue -func (v values) Len() int { return len(v) } -func (v values) Less(i, j int) bool { +func (v sortByString) Len() int { return len(v) } +func (v sortByString) Less(i, j int) bool { return v[i].str < v[j].str } -func (v values) Swap(i, j int) { v[i], v[j] = v[j], v[i] } +func (v sortByString) Swap(i, j int) { v[i], v[j] = v[j], v[i] } type sortNext struct { - namer graph.Namer - subIt graph.Scanner - ordered values - result result - err error - index int + namer graph.Namer + subIt graph.Scanner + ordered sortByString + result result + err error + index int + pathIndex int } func newSortNext(namer graph.Namer, subIt graph.Scanner) *sortNext { return &sortNext{ - namer: namer, - subIt: subIt, + namer: namer, + subIt: subIt, + pathIndex: -1, } } @@ -139,18 +140,30 @@ func (it *sortNext) Next(ctx context.Context) bool { v, err := getSortedValues(ctx, it.namer, it.subIt) it.ordered = v it.err = err + if it.err != nil { + return false + } } - ordered := it.ordered - if it.index < len(ordered) { - it.result = ordered[it.index].result - it.index++ - return true + if it.index >= len(it.ordered) { + return false } - return false + it.pathIndex = -1 + it.result = it.ordered[it.index].result + it.index++ + return true } func (it *sortNext) NextPath(ctx context.Context) bool { - return false + if it.index >= len(it.ordered) { + return false + } + r := it.ordered[it.index] + if it.pathIndex+1 >= len(r.paths) { + return false + } + it.pathIndex++ + it.result = r.paths[it.pathIndex] + return true } func (it *sortNext) Close() error { @@ -162,25 +175,29 @@ func (it *sortNext) String() string { return "SortNext" } -func getSortedValues(ctx context.Context, namer graph.Namer, it graph.Scanner) (values, error) { - var v values - +func getSortedValues(ctx context.Context, namer graph.Namer, it graph.Scanner) (sortByString, error) { + var v sortByString for it.Next(ctx) { - var id = it.Result() - var name = namer.NameOf(id) - var str = name.String() - var tags = make(map[string]graph.Value) + id := it.Result() + // TODO(dennwc): batch and use graph.ValuesOf + name := namer.NameOf(id) + str := name.String() + tags := make(map[string]graph.Ref) it.TagResults(tags) - result := result{id, tags} - value := value{result, name, str} - v = append(v, value) - err := it.Err() - if err != nil { - return v, err + val := sortValue{ + result: result{id, tags}, + str: str, + } + for it.NextPath(ctx) { + tags = make(map[string]graph.Ref) + it.TagResults(tags) + val.paths = append(val.paths, result{id, tags}) } + v = append(v, val) + } + if err := it.Err(); err != nil { + return v, err } - sort.Sort(v) - return v, nil } From fcaed7fbe0f6cde214d949b27be3375573ab026d Mon Sep 17 00:00:00 2001 From: Denys Smirnov Date: Tue, 8 Oct 2019 14:51:06 +0300 Subject: [PATCH 10/11] break natural sorting in the test dataset --- data/testdata.nq | 2 +- query/gizmo/gizmo_test.go | 4 ++-- query/graphql/graphql_test.go | 4 ++-- query/mql/mql_test.go | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/data/testdata.nq b/data/testdata.nq index 7c1dce31d..d8fc62ebc 100644 --- a/data/testdata.nq +++ b/data/testdata.nq @@ -1,9 +1,9 @@ . . "cool_person" . + . . . - . . "cool_person" . . diff --git a/query/gizmo/gizmo_test.go b/query/gizmo/gizmo_test.go index 5d6ddaf1f..44060a90a 100644 --- a/query/gizmo/gizmo_test.go +++ b/query/gizmo/gizmo_test.go @@ -540,7 +540,7 @@ var testQueries = []struct { arr = g.V("").in("").toArray(2) for (i in arr) g.emit(arr[i]); `, - expect: []string{"", ""}, + expect: []string{"", ""}, }, { message: "show ForEach", @@ -554,7 +554,7 @@ var testQueries = []struct { query: ` g.V("").in("").forEach(2, function(o){g.emit(o.id)}); `, - expect: []string{"", ""}, + expect: []string{"", ""}, }, { message: "clone paths", diff --git a/query/graphql/graphql_test.go b/query/graphql/graphql_test.go index 4547f4f4c..ec3f8a9de 100644 --- a/query/graphql/graphql_test.go +++ b/query/graphql/graphql_test.go @@ -123,8 +123,8 @@ var casesExecute = []struct { "follows": nil, "followed": []M{ {ValueKey: quad.IRI("alice")}, - {ValueKey: quad.IRI("charlie")}, {ValueKey: quad.IRI("dani")}, + {ValueKey: quad.IRI("charlie")}, }, }, { @@ -283,8 +283,8 @@ var casesExecute = []struct { {"id": quad.IRI("fred")}, {"id": quad.IRI("status")}, {"id": quad.String("cool_person")}, - {"id": quad.IRI("charlie")}, {"id": quad.IRI("dani"), "status": quad.String("cool_person")}, + {"id": quad.IRI("charlie")}, {"id": quad.IRI("greg"), "status": []quad.Value{ quad.String("cool_person"), quad.String("smart_person"), diff --git a/query/mql/mql_test.go b/query/mql/mql_test.go index 5e4165397..205238b65 100644 --- a/query/mql/mql_test.go +++ b/query/mql/mql_test.go @@ -69,8 +69,8 @@ var testQueries = []struct { {"id": ""}, {"id": ""}, {"id": "cool_person"}, - {"id": ""}, {"id": ""}, + {"id": ""}, {"id": ""}, {"id": ""}, {"id": ""}, @@ -124,8 +124,8 @@ var testQueries = []struct { expect: ` [ {"id": "", "": {"id": "", "": "cool_person"}}, - {"id": "", "": {"id": "", "": "cool_person"}}, {"id": "", "": {"id": "", "": "cool_person"}}, + {"id": "", "": {"id": "", "": "cool_person"}}, {"id": "", "": {"id": "", "": "cool_person"}} ] `, From 7ce207149cbdb801d052d6d03dadb56b8200f8ee Mon Sep 17 00:00:00 2001 From: Denys Smirnov Date: Tue, 8 Oct 2019 14:52:08 +0300 Subject: [PATCH 11/11] path: add tests for sorting optimization --- graph/path/pathtest/pathtest.go | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/graph/path/pathtest/pathtest.go b/graph/path/pathtest/pathtest.go index 4a5b99a8b..a15e7cfc8 100644 --- a/graph/path/pathtest/pathtest.go +++ b/graph/path/pathtest/pathtest.go @@ -92,6 +92,7 @@ type test struct { expect []quad.Value expectAlt [][]quad.Value tag string + unsorted bool } // Define morphisms without a QuadStore @@ -480,6 +481,13 @@ func testSet(qs graph.QuadStore) []test { tag: "target", expect: []quad.Value{vBob, vFred, vGreg}, }, + { + message: "order with a next path", + path: StartPath(qs).Order().Has(vFollows, vBob), + expect: []quad.Value{vAlice, vCharlie, vDani}, + unsorted: true, + skip: true, // TODO(dennwc): optimize Order in And properly + }, } } @@ -517,20 +525,26 @@ func RunTestMorphisms(t *testing.T, fnc testutil.DatabaseFunc) { t.Error(err) return } - sort.Sort(quad.ByValueString(got)) + if !test.unsorted { + sort.Sort(quad.ByValueString(got)) + } var eq bool exp := test.expect if test.expectAlt != nil { for _, alt := range test.expectAlt { exp = alt - sort.Sort(quad.ByValueString(exp)) + if !test.unsorted { + sort.Sort(quad.ByValueString(exp)) + } eq = reflect.DeepEqual(got, exp) if eq { break } } } else { - sort.Sort(quad.ByValueString(test.expect)) + if !test.unsorted { + sort.Sort(quad.ByValueString(test.expect)) + } eq = reflect.DeepEqual(got, test.expect) } if !eq {