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 1/6] 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 2/6] 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 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 3/6] 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 4/6] 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 5/6] 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 6/6] 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