Add named path symbols to new symbols of a match

Summary:
Previously, named path symbols remained untracked as `new_symbols` during planning. This meant that
operator `Optional` would be left unaware of those symbols, and therefore not reset them to `Null`
if optional matching failed.

Test Optional operator will be aware of path symbols

Reviewers: florijan, mislav.bradac

Reviewed By: mislav.bradac

Subscribers: pullbot

Differential Revision: https://phabricator.memgraph.io/D974

src/query/plan/rule_based_planner.hpp

@@ -441,6 +441,13 @@ class RuleBasedPlanner {
         last_op = impl::GenFilters(last_op, bound_symbols, filters, storage);
       }
     }
+    DCHECK(named_paths.empty()) << "Expected to generate all named paths";
+    // We bound all named path symbols, so just add them to new_symbols.
+    for (const auto &named_path : matching.named_paths) {
+      DCHECK(utils::Contains(bound_symbols, named_path.first))
+          << "Expected generated named path to have bound symbol";
+      match_context.new_symbols.emplace_back(named_path.first);
+    }
     DCHECK(filters.empty()) << "Expected to generate all filters";
     return last_op;
   }

tests/unit/query_planner.cpp

@@ -2,6 +2,7 @@
 #include <tuple>
 #include <unordered_set>
 
+#include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
 #include "query/frontend/ast/ast.hpp"
@@ -219,12 +220,21 @@ class ExpectOptional : public OpChecker<Optional> {
   ExpectOptional(const std::list<BaseOpChecker *> &optional)
       : optional_(optional) {}
 
+  ExpectOptional(const std::vector<Symbol> &optional_symbols,
+                 const std::list<BaseOpChecker *> &optional)
+      : optional_symbols_(optional_symbols), optional_(optional) {}
+
   void ExpectOp(Optional &optional, const SymbolTable &symbol_table) override {
+    if (!optional_symbols_.empty()) {
+      EXPECT_THAT(optional.optional_symbols(),
+                  testing::UnorderedElementsAreArray(optional_symbols_));
+    }
     PlanChecker check_optional(optional_, symbol_table);
     optional.optional()->Accept(check_optional);
   }
 
  private:
+  std::vector<Symbol> optional_symbols_;
   const std::list<BaseOpChecker *> &optional_;
 };
 
@@ -452,6 +462,31 @@ TEST(TestLogicalPlanner, MatchNamedPatternWithPredicateReturn) {
             ExpectConstructNamedPath(), ExpectFilter(), ExpectProduce());
 }
 
+TEST(TestLogicalPlanner, OptionalMatchNamedPatternReturn) {
+  // Test OPTIONAL MATCH p = (n) -[r]- (m) RETURN p
+  GraphDb db;
+  GraphDbAccessor dba(db);
+  AstTreeStorage storage;
+  auto node_n = NODE("n");
+  auto edge = EDGE("r");
+  auto node_m = NODE("m");
+  auto pattern = NAMED_PATTERN("p", node_n, edge, node_m);
+  QUERY(OPTIONAL_MATCH(pattern), RETURN("p"));
+  auto symbol_table = MakeSymbolTable(*storage.query());
+  auto planning_context = MakePlanningContext(storage, symbol_table, dba);
+  auto plan = MakeLogicalPlan<RuleBasedPlanner>(planning_context);
+  std::list<BaseOpChecker *> optional{new ExpectScanAll(), new ExpectExpand(),
+                                      new ExpectConstructNamedPath()};
+  auto get_symbol = [&symbol_table](const auto *ast_node) {
+    return symbol_table.at(*ast_node->identifier_);
+  };
+  std::vector<Symbol> optional_symbols{get_symbol(pattern), get_symbol(node_n),
+                                       get_symbol(edge), get_symbol(node_m)};
+  CheckPlan(*plan, symbol_table, ExpectOptional(optional_symbols, optional),
+            ExpectProduce());
+}
+
+
 TEST(TestLogicalPlanner, MatchWhereReturn) {
   // Test MATCH (n) WHERE n.property < 42 RETURN n
   AstTreeStorage storage;