Fix bug on all shortest paths with an upper bound (#737)

src/query/plan/operator.cpp

@@ -1,4 +1,4 @@
-// Copyright 2022 Memgraph Ltd.
+// Copyright 2023 Memgraph Ltd.
 //
 // Use of this software is governed by the Business Source License
 // included in the file licenses/BSL.txt; by using this file, you agree to be bound by the terms of the Business Source
@@ -2020,7 +2020,7 @@ class ExpandAllShortestPathsCursor : public query::plan::Cursor {
         next_edges_.clear();
         traversal_stack_.clear();
 
-        pq_.push({TypedValue(), 0, *start_vertex, std::nullopt});
+        expand_from_vertex(*start_vertex, TypedValue(), 0);
         visited_cost_.emplace(*start_vertex, 0);
         frame[self_.common_.edge_symbol] = TypedValue::TVector(memory);
       }
@@ -2029,33 +2029,28 @@ class ExpandAllShortestPathsCursor : public query::plan::Cursor {
       while (!pq_.empty()) {
         if (MustAbort(context)) throw HintedAbortError();
 
-        auto [current_weight, current_depth, current_vertex, maybe_directed_edge] = pq_.top();
+        const auto [current_weight, current_depth, current_vertex, directed_edge] = pq_.top();
         pq_.pop();
 
+        const auto &[current_edge, direction, weight] = directed_edge;
+        if (expanded_.contains(current_edge)) continue;
+        expanded_.emplace(current_edge);
+
         // Expand only if what we've just expanded is less than max depth.
         if (current_depth < upper_bound_) {
-          if (maybe_directed_edge) {
-            auto &[current_edge, direction, weight] = *maybe_directed_edge;
-            if (expanded_.find(current_edge) != expanded_.end()) continue;
-            expanded_.emplace(current_edge);
-          }
           expand_from_vertex(current_vertex, current_weight, current_depth);
         }
 
-        // if current vertex is not starting vertex, maybe_directed_edge will not be nullopt
+        // Searching for a previous vertex in the expansion
-        if (maybe_directed_edge) {
+        auto prev_vertex = direction == EdgeAtom::Direction::IN ? current_edge.To() : current_edge.From();
-          auto &[current_edge, direction, weight] = *maybe_directed_edge;
-          // Searching for a previous vertex in the expansion
-          auto prev_vertex = direction == EdgeAtom::Direction::IN ? current_edge.To() : current_edge.From();
 
-          // Update the parent
+        // Update the parent
-          if (next_edges_.find({prev_vertex, current_depth - 1}) == next_edges_.end()) {
+        if (next_edges_.find({prev_vertex, current_depth - 1}) == next_edges_.end()) {
-            utils::pmr::list<DirectedEdge> empty(memory);
+          utils::pmr::list<DirectedEdge> empty(memory);
-            next_edges_[{prev_vertex, current_depth - 1}] = std::move(empty);
+          next_edges_[{prev_vertex, current_depth - 1}] = std::move(empty);
-          }
-
-          next_edges_.at({prev_vertex, current_depth - 1}).emplace_back(*maybe_directed_edge);
         }
+
+        next_edges_.at({prev_vertex, current_depth - 1}).emplace_back(directed_edge);
       }
 
       if (start_vertex && next_edges_.find({*start_vertex, 0}) != next_edges_.end()) {
@@ -2112,8 +2107,8 @@ class ExpandAllShortestPathsCursor : public query::plan::Cursor {
   // Priority queue comparator. Keep lowest weight on top of the queue.
   class PriorityQueueComparator {
    public:
-    bool operator()(const std::tuple<TypedValue, int64_t, VertexAccessor, std::optional<DirectedEdge>> &lhs,
+    bool operator()(const std::tuple<TypedValue, int64_t, VertexAccessor, DirectedEdge> &lhs,
-                    const std::tuple<TypedValue, int64_t, VertexAccessor, std::optional<DirectedEdge>> &rhs) {
+                    const std::tuple<TypedValue, int64_t, VertexAccessor, DirectedEdge> &rhs) {
       const auto &lhs_weight = std::get<0>(lhs);
       const auto &rhs_weight = std::get<0>(rhs);
       // Null defines minimum value for all types
@@ -2132,8 +2127,8 @@ class ExpandAllShortestPathsCursor : public query::plan::Cursor {
 
   // Priority queue - core element of the algorithm.
   // Stores: {weight, depth, next vertex, edge and direction}
-  std::priority_queue<std::tuple<TypedValue, int64_t, VertexAccessor, std::optional<DirectedEdge>>,
+  std::priority_queue<std::tuple<TypedValue, int64_t, VertexAccessor, DirectedEdge>,
-                      utils::pmr::vector<std::tuple<TypedValue, int64_t, VertexAccessor, std::optional<DirectedEdge>>>,
+                      utils::pmr::vector<std::tuple<TypedValue, int64_t, VertexAccessor, DirectedEdge>>,
                       PriorityQueueComparator>
       pq_;
 

tests/gql_behave/tests/memgraph_V1/features/memgraph_allshortest.feature

@@ -15,6 +15,26 @@ Feature: All Shortest Path
           | '1' |
           | '3' |
 
+  Scenario: Test match allShortest upper bound 2
+      Given an empty graph
+      And having executed:
+          """
+          CREATE (a {a:'0'})-[:r {w: 2}]->(b {a:'1'})-[:r {w: 3}]->(c {a:'2'}),
+            (a)-[:re {w: 2}]->(b),
+            (b)-[:re {w:3}]->(c),
+            ({a: '4'})<-[:r {w: 1}]-(a),
+            ({a: '5'})<-[:r {w: 1}]-(a),
+            (c)-[:r {w: 1}]->({a: '6'}),
+            (c)-[:r {w: 1}]->({a: '7'})
+          """
+      When executing query:
+          """
+          MATCH path=(n {a:'0'})-[r *allShortest ..2 (e, n | 1 ) w]->(m {a:'2'}) RETURN COUNT(path) AS c
+          """
+      Then the result should be:
+          | c |
+          | 4 |
+
   Scenario: Test match allShortest filtered
       Given an empty graph
       And having executed: