Fix accessing a variable bound to a list within BFS function (#1380)

src/query/frame_change.hpp

@@ -43,22 +43,40 @@ struct CachedValue {
     return cache_.get_allocator().GetMemoryResource();
   }
 
-  bool CacheValue(const TypedValue &maybe_list) {
+  // Func to check if cache_ contains value
+  bool CacheValue(TypedValue &&maybe_list) {
     if (!maybe_list.IsList()) {
       return false;
     }
-    const auto &list = maybe_list.ValueList();
+    auto &list = maybe_list.ValueList();
     TypedValue::Hash hash{};
-    for (const TypedValue &list_elem : list) {
+    for (auto &element : list) {
-      const auto list_elem_key = hash(list_elem);
+      const auto key = hash(element);
-      auto &vector_values = cache_[list_elem_key];
+      auto &vector_values = cache_[key];
-      if (!IsValueInVec(vector_values, list_elem)) {
+      if (!IsValueInVec(vector_values, element)) {
-        vector_values.push_back(list_elem);
+        vector_values.emplace_back(std::move(element));
       }
     }
     return true;
   }
 
+  bool CacheValue(const TypedValue &maybe_list) {
+    if (!maybe_list.IsList()) {
+      return false;
+    }
+    auto &list = maybe_list.ValueList();
+    TypedValue::Hash hash{};
+    for (auto &element : list) {
+      const auto key = hash(element);
+      auto &vector_values = cache_[key];
+      if (!IsValueInVec(vector_values, element)) {
+        vector_values.emplace_back(element);
+      }
+    }
+    return true;
+  }
+
+  // Func to check if cache_ contains value
   bool ContainsValue(const TypedValue &value) const {
     TypedValue::Hash hash{};
     const auto key = hash(value);

src/query/interpret/eval.hpp

@@ -270,36 +270,33 @@ class ExpressionEvaluator : public ExpressionVisitor<TypedValue> {
   }
 
   TypedValue Visit(InListOperator &in_list) override {
-    TypedValue *_list_ptr = nullptr;
-    TypedValue _list;
     auto literal = in_list.expression1_->Accept(*this);
 
-    auto get_list_literal = [this, &in_list, &_list, &_list_ptr]() -> void {
+    auto get_list_literal = [this, &in_list]() -> TypedValue {
       ReferenceExpressionEvaluator reference_expression_evaluator{frame_, symbol_table_, ctx_};
-      _list_ptr = in_list.expression2_->Accept(reference_expression_evaluator);
+      auto *list_ptr = in_list.expression2_->Accept(reference_expression_evaluator);
-      if (nullptr == _list_ptr) {
+      if (nullptr == list_ptr) {
-        _list = in_list.expression2_->Accept(*this);
+        return in_list.expression2_->Accept(*this);
-        _list_ptr = &_list;
       }
+      return *list_ptr;
     };
 
-    auto do_list_literal_checks = [this, &literal, &_list_ptr]() -> std::optional<TypedValue> {
+    auto do_list_literal_checks = [this, &literal](const TypedValue &list) -> std::optional<TypedValue> {
-      MG_ASSERT(_list_ptr, "List literal should have been defined");
+      if (list.IsNull()) {
-      if (_list_ptr->IsNull()) {
         return TypedValue(ctx_->memory);
       }
       // Exceptions have higher priority than returning nulls when list expression
       // is not null.
-      if (_list_ptr->type() != TypedValue::Type::List) {
+      if (list.type() != TypedValue::Type::List) {
-        throw QueryRuntimeException("IN expected a list, got {}.", _list_ptr->type());
+        throw QueryRuntimeException("IN expected a list, got {}.", list.type());
       }
-      const auto &list = _list_ptr->ValueList();
+      const auto &list_value = list.ValueList();
 
       // If literal is NULL there is no need to try to compare it with every
       // element in the list since result of every comparison will be NULL. There
       // is one special case that we must test explicitly: if list is empty then
       // result is false since no comparison will be performed.
-      if (list.empty()) return TypedValue(false, ctx_->memory);
+      if (list_value.empty()) return TypedValue(false, ctx_->memory);
       if (literal.IsNull()) return TypedValue(ctx_->memory);
       return {};
     };
@@ -312,13 +309,13 @@ class ExpressionEvaluator : public ExpressionVisitor<TypedValue> {
       if (!frame_change_collector_->IsKeyValueCached(*cached_id)) {
         // Check only first time if everything is okay, later when we use
         // cache there is no need to check again as we did check first time
-        get_list_literal();
+        auto list = get_list_literal();
-        auto preoperational_checks = do_list_literal_checks();
+        auto preoperational_checks = do_list_literal_checks(list);
         if (preoperational_checks) {
           return std::move(*preoperational_checks);
         }
         auto &cached_value = frame_change_collector_->GetCachedValue(*cached_id);
-        cached_value.CacheValue(*_list_ptr);
+        cached_value.CacheValue(std::move(list));
         spdlog::trace("Value cached {}", *cached_id);
       }
       const auto &cached_value = frame_change_collector_->GetCachedValue(*cached_id);
@@ -334,16 +331,16 @@ class ExpressionEvaluator : public ExpressionVisitor<TypedValue> {
     }
     // When caching is not an option, we need to evaluate list literal every time
     // and do the checks
-    get_list_literal();
+    const auto list = get_list_literal();
-    auto preoperational_checks = do_list_literal_checks();
+    auto preoperational_checks = do_list_literal_checks(list);
     if (preoperational_checks) {
       return std::move(*preoperational_checks);
     }
 
-    const auto &list = _list.ValueList();
+    const auto &list_value = list.ValueList();
     spdlog::trace("Not using cache on IN LIST operator");
     auto has_null = false;
-    for (const auto &element : list) {
+    for (const auto &element : list_value) {
       auto result = literal == element;
       if (result.IsNull()) {
         has_null = true;

tests/gql_behave/tests/memgraph_V1/features/memgraph_bfs.feature

@@ -103,3 +103,21 @@ Feature: Bfs
       Then the result should be:
           | s | r0 |
           | 1 | 1  |
+
+  Scenario: Test accessing a variable bound to a list within BFS function
+      Given an empty graph
+      And having executed:
+          """
+          CREATE (:Node {id: 0})-[:LINK {date: '2023-03'}]->(:Node {id: 1}),
+          (:Node {id: 1})-[:LINK {date: '2023-04'}]->(:Node {id: 2}),
+          (:Node {id: 2})-[:LINK {date: '2023-03'}]->(:Node {id: 3});
+          """
+      When executing query:
+          """
+          WITH ['2023-03'] AS date
+          MATCH p = (:Node)-[*BFS ..2 (r, n | r.date IN date)]->(:Node {id: 3})
+          RETURN p
+          """
+      Then the result should be:
+         | p                                                              |
+         | <(:Node {id: 2})-[:LINK {date: '2023-03'}]->(:Node {id: 3})>   |