From df9d45db2f30ccbbf4004b1ba82d9d82d2d63346 Mon Sep 17 00:00:00 2001 From: Anurag Date: Tue, 9 Jun 2020 15:12:31 +0000 Subject: [PATCH] release/v20.03 K shortest paths queries fix and documentation update (#5548) * K-Shortest path query fix (#5410) * Bug fix for depth parameter * Handle cycles in K - Shortest path queries (cherry picked from commit 6332254bdbeb8d2f80cd12cc41da1fdbd60cebd1) * Edit test cases to remove flaky behavior (#5464) (cherry picked from commit bae2b1bb2e52bdf4189871831f3cc44266079980) * Added details of Shortest path queries in documentation (#5533) (cherry picked from commit abb57c027fccb7efc241f9faa8aaacf953535384) --- query/common_test.go | 4 +- query/query3_test.go | 421 +++++++++++++++++++++++++-- query/shortest.go | 21 +- wiki/content/query-language/index.md | 9 +- 4 files changed, 414 insertions(+), 41 deletions(-) diff --git a/query/common_test.go b/query/common_test.go index a3892a5c9ed..c5507795944 100644 --- a/query/common_test.go +++ b/query/common_test.go @@ -686,7 +686,7 @@ func populateCluster() { <510> <511> . <510> <512> . - <51> <52> (weight=10) . + <51> <52> (weight=11) . <51> <53> (weight=1) . <51> <54> (weight=10) . @@ -699,7 +699,7 @@ func populateCluster() { <52> <54> (weight=10) . <54> <51> (weight=10) . - <54> <52> (weight=1) . + <54> <52> (weight=2) . <54> <53> (weight=10) . <54> <55> (weight=1) . diff --git a/query/query3_test.go b/query/query3_test.go index 6c223b857d2..cea13941107 100644 --- a/query/query3_test.go +++ b/query/query3_test.go @@ -23,6 +23,7 @@ import ( "strings" "testing" + "github.com/dgraph-io/dgraph/testutil" "github.com/stretchr/testify/require" "google.golang.org/grpc/metadata" ) @@ -561,6 +562,298 @@ func TestKShortestPathWeighted1MinMaxWeight(t *testing.T) { `, js) } +func TestKShortestPathDepth(t *testing.T) { + // Shortest path between 1 and 1000 is the path 1 => 31 => 1001 => 1000 + // but if the depth is less than 3 then there is no direct path between + // 1 and 1000. Also if depth >=5 there is another path + // 1 => 31 => 1001 => 1003 => 1002 => 1000 + query := ` + query test ($depth: int, $numpaths: int) { + path as shortest(from: 1, to: 1000, depth: $depth, numpaths: $numpaths) { + follow + } + me(func: uid(path)) { + name + } + }` + + emptyPath := `{"data": {"me":[]}}` + + onePath := `{ + "data": { + "me": [ + {"name": "Michonne"}, + {"name": "Andrea"}, + {"name": "Bob"}, + {"name": "Alice"} + ], + "_path_": [ + { + "follow": { + "follow": { + "follow": { + "uid": "0x3e8" + }, + "uid": "0x3e9" + }, + "uid": "0x1f" + }, + "uid": "0x1", + "_weight_": 3 + } + ] + } + }` + twoPaths := `{ + "data": { + "me": [ + {"name": "Michonne"}, + {"name": "Andrea"}, + {"name": "Bob"}, + {"name": "Alice"} + ], + "_path_": [ + { + "follow": { + "follow": { + "follow": { + "uid": "0x3e8" + }, + "uid": "0x3e9" + }, + "uid": "0x1f" + }, + "uid": "0x1", + "_weight_": 3 + }, + { + "follow": { + "follow": { + "follow": { + "follow": { + "follow": { + "uid": "0x3e8" + }, + "uid": "0x3ea" + }, + "uid": "0x3eb" + }, + "uid": "0x3e9" + }, + "uid": "0x1f" + }, + "uid": "0x1", + "_weight_": 5 + } + ] + } + }` + tests := []struct { + depth, numpaths, output string + }{ + { + "2", + "4", + emptyPath, + }, + { + "3", + "4", + onePath, + }, + { + "4", + "4", + onePath, + }, + { + "5", + "4", + twoPaths, + }, + { + "6", + "4", + twoPaths, + }, + } + + t.Parallel() + for _, tc := range tests { + t.Run(fmt.Sprintf("depth_%s_numpaths_%s", tc.depth, tc.numpaths), func(t *testing.T) { + js, err := processQueryWithVars(t, query, map[string]string{"$depth": tc.depth, + "$numpaths": tc.numpaths}) + require.NoError(t, err) + require.JSONEq(t, tc.output, js) + }) + } +} + +func TestKShortestPathTwoPaths(t *testing.T) { + query := ` + { + A as shortest(from: 51, to:55, numpaths: 2, depth:2) { + connects @facets(weight) + } + me(func: uid(A)) { + name + } + }` + js := processQueryNoErr(t, query) + require.JSONEq(t, `{ + "data": { + "me": [ + {"name": "A"}, + {"name": "C"}, + {"name": "D"}, + {"name": "E"} + ], + "_path_": [ + { + "connects": { + "connects": { + "connects": { + "uid": "0x37" + }, + "connects|weight": 1, + "uid": "0x36" + }, + "connects|weight": 1, + "uid": "0x35" + }, + "connects|weight": 1, + "uid": "0x33", + "_weight_": 3 + }, + { + "connects": { + "connects": { + "uid": "0x37" + }, + "connects|weight": 1, + "uid": "0x36" + }, + "connects|weight": 10, + "uid": "0x33", + "_weight_": 11 + } + ] + } + }`, js) +} + +// There are 5 paths between 51 to 55 under "connects" predicate. +// This tests checks if the algorithm finds only 5 paths and doesn't add +// cyclical paths when forced to search for 6 or more paths. +func TestKShortestPathAllPaths(t *testing.T) { + for _, q := range []string{ + `{A as shortest(from: 51, to:55, numpaths: 5) {connects @facets(weight)} + me(func: uid(A)) {name}}`, + `{A as shortest(from: 51, to:55, numpaths: 6) {connects @facets(weight)} + me(func: uid(A)) {name}}`, + `{A as shortest(from: 51, to:55, numpaths: 10) {connects @facets(weight)} + me(func: uid(A)) {name}}`, + } { + js := processQueryNoErr(t, q) + expected := `{ + "data": { + "me": [ + {"name": "A"}, + {"name": "C"}, + {"name": "D"}, + {"name": "E"} + ], + "_path_": [ + { + "connects": { + "connects": { + "connects": { + "uid": "0x37" + }, + "connects|weight": 1, + "uid": "0x36" + }, + "connects|weight": 1, + "uid": "0x35" + }, + "connects|weight": 1, + "uid": "0x33", + "_weight_": 3 + }, + { + "connects": { + "connects": { + "uid": "0x37" + }, + "connects|weight": 1, + "uid": "0x36" + }, + "connects|weight": 10, + "uid": "0x33", + "_weight_": 11 + }, + { + "connects": { + "connects": { + "connects": { + "connects": { + "uid": "0x37" + }, + "connects|weight": 1, + "uid": "0x36" + }, + "connects|weight": 10, + "uid": "0x34" + }, + "connects|weight": 10, + "uid": "0x35" + }, + "connects|weight": 1, + "uid": "0x33", + "_weight_": 22 + }, + { + "connects": { + "connects": { + "connects": { + "uid": "0x37" + }, + "connects|weight": 1, + "uid": "0x36" + }, + "connects|weight": 10, + "uid": "0x34" + }, + "connects|weight": 11, + "uid": "0x33", + "_weight_": 22 + }, + { + "connects": { + "connects": { + "connects": { + "connects": { + "uid": "0x37" + }, + "connects|weight": 1, + "uid": "0x36" + }, + "connects|weight": 1, + "uid": "0x35" + }, + "connects|weight": 10, + "uid": "0x34" + }, + "connects|weight": 11, + "uid": "0x33", + "_weight_": 23 + } + ] + } + }` + testutil.CompareJSON(t, expected, js) + } +} func TestTwoShortestPath(t *testing.T) { query := ` @@ -746,7 +1039,6 @@ func TestShortestPathWithUidVariableNoMatchForFrom(t *testing.T) { require.JSONEq(t, `{"data":{}}`, js) } -// TODO - Later also extend this to k-shortest path. func TestShortestPathWithDepth(t *testing.T) { // Shortest path between A and B is the path A => C => D => B but if the depth is less than 3 // then the direct path between A and B should be returned. @@ -783,9 +1075,9 @@ func TestShortestPathWithDepth(t *testing.T) { "connects": { "uid": "0x34" }, - "connects|weight": 10, + "connects|weight": 11, "uid": "0x33", - "_weight_": 10 + "_weight_": 11 } ] } @@ -819,7 +1111,7 @@ func TestShortestPathWithDepth(t *testing.T) { "connects": { "uid": "0x34" }, - "connects|weight": 1, + "connects|weight": 2, "uid": "0x36" }, "connects|weight": 1, @@ -827,7 +1119,7 @@ func TestShortestPathWithDepth(t *testing.T) { }, "connects|weight": 1, "uid": "0x33", - "_weight_": 3 + "_weight_": 4 } ] } @@ -835,6 +1127,83 @@ func TestShortestPathWithDepth(t *testing.T) { emptyPath := `{"data":{"path":[]}}` + allPaths := `{ + "data": { + "path": [ + {"uid": "0x33","name": "A"}, + {"uid": "0x35","name": "C"}, + {"uid": "0x36","name": "D"}, + {"uid": "0x34","name": "B"} + ], + "_path_": [ + { + "connects": { + "connects": { + "connects": { + "uid": "0x34" + }, + "connects|weight": 2, + "uid": "0x36" + }, + "connects|weight": 1, + "uid": "0x35" + }, + "connects|weight": 1, + "uid": "0x33", + "_weight_": 4 + }, + { + "connects": { + "connects": { + "uid": "0x34" + }, + "connects|weight": 10, + "uid": "0x35" + }, + "connects|weight": 1, + "uid": "0x33", + "_weight_": 11 + }, + { + "connects": { + "uid": "0x34" + }, + "connects|weight": 11, + "uid": "0x33", + "_weight_": 11 + }, + { + "connects": { + "connects": { + "uid": "0x34" + }, + "connects|weight": 2, + "uid": "0x36" + }, + "connects|weight": 10, + "uid": "0x33", + "_weight_": 12 + }, + { + "connects": { + "connects": { + "connects": { + "uid": "0x34" + }, + "connects|weight": 10, + "uid": "0x35" + }, + "connects|weight": 10, + "uid": "0x36" + }, + "connects|weight": 10, + "uid": "0x33", + "_weight_": 30 + } + ] + } + }` + tests := []struct { depth, numpaths, output string }{ @@ -863,33 +1232,27 @@ func TestShortestPathWithDepth(t *testing.T) { "1", shortestPath, }, + //The test cases below are for k-shortest path queries with varying depths. { "0", "10", emptyPath, }, - // The test cases below are for k-shortest path queries with varying depths. They don't pass - // right now and hence are commented out... - // { - // "1", - // "10", - // directPath, - // }, - // { - // "2", - // "10", - // directPath, - // }, - // { - // "3", - // "10", - // shortestPath, - // }, - // { - // "10", - // "10", - // shortestPath, - // }, + { + "1", + "10", + directPath, + }, + { + "2", + "10", + allPaths, + }, + { + "10", + "10", + allPaths, + }, } t.Parallel() @@ -939,9 +1302,9 @@ func TestShortestPathWithDepth_direct_path_is_shortest(t *testing.T) { "connects": { "uid": "0x34" }, - "connects|weight": 1, + "connects|weight": 2, "uid": "0x36", - "_weight_": 1 + "_weight_": 2 } ] } diff --git a/query/shortest.go b/query/shortest.go index d05c3dab6f0..35505af5e35 100644 --- a/query/shortest.go +++ b/query/shortest.go @@ -63,6 +63,15 @@ var errFacet = errors.Errorf("Skip the edge") type priorityQueue []*queueItem +func (r *route) indexOf(uid uint64) int { + for i, val := range *r.route { + if val.uid == uid { + return i + } + } + return -1 +} + func (h priorityQueue) Len() int { return len(h) } func (h priorityQueue) Less(i, j int) bool { return h[i].cost < h[j].cost } @@ -303,7 +312,7 @@ func runKShortestPaths(ctx context.Context, sg *SubGraph) ([]*SubGraph, error) { } heap.Push(&pq, srcNode) - numHops := -1 + numHops := 0 maxHops := math.MaxInt32 if sg.Params.ExploreDepth != nil { maxHops = int(*sg.Params.ExploreDepth) @@ -340,7 +349,7 @@ func runKShortestPaths(ctx context.Context, sg *SubGraph) ([]*SubGraph, error) { break } } - if item.hop > numHops && numHops < maxHops { + if item.hop > numHops-1 && numHops < maxHops { // Explore the next level by calling processGraph and add them // to the queue. if !stopExpansion { @@ -364,9 +373,6 @@ func runKShortestPaths(ctx context.Context, sg *SubGraph) ([]*SubGraph, error) { case <-ctx.Done(): return nil, ctx.Err() default: - if stopExpansion { - continue - } } neighbours := adjacencyMap[item.uid] for toUid, info := range neighbours { @@ -375,7 +381,10 @@ func runKShortestPaths(ctx context.Context, sg *SubGraph) ([]*SubGraph, error) { if item.cost+cost > maxWeight { continue } - + // Skip neighbour if it present in current path to remove cyclical paths + if len(*item.path.route) > 0 && item.path.indexOf(toUid) != -1 { + continue + } curPath := pathPool.Get().(*[]pathInfo) if curPath == nil { return nil, errors.Errorf("Sync pool returned a nil pointer") diff --git a/wiki/content/query-language/index.md b/wiki/content/query-language/index.md index 51b8c5bc276..8cd8c0c0de2 100644 --- a/wiki/content/query-language/index.md +++ b/wiki/content/query-language/index.md @@ -3087,11 +3087,11 @@ Calculating the average ratings of users requires a variable that maps users to } {{}} -## K-Shortest Path Queries +## Shortest Path Queries The shortest path between a source (`from`) node and destination (`to`) node can be found using the keyword `shortest` for the query block name. It requires the source node UID, destination node UID and the predicates (at least one) that have to be considered for traversal. A `shortest` query block returns the shortest path under `_path_` in the query response. The path can also be stored in a variable which is used in other query blocks. -By default the shortest path is returned. With `numpaths: k`, the k-shortest paths are returned. With `depth: n`, the shortest paths up to `n` hops away are returned. +**K-Shortest Path queries:** By default the shortest path is returned. With `numpaths: k`, and `k > 1`, the k-shortest paths are returned. Cyclical paths are pruned out from the result of k-shortest path query. With `depth: n`, the paths up to `n` depth away are returned. {{% notice "note" %}} - If no predicates are specified in the `shortest` block, no path can be fetched as no edge is traversed. @@ -3277,8 +3277,9 @@ Some points to keep in mind for shortest path queries: - Weights must be non-negative. Dijkstra's algorithm is used to calculate the shortest paths. - Only one facet per predicate in the shortest query block is allowed. -- Only one `shortest` path block is allowed per query. Only one `_path_` is returned in the result. -- For k-shortest paths (when `numpaths` > 1), the result of the shortest path query variable will only return a single path. All k paths are returned in `_path_`. +- Only one `shortest` path block is allowed per query. Only one `_path_` is returned in the result. For queries with `numpaths` > 1, `_path_` contains all the paths. +- Cyclical paths are not included in the result of k-shortest path query. +- For k-shortest paths (when `numpaths` > 1), the result of the shortest path query variable will only return a single path which will be the shortest path among the k paths. All k paths are returned in `_path_`. ## Recurse Query