From 89f42ef73eeca486f97137db125fccd8b23e228a Mon Sep 17 00:00:00 2001 From: Kostas Kyrimis Date: Wed, 7 Dec 2022 18:33:47 +0200 Subject: [PATCH 01/22] Add CreateExpand PullMultiple and prototype mocks for testing --- src/query/v2/multiframe.cpp | 2 +- src/query/v2/multiframe.hpp | 2 +- src/query/v2/plan/operator.cpp | 74 ++++++++++++++++ tests/unit/CMakeLists.txt | 4 + tests/unit/mock_helpers.hpp | 63 +++++++++++++ .../query_v2_create_expand_multiframe.cpp | 88 +++++++++++++++++++ 6 files changed, 231 insertions(+), 2 deletions(-) create mode 100644 tests/unit/mock_helpers.hpp create mode 100644 tests/unit/query_v2_create_expand_multiframe.cpp diff --git a/src/query/v2/multiframe.cpp b/src/query/v2/multiframe.cpp index 4829addb2..88262e514 100644 --- a/src/query/v2/multiframe.cpp +++ b/src/query/v2/multiframe.cpp @@ -24,7 +24,7 @@ static_assert(std::forward_iterator); static_assert(std::forward_iterator); static_assert(std::forward_iterator); -MultiFrame::MultiFrame(int64_t size_of_frame, size_t number_of_frames, utils::MemoryResource *execution_memory) +MultiFrame::MultiFrame(size_t size_of_frame, size_t number_of_frames, utils::MemoryResource *execution_memory) : frames_(utils::pmr::vector( number_of_frames, FrameWithValidity(size_of_frame, execution_memory), execution_memory)) { MG_ASSERT(number_of_frames > 0); diff --git a/src/query/v2/multiframe.hpp b/src/query/v2/multiframe.hpp index 0b6896422..37e6fea11 100644 --- a/src/query/v2/multiframe.hpp +++ b/src/query/v2/multiframe.hpp @@ -30,7 +30,7 @@ class MultiFrame { friend class ValidFramesReader; friend class InvalidFramesPopulator; - MultiFrame(int64_t size_of_frame, size_t number_of_frames, utils::MemoryResource *execution_memory); + MultiFrame(size_t size_of_frame, size_t number_of_frames, utils::MemoryResource *execution_memory); ~MultiFrame() = default; MultiFrame(const MultiFrame &other); diff --git a/src/query/v2/plan/operator.cpp b/src/query/v2/plan/operator.cpp index eeb5cd6b4..c909cc466 100644 --- a/src/query/v2/plan/operator.cpp +++ b/src/query/v2/plan/operator.cpp @@ -2376,6 +2376,22 @@ class DistributedCreateExpandCursor : public Cursor { return true; } + void PullMultiple(MultiFrame &multi_frame, ExecutionContext &context) override { + SCOPED_PROFILE_OP("CreateExpandMF"); + input_cursor_->PullMultiple(multi_frame, context); + auto request_vertices = ExpandCreationInfoToRequests(multi_frame, context); + { + SCOPED_REQUEST_WAIT_PROFILE; + auto &request_router = context.request_router; + auto results = request_router->CreateExpand(std::move(request_vertices)); + for (const auto &result : results) { + if (result.error) { + throw std::runtime_error("CreateExpand Request failed"); + } + } + } + } + void Shutdown() override { input_cursor_->Shutdown(); } void Reset() override { @@ -2450,6 +2466,64 @@ class DistributedCreateExpandCursor : public Cursor { return edge_requests; } + std::vector ExpandCreationInfoToRequests(MultiFrame &multi_frame, ExecutionContext &context) const { + std::vector edge_requests; + auto reader = multi_frame.GetValidFramesConsumer(); + + for (auto &frame : reader) { + const auto &edge_info = self_.edge_info_; + msgs::NewExpand request{.id = {context.edge_ids_alloc->AllocateId()}}; + ExpressionEvaluator evaluator(&frame, context.symbol_table, context.evaluation_context, nullptr, + storage::v3::View::NEW); + request.type = {edge_info.edge_type}; + if (const auto *edge_info_properties = std::get_if(&edge_info.properties)) { + for (const auto &[property, value_expression] : *edge_info_properties) { + TypedValue val = value_expression->Accept(evaluator); + request.properties.emplace_back(property, storage::v3::TypedValueToValue(val)); + } + } else { + // handle parameter + auto property_map = evaluator.Visit(*std::get(edge_info.properties)).ValueMap(); + for (const auto &[property, value] : property_map) { + const auto property_id = context.request_router->NameToProperty(std::string(property)); + request.properties.emplace_back(property_id, storage::v3::TypedValueToValue(value)); + } + } + // src, dest + TypedValue &v1_value = frame[self_.input_symbol_]; + const auto &v1 = v1_value.ValueVertex(); + const auto &v2 = OtherVertex(frame); + + // Set src and dest vertices + // TODO(jbajic) Currently we are only handling scenario where vertices + // are matched + const auto set_vertex = [](const auto &vertex, auto &vertex_id) { + vertex_id.first = vertex.PrimaryLabel(); + vertex_id.second = vertex.GetVertex().id.second; + }; + + std::invoke([&]() { + switch (edge_info.direction) { + case EdgeAtom::Direction::IN: { + set_vertex(v2, request.src_vertex); + set_vertex(v1, request.dest_vertex); + break; + } + case EdgeAtom::Direction::OUT: { + set_vertex(v1, request.src_vertex); + set_vertex(v2, request.dest_vertex); + break; + } + case EdgeAtom::Direction::BOTH: + LOG_FATAL("Must indicate exact expansion direction here"); + } + }); + + edge_requests.push_back(std::move(request)); + } + return edge_requests; + } + private: void ResetExecutionState() {} diff --git a/tests/unit/CMakeLists.txt b/tests/unit/CMakeLists.txt index 188c6c1b0..b1c5c9c6f 100644 --- a/tests/unit/CMakeLists.txt +++ b/tests/unit/CMakeLists.txt @@ -410,3 +410,7 @@ target_link_libraries(${test_prefix}high_density_shard_create_scan mg-io mg-coor # Tests for awesome_memgraph_functions add_unit_test(query_v2_expression_evaluator.cpp) target_link_libraries(${test_prefix}query_v2_expression_evaluator mg-query-v2) + +# Tests for multiframes +add_unit_test(query_v2_create_expand_multiframe.cpp) +target_link_libraries(${test_prefix}query_v2_create_expand_multiframe mg-query-v2) diff --git a/tests/unit/mock_helpers.hpp b/tests/unit/mock_helpers.hpp new file mode 100644 index 000000000..5201aa210 --- /dev/null +++ b/tests/unit/mock_helpers.hpp @@ -0,0 +1,63 @@ +// Copyright 2022 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 +// License, and you may not use this file except in compliance with the Business Source License. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +#pragma once + +#include +#include +#include "query/v2/plan/operator.hpp" +#include "query/v2/request_router.hpp" + +namespace memgraph { +class MockedRequestRouter : public query::v2::RequestRouterInterface { + public: + MOCK_METHOD1(ScanVertices, std::vector(std::optional label)); + MOCK_METHOD1(CreateVertices, std::vector(std::vector)); + MOCK_METHOD1(ExpandOne, std::vector(msgs::ExpandOneRequest)); + MOCK_METHOD1(CreateExpand, std::vector(std::vector)); + MOCK_METHOD1(GetProperties, std::vector(msgs::GetPropertiesRequest)); + MOCK_METHOD0(StartTransaction, void()); + MOCK_METHOD0(Commit, void()); + + MOCK_CONST_METHOD1(NameToEdgeType, storage::v3::EdgeTypeId(const std::string &)); + MOCK_CONST_METHOD1(NameToProperty, storage::v3::PropertyId(const std::string &)); + MOCK_CONST_METHOD1(NameToLabel, storage::v3::LabelId(const std::string &)); + MOCK_CONST_METHOD1(LabelToName, storage::v3::LabelId(const std::string &)); + MOCK_CONST_METHOD1(PropertyToName, const std::string &(storage::v3::PropertyId)); + MOCK_CONST_METHOD1(LabelToName, const std::string &(storage::v3::LabelId label)); + MOCK_CONST_METHOD1(EdgeTypeToName, const std::string &(storage::v3::EdgeTypeId type)); + MOCK_CONST_METHOD1(MaybeNameToProperty, std::optional(const std::string &)); + MOCK_CONST_METHOD1(MaybeNameToEdgeType, std::optional(const std::string &)); + MOCK_CONST_METHOD1(MaybeNameToLabel, std::optional(const std::string &)); + MOCK_CONST_METHOD1(IsPrimaryLabel, bool(storage::v3::LabelId)); + MOCK_CONST_METHOD2(IsPrimaryKey, bool(storage::v3::LabelId, storage::v3::PropertyId)); +}; + +class MockedLogicalOperator : query::v2::plan::LogicalOperator { + public: + MOCK_CONST_METHOD1(MakeCursor, query::v2::plan::UniqueCursorPtr(utils::MemoryResource *)); + MOCK_CONST_METHOD1(OutputSymbols, std::vector(const expr::SymbolTable &)); + MOCK_CONST_METHOD1(ModifiedSymbols, std::vector(const expr::SymbolTable &)); + MOCK_CONST_METHOD0(HasSingleInput, bool()); + MOCK_CONST_METHOD0(input, std::shared_ptr()); + MOCK_METHOD1(set_input, void(std::shared_ptr)); + MOCK_CONST_METHOD1(Clone, std::unique_ptr(query::v2::AstStorage *storage)); +}; + +class MockedCursor : memgraph::query::v2::plan::Cursor { + public: + MOCK_METHOD2(Pull, bool(query::v2::Frame &, expr::ExecutionContext &)); + MOCK_METHOD2(PullMultiple, void(query::v2::MultiFrame &, expr::ExecutionContext &)); + MOCK_METHOD0(Reset, void()); + MOCK_METHOD0(Shutdown, void()); +}; + +} // namespace memgraph diff --git a/tests/unit/query_v2_create_expand_multiframe.cpp b/tests/unit/query_v2_create_expand_multiframe.cpp new file mode 100644 index 000000000..f6f6567db --- /dev/null +++ b/tests/unit/query_v2_create_expand_multiframe.cpp @@ -0,0 +1,88 @@ +// Copyright 2022 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 +// License, and you may not use this file except in compliance with the Business Source License. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +#include "mock_helpers.hpp" + +#include "query/v2/bindings/frame.hpp" +#include "query/v2/bindings/symbol_table.hpp" +#include "query/v2/common.hpp" +#include "query/v2/context.hpp" +#include "query/v2/plan/operator.hpp" +#include "query/v2/requests.hpp" +#include "storage/v3/property_value.hpp" +#include "storage/v3/shard.hpp" +#include "utils/logging.hpp" +#include "utils/memory.hpp" + +using namespace memgraph::query::v2; +using namespace memgraph::query::v2::plan; +namespace memgraph { +class TestTemplate : public testing::Test { + protected: + void SetUp() override {} +}; + +ExecutionContext MakeContext(const AstStorage &storage, const SymbolTable &symbol_table, RequestRouterInterface *router, + IdAllocator *id_alloc) { + ExecutionContext context; + context.symbol_table = symbol_table; + context.evaluation_context.properties = NamesToProperties(storage.properties_, router); + context.evaluation_context.labels = NamesToLabels(storage.labels_, router); + context.edge_ids_alloc = id_alloc; + context.request_router = router; + return context; +} + +MultiFrame CreateMultiFrame(const size_t max_pos, const Symbol &src, const Symbol &dst, MockedRequestRouter *router) { + static constexpr size_t frame_size = 100; + MultiFrame multi_frame(max_pos, frame_size, utils::NewDeleteResource()); + auto frames_populator = multi_frame.GetInvalidFramesPopulator(); + size_t i = 0; + for (auto &frame : frames_populator) { + frame.MakeValid(); + auto &src_acc = frame.at(src); + auto &dst_acc = frame.at(dst); + auto v1 = msgs::Vertex{.id = {{msgs::LabelId::FromUint(1)}, {msgs::Value(static_cast(i++))}}}; + auto v2 = msgs::Vertex{.id = {{msgs::LabelId::FromUint(1)}, {msgs::Value(static_cast(i++))}}}; + std::map mp; + src_acc = TypedValue(query::v2::accessors::VertexAccessor(v1, mp, router)); + dst_acc = TypedValue(query::v2::accessors::VertexAccessor(v2, mp, router)); + } + + return multi_frame; +} + +TEST_F(TestTemplate, CreateExpand) { + MockedRequestRouter router; + + AstStorage ast; + SymbolTable symbol_table; + + query::v2::plan::NodeCreationInfo node; + query::v2::plan::EdgeCreationInfo edge; + edge.edge_type = msgs::EdgeTypeId::FromUint(1); + edge.direction = EdgeAtom::Direction::IN; + auto id_alloc = IdAllocator(0, 100); + + const auto &src = symbol_table.CreateSymbol("n", true); + node.symbol = symbol_table.CreateSymbol("u", true); + + auto create_expand = query::v2::plan::CreateExpand(node, edge, nullptr, src, true); + auto cursor = create_expand.MakeCursor(utils::NewDeleteResource()); + + EXPECT_CALL(router, CreateExpand(testing::_)) + .Times(1) + .WillOnce(::testing::Return(std::vector{})); + auto context = MakeContext(ast, symbol_table, &router, &id_alloc); + auto multi_frame = CreateMultiFrame(context.symbol_table.max_position(), src, node.symbol, &router); + cursor->PullMultiple(multi_frame, context); +} +} // namespace memgraph From 4ed20f0247bc3510eaeca16d97df80423bbca7ba Mon Sep 17 00:00:00 2001 From: Kostas Kyrimis Date: Thu, 8 Dec 2022 18:46:30 +0200 Subject: [PATCH 02/22] Add prototype for CreateNode multiframe --- src/query/v2/plan/operator.cpp | 70 +++++++++++++++ tests/unit/CMakeLists.txt | 3 + .../unit/query_v2_create_node_multiframe.cpp | 87 +++++++++++++++++++ 3 files changed, 160 insertions(+) create mode 100644 tests/unit/query_v2_create_node_multiframe.cpp diff --git a/src/query/v2/plan/operator.cpp b/src/query/v2/plan/operator.cpp index c909cc466..da0913376 100644 --- a/src/query/v2/plan/operator.cpp +++ b/src/query/v2/plan/operator.cpp @@ -189,6 +189,17 @@ class DistributedCreateNodeCursor : public Cursor { return false; } + void PullMultiple(MultiFrame &multi_frame, ExecutionContext &context) override { + SCOPED_PROFILE_OP("CreateNodeMF"); + input_cursor_->PullMultiple(multi_frame, context); + auto &request_router = context.request_router; + { + SCOPED_REQUEST_WAIT_PROFILE; + request_router->CreateVertices(NodeCreationInfoToRequests(context, multi_frame)); + } + PlaceNodesOnTheMultiFrame(multi_frame, context); + } + void Shutdown() override { input_cursor_->Shutdown(); } void Reset() override {} @@ -247,6 +258,65 @@ class DistributedCreateNodeCursor : public Cursor { return requests; } + void PlaceNodesOnTheMultiFrame(MultiFrame &multi_frame, ExecutionContext &context) { + auto multi_frame_reader = multi_frame.GetValidFramesConsumer(); + size_t i = 0; + MG_ASSERT(std::distance(multi_frame_reader.begin(), multi_frame_reader.end())); + for (auto &frame : multi_frame_reader) { + const auto primary_label = msgs::Label{.id = nodes_info_[0]->labels[0]}; + msgs::Vertex v{.id = std::make_pair(primary_label, primary_keys_[i])}; + frame[nodes_info_.front()->symbol] = TypedValue( + query::v2::accessors::VertexAccessor(std::move(v), src_vertex_props_[i++], context.request_router)); + } + } + + std::vector NodeCreationInfoToRequests(ExecutionContext &context, MultiFrame &multi_frame) { + std::vector requests; + auto multi_frame_reader = multi_frame.GetValidFramesConsumer(); + for (auto &frame : multi_frame_reader) { + msgs::PrimaryKey pk; + for (const auto &node_info : nodes_info_) { + msgs::NewVertex rqst; + MG_ASSERT(!node_info->labels.empty(), "Cannot determine primary label"); + const auto primary_label = node_info->labels[0]; + // TODO(jbajic) Fix properties not send, + // suggestion: ignore distinction between properties and primary keys + // since schema validation is done on storage side + ExpressionEvaluator evaluator(&frame, context.symbol_table, context.evaluation_context, nullptr, + storage::v3::View::NEW); + if (const auto *node_info_properties = std::get_if(&node_info->properties)) { + for (const auto &[key, value_expression] : *node_info_properties) { + TypedValue val = value_expression->Accept(evaluator); + if (context.request_router->IsPrimaryKey(primary_label, key)) { + rqst.primary_key.push_back(TypedValueToValue(val)); + pk.push_back(TypedValueToValue(val)); + } + } + } else { + auto property_map = evaluator.Visit(*std::get(node_info->properties)).ValueMap(); + for (const auto &[key, value] : property_map) { + auto key_str = std::string(key); + auto property_id = context.request_router->NameToProperty(key_str); + if (context.request_router->IsPrimaryKey(primary_label, property_id)) { + rqst.primary_key.push_back(TypedValueToValue(value)); + pk.push_back(TypedValueToValue(value)); + } + } + } + + if (node_info->labels.empty()) { + throw QueryRuntimeException("Primary label must be defined!"); + } + // TODO(kostasrim) Copy non primary labels as well + rqst.label_ids.push_back(msgs::Label{.id = primary_label}); + src_vertex_props_.push_back(rqst.properties); + requests.push_back(std::move(rqst)); + } + primary_keys_.push_back(std::move(pk)); + } + return requests; + } + private: const UniqueCursorPtr input_cursor_; std::vector nodes_info_; diff --git a/tests/unit/CMakeLists.txt b/tests/unit/CMakeLists.txt index b1c5c9c6f..731c9ea1e 100644 --- a/tests/unit/CMakeLists.txt +++ b/tests/unit/CMakeLists.txt @@ -414,3 +414,6 @@ target_link_libraries(${test_prefix}query_v2_expression_evaluator mg-query-v2) # Tests for multiframes add_unit_test(query_v2_create_expand_multiframe.cpp) target_link_libraries(${test_prefix}query_v2_create_expand_multiframe mg-query-v2) + +add_unit_test(query_v2_create_node_multiframe.cpp) +target_link_libraries(${test_prefix}query_v2_create_node_multiframe mg-query-v2) diff --git a/tests/unit/query_v2_create_node_multiframe.cpp b/tests/unit/query_v2_create_node_multiframe.cpp new file mode 100644 index 000000000..b4a6bdbaf --- /dev/null +++ b/tests/unit/query_v2_create_node_multiframe.cpp @@ -0,0 +1,87 @@ +// Copyright 2022 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 +// License, and you may not use this file except in compliance with the Business Source License. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +#include "mock_helpers.hpp" + +#include "query/v2/bindings/frame.hpp" +#include "query/v2/bindings/symbol_table.hpp" +#include "query/v2/common.hpp" +#include "query/v2/context.hpp" +#include "query/v2/frontend/ast/ast.hpp" +#include "query/v2/plan/operator.hpp" +#include "query/v2/requests.hpp" +#include "storage/v3/property_value.hpp" +#include "storage/v3/shard.hpp" +#include "utils/logging.hpp" +#include "utils/memory.hpp" + +using namespace memgraph::query::v2; +using namespace memgraph::query::v2::plan; +namespace memgraph { +class TestTemplate : public testing::Test { + protected: + void SetUp() override {} +}; + +ExecutionContext MakeContext(const AstStorage &storage, const SymbolTable &symbol_table, RequestRouterInterface *router, + IdAllocator *id_alloc) { + ExecutionContext context; + context.symbol_table = symbol_table; + context.evaluation_context.properties = NamesToProperties(storage.properties_, router); + context.evaluation_context.labels = NamesToLabels(storage.labels_, router); + context.edge_ids_alloc = id_alloc; + context.request_router = router; + return context; +} + +MultiFrame CreateMultiFrame(const size_t max_pos) { + static constexpr size_t frame_size = 100; + MultiFrame multi_frame(max_pos, frame_size, utils::NewDeleteResource()); + auto frames_populator = multi_frame.GetInvalidFramesPopulator(); + for (auto &frame : frames_populator) { + frame.MakeValid(); + } + + return multi_frame; +} + +TEST_F(TestTemplate, CreateNode) { + MockedRequestRouter router; + + AstStorage ast; + SymbolTable symbol_table; + + query::v2::plan::NodeCreationInfo node; + query::v2::plan::EdgeCreationInfo edge; + edge.edge_type = msgs::EdgeTypeId::FromUint(1); + edge.direction = EdgeAtom::Direction::IN; + auto id_alloc = IdAllocator(0, 100); + + node.symbol = symbol_table.CreateSymbol("n", true); + node.labels.push_back(msgs::LabelId::FromUint(2)); + auto literal = query::v2::PrimitiveLiteral(); + literal.value_ = TypedValue(static_cast(200)); + auto p = query::v2::plan::PropertiesMapList{}; + p.push_back(std::make_pair(msgs::PropertyId::FromUint(2), &literal)); + node.properties.emplace<0>(std::move(p)); + + auto create_expand = query::v2::plan::CreateNode(nullptr, node); + auto cursor = create_expand.MakeCursor(utils::NewDeleteResource()); + + EXPECT_CALL(router, CreateVertices(testing::_)) + .Times(1) + .WillOnce(::testing::Return(std::vector{})); + EXPECT_CALL(router, IsPrimaryKey(testing::_, testing::_)).WillRepeatedly(::testing::Return(true)); + auto context = MakeContext(ast, symbol_table, &router, &id_alloc); + auto multi_frame = CreateMultiFrame(context.symbol_table.max_position()); + cursor->PullMultiple(multi_frame, context); +} +} // namespace memgraph From 2e4e975102ecedb8f73a49b09c3fd336b1f1cd7c Mon Sep 17 00:00:00 2001 From: Kostas Kyrimis Date: Mon, 12 Dec 2022 19:15:28 +0200 Subject: [PATCH 03/22] Update GoogleTest lib to latest release version 1.12.1 --- libs/setup.sh | 2 +- tests/unit/utils_settings.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/libs/setup.sh b/libs/setup.sh index bb737d432..55288f535 100755 --- a/libs/setup.sh +++ b/libs/setup.sh @@ -171,7 +171,7 @@ benchmark_tag="v1.6.0" repo_clone_try_double "${primary_urls[gbenchmark]}" "${secondary_urls[gbenchmark]}" "benchmark" "$benchmark_tag" true # google test -googletest_tag="release-1.8.0" +googletest_tag="release-1.12.1" repo_clone_try_double "${primary_urls[gtest]}" "${secondary_urls[gtest]}" "googletest" "$googletest_tag" true # libbcrypt diff --git a/tests/unit/utils_settings.cpp b/tests/unit/utils_settings.cpp index 388e467ed..20262e1f5 100644 --- a/tests/unit/utils_settings.cpp +++ b/tests/unit/utils_settings.cpp @@ -11,7 +11,7 @@ #include -#include +#include #include #include "utils/settings.hpp" From f04ed3c1372d15d1b0d10e4f1cd3f2c756317a9c Mon Sep 17 00:00:00 2001 From: Kostas Kyrimis Date: Mon, 12 Dec 2022 19:15:49 +0200 Subject: [PATCH 04/22] Simplify Mocks and test --- src/query/v2/plan/operator.cpp | 2 +- tests/unit/mock_helpers.hpp | 89 +++++++++++-------- .../query_v2_create_expand_multiframe.cpp | 45 ++++------ 3 files changed, 73 insertions(+), 63 deletions(-) diff --git a/src/query/v2/plan/operator.cpp b/src/query/v2/plan/operator.cpp index c909cc466..012981e44 100644 --- a/src/query/v2/plan/operator.cpp +++ b/src/query/v2/plan/operator.cpp @@ -2468,7 +2468,7 @@ class DistributedCreateExpandCursor : public Cursor { std::vector ExpandCreationInfoToRequests(MultiFrame &multi_frame, ExecutionContext &context) const { std::vector edge_requests; - auto reader = multi_frame.GetValidFramesConsumer(); + auto reader = multi_frame.GetValidFramesModifier(); for (auto &frame : reader) { const auto &edge_info = self_.edge_info_; diff --git a/tests/unit/mock_helpers.hpp b/tests/unit/mock_helpers.hpp index 5201aa210..0010d5986 100644 --- a/tests/unit/mock_helpers.hpp +++ b/tests/unit/mock_helpers.hpp @@ -13,51 +13,70 @@ #include #include +#include "query/v2/common.hpp" +#include "query/v2/context.hpp" #include "query/v2/plan/operator.hpp" #include "query/v2/request_router.hpp" -namespace memgraph { -class MockedRequestRouter : public query::v2::RequestRouterInterface { +namespace memgraph::query::v2 { +class MockedRequestRouter : public RequestRouterInterface { public: - MOCK_METHOD1(ScanVertices, std::vector(std::optional label)); - MOCK_METHOD1(CreateVertices, std::vector(std::vector)); - MOCK_METHOD1(ExpandOne, std::vector(msgs::ExpandOneRequest)); - MOCK_METHOD1(CreateExpand, std::vector(std::vector)); - MOCK_METHOD1(GetProperties, std::vector(msgs::GetPropertiesRequest)); - MOCK_METHOD0(StartTransaction, void()); - MOCK_METHOD0(Commit, void()); + MOCK_METHOD(std::vector, ScanVertices, (std::optional label)); + MOCK_METHOD(std::vector, CreateVertices, (std::vector)); + MOCK_METHOD(std::vector, ExpandOne, (msgs::ExpandOneRequest)); + MOCK_METHOD(std::vector, CreateExpand, (std::vector)); + MOCK_METHOD(std::vector, GetProperties, (msgs::GetPropertiesRequest)); + MOCK_METHOD(void, StartTransaction, ()); + MOCK_METHOD(void, Commit, ()); - MOCK_CONST_METHOD1(NameToEdgeType, storage::v3::EdgeTypeId(const std::string &)); - MOCK_CONST_METHOD1(NameToProperty, storage::v3::PropertyId(const std::string &)); - MOCK_CONST_METHOD1(NameToLabel, storage::v3::LabelId(const std::string &)); - MOCK_CONST_METHOD1(LabelToName, storage::v3::LabelId(const std::string &)); - MOCK_CONST_METHOD1(PropertyToName, const std::string &(storage::v3::PropertyId)); - MOCK_CONST_METHOD1(LabelToName, const std::string &(storage::v3::LabelId label)); - MOCK_CONST_METHOD1(EdgeTypeToName, const std::string &(storage::v3::EdgeTypeId type)); - MOCK_CONST_METHOD1(MaybeNameToProperty, std::optional(const std::string &)); - MOCK_CONST_METHOD1(MaybeNameToEdgeType, std::optional(const std::string &)); - MOCK_CONST_METHOD1(MaybeNameToLabel, std::optional(const std::string &)); - MOCK_CONST_METHOD1(IsPrimaryLabel, bool(storage::v3::LabelId)); - MOCK_CONST_METHOD2(IsPrimaryKey, bool(storage::v3::LabelId, storage::v3::PropertyId)); + MOCK_METHOD(storage::v3::EdgeTypeId, NameToEdgeType, (const std::string &), (const)); + MOCK_METHOD(storage::v3::PropertyId, NameToProperty, (const std::string &), (const)); + MOCK_METHOD(storage::v3::LabelId, NameToLabel, (const std::string &), (const)); + MOCK_METHOD(storage::v3::LabelId, LabelToName, (const std::string &), (const)); + MOCK_METHOD(const std::string &, PropertyToName, (storage::v3::PropertyId), (const)); + MOCK_METHOD(const std::string &, LabelToName, (storage::v3::LabelId label), (const)); + MOCK_METHOD(const std::string &, EdgeTypeToName, (storage::v3::EdgeTypeId type), (const)); + MOCK_METHOD(std::optional, MaybeNameToProperty, (const std::string &), (const)); + MOCK_METHOD(std::optional, MaybeNameToEdgeType, (const std::string &), (const)); + MOCK_METHOD(std::optional, MaybeNameToLabel, (const std::string &), (const)); + MOCK_METHOD(bool, IsPrimaryLabel, (storage::v3::LabelId), (const)); + MOCK_METHOD(bool, IsPrimaryKey, (storage::v3::LabelId, storage::v3::PropertyId), (const)); }; -class MockedLogicalOperator : query::v2::plan::LogicalOperator { +class MockedLogicalOperator : public plan::LogicalOperator { public: - MOCK_CONST_METHOD1(MakeCursor, query::v2::plan::UniqueCursorPtr(utils::MemoryResource *)); - MOCK_CONST_METHOD1(OutputSymbols, std::vector(const expr::SymbolTable &)); - MOCK_CONST_METHOD1(ModifiedSymbols, std::vector(const expr::SymbolTable &)); - MOCK_CONST_METHOD0(HasSingleInput, bool()); - MOCK_CONST_METHOD0(input, std::shared_ptr()); - MOCK_METHOD1(set_input, void(std::shared_ptr)); - MOCK_CONST_METHOD1(Clone, std::unique_ptr(query::v2::AstStorage *storage)); + MOCK_METHOD(plan::UniqueCursorPtr, MakeCursor, (utils::MemoryResource *), (const)); + MOCK_METHOD(std::vector, ModifiedSymbols, (const expr::SymbolTable &), (const)); + MOCK_METHOD(bool, HasSingleInput, (), (const)); + MOCK_METHOD(std::shared_ptr, input, (), (const)); + MOCK_METHOD(void, set_input, (std::shared_ptr)); + MOCK_METHOD(std::unique_ptr, Clone, (AstStorage * storage), (const)); + MOCK_METHOD(bool, Accept, (plan::HierarchicalLogicalOperatorVisitor & visitor)); }; -class MockedCursor : memgraph::query::v2::plan::Cursor { +class MockedCursor : public plan::Cursor { public: - MOCK_METHOD2(Pull, bool(query::v2::Frame &, expr::ExecutionContext &)); - MOCK_METHOD2(PullMultiple, void(query::v2::MultiFrame &, expr::ExecutionContext &)); - MOCK_METHOD0(Reset, void()); - MOCK_METHOD0(Shutdown, void()); + MOCK_METHOD(bool, Pull, (Frame &, expr::ExecutionContext &)); + MOCK_METHOD(void, PullMultiple, (MultiFrame &, expr::ExecutionContext &)); + MOCK_METHOD(void, Reset, ()); + MOCK_METHOD(void, Shutdown, ()); }; -} // namespace memgraph +inline expr::ExecutionContext MakeContext(const expr::AstStorage &storage, const expr::SymbolTable &symbol_table, + RequestRouterInterface *router, IdAllocator *id_alloc) { + expr::ExecutionContext context; + context.symbol_table = symbol_table; + context.evaluation_context.properties = NamesToProperties(storage.properties_, router); + context.evaluation_context.labels = NamesToLabels(storage.labels_, router); + context.edge_ids_alloc = id_alloc; + context.request_router = router; + return context; +} + +inline MockedLogicalOperator &BaseToMock(plan::LogicalOperator *op) { + return *static_cast(op); +} + +inline MockedCursor &BaseToMock(plan::Cursor *cursor) { return *static_cast(cursor); } + +} // namespace memgraph::query::v2 diff --git a/tests/unit/query_v2_create_expand_multiframe.cpp b/tests/unit/query_v2_create_expand_multiframe.cpp index f6f6567db..5b4c1e2ec 100644 --- a/tests/unit/query_v2_create_expand_multiframe.cpp +++ b/tests/unit/query_v2_create_expand_multiframe.cpp @@ -9,6 +9,7 @@ // by the Apache License, Version 2.0, included in the file // licenses/APL.txt. +#include #include "mock_helpers.hpp" #include "query/v2/bindings/frame.hpp" @@ -22,24 +23,7 @@ #include "utils/logging.hpp" #include "utils/memory.hpp" -using namespace memgraph::query::v2; -using namespace memgraph::query::v2::plan; -namespace memgraph { -class TestTemplate : public testing::Test { - protected: - void SetUp() override {} -}; - -ExecutionContext MakeContext(const AstStorage &storage, const SymbolTable &symbol_table, RequestRouterInterface *router, - IdAllocator *id_alloc) { - ExecutionContext context; - context.symbol_table = symbol_table; - context.evaluation_context.properties = NamesToProperties(storage.properties_, router); - context.evaluation_context.labels = NamesToLabels(storage.labels_, router); - context.edge_ids_alloc = id_alloc; - context.request_router = router; - return context; -} +namespace memgraph::query::v2 { MultiFrame CreateMultiFrame(const size_t max_pos, const Symbol &src, const Symbol &dst, MockedRequestRouter *router) { static constexpr size_t frame_size = 100; @@ -60,14 +44,15 @@ MultiFrame CreateMultiFrame(const size_t max_pos, const Symbol &src, const Symbo return multi_frame; } -TEST_F(TestTemplate, CreateExpand) { - MockedRequestRouter router; +TEST(CreateExpandTest, Cursor) { + using testing::_; + using testing::Return; AstStorage ast; SymbolTable symbol_table; - query::v2::plan::NodeCreationInfo node; - query::v2::plan::EdgeCreationInfo edge; + plan::NodeCreationInfo node; + plan::EdgeCreationInfo edge; edge.edge_type = msgs::EdgeTypeId::FromUint(1); edge.direction = EdgeAtom::Direction::IN; auto id_alloc = IdAllocator(0, 100); @@ -75,14 +60,20 @@ TEST_F(TestTemplate, CreateExpand) { const auto &src = symbol_table.CreateSymbol("n", true); node.symbol = symbol_table.CreateSymbol("u", true); - auto create_expand = query::v2::plan::CreateExpand(node, edge, nullptr, src, true); + auto once_cur = plan::MakeUniqueCursorPtr(utils::NewDeleteResource()); + EXPECT_CALL(BaseToMock(once_cur.get()), PullMultiple(_, _)).Times(1); + + std::shared_ptr once_op = std::make_shared(); + EXPECT_CALL(BaseToMock(once_op.get()), MakeCursor(_)).Times(1).WillOnce(Return(std::move(once_cur))); + + auto create_expand = plan::CreateExpand(node, edge, once_op, src, true); auto cursor = create_expand.MakeCursor(utils::NewDeleteResource()); - EXPECT_CALL(router, CreateExpand(testing::_)) - .Times(1) - .WillOnce(::testing::Return(std::vector{})); + MockedRequestRouter router; + EXPECT_CALL(router, CreateExpand(_)).Times(1).WillOnce(Return(std::vector{})); auto context = MakeContext(ast, symbol_table, &router, &id_alloc); auto multi_frame = CreateMultiFrame(context.symbol_table.max_position(), src, node.symbol, &router); cursor->PullMultiple(multi_frame, context); } -} // namespace memgraph + +} // namespace memgraph::query::v2 From 04450dada7f7bc1c301678044d12edb4925161f4 Mon Sep 17 00:00:00 2001 From: Kostas Kyrimis Date: Mon, 12 Dec 2022 19:23:40 +0200 Subject: [PATCH 05/22] Simplify tests --- .../unit/query_v2_create_node_multiframe.cpp | 46 +++++++------------ 1 file changed, 17 insertions(+), 29 deletions(-) diff --git a/tests/unit/query_v2_create_node_multiframe.cpp b/tests/unit/query_v2_create_node_multiframe.cpp index b4a6bdbaf..f19082783 100644 --- a/tests/unit/query_v2_create_node_multiframe.cpp +++ b/tests/unit/query_v2_create_node_multiframe.cpp @@ -20,28 +20,9 @@ #include "query/v2/requests.hpp" #include "storage/v3/property_value.hpp" #include "storage/v3/shard.hpp" -#include "utils/logging.hpp" #include "utils/memory.hpp" -using namespace memgraph::query::v2; -using namespace memgraph::query::v2::plan; -namespace memgraph { -class TestTemplate : public testing::Test { - protected: - void SetUp() override {} -}; - -ExecutionContext MakeContext(const AstStorage &storage, const SymbolTable &symbol_table, RequestRouterInterface *router, - IdAllocator *id_alloc) { - ExecutionContext context; - context.symbol_table = symbol_table; - context.evaluation_context.properties = NamesToProperties(storage.properties_, router); - context.evaluation_context.labels = NamesToLabels(storage.labels_, router); - context.edge_ids_alloc = id_alloc; - context.request_router = router; - return context; -} - +namespace memgraph::query::v2 { MultiFrame CreateMultiFrame(const size_t max_pos) { static constexpr size_t frame_size = 100; MultiFrame multi_frame(max_pos, frame_size, utils::NewDeleteResource()); @@ -53,29 +34,36 @@ MultiFrame CreateMultiFrame(const size_t max_pos) { return multi_frame; } -TEST_F(TestTemplate, CreateNode) { - MockedRequestRouter router; +TEST(CreateNodeTest, CreateNodeCursor) { + using testing::_; + using testing::Return; AstStorage ast; SymbolTable symbol_table; - query::v2::plan::NodeCreationInfo node; - query::v2::plan::EdgeCreationInfo edge; + plan::NodeCreationInfo node; + plan::EdgeCreationInfo edge; edge.edge_type = msgs::EdgeTypeId::FromUint(1); edge.direction = EdgeAtom::Direction::IN; auto id_alloc = IdAllocator(0, 100); node.symbol = symbol_table.CreateSymbol("n", true); node.labels.push_back(msgs::LabelId::FromUint(2)); - auto literal = query::v2::PrimitiveLiteral(); + auto literal = PrimitiveLiteral(); literal.value_ = TypedValue(static_cast(200)); - auto p = query::v2::plan::PropertiesMapList{}; + auto p = plan::PropertiesMapList{}; p.push_back(std::make_pair(msgs::PropertyId::FromUint(2), &literal)); node.properties.emplace<0>(std::move(p)); - auto create_expand = query::v2::plan::CreateNode(nullptr, node); - auto cursor = create_expand.MakeCursor(utils::NewDeleteResource()); + auto once_cur = plan::MakeUniqueCursorPtr(utils::NewDeleteResource()); + EXPECT_CALL(BaseToMock(once_cur.get()), PullMultiple(_, _)).Times(1); + std::shared_ptr once_op = std::make_shared(); + EXPECT_CALL(BaseToMock(once_op.get()), MakeCursor(_)).Times(1).WillOnce(Return(std::move(once_cur))); + + auto create_expand = plan::CreateNode(once_op, node); + auto cursor = create_expand.MakeCursor(utils::NewDeleteResource()); + MockedRequestRouter router; EXPECT_CALL(router, CreateVertices(testing::_)) .Times(1) .WillOnce(::testing::Return(std::vector{})); @@ -84,4 +72,4 @@ TEST_F(TestTemplate, CreateNode) { auto multi_frame = CreateMultiFrame(context.symbol_table.max_position()); cursor->PullMultiple(multi_frame, context); } -} // namespace memgraph +} // namespace memgraph::query::v2 From a9eca651df72cd3041fae6e23bdf69e05356cd0c Mon Sep 17 00:00:00 2001 From: Kostas Kyrimis Date: Wed, 14 Dec 2022 18:26:40 +0200 Subject: [PATCH 06/22] Address GH comments and fix a bug in ValidFramesModifier postincrement --- src/query/v2/multiframe.hpp | 2 +- src/query/v2/plan/operator.cpp | 35 ++++++------------- tests/unit/mock_helpers.hpp | 6 ++-- .../query_v2_create_expand_multiframe.cpp | 16 +++++++-- 4 files changed, 28 insertions(+), 31 deletions(-) diff --git a/src/query/v2/multiframe.hpp b/src/query/v2/multiframe.hpp index e13eb07ac..7d9b73700 100644 --- a/src/query/v2/multiframe.hpp +++ b/src/query/v2/multiframe.hpp @@ -168,7 +168,7 @@ class ValidFramesModifier { Iterator &operator++() { do { ptr_++; - } while (*this != iterator_wrapper_->end() && ptr_->IsValid()); + } while (*this != iterator_wrapper_->end() && !ptr_->IsValid()); return *this; } diff --git a/src/query/v2/plan/operator.cpp b/src/query/v2/plan/operator.cpp index 012981e44..805b3ec51 100644 --- a/src/query/v2/plan/operator.cpp +++ b/src/query/v2/plan/operator.cpp @@ -2468,9 +2468,9 @@ class DistributedCreateExpandCursor : public Cursor { std::vector ExpandCreationInfoToRequests(MultiFrame &multi_frame, ExecutionContext &context) const { std::vector edge_requests; - auto reader = multi_frame.GetValidFramesModifier(); + auto frames_modifier = multi_frame.GetValidFramesModifier(); - for (auto &frame : reader) { + for (auto &frame : frames_modifier) { const auto &edge_info = self_.edge_info_; msgs::NewExpand request{.id = {context.edge_ids_alloc->AllocateId()}}; ExpressionEvaluator evaluator(&frame, context.symbol_table, context.evaluation_context, nullptr, @@ -2489,35 +2489,22 @@ class DistributedCreateExpandCursor : public Cursor { request.properties.emplace_back(property_id, storage::v3::TypedValueToValue(value)); } } - // src, dest + TypedValue &v1_value = frame[self_.input_symbol_]; const auto &v1 = v1_value.ValueVertex(); const auto &v2 = OtherVertex(frame); + msgs::Edge edge{.src = request.src_vertex, + .dst = request.dest_vertex, + .properties = request.properties, + .id = request.id, + .type = request.type}; + frame[self_.edge_info_.symbol] = TypedValue(accessors::EdgeAccessor(std::move(edge), context.request_router)); // Set src and dest vertices // TODO(jbajic) Currently we are only handling scenario where vertices // are matched - const auto set_vertex = [](const auto &vertex, auto &vertex_id) { - vertex_id.first = vertex.PrimaryLabel(); - vertex_id.second = vertex.GetVertex().id.second; - }; - - std::invoke([&]() { - switch (edge_info.direction) { - case EdgeAtom::Direction::IN: { - set_vertex(v2, request.src_vertex); - set_vertex(v1, request.dest_vertex); - break; - } - case EdgeAtom::Direction::OUT: { - set_vertex(v1, request.src_vertex); - set_vertex(v2, request.dest_vertex); - break; - } - case EdgeAtom::Direction::BOTH: - LOG_FATAL("Must indicate exact expansion direction here"); - } - }); + request.src_vertex = v1.Id(); + request.dest_vertex = v2.Id(); edge_requests.push_back(std::move(request)); } diff --git a/tests/unit/mock_helpers.hpp b/tests/unit/mock_helpers.hpp index 0010d5986..6c03889ba 100644 --- a/tests/unit/mock_helpers.hpp +++ b/tests/unit/mock_helpers.hpp @@ -73,10 +73,10 @@ inline expr::ExecutionContext MakeContext(const expr::AstStorage &storage, const return context; } -inline MockedLogicalOperator &BaseToMock(plan::LogicalOperator *op) { - return *static_cast(op); +inline MockedLogicalOperator &BaseToMock(plan::LogicalOperator &op) { + return dynamic_cast(op); } -inline MockedCursor &BaseToMock(plan::Cursor *cursor) { return *static_cast(cursor); } +inline MockedCursor &BaseToMock(plan::Cursor &cursor) { return dynamic_cast(cursor); } } // namespace memgraph::query::v2 diff --git a/tests/unit/query_v2_create_expand_multiframe.cpp b/tests/unit/query_v2_create_expand_multiframe.cpp index 5b4c1e2ec..05b631dae 100644 --- a/tests/unit/query_v2_create_expand_multiframe.cpp +++ b/tests/unit/query_v2_create_expand_multiframe.cpp @@ -55,25 +55,35 @@ TEST(CreateExpandTest, Cursor) { plan::EdgeCreationInfo edge; edge.edge_type = msgs::EdgeTypeId::FromUint(1); edge.direction = EdgeAtom::Direction::IN; + edge.symbol = symbol_table.CreateSymbol("e", true); auto id_alloc = IdAllocator(0, 100); const auto &src = symbol_table.CreateSymbol("n", true); node.symbol = symbol_table.CreateSymbol("u", true); auto once_cur = plan::MakeUniqueCursorPtr(utils::NewDeleteResource()); - EXPECT_CALL(BaseToMock(once_cur.get()), PullMultiple(_, _)).Times(1); + EXPECT_CALL(BaseToMock(*once_cur), PullMultiple(_, _)).Times(1); std::shared_ptr once_op = std::make_shared(); - EXPECT_CALL(BaseToMock(once_op.get()), MakeCursor(_)).Times(1).WillOnce(Return(std::move(once_cur))); + EXPECT_CALL(BaseToMock(*once_op), MakeCursor(_)).Times(1).WillOnce(Return(std::move(once_cur))); auto create_expand = plan::CreateExpand(node, edge, once_op, src, true); auto cursor = create_expand.MakeCursor(utils::NewDeleteResource()); MockedRequestRouter router; - EXPECT_CALL(router, CreateExpand(_)).Times(1).WillOnce(Return(std::vector{})); + EXPECT_CALL(router, CreateExpand(_)) + .Times(1) + .WillOnce(Return(std::vector{msgs::CreateExpandResponse{}})); auto context = MakeContext(ast, symbol_table, &router, &id_alloc); auto multi_frame = CreateMultiFrame(context.symbol_table.max_position(), src, node.symbol, &router); cursor->PullMultiple(multi_frame, context); + + auto frames = multi_frame.GetValidFramesReader(); + for (auto &frame : frames) { + EXPECT_EQ(frame[edge.symbol].IsEdge(), true); + const auto &e = frame[edge.symbol].ValueEdge(); + EXPECT_EQ(e.EdgeType(), edge.edge_type); + } } } // namespace memgraph::query::v2 From 7e217e94b3bde975a8ce1c9cde930ffd249e64c7 Mon Sep 17 00:00:00 2001 From: jeremy Date: Mon, 19 Dec 2022 15:44:01 +0100 Subject: [PATCH 07/22] Tests: CreateMultiFrame create invalid frames Test uses real "once" instead of mocked version --- .../query_v2_create_expand_multiframe.cpp | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/tests/unit/query_v2_create_expand_multiframe.cpp b/tests/unit/query_v2_create_expand_multiframe.cpp index 05b631dae..8720656fd 100644 --- a/tests/unit/query_v2_create_expand_multiframe.cpp +++ b/tests/unit/query_v2_create_expand_multiframe.cpp @@ -26,12 +26,11 @@ namespace memgraph::query::v2 { MultiFrame CreateMultiFrame(const size_t max_pos, const Symbol &src, const Symbol &dst, MockedRequestRouter *router) { - static constexpr size_t frame_size = 100; - MultiFrame multi_frame(max_pos, frame_size, utils::NewDeleteResource()); + static constexpr size_t number_of_frames = 100; + MultiFrame multi_frame(max_pos, number_of_frames, utils::NewDeleteResource()); auto frames_populator = multi_frame.GetInvalidFramesPopulator(); size_t i = 0; for (auto &frame : frames_populator) { - frame.MakeValid(); auto &src_acc = frame.at(src); auto &dst_acc = frame.at(dst); auto v1 = msgs::Vertex{.id = {{msgs::LabelId::FromUint(1)}, {msgs::Value(static_cast(i++))}}}; @@ -41,6 +40,8 @@ MultiFrame CreateMultiFrame(const size_t max_pos, const Symbol &src, const Symbo dst_acc = TypedValue(query::v2::accessors::VertexAccessor(v2, mp, router)); } + multi_frame.MakeAllFramesInvalid(); + return multi_frame; } @@ -61,11 +62,8 @@ TEST(CreateExpandTest, Cursor) { const auto &src = symbol_table.CreateSymbol("n", true); node.symbol = symbol_table.CreateSymbol("u", true); - auto once_cur = plan::MakeUniqueCursorPtr(utils::NewDeleteResource()); - EXPECT_CALL(BaseToMock(*once_cur), PullMultiple(_, _)).Times(1); - - std::shared_ptr once_op = std::make_shared(); - EXPECT_CALL(BaseToMock(*once_op), MakeCursor(_)).Times(1).WillOnce(Return(std::move(once_cur))); + auto once_op = std::make_shared(); + auto once_cur = once_op->MakeCursor(utils::NewDeleteResource()); auto create_expand = plan::CreateExpand(node, edge, once_op, src, true); auto cursor = create_expand.MakeCursor(utils::NewDeleteResource()); @@ -79,11 +77,18 @@ TEST(CreateExpandTest, Cursor) { cursor->PullMultiple(multi_frame, context); auto frames = multi_frame.GetValidFramesReader(); + auto number_of_valid_frames = 0; for (auto &frame : frames) { + ++number_of_valid_frames; EXPECT_EQ(frame[edge.symbol].IsEdge(), true); const auto &e = frame[edge.symbol].ValueEdge(); EXPECT_EQ(e.EdgeType(), edge.edge_type); } + EXPECT_EQ(number_of_valid_frames, 1); + + auto invalid_frames = multi_frame.GetInvalidFramesPopulator(); + auto number_of_invalid_frames = std::distance(invalid_frames.begin(), invalid_frames.end()); + EXPECT_EQ(number_of_invalid_frames, 99); } } // namespace memgraph::query::v2 From 599b133a55ba56bd9725d9462b428758bbb582f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A1nos=20Benjamin=20Antal?= Date: Thu, 12 Jan 2023 09:04:18 +0100 Subject: [PATCH 08/22] Fix edge direction when creating edges --- src/query/v2/plan/operator.cpp | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/src/query/v2/plan/operator.cpp b/src/query/v2/plan/operator.cpp index 805b3ec51..8681c6a2a 100644 --- a/src/query/v2/plan/operator.cpp +++ b/src/query/v2/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 @@ -2503,8 +2503,20 @@ class DistributedCreateExpandCursor : public Cursor { // Set src and dest vertices // TODO(jbajic) Currently we are only handling scenario where vertices // are matched - request.src_vertex = v1.Id(); - request.dest_vertex = v2.Id(); + switch (edge_info.direction) { + case EdgeAtom::Direction::IN: { + request.src_vertex = v2.Id(); + request.dest_vertex = v1.Id(); + break; + } + case EdgeAtom::Direction::OUT: { + request.src_vertex = v1.Id(); + request.dest_vertex = v2.Id(); + break; + } + case EdgeAtom::Direction::BOTH: + LOG_FATAL("Must indicate exact expansion direction here"); + } edge_requests.push_back(std::move(request)); } From d7bd2cc7549b428ab7b850115d6d1ed3301cc219 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A1nos=20Benjamin=20Antal?= Date: Thu, 12 Jan 2023 09:05:29 +0100 Subject: [PATCH 09/22] Eliminate copying expands --- src/query/v2/request_router.hpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/query/v2/request_router.hpp b/src/query/v2/request_router.hpp index 3dd2f164b..96d51b05f 100644 --- a/src/query/v2/request_router.hpp +++ b/src/query/v2/request_router.hpp @@ -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 @@ -299,7 +299,8 @@ class RequestRouter : public RequestRouterInterface { MG_ASSERT(!new_edges.empty()); // create requests - std::vector> requests_to_be_sent = RequestsForCreateExpand(new_edges); + std::vector> requests_to_be_sent = + RequestsForCreateExpand(std::move(new_edges)); // begin all requests in parallel RunningRequests running_requests = {}; @@ -430,7 +431,7 @@ class RequestRouter : public RequestRouterInterface { } std::vector> RequestsForCreateExpand( - const std::vector &new_expands) { + std::vector new_expands) { std::map per_shard_request_table; auto ensure_shard_exists_in_table = [&per_shard_request_table, transaction_id = transaction_id_](const ShardMetadata &shard) { From ace1eb401f4aab75a5b955849957dcb96f149897 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A1nos=20Benjamin=20Antal?= Date: Sun, 15 Jan 2023 18:25:32 +0100 Subject: [PATCH 10/22] Make unit tests compile with new gtest version --- tests/unit/query_v2_create_node_multiframe.cpp | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/tests/unit/query_v2_create_node_multiframe.cpp b/tests/unit/query_v2_create_node_multiframe.cpp index f19082783..39ce17b43 100644 --- a/tests/unit/query_v2_create_node_multiframe.cpp +++ b/tests/unit/query_v2_create_node_multiframe.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 @@ -55,11 +55,7 @@ TEST(CreateNodeTest, CreateNodeCursor) { p.push_back(std::make_pair(msgs::PropertyId::FromUint(2), &literal)); node.properties.emplace<0>(std::move(p)); - auto once_cur = plan::MakeUniqueCursorPtr(utils::NewDeleteResource()); - EXPECT_CALL(BaseToMock(once_cur.get()), PullMultiple(_, _)).Times(1); - - std::shared_ptr once_op = std::make_shared(); - EXPECT_CALL(BaseToMock(once_op.get()), MakeCursor(_)).Times(1).WillOnce(Return(std::move(once_cur))); + auto once_op = std::make_shared(); auto create_expand = plan::CreateNode(once_op, node); auto cursor = create_expand.MakeCursor(utils::NewDeleteResource()); From b2b9b1d5cbc06a110f34bbb250591ecb5d5d22e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A1nos=20Benjamin=20Antal?= Date: Sun, 15 Jan 2023 18:25:48 +0100 Subject: [PATCH 11/22] Eliminate warnings about deprecated methods --- tests/unit/bfs_single_node.cpp | 32 +++++++++---------- tests/unit/cypher_main_visitor.cpp | 6 ++-- ..._print_ast_to_original_expression_test.cpp | 4 +-- tests/unit/query_plan.cpp | 4 +-- .../query_v2_create_expand_multiframe.cpp | 3 +- tests/unit/query_v2_cypher_main_visitor.cpp | 6 ++-- tests/unit/storage_v3.cpp | 6 ++-- tests/unit/storage_v3_edge.cpp | 6 ++-- tests/unit/storage_v3_isolation_level.cpp | 6 ++-- tests/unit/utils_csv_parsing.cpp | 4 +-- tests/unit/utils_file_locker.cpp | 8 ++--- tests/unit/utils_memory.cpp | 4 +-- 12 files changed, 44 insertions(+), 45 deletions(-) diff --git a/tests/unit/bfs_single_node.cpp b/tests/unit/bfs_single_node.cpp index 93002eef5..eab4f3973 100644 --- a/tests/unit/bfs_single_node.cpp +++ b/tests/unit/bfs_single_node.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 @@ -88,14 +88,14 @@ TEST_P(SingleNodeBfsTest, All) { std::unique_ptr SingleNodeBfsTest::db_{nullptr}; -INSTANTIATE_TEST_CASE_P(DirectionAndExpansionDepth, SingleNodeBfsTest, - testing::Combine(testing::Range(-1, kVertexCount), testing::Range(-1, kVertexCount), - testing::Values(EdgeAtom::Direction::OUT, EdgeAtom::Direction::IN, - EdgeAtom::Direction::BOTH), - testing::Values(std::vector{}), testing::Bool(), - testing::Values(FilterLambdaType::NONE))); +INSTANTIATE_TEST_SUITE_P(DirectionAndExpansionDepth, SingleNodeBfsTest, + testing::Combine(testing::Range(-1, kVertexCount), testing::Range(-1, kVertexCount), + testing::Values(EdgeAtom::Direction::OUT, EdgeAtom::Direction::IN, + EdgeAtom::Direction::BOTH), + testing::Values(std::vector{}), testing::Bool(), + testing::Values(FilterLambdaType::NONE))); -INSTANTIATE_TEST_CASE_P( +INSTANTIATE_TEST_SUITE_P( EdgeType, SingleNodeBfsTest, testing::Combine(testing::Values(-1), testing::Values(-1), testing::Values(EdgeAtom::Direction::OUT, EdgeAtom::Direction::IN, EdgeAtom::Direction::BOTH), @@ -103,11 +103,11 @@ INSTANTIATE_TEST_CASE_P( std::vector{"b"}, std::vector{"a", "b"}), testing::Bool(), testing::Values(FilterLambdaType::NONE))); -INSTANTIATE_TEST_CASE_P(FilterLambda, SingleNodeBfsTest, - testing::Combine(testing::Values(-1), testing::Values(-1), - testing::Values(EdgeAtom::Direction::OUT, EdgeAtom::Direction::IN, - EdgeAtom::Direction::BOTH), - testing::Values(std::vector{}), testing::Bool(), - testing::Values(FilterLambdaType::NONE, FilterLambdaType::USE_FRAME, - FilterLambdaType::USE_FRAME_NULL, FilterLambdaType::USE_CTX, - FilterLambdaType::ERROR))); +INSTANTIATE_TEST_SUITE_P(FilterLambda, SingleNodeBfsTest, + testing::Combine(testing::Values(-1), testing::Values(-1), + testing::Values(EdgeAtom::Direction::OUT, EdgeAtom::Direction::IN, + EdgeAtom::Direction::BOTH), + testing::Values(std::vector{}), testing::Bool(), + testing::Values(FilterLambdaType::NONE, FilterLambdaType::USE_FRAME, + FilterLambdaType::USE_FRAME_NULL, FilterLambdaType::USE_CTX, + FilterLambdaType::ERROR))); diff --git a/tests/unit/cypher_main_visitor.cpp b/tests/unit/cypher_main_visitor.cpp index 40aeb0161..93f0653b4 100644 --- a/tests/unit/cypher_main_visitor.cpp +++ b/tests/unit/cypher_main_visitor.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 @@ -294,7 +294,7 @@ std::shared_ptr gAstGeneratorTypes[] = { std::make_shared(), }; -INSTANTIATE_TEST_CASE_P(AstGeneratorTypes, CypherMainVisitorTest, ::testing::ValuesIn(gAstGeneratorTypes)); +INSTANTIATE_TEST_SUITE_P(AstGeneratorTypes, CypherMainVisitorTest, ::testing::ValuesIn(gAstGeneratorTypes)); // NOTE: The above used to use *Typed Tests* functionality of gtest library. // Unfortunately, the compilation time of this test increased to full 2 minutes! @@ -308,7 +308,7 @@ INSTANTIATE_TEST_CASE_P(AstGeneratorTypes, CypherMainVisitorTest, ::testing::Val // ClonedAstGenerator, CachedAstGenerator> // AstGeneratorTypes; // -// TYPED_TEST_CASE(CypherMainVisitorTest, AstGeneratorTypes); +// TYPED_TEST_SUITE(CypherMainVisitorTest, AstGeneratorTypes); TEST_P(CypherMainVisitorTest, SyntaxException) { auto &ast_generator = *GetParam(); diff --git a/tests/unit/pretty_print_ast_to_original_expression_test.cpp b/tests/unit/pretty_print_ast_to_original_expression_test.cpp index e5d77ae0e..a2144c8aa 100644 --- a/tests/unit/pretty_print_ast_to_original_expression_test.cpp +++ b/tests/unit/pretty_print_ast_to_original_expression_test.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 @@ -67,7 +67,7 @@ TEST_P(ExpressiontoStringTest, Example) { EXPECT_EQ(rewritten_expression, rewritten_expression2); } -INSTANTIATE_TEST_CASE_P( +INSTANTIATE_TEST_SUITE_P( PARAMETER, ExpressiontoStringTest, ::testing::Values( std::make_pair(std::string("2 / 1"), std::string("(2 / 1)")), diff --git a/tests/unit/query_plan.cpp b/tests/unit/query_plan.cpp index 93d2f33c7..935709cec 100644 --- a/tests/unit/query_plan.cpp +++ b/tests/unit/query_plan.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 @@ -90,7 +90,7 @@ void DeleteListContent(std::list *list) { delete ptr; } } -TYPED_TEST_CASE(TestPlanner, PlannerTypes); +TYPED_TEST_SUITE(TestPlanner, PlannerTypes); TYPED_TEST(TestPlanner, MatchNodeReturn) { // Test MATCH (n) RETURN n diff --git a/tests/unit/query_v2_create_expand_multiframe.cpp b/tests/unit/query_v2_create_expand_multiframe.cpp index 8720656fd..e978a6bb5 100644 --- a/tests/unit/query_v2_create_expand_multiframe.cpp +++ b/tests/unit/query_v2_create_expand_multiframe.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 @@ -63,7 +63,6 @@ TEST(CreateExpandTest, Cursor) { node.symbol = symbol_table.CreateSymbol("u", true); auto once_op = std::make_shared(); - auto once_cur = once_op->MakeCursor(utils::NewDeleteResource()); auto create_expand = plan::CreateExpand(node, edge, once_op, src, true); auto cursor = create_expand.MakeCursor(utils::NewDeleteResource()); diff --git a/tests/unit/query_v2_cypher_main_visitor.cpp b/tests/unit/query_v2_cypher_main_visitor.cpp index d7e4169e2..4d5311a6e 100644 --- a/tests/unit/query_v2_cypher_main_visitor.cpp +++ b/tests/unit/query_v2_cypher_main_visitor.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 @@ -299,7 +299,7 @@ std::shared_ptr gAstGeneratorTypes[] = { std::make_shared(), }; -INSTANTIATE_TEST_CASE_P(AstGeneratorTypes, CypherMainVisitorTest, ::testing::ValuesIn(gAstGeneratorTypes)); +INSTANTIATE_TEST_SUITE_P(AstGeneratorTypes, CypherMainVisitorTest, ::testing::ValuesIn(gAstGeneratorTypes)); // NOTE: The above used to use *Typed Tests* functionality of gtest library. // Unfortunately, the compilation time of this test increased to full 2 minutes! @@ -313,7 +313,7 @@ INSTANTIATE_TEST_CASE_P(AstGeneratorTypes, CypherMainVisitorTest, ::testing::Val // ClonedAstGenerator, CachedAstGenerator> // AstGeneratorTypes; // -// TYPED_TEST_CASE(CypherMainVisitorTest, AstGeneratorTypes); +// TYPED_TEST_SUITE(CypherMainVisitorTest, AstGeneratorTypes); TEST_P(CypherMainVisitorTest, SyntaxException) { auto &ast_generator = *GetParam(); diff --git a/tests/unit/storage_v3.cpp b/tests/unit/storage_v3.cpp index 6f6902d1c..1832165f9 100644 --- a/tests/unit/storage_v3.cpp +++ b/tests/unit/storage_v3.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 @@ -82,8 +82,8 @@ class StorageV3 : public ::testing::TestWithParam { Config{.gc = {.reclamation_interval = reclamation_interval}}}; coordinator::Hlc last_hlc{0, io::Time{}}; }; -INSTANTIATE_TEST_CASE_P(WithGc, StorageV3, ::testing::Values(true)); -INSTANTIATE_TEST_CASE_P(WithoutGc, StorageV3, ::testing::Values(false)); +INSTANTIATE_TEST_SUITE_P(WithGc, StorageV3, ::testing::Values(true)); +INSTANTIATE_TEST_SUITE_P(WithoutGc, StorageV3, ::testing::Values(false)); // NOLINTNEXTLINE(hicpp-special-member-functions) TEST_P(StorageV3, Commit) { diff --git a/tests/unit/storage_v3_edge.cpp b/tests/unit/storage_v3_edge.cpp index 3d2ab8bbd..8637db11c 100644 --- a/tests/unit/storage_v3_edge.cpp +++ b/tests/unit/storage_v3_edge.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 @@ -60,8 +60,8 @@ class StorageEdgeTest : public ::testing::TestWithParam { coordinator::Hlc last_hlc{0, io::Time{}}; }; -INSTANTIATE_TEST_CASE_P(EdgesWithProperties, StorageEdgeTest, ::testing::Values(true)); -INSTANTIATE_TEST_CASE_P(EdgesWithoutProperties, StorageEdgeTest, ::testing::Values(false)); +INSTANTIATE_TEST_SUITE_P(EdgesWithProperties, StorageEdgeTest, ::testing::Values(true)); +INSTANTIATE_TEST_SUITE_P(EdgesWithoutProperties, StorageEdgeTest, ::testing::Values(false)); // NOLINTNEXTLINE(hicpp-special-member-functions) TEST_P(StorageEdgeTest, EdgeCreateFromSmallerCommit) { diff --git a/tests/unit/storage_v3_isolation_level.cpp b/tests/unit/storage_v3_isolation_level.cpp index 6b661f632..bd0f27b6b 100644 --- a/tests/unit/storage_v3_isolation_level.cpp +++ b/tests/unit/storage_v3_isolation_level.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 @@ -135,6 +135,6 @@ TEST_P(StorageIsolationLevelTest, Visibility) { } } -INSTANTIATE_TEST_CASE_P(ParameterizedStorageIsolationLevelTests, StorageIsolationLevelTest, - ::testing::ValuesIn(isolation_levels), StorageIsolationLevelTest::PrintToStringParamName()); +INSTANTIATE_TEST_SUITE_P(ParameterizedStorageIsolationLevelTests, StorageIsolationLevelTest, + ::testing::ValuesIn(isolation_levels), StorageIsolationLevelTest::PrintToStringParamName()); } // namespace memgraph::storage::v3::tests diff --git a/tests/unit/utils_csv_parsing.cpp b/tests/unit/utils_csv_parsing.cpp index 3c852b171..e8d6c2241 100644 --- a/tests/unit/utils_csv_parsing.cpp +++ b/tests/unit/utils_csv_parsing.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 @@ -330,4 +330,4 @@ TEST_P(CsvReaderTest, EmptyColumns) { } } -INSTANTIATE_TEST_CASE_P(NewlineParameterizedTest, CsvReaderTest, ::testing::Values("\n", "\r\n")); +INSTANTIATE_TEST_SUITE_P(NewlineParameterizedTest, CsvReaderTest, ::testing::Values("\n", "\r\n")); diff --git a/tests/unit/utils_file_locker.cpp b/tests/unit/utils_file_locker.cpp index 21c71b1a3..f2217953e 100644 --- a/tests/unit/utils_file_locker.cpp +++ b/tests/unit/utils_file_locker.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 @@ -190,9 +190,9 @@ TEST_P(FileLockerParameterizedTest, RemovePath) { std::filesystem::current_path(save_path); } -INSTANTIATE_TEST_CASE_P(FileLockerPathVariantTests, FileLockerParameterizedTest, - ::testing::Values(std::make_tuple(false, false), std::make_tuple(false, true), - std::make_tuple(true, false), std::make_tuple(true, true))); +INSTANTIATE_TEST_SUITE_P(FileLockerPathVariantTests, FileLockerParameterizedTest, + ::testing::Values(std::make_tuple(false, false), std::make_tuple(false, true), + std::make_tuple(true, false), std::make_tuple(true, true))); TEST_F(FileLockerTest, MultipleLockers) { CreateFiles(3); diff --git a/tests/unit/utils_memory.cpp b/tests/unit/utils_memory.cpp index 70bf85653..73f78d545 100644 --- a/tests/unit/utils_memory.cpp +++ b/tests/unit/utils_memory.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 @@ -393,7 +393,7 @@ class AllocatorTest : public ::testing::Test {}; using ContainersWithAllocators = ::testing::Types; -TYPED_TEST_CASE(AllocatorTest, ContainersWithAllocators); +TYPED_TEST_SUITE(AllocatorTest, ContainersWithAllocators); TYPED_TEST(AllocatorTest, PropagatesToStdUsesAllocator) { std::vector> vec(memgraph::utils::NewDeleteResource()); From b30137ab7ac7bdd5005b6f5ffc23d45131e6fa6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A1nos=20Benjamin=20Antal?= Date: Sun, 15 Jan 2023 18:39:58 +0100 Subject: [PATCH 12/22] Improve unit tests to catch bug --- src/query/v2/requests.hpp | 3 ++- tests/unit/query_v2_create_node_multiframe.cpp | 18 +++++++++++++++--- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/src/query/v2/requests.hpp b/src/query/v2/requests.hpp index 9ff9a1bae..2335fea7d 100644 --- a/src/query/v2/requests.hpp +++ b/src/query/v2/requests.hpp @@ -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 @@ -36,6 +36,7 @@ struct Value; struct Label { LabelId id; friend bool operator==(const Label &lhs, const Label &rhs) { return lhs.id == rhs.id; } + friend bool operator==(const Label &lhs, const LabelId &rhs) { return lhs.id == rhs; } }; // TODO(kostasrim) update this with CompoundKey, same for the rest of the file. diff --git a/tests/unit/query_v2_create_node_multiframe.cpp b/tests/unit/query_v2_create_node_multiframe.cpp index 39ce17b43..5e5d83249 100644 --- a/tests/unit/query_v2_create_node_multiframe.cpp +++ b/tests/unit/query_v2_create_node_multiframe.cpp @@ -36,15 +36,13 @@ MultiFrame CreateMultiFrame(const size_t max_pos) { TEST(CreateNodeTest, CreateNodeCursor) { using testing::_; + using testing::ElementsAre; using testing::Return; AstStorage ast; SymbolTable symbol_table; plan::NodeCreationInfo node; - plan::EdgeCreationInfo edge; - edge.edge_type = msgs::EdgeTypeId::FromUint(1); - edge.direction = EdgeAtom::Direction::IN; auto id_alloc = IdAllocator(0, 100); node.symbol = symbol_table.CreateSymbol("n", true); @@ -67,5 +65,19 @@ TEST(CreateNodeTest, CreateNodeCursor) { auto context = MakeContext(ast, symbol_table, &router, &id_alloc); auto multi_frame = CreateMultiFrame(context.symbol_table.max_position()); cursor->PullMultiple(multi_frame, context); + + auto frames = multi_frame.GetValidFramesReader(); + auto number_of_valid_frames = 0; + for (auto &frame : frames) { + ++number_of_valid_frames; + EXPECT_EQ(frame[node.symbol].IsEdge(), true); + const auto &n = frame[node.symbol].ValueVertex(); + EXPECT_THAT(n.Labels(), ElementsAre(msgs::Label{msgs::LabelId::FromUint(2)})); + } + EXPECT_EQ(number_of_valid_frames, 1); + + auto invalid_frames = multi_frame.GetInvalidFramesPopulator(); + auto number_of_invalid_frames = std::distance(invalid_frames.begin(), invalid_frames.end()); + EXPECT_EQ(number_of_invalid_frames, 99); } } // namespace memgraph::query::v2 From c139856b2a9b17ea52bc9c38107eeaa7f3b966f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A1nos=20Benjamin=20Antal?= Date: Sun, 15 Jan 2023 18:52:36 +0100 Subject: [PATCH 13/22] Fix unit tests --- src/query/v2/plan/operator.cpp | 14 +++++++------- tests/unit/query_v2_create_node_multiframe.cpp | 18 +++++++++--------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/query/v2/plan/operator.cpp b/src/query/v2/plan/operator.cpp index da282387c..630599276 100644 --- a/src/query/v2/plan/operator.cpp +++ b/src/query/v2/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 @@ -192,7 +192,7 @@ class DistributedCreateNodeCursor : public Cursor { void PullMultiple(MultiFrame &multi_frame, ExecutionContext &context) override { SCOPED_PROFILE_OP("CreateNodeMF"); input_cursor_->PullMultiple(multi_frame, context); - auto &request_router = context.request_router; + auto *request_router = context.request_router; { SCOPED_REQUEST_WAIT_PROFILE; request_router->CreateVertices(NodeCreationInfoToRequests(context, multi_frame)); @@ -259,10 +259,10 @@ class DistributedCreateNodeCursor : public Cursor { } void PlaceNodesOnTheMultiFrame(MultiFrame &multi_frame, ExecutionContext &context) { - auto multi_frame_reader = multi_frame.GetValidFramesConsumer(); + auto multi_frame_modifier = multi_frame.GetValidFramesModifier(); size_t i = 0; - MG_ASSERT(std::distance(multi_frame_reader.begin(), multi_frame_reader.end())); - for (auto &frame : multi_frame_reader) { + MG_ASSERT(std::distance(multi_frame_modifier.begin(), multi_frame_modifier.end())); + for (auto &frame : multi_frame_modifier) { const auto primary_label = msgs::Label{.id = nodes_info_[0]->labels[0]}; msgs::Vertex v{.id = std::make_pair(primary_label, primary_keys_[i])}; frame[nodes_info_.front()->symbol] = TypedValue( @@ -272,8 +272,8 @@ class DistributedCreateNodeCursor : public Cursor { std::vector NodeCreationInfoToRequests(ExecutionContext &context, MultiFrame &multi_frame) { std::vector requests; - auto multi_frame_reader = multi_frame.GetValidFramesConsumer(); - for (auto &frame : multi_frame_reader) { + auto multi_frame_modifier = multi_frame.GetValidFramesModifier(); + for (auto &frame : multi_frame_modifier) { msgs::PrimaryKey pk; for (const auto &node_info : nodes_info_) { msgs::NewVertex rqst; diff --git a/tests/unit/query_v2_create_node_multiframe.cpp b/tests/unit/query_v2_create_node_multiframe.cpp index 5e5d83249..b04b00e96 100644 --- a/tests/unit/query_v2_create_node_multiframe.cpp +++ b/tests/unit/query_v2_create_node_multiframe.cpp @@ -9,6 +9,7 @@ // by the Apache License, Version 2.0, included in the file // licenses/APL.txt. +#include "gmock/gmock.h" #include "mock_helpers.hpp" #include "query/v2/bindings/frame.hpp" @@ -26,17 +27,13 @@ namespace memgraph::query::v2 { MultiFrame CreateMultiFrame(const size_t max_pos) { static constexpr size_t frame_size = 100; MultiFrame multi_frame(max_pos, frame_size, utils::NewDeleteResource()); - auto frames_populator = multi_frame.GetInvalidFramesPopulator(); - for (auto &frame : frames_populator) { - frame.MakeValid(); - } return multi_frame; } TEST(CreateNodeTest, CreateNodeCursor) { using testing::_; - using testing::ElementsAre; + using testing::IsEmpty; using testing::Return; AstStorage ast; @@ -46,11 +43,12 @@ TEST(CreateNodeTest, CreateNodeCursor) { auto id_alloc = IdAllocator(0, 100); node.symbol = symbol_table.CreateSymbol("n", true); - node.labels.push_back(msgs::LabelId::FromUint(2)); + const auto primary_label_id = msgs::LabelId::FromUint(2); + node.labels.push_back(primary_label_id); auto literal = PrimitiveLiteral(); literal.value_ = TypedValue(static_cast(200)); auto p = plan::PropertiesMapList{}; - p.push_back(std::make_pair(msgs::PropertyId::FromUint(2), &literal)); + p.push_back(std::make_pair(msgs::PropertyId::FromUint(3), &literal)); node.properties.emplace<0>(std::move(p)); auto once_op = std::make_shared(); @@ -70,9 +68,11 @@ TEST(CreateNodeTest, CreateNodeCursor) { auto number_of_valid_frames = 0; for (auto &frame : frames) { ++number_of_valid_frames; - EXPECT_EQ(frame[node.symbol].IsEdge(), true); + EXPECT_EQ(frame[node.symbol].IsVertex(), true); const auto &n = frame[node.symbol].ValueVertex(); - EXPECT_THAT(n.Labels(), ElementsAre(msgs::Label{msgs::LabelId::FromUint(2)})); + EXPECT_THAT(n.Labels(), IsEmpty()); + EXPECT_EQ(n.PrimaryLabel(), primary_label_id); + // TODO(antaljanosbenjamin): Check primary key } EXPECT_EQ(number_of_valid_frames, 1); From e40f7f507b422ddb0daa4379bd529fce2234cfa0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A1nos=20Benjamin=20Antal?= Date: Mon, 16 Jan 2023 08:40:43 +0100 Subject: [PATCH 14/22] Fix pull logic for multiframe --- src/query/v2/interpreter.cpp | 4 ++-- src/query/v2/multiframe.cpp | 6 +++++- src/query/v2/multiframe.hpp | 3 ++- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/query/v2/interpreter.cpp b/src/query/v2/interpreter.cpp index fde11ac00..594942aec 100644 --- a/src/query/v2/interpreter.cpp +++ b/src/query/v2/interpreter.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 @@ -734,7 +734,7 @@ std::optional PullPlan::PullMultiple(AnyStrea // Returns true if a result was pulled. const auto pull_result = [&]() -> bool { cursor_->PullMultiple(multi_frame_, ctx_); - return multi_frame_.HasValidFrame(); + return !multi_frame_.HasInvalidFrame(); }; const auto stream_values = [&output_symbols, &stream](const Frame &frame) { diff --git a/src/query/v2/multiframe.cpp b/src/query/v2/multiframe.cpp index 2cb591153..0ddfd3aa7 100644 --- a/src/query/v2/multiframe.cpp +++ b/src/query/v2/multiframe.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 @@ -48,6 +48,10 @@ bool MultiFrame::HasValidFrame() const noexcept { return std::any_of(frames_.begin(), frames_.end(), [](auto &frame) { return frame.IsValid(); }); } +bool MultiFrame::HasInvalidFrame() const noexcept { + return std::any_of(frames_.rbegin(), frames_.rend(), [](auto &frame) { return !frame.IsValid(); }); +} + // NOLINTNEXTLINE (bugprone-exception-escape) void MultiFrame::DefragmentValidFrames() noexcept { /* diff --git a/src/query/v2/multiframe.hpp b/src/query/v2/multiframe.hpp index 7d9b73700..b46be171a 100644 --- a/src/query/v2/multiframe.hpp +++ b/src/query/v2/multiframe.hpp @@ -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 @@ -81,6 +81,7 @@ class MultiFrame { void MakeAllFramesInvalid() noexcept; bool HasValidFrame() const noexcept; + bool HasInvalidFrame() const noexcept; inline utils::MemoryResource *GetMemoryResource() { return frames_[0].GetMemoryResource(); } From 392f6e2b730f644f7707a7861b81dcc43ee71280 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A1nos=20Benjamin=20Antal?= Date: Mon, 16 Jan 2023 08:57:23 +0100 Subject: [PATCH 15/22] Reduce the number of node infos to a maximum of one --- src/query/v2/plan/operator.cpp | 141 +++++++++++++++------------------ 1 file changed, 65 insertions(+), 76 deletions(-) diff --git a/src/query/v2/plan/operator.cpp b/src/query/v2/plan/operator.cpp index 630599276..2f191c113 100644 --- a/src/query/v2/plan/operator.cpp +++ b/src/query/v2/plan/operator.cpp @@ -170,9 +170,8 @@ uint64_t ComputeProfilingKey(const T *obj) { class DistributedCreateNodeCursor : public Cursor { public: using InputOperator = std::shared_ptr; - DistributedCreateNodeCursor(const InputOperator &op, utils::MemoryResource *mem, - std::vector nodes_info) - : input_cursor_(op->MakeCursor(mem)), nodes_info_(std::move(nodes_info)) {} + DistributedCreateNodeCursor(const InputOperator &op, utils::MemoryResource *mem, const NodeCreationInfo &node_info) + : input_cursor_(op->MakeCursor(mem)), node_info_(node_info) {} bool Pull(Frame &frame, ExecutionContext &context) override { SCOPED_PROFILE_OP("CreateNode"); @@ -206,27 +205,78 @@ class DistributedCreateNodeCursor : public Cursor { void PlaceNodeOnTheFrame(Frame &frame, ExecutionContext &context) { // TODO(kostasrim) Make this work with batching - const auto primary_label = msgs::Label{.id = nodes_info_[0]->labels[0]}; + const auto primary_label = msgs::Label{.id = node_info_.labels[0]}; msgs::Vertex v{.id = std::make_pair(primary_label, primary_keys_[0])}; - frame[nodes_info_.front()->symbol] = + frame[node_info_.symbol] = TypedValue(query::v2::accessors::VertexAccessor(std::move(v), src_vertex_props_[0], context.request_router)); } std::vector NodeCreationInfoToRequest(ExecutionContext &context, Frame &frame) { std::vector requests; - // TODO(kostasrim) this assertion should be removed once we support multiple vertex creation - MG_ASSERT(nodes_info_.size() == 1); msgs::PrimaryKey pk; - for (const auto &node_info : nodes_info_) { + msgs::NewVertex rqst; + MG_ASSERT(!node_info_.labels.empty(), "Cannot determine primary label"); + const auto primary_label = node_info_.labels[0]; + // TODO(jbajic) Fix properties not send, + // suggestion: ignore distinction between properties and primary keys + // since schema validation is done on storage side + ExpressionEvaluator evaluator(&frame, context.symbol_table, context.evaluation_context, nullptr, + storage::v3::View::NEW); + if (const auto *node_info_properties = std::get_if(&node_info_.properties)) { + for (const auto &[key, value_expression] : *node_info_properties) { + TypedValue val = value_expression->Accept(evaluator); + if (context.request_router->IsPrimaryKey(primary_label, key)) { + rqst.primary_key.push_back(TypedValueToValue(val)); + pk.push_back(TypedValueToValue(val)); + } + } + } else { + auto property_map = evaluator.Visit(*std::get(node_info_.properties)).ValueMap(); + for (const auto &[key, value] : property_map) { + auto key_str = std::string(key); + auto property_id = context.request_router->NameToProperty(key_str); + if (context.request_router->IsPrimaryKey(primary_label, property_id)) { + rqst.primary_key.push_back(TypedValueToValue(value)); + pk.push_back(TypedValueToValue(value)); + } + } + } + + // TODO(kostasrim) Copy non primary labels as well + rqst.label_ids.push_back(msgs::Label{.id = primary_label}); + src_vertex_props_.push_back(rqst.properties); + requests.push_back(std::move(rqst)); + + primary_keys_.push_back(std::move(pk)); + return requests; + } + + void PlaceNodesOnTheMultiFrame(MultiFrame &multi_frame, ExecutionContext &context) { + auto multi_frame_modifier = multi_frame.GetValidFramesModifier(); + size_t i = 0; + MG_ASSERT(std::distance(multi_frame_modifier.begin(), multi_frame_modifier.end())); + for (auto &frame : multi_frame_modifier) { + const auto primary_label = msgs::Label{.id = node_info_.labels[0]}; + msgs::Vertex v{.id = std::make_pair(primary_label, primary_keys_[i])}; + frame[node_info_.symbol] = TypedValue( + query::v2::accessors::VertexAccessor(std::move(v), src_vertex_props_[i++], context.request_router)); + } + } + + std::vector NodeCreationInfoToRequests(ExecutionContext &context, MultiFrame &multi_frame) { + std::vector requests; + auto multi_frame_modifier = multi_frame.GetValidFramesModifier(); + for (auto &frame : multi_frame_modifier) { + msgs::PrimaryKey pk; msgs::NewVertex rqst; - MG_ASSERT(!node_info->labels.empty(), "Cannot determine primary label"); - const auto primary_label = node_info->labels[0]; + MG_ASSERT(!node_info_.labels.empty(), "Cannot determine primary label"); + const auto primary_label = node_info_.labels[0]; // TODO(jbajic) Fix properties not send, // suggestion: ignore distinction between properties and primary keys // since schema validation is done on storage side ExpressionEvaluator evaluator(&frame, context.symbol_table, context.evaluation_context, nullptr, storage::v3::View::NEW); - if (const auto *node_info_properties = std::get_if(&node_info->properties)) { + if (const auto *node_info_properties = std::get_if(&node_info_.properties)) { for (const auto &[key, value_expression] : *node_info_properties) { TypedValue val = value_expression->Accept(evaluator); if (context.request_router->IsPrimaryKey(primary_label, key)) { @@ -235,7 +285,7 @@ class DistributedCreateNodeCursor : public Cursor { } } } else { - auto property_map = evaluator.Visit(*std::get(node_info->properties)).ValueMap(); + auto property_map = evaluator.Visit(*std::get(node_info_.properties)).ValueMap(); for (const auto &[key, value] : property_map) { auto key_str = std::string(key); auto property_id = context.request_router->NameToProperty(key_str); @@ -246,80 +296,19 @@ class DistributedCreateNodeCursor : public Cursor { } } - if (node_info->labels.empty()) { - throw QueryRuntimeException("Primary label must be defined!"); - } // TODO(kostasrim) Copy non primary labels as well rqst.label_ids.push_back(msgs::Label{.id = primary_label}); src_vertex_props_.push_back(rqst.properties); requests.push_back(std::move(rqst)); - } - primary_keys_.push_back(std::move(pk)); - return requests; - } - - void PlaceNodesOnTheMultiFrame(MultiFrame &multi_frame, ExecutionContext &context) { - auto multi_frame_modifier = multi_frame.GetValidFramesModifier(); - size_t i = 0; - MG_ASSERT(std::distance(multi_frame_modifier.begin(), multi_frame_modifier.end())); - for (auto &frame : multi_frame_modifier) { - const auto primary_label = msgs::Label{.id = nodes_info_[0]->labels[0]}; - msgs::Vertex v{.id = std::make_pair(primary_label, primary_keys_[i])}; - frame[nodes_info_.front()->symbol] = TypedValue( - query::v2::accessors::VertexAccessor(std::move(v), src_vertex_props_[i++], context.request_router)); - } - } - - std::vector NodeCreationInfoToRequests(ExecutionContext &context, MultiFrame &multi_frame) { - std::vector requests; - auto multi_frame_modifier = multi_frame.GetValidFramesModifier(); - for (auto &frame : multi_frame_modifier) { - msgs::PrimaryKey pk; - for (const auto &node_info : nodes_info_) { - msgs::NewVertex rqst; - MG_ASSERT(!node_info->labels.empty(), "Cannot determine primary label"); - const auto primary_label = node_info->labels[0]; - // TODO(jbajic) Fix properties not send, - // suggestion: ignore distinction between properties and primary keys - // since schema validation is done on storage side - ExpressionEvaluator evaluator(&frame, context.symbol_table, context.evaluation_context, nullptr, - storage::v3::View::NEW); - if (const auto *node_info_properties = std::get_if(&node_info->properties)) { - for (const auto &[key, value_expression] : *node_info_properties) { - TypedValue val = value_expression->Accept(evaluator); - if (context.request_router->IsPrimaryKey(primary_label, key)) { - rqst.primary_key.push_back(TypedValueToValue(val)); - pk.push_back(TypedValueToValue(val)); - } - } - } else { - auto property_map = evaluator.Visit(*std::get(node_info->properties)).ValueMap(); - for (const auto &[key, value] : property_map) { - auto key_str = std::string(key); - auto property_id = context.request_router->NameToProperty(key_str); - if (context.request_router->IsPrimaryKey(primary_label, property_id)) { - rqst.primary_key.push_back(TypedValueToValue(value)); - pk.push_back(TypedValueToValue(value)); - } - } - } - - if (node_info->labels.empty()) { - throw QueryRuntimeException("Primary label must be defined!"); - } - // TODO(kostasrim) Copy non primary labels as well - rqst.label_ids.push_back(msgs::Label{.id = primary_label}); - src_vertex_props_.push_back(rqst.properties); - requests.push_back(std::move(rqst)); - } primary_keys_.push_back(std::move(pk)); } + return requests; } private: const UniqueCursorPtr input_cursor_; - std::vector nodes_info_; + NodeCreationInfo node_info_; std::vector>> src_vertex_props_; std::vector primary_keys_; }; @@ -364,7 +353,7 @@ ACCEPT_WITH_INPUT(CreateNode) UniqueCursorPtr CreateNode::MakeCursor(utils::MemoryResource *mem) const { EventCounter::IncrementCounter(EventCounter::CreateNodeOperator); - return MakeUniqueCursorPtr(mem, input_, mem, std::vector{&this->node_info_}); + return MakeUniqueCursorPtr(mem, input_, mem, this->node_info_); } std::vector CreateNode::ModifiedSymbols(const SymbolTable &table) const { From 920ad277a5e44a34083f0030d3618695a7eb42bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A1nos=20Benjamin=20Antal?= Date: Mon, 16 Jan 2023 09:03:35 +0100 Subject: [PATCH 16/22] Add assertion about primary label --- src/query/v2/plan/operator.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/query/v2/plan/operator.cpp b/src/query/v2/plan/operator.cpp index 2f191c113..4ea189772 100644 --- a/src/query/v2/plan/operator.cpp +++ b/src/query/v2/plan/operator.cpp @@ -271,6 +271,7 @@ class DistributedCreateNodeCursor : public Cursor { msgs::NewVertex rqst; MG_ASSERT(!node_info_.labels.empty(), "Cannot determine primary label"); const auto primary_label = node_info_.labels[0]; + MG_ASSERT(context.request_router->IsPrimaryLabel(primary_label), "First label has to be a primary label!"); // TODO(jbajic) Fix properties not send, // suggestion: ignore distinction between properties and primary keys // since schema validation is done on storage side From 775e950dbae0bc34339c91bd9def081c89f66308 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A1nos=20Benjamin=20Antal?= Date: Mon, 16 Jan 2023 10:16:12 +0100 Subject: [PATCH 17/22] Update unit tests to test the new logic --- tests/unit/query_v2_create_node_multiframe.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/unit/query_v2_create_node_multiframe.cpp b/tests/unit/query_v2_create_node_multiframe.cpp index b04b00e96..014fff6ce 100644 --- a/tests/unit/query_v2_create_node_multiframe.cpp +++ b/tests/unit/query_v2_create_node_multiframe.cpp @@ -56,10 +56,9 @@ TEST(CreateNodeTest, CreateNodeCursor) { auto create_expand = plan::CreateNode(once_op, node); auto cursor = create_expand.MakeCursor(utils::NewDeleteResource()); MockedRequestRouter router; - EXPECT_CALL(router, CreateVertices(testing::_)) - .Times(1) - .WillOnce(::testing::Return(std::vector{})); - EXPECT_CALL(router, IsPrimaryKey(testing::_, testing::_)).WillRepeatedly(::testing::Return(true)); + EXPECT_CALL(router, CreateVertices(_)).Times(1).WillOnce(Return(std::vector{})); + EXPECT_CALL(router, IsPrimaryLabel(_)).WillRepeatedly(Return(true)); + EXPECT_CALL(router, IsPrimaryKey(_, _)).WillRepeatedly(Return(true)); auto context = MakeContext(ast, symbol_table, &router, &id_alloc); auto multi_frame = CreateMultiFrame(context.symbol_table.max_position()); cursor->PullMultiple(multi_frame, context); From b38a9b9c901c24c3bb99d62465c42282398aea45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A1nos=20Benjamin=20Antal?= Date: Tue, 17 Jan 2023 20:25:10 +0100 Subject: [PATCH 18/22] Use `tests` namespace for tests --- tests/unit/mock_helpers.hpp | 6 +++--- tests/unit/query_v2_create_expand_multiframe.cpp | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/unit/mock_helpers.hpp b/tests/unit/mock_helpers.hpp index 6c03889ba..c7994586a 100644 --- a/tests/unit/mock_helpers.hpp +++ b/tests/unit/mock_helpers.hpp @@ -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 @@ -18,7 +18,7 @@ #include "query/v2/plan/operator.hpp" #include "query/v2/request_router.hpp" -namespace memgraph::query::v2 { +namespace memgraph::query::v2::tests { class MockedRequestRouter : public RequestRouterInterface { public: MOCK_METHOD(std::vector, ScanVertices, (std::optional label)); @@ -79,4 +79,4 @@ inline MockedLogicalOperator &BaseToMock(plan::LogicalOperator &op) { inline MockedCursor &BaseToMock(plan::Cursor &cursor) { return dynamic_cast(cursor); } -} // namespace memgraph::query::v2 +} // namespace memgraph::query::v2::tests diff --git a/tests/unit/query_v2_create_expand_multiframe.cpp b/tests/unit/query_v2_create_expand_multiframe.cpp index 8720656fd..ebdc4a9a7 100644 --- a/tests/unit/query_v2_create_expand_multiframe.cpp +++ b/tests/unit/query_v2_create_expand_multiframe.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 @@ -23,7 +23,7 @@ #include "utils/logging.hpp" #include "utils/memory.hpp" -namespace memgraph::query::v2 { +namespace memgraph::query::v2::tests { MultiFrame CreateMultiFrame(const size_t max_pos, const Symbol &src, const Symbol &dst, MockedRequestRouter *router) { static constexpr size_t number_of_frames = 100; @@ -91,4 +91,4 @@ TEST(CreateExpandTest, Cursor) { EXPECT_EQ(number_of_invalid_frames, 99); } -} // namespace memgraph::query::v2 +} // namespace memgraph::query::v2::tests From a3b1676c42bb8a6a69d872ace5f0b8d75891ea63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A1nos=20Benjamin=20Antal?= Date: Tue, 17 Jan 2023 20:25:28 +0100 Subject: [PATCH 19/22] Separate include blocks --- tests/unit/mock_helpers.hpp | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/unit/mock_helpers.hpp b/tests/unit/mock_helpers.hpp index c7994586a..c522b8602 100644 --- a/tests/unit/mock_helpers.hpp +++ b/tests/unit/mock_helpers.hpp @@ -13,6 +13,7 @@ #include #include + #include "query/v2/common.hpp" #include "query/v2/context.hpp" #include "query/v2/plan/operator.hpp" From 7fb828bca38ab8e8929bc40494255f70645797b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A1nos=20Benjamin=20Antal?= Date: Tue, 17 Jan 2023 20:32:00 +0100 Subject: [PATCH 20/22] Update outdated comments --- src/query/v2/plan/operator.cpp | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/query/v2/plan/operator.cpp b/src/query/v2/plan/operator.cpp index 7ac02b451..49d25f4e2 100644 --- a/src/query/v2/plan/operator.cpp +++ b/src/query/v2/plan/operator.cpp @@ -217,9 +217,7 @@ class DistributedCreateNodeCursor : public Cursor { msgs::NewVertex rqst; MG_ASSERT(!node_info_.labels.empty(), "Cannot determine primary label"); const auto primary_label = node_info_.labels[0]; - // TODO(jbajic) Fix properties not send, - // suggestion: ignore distinction between properties and primary keys - // since schema validation is done on storage side + // TODO(jbajic) Send also the properties that are not part of primary key ExpressionEvaluator evaluator(&frame, context.symbol_table, context.evaluation_context, nullptr, storage::v3::View::NEW); if (const auto *node_info_properties = std::get_if(&node_info_.properties)) { @@ -272,9 +270,7 @@ class DistributedCreateNodeCursor : public Cursor { MG_ASSERT(!node_info_.labels.empty(), "Cannot determine primary label"); const auto primary_label = node_info_.labels[0]; MG_ASSERT(context.request_router->IsPrimaryLabel(primary_label), "First label has to be a primary label!"); - // TODO(jbajic) Fix properties not send, - // suggestion: ignore distinction between properties and primary keys - // since schema validation is done on storage side + // TODO(jbajic) Send also the properties that are not part of primary key ExpressionEvaluator evaluator(&frame, context.symbol_table, context.evaluation_context, nullptr, storage::v3::View::NEW); if (const auto *node_info_properties = std::get_if(&node_info_.properties)) { From 81675106fd9c1b00c779f5157bc97265d3214489 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A1nos=20Benjamin=20Antal?= Date: Tue, 17 Jan 2023 20:33:14 +0100 Subject: [PATCH 21/22] Use `tests` namespace for tests --- tests/unit/query_v2_create_node_multiframe.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/query_v2_create_node_multiframe.cpp b/tests/unit/query_v2_create_node_multiframe.cpp index 014fff6ce..b298d2781 100644 --- a/tests/unit/query_v2_create_node_multiframe.cpp +++ b/tests/unit/query_v2_create_node_multiframe.cpp @@ -23,7 +23,7 @@ #include "storage/v3/shard.hpp" #include "utils/memory.hpp" -namespace memgraph::query::v2 { +namespace memgraph::query::v2::tests { MultiFrame CreateMultiFrame(const size_t max_pos) { static constexpr size_t frame_size = 100; MultiFrame multi_frame(max_pos, frame_size, utils::NewDeleteResource()); @@ -79,4 +79,4 @@ TEST(CreateNodeTest, CreateNodeCursor) { auto number_of_invalid_frames = std::distance(invalid_frames.begin(), invalid_frames.end()); EXPECT_EQ(number_of_invalid_frames, 99); } -} // namespace memgraph::query::v2 +} // namespace memgraph::query::v2::tests From 8f6fac3cdec1267fd619afe83a7602889b558ee7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A1nos=20Benjamin=20Antal?= Date: Thu, 19 Jan 2023 14:35:25 +0100 Subject: [PATCH 22/22] Make arguments const --- src/query/v2/multiframe.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/query/v2/multiframe.cpp b/src/query/v2/multiframe.cpp index 0ddfd3aa7..38cc7549a 100644 --- a/src/query/v2/multiframe.cpp +++ b/src/query/v2/multiframe.cpp @@ -45,11 +45,11 @@ void MultiFrame::MakeAllFramesInvalid() noexcept { } bool MultiFrame::HasValidFrame() const noexcept { - return std::any_of(frames_.begin(), frames_.end(), [](auto &frame) { return frame.IsValid(); }); + return std::any_of(frames_.begin(), frames_.end(), [](const auto &frame) { return frame.IsValid(); }); } bool MultiFrame::HasInvalidFrame() const noexcept { - return std::any_of(frames_.rbegin(), frames_.rend(), [](auto &frame) { return !frame.IsValid(); }); + return std::any_of(frames_.rbegin(), frames_.rend(), [](const auto &frame) { return !frame.IsValid(); }); } // NOLINTNEXTLINE (bugprone-exception-escape)