diff --git a/src/query/frame_change.hpp b/src/query/frame_change.hpp index b522e5920..535d5ddb9 100644 --- a/src/query/frame_change.hpp +++ b/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) { - const auto list_elem_key = hash(list_elem); - auto &vector_values = cache_[list_elem_key]; - if (!IsValueInVec(vector_values, list_elem)) { - vector_values.push_back(list_elem); + for (auto &element : list) { + const auto key = hash(element); + auto &vector_values = cache_[key]; + if (!IsValueInVec(vector_values, element)) { + 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); diff --git a/src/query/interpret/eval.hpp b/src/query/interpret/eval.hpp index d4c2c8bc0..f9ebf5467 100644 --- a/src/query/interpret/eval.hpp +++ b/src/query/interpret/eval.hpp @@ -270,36 +270,33 @@ class ExpressionEvaluator : public ExpressionVisitor { } 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); - if (nullptr == _list_ptr) { - _list = in_list.expression2_->Accept(*this); - _list_ptr = &_list; + auto *list_ptr = in_list.expression2_->Accept(reference_expression_evaluator); + if (nullptr == list_ptr) { + return in_list.expression2_->Accept(*this); } + return *list_ptr; }; - auto do_list_literal_checks = [this, &literal, &_list_ptr]() -> std::optional { - MG_ASSERT(_list_ptr, "List literal should have been defined"); - if (_list_ptr->IsNull()) { + auto do_list_literal_checks = [this, &literal](const TypedValue &list) -> std::optional { + if (list.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) { - throw QueryRuntimeException("IN expected a list, got {}.", _list_ptr->type()); + if (list.type() != TypedValue::Type::List) { + 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 { 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 preoperational_checks = do_list_literal_checks(); + auto list = get_list_literal(); + 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 { } // When caching is not an option, we need to evaluate list literal every time // and do the checks - get_list_literal(); - auto preoperational_checks = do_list_literal_checks(); + const auto list = get_list_literal(); + 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; diff --git a/tests/gql_behave/tests/memgraph_V1/features/memgraph_bfs.feature b/tests/gql_behave/tests/memgraph_V1/features/memgraph_bfs.feature index 5634efd7f..d47566012 100644 --- a/tests/gql_behave/tests/memgraph_V1/features/memgraph_bfs.feature +++ b/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})> |