From 55e0dbca804201bac4ab30c127b43c6e1890994c Mon Sep 17 00:00:00 2001 From: jeremy <jeremy.bailleux@memgraph.io> Date: Fri, 21 Oct 2022 16:32:49 +0200 Subject: [PATCH 01/63] Add limit to ExpandOne Add missing pragma Add test Merge conflicts --- src/storage/v3/expr.hpp | 2 + src/storage/v3/request_helper.cpp | 77 ++++++++++----------- src/storage/v3/request_helper.hpp | 65 +++++++++++++++-- src/storage/v3/shard_rsm.cpp | 111 +++++++++++++++++++++++------- tests/simulation/shard_rsm.cpp | 65 +++++++++++++++++ 5 files changed, 248 insertions(+), 72 deletions(-) diff --git a/src/storage/v3/expr.hpp b/src/storage/v3/expr.hpp index c3199abf1..7eefdd71c 100644 --- a/src/storage/v3/expr.hpp +++ b/src/storage/v3/expr.hpp @@ -9,6 +9,8 @@ // by the Apache License, Version 2.0, included in the file // licenses/APL.txt. +#pragma once + #include <vector> #include "db_accessor.hpp" diff --git a/src/storage/v3/request_helper.cpp b/src/storage/v3/request_helper.cpp index bb1c8bca4..7dab4c5c1 100644 --- a/src/storage/v3/request_helper.cpp +++ b/src/storage/v3/request_helper.cpp @@ -19,43 +19,6 @@ namespace memgraph::storage::v3 { -std::vector<Element> OrderByElements(Shard::Accessor &acc, DbAccessor &dba, VerticesIterable &vertices_iterable, - std::vector<msgs::OrderBy> &order_bys) { - std::vector<Element> ordered; - ordered.reserve(acc.ApproximateVertexCount()); - std::vector<Ordering> ordering; - ordering.reserve(order_bys.size()); - for (const auto &order : order_bys) { - switch (order.direction) { - case memgraph::msgs::OrderingDirection::ASCENDING: { - ordering.push_back(Ordering::ASC); - break; - } - case memgraph::msgs::OrderingDirection::DESCENDING: { - ordering.push_back(Ordering::DESC); - break; - } - } - } - auto compare_typed_values = TypedValueVectorCompare(ordering); - for (auto it = vertices_iterable.begin(); it != vertices_iterable.end(); ++it) { - std::vector<TypedValue> properties_order_by; - properties_order_by.reserve(order_bys.size()); - - for (const auto &order_by : order_bys) { - const auto val = - ComputeExpression(dba, *it, std::nullopt, order_by.expression.expression, expr::identifier_node_symbol, ""); - properties_order_by.push_back(val); - } - ordered.push_back({std::move(properties_order_by), *it}); - } - - std::sort(ordered.begin(), ordered.end(), [compare_typed_values](const auto &pair1, const auto &pair2) { - return compare_typed_values(pair1.properties_order_by, pair2.properties_order_by); - }); - return ordered; -} - VerticesIterable::Iterator GetStartVertexIterator(VerticesIterable &vertex_iterable, const std::vector<PropertyValue> &start_ids, const View view) { auto it = vertex_iterable.begin(); @@ -68,15 +31,47 @@ VerticesIterable::Iterator GetStartVertexIterator(VerticesIterable &vertex_itera return it; } -std::vector<Element>::const_iterator GetStartOrderedElementsIterator(const std::vector<Element> &ordered_elements, - const std::vector<PropertyValue> &start_ids, - const View view) { +std::vector<Element<VertexAccessor>>::const_iterator GetStartOrderedElementsIterator( + const std::vector<Element<VertexAccessor>> &ordered_elements, const std::vector<PropertyValue> &start_ids, + const View view) { for (auto it = ordered_elements.begin(); it != ordered_elements.end(); ++it) { - if (const auto &vertex = it->vertex_acc; start_ids <= vertex.PrimaryKey(view).GetValue()) { + if (const auto &vertex = it->object_acc; start_ids <= vertex.PrimaryKey(view).GetValue()) { return it; } } return ordered_elements.end(); } +std::vector<EdgeAccessor> GetEdgesFromVertex(const VertexAccessor &vertex_accessor, + const msgs::EdgeDirection direction) { + switch (direction) { + case memgraph::msgs::EdgeDirection::IN: { + auto edges = vertex_accessor.InEdges(View::OLD); + if (edges.HasValue()) { + return edges.GetValue(); + } + return {}; + } + case memgraph::msgs::EdgeDirection::OUT: { + auto edges = vertex_accessor.OutEdges(View::OLD); + if (edges.HasValue()) { + return edges.GetValue(); + } + return {}; + } + case memgraph::msgs::EdgeDirection::BOTH: { + auto maybe_in_edges = vertex_accessor.InEdges(View::OLD); + auto maybe_out_edges = vertex_accessor.OutEdges(View::OLD); + std::vector<EdgeAccessor> edges; + if (maybe_in_edges.HasValue()) { + edges = maybe_in_edges.GetValue(); + } + if (maybe_out_edges.HasValue()) { + edges.insert(edges.end(), maybe_out_edges.GetValue().begin(), maybe_out_edges.GetValue().end()); + } + return edges; + } + } +} + } // namespace memgraph::storage::v3 diff --git a/src/storage/v3/request_helper.hpp b/src/storage/v3/request_helper.hpp index 24ed40f8c..6212c4a69 100644 --- a/src/storage/v3/request_helper.hpp +++ b/src/storage/v3/request_helper.hpp @@ -9,14 +9,21 @@ // by the Apache License, Version 2.0, included in the file // licenses/APL.txt. +#pragma once + #include <vector> #include "ast/ast.hpp" +#include "pretty_print_ast_to_original_expression.hpp" // #NoCommit why like this? +#include "query/v2/requests.hpp" #include "storage/v3/bindings/typed_value.hpp" +#include "storage/v3/edge_accessor.hpp" +#include "storage/v3/expr.hpp" #include "storage/v3/shard.hpp" #include "storage/v3/vertex_accessor.hpp" - namespace memgraph::storage::v3 { +template <typename T> +concept ObjectAccessor = std::is_same_v<T, VertexAccessor> || std::is_same_v<T, EdgeAccessor>; inline bool TypedValueCompare(const TypedValue &a, const TypedValue &b) { // in ordering null comes after everything else @@ -99,18 +106,62 @@ class TypedValueVectorCompare final { std::vector<Ordering> ordering_; }; +template <ObjectAccessor TObjectAccessor> struct Element { std::vector<TypedValue> properties_order_by; - VertexAccessor vertex_acc; + TObjectAccessor object_acc; }; -std::vector<Element> OrderByElements(Shard::Accessor &acc, DbAccessor &dba, VerticesIterable &vertices_iterable, - std::vector<msgs::OrderBy> &order_bys); +// #NoCommit in cpp +template <ObjectAccessor TObjectAccessor, typename TIterable> +std::vector<Element<TObjectAccessor>> OrderByElements(Shard::Accessor &acc, DbAccessor &dba, TIterable &iterable, + std::vector<msgs::OrderBy> &order_bys) { + std::vector<Element<TObjectAccessor>> ordered; + ordered.reserve(acc.ApproximateVertexCount()); + std::vector<Ordering> ordering; + ordering.reserve(order_bys.size()); + for (const auto &order : order_bys) { + switch (order.direction) { + case memgraph::msgs::OrderingDirection::ASCENDING: { + ordering.push_back(Ordering::ASC); + break; + } + case memgraph::msgs::OrderingDirection::DESCENDING: { + ordering.push_back(Ordering::DESC); + break; + } + } + } + auto compare_typed_values = TypedValueVectorCompare(ordering); + auto it = iterable.begin(); + for (; it != iterable.end(); ++it) { + std::vector<TypedValue> properties_order_by; + properties_order_by.reserve(order_bys.size()); + + for (const auto &order_by : order_bys) { + if constexpr (std::is_same_v<TIterable, VerticesIterable>) { + properties_order_by.push_back(ComputeExpression(dba, *it, std::nullopt, order_by.expression.expression, + expr::identifier_node_symbol, expr::identifier_edge_symbol)); + } else { + properties_order_by.push_back(ComputeExpression(dba, std::nullopt, *it, order_by.expression.expression, + expr::identifier_node_symbol, expr::identifier_edge_symbol)); + } + } + ordered.push_back({std::move(properties_order_by), *it}); + } + + std::sort(ordered.begin(), ordered.end(), [compare_typed_values](const auto &pair1, const auto &pair2) { + return compare_typed_values(pair1.properties_order_by, pair2.properties_order_by); + }); + return ordered; +} VerticesIterable::Iterator GetStartVertexIterator(VerticesIterable &vertex_iterable, const std::vector<PropertyValue> &start_ids, View view); -std::vector<Element>::const_iterator GetStartOrderedElementsIterator(const std::vector<Element> &ordered_elements, - const std::vector<PropertyValue> &start_ids, - View view); +std::vector<Element<VertexAccessor>>::const_iterator GetStartOrderedElementsIterator( + const std::vector<Element<VertexAccessor>> &ordered_elements, const std::vector<PropertyValue> &start_ids, + View view); + +std::vector<EdgeAccessor> GetEdgesFromVertex(const VertexAccessor &vertex_accessor, msgs::EdgeDirection direction); } // namespace memgraph::storage::v3 diff --git a/src/storage/v3/shard_rsm.cpp b/src/storage/v3/shard_rsm.cpp index 679f95172..97b275530 100644 --- a/src/storage/v3/shard_rsm.cpp +++ b/src/storage/v3/shard_rsm.cpp @@ -15,6 +15,7 @@ #include <unordered_set> #include <utility> +#include <unordered_map> #include "parser/opencypher/parser.hpp" #include "query/v2/requests.hpp" #include "storage/v3/bindings/ast/ast.hpp" @@ -26,6 +27,7 @@ #include "storage/v3/bindings/symbol_generator.hpp" #include "storage/v3/bindings/symbol_table.hpp" #include "storage/v3/bindings/typed_value.hpp" +#include "storage/v3/edge_accessor.hpp" #include "storage/v3/expr.hpp" #include "storage/v3/id_types.hpp" #include "storage/v3/key_store.hpp" @@ -802,18 +804,18 @@ msgs::ReadResponses ShardRsm::HandleRead(msgs::ScanVerticesRequest &&req) { uint64_t sample_counter{0}; auto vertex_iterable = acc.Vertices(view); if (!req.order_bys.empty()) { - const auto ordered = OrderByElements(acc, dba, vertex_iterable, req.order_bys); + const auto ordered = OrderByElements<VertexAccessor>(acc, dba, vertex_iterable, req.order_bys); // we are traversing Elements auto it = GetStartOrderedElementsIterator(ordered, start_id, View(req.storage_view)); for (; it != ordered.end(); ++it) { - emplace_scan_result(it->vertex_acc); + emplace_scan_result(it->object_acc); ++sample_counter; if (req.batch_limit && sample_counter == req.batch_limit) { // Reached the maximum specified batch size. // Get the next element before exiting. ++it; if (it != ordered.end()) { - const auto &next_vertex = it->vertex_acc; + const auto &next_vertex = it->object_acc; next_start_id = ConstructValueVertex(next_vertex, view).vertex_v.id; } @@ -854,36 +856,97 @@ msgs::ReadResponses ShardRsm::HandleRead(msgs::ExpandOneRequest &&req) { bool action_successful = true; std::vector<msgs::ExpandOneResultRow> results; + auto batch_limit = req.limit; + auto dba = DbAccessor{&acc}; auto maybe_filter_based_on_edge_uniquness = InitializeEdgeUniqunessFunction(req.only_unique_neighbor_rows); auto edge_filler = InitializeEdgeFillerFunction(req); - for (auto &src_vertex : req.src_vertices) { - // Get Vertex acc - auto src_vertex_acc_opt = acc.FindVertex(ConvertPropertyVector((src_vertex.second)), View::NEW); - if (!src_vertex_acc_opt) { - action_successful = false; - spdlog::debug("Encountered an error while trying to obtain VertexAccessor. Transaction id: {}", - req.transaction_id.logical_id); - break; - } + //#NoCommit code below is duplicated, one needs to factor it once it's clear + if (req.order_by) { + auto vertex_iterable = acc.Vertices(View::OLD); + const auto ordered_vertices = OrderByElements<VertexAccessor>(acc, dba, vertex_iterable, *req.order_by); + std::vector<std::pair<VertexAccessor, std::vector<Element<EdgeAccessor>>>> vertex_ordered_edges; + vertex_ordered_edges.reserve(req.src_vertices.size()); - if (!req.filters.empty()) { - // NOTE - DbAccessor might get removed in the future. - auto dba = DbAccessor{&acc}; - const bool eval = FilterOnVertex(dba, src_vertex_acc_opt.value(), req.filters, expr::identifier_node_symbol); - if (!eval) { - continue; + for (auto &src_vertex : req.src_vertices) { + // Get Vertex acc + auto src_vertex_acc_opt = acc.FindVertex(ConvertPropertyVector((src_vertex.second)), View::NEW); + if (!src_vertex_acc_opt) { + action_successful = false; + spdlog::debug("Encountered an error while trying to obtain VertexAccessor. Transaction id: {}", + req.transaction_id.logical_id); + break; + } + + if (!req.filters.empty()) { + // NOTE - DbAccessor might get removed in the future. + auto dba = DbAccessor{&acc}; + const bool eval = FilterOnVertex(dba, src_vertex_acc_opt.value(), req.filters, expr::identifier_node_symbol); + if (!eval) { + continue; + } + } + auto result = GetExpandOneResult(acc, src_vertex, req, maybe_filter_based_on_edge_uniquness, edge_filler); + + if (!result) { + action_successful = false; + break; + } + + results.emplace_back(result.value()); + if (batch_limit) { // #NoCommit can ebd one differently + --*batch_limit; + if (batch_limit < 0) { + break; + } } } - auto result = GetExpandOneResult(acc, src_vertex, req, maybe_filter_based_on_edge_uniquness, edge_filler); + // Now we have to construct response like this, and here we will care about + // limit: + // v1 e1 + // v1 e2 + // v1 e3 + // v2 e4 + // v2 e5 + // v2 e6 + // v2 e7 + // v3 e8 + // v3 e9 + } else { + for (auto &src_vertex : req.src_vertices) { + // Get Vertex acc + auto src_vertex_acc_opt = acc.FindVertex(ConvertPropertyVector((src_vertex.second)), View::NEW); + if (!src_vertex_acc_opt) { + action_successful = false; + spdlog::debug("Encountered an error while trying to obtain VertexAccessor. Transaction id: {}", + req.transaction_id.logical_id); + break; + } - if (!result) { - action_successful = false; - break; + if (!req.filters.empty()) { + // NOTE - DbAccessor might get removed in the future. + auto dba = DbAccessor{&acc}; + const bool eval = FilterOnVertex(dba, src_vertex_acc_opt.value(), req.filters, expr::identifier_node_symbol); + if (!eval) { + continue; + } + } + auto result = GetExpandOneResult(acc, src_vertex, req, maybe_filter_based_on_edge_uniquness, edge_filler); + + if (!result) { + action_successful = false; + break; + } + + results.emplace_back(result.value()); + if (batch_limit) { // #NoCommit can ebd one differently + --*batch_limit; + if (batch_limit < 0) { + break; + } + } } - - results.emplace_back(result.value()); } msgs::ExpandOneResponse resp{}; diff --git a/tests/simulation/shard_rsm.cpp b/tests/simulation/shard_rsm.cpp index d5f8f2775..6c3ea7d60 100644 --- a/tests/simulation/shard_rsm.cpp +++ b/tests/simulation/shard_rsm.cpp @@ -680,6 +680,61 @@ void AttemptToExpandOneWithUniqueEdges(ShardClient &client, uint64_t src_vertex_ } } +void AttemptToExpandOneLimitAndOrderBy(ShardClient &client, uint64_t src_vertex_val, EdgeTypeId edge_type_id) { + // Source vertex + msgs::Label label = {.id = get_primary_label()}; + auto src_vertex = std::make_pair(label, GetPrimaryKey(src_vertex_val)); + + // Edge type + auto edge_type = msgs::EdgeType{}; + edge_type.id = edge_type_id; + + // Edge direction + auto edge_direction = msgs::EdgeDirection::OUT; + + // Source Vertex properties to look for + std::optional<std::vector<PropertyId>> src_vertex_properties = {}; + + // Edge properties to look for + std::optional<std::vector<PropertyId>> edge_properties = {}; + + std::vector<std::string> expressions; + std::optional<std::vector<msgs::OrderBy>> order_by = {}; + // std::optional<size_t> limit = 5; + // std::optional<msgs::Filter> filter = {}; + + msgs::ExpandOneRequest expand_one_req{}; + + expand_one_req.direction = edge_direction; + expand_one_req.edge_properties = edge_properties; + expand_one_req.edge_types = {edge_type}; + // expand_one_req.expressions = expressions; #NoCommit not existing? + // expand_one_req.filter = filter; + // expand_one_req.limit = limit; + expand_one_req.order_by = order_by; + expand_one_req.src_vertex_properties = src_vertex_properties; + expand_one_req.src_vertices = {src_vertex}; + expand_one_req.transaction_id.logical_id = GetTransactionId(); + + while (true) { + auto read_res = client.SendReadRequest(expand_one_req); + if (read_res.HasError()) { + continue; + } + + auto write_response_result = read_res.GetValue(); + auto write_response = std::get<msgs::ExpandOneResponse>(write_response_result); + MG_ASSERT(write_response.result.size() == 1); + // MG_ASSERT(write_response.result[0].edges_with_all_properties->size() == 10); // #NoCommit where does that come + // from? + // auto number_of_properties_on_edge = + // (std::get<std::map<PropertyId, msgs::Value>>(write_response.result[0].edges_with_all_properties.value()[0])) + // .size(); + // MG_ASSERT(number_of_properties_on_edge == 1); + break; + } +} + void AttemptToExpandOneWithSpecifiedSrcVertexProperties(ShardClient &client, uint64_t src_vertex_val, EdgeTypeId edge_type_id) { // Source vertex @@ -1021,6 +1076,9 @@ void TestExpandOneGraphOne(ShardClient &client) { auto edge_prop_id = GetUniqueInteger(); auto edge_prop_val = GetUniqueInteger(); + std::vector<uint64_t> edges_ids(10); + std::generate(edges_ids.begin(), edges_ids.end(), GetUniqueInteger); + // (V1)-[edge_type_id]->(V2) MG_ASSERT(AttemptToAddEdgeWithProperties(client, unique_prop_val_1, unique_prop_val_2, edge_gid_1, edge_prop_id, edge_prop_val, {edge_type_id})); @@ -1028,6 +1086,13 @@ void TestExpandOneGraphOne(ShardClient &client) { MG_ASSERT(AttemptToAddEdgeWithProperties(client, unique_prop_val_1, unique_prop_val_3, edge_gid_2, edge_prop_id, edge_prop_val, {edge_type_id})); + // (V2)-[edge_type_id]->(V3) x 10 + std::for_each(edges_ids.begin(), edges_ids.end(), [&](const auto &edge_id) { + MG_ASSERT(AttemptToAddEdgeWithProperties(client, unique_prop_val_2, unique_prop_val_3, edge_id, edge_prop_id, + edge_prop_val, {edge_type_id})); + }); + AttemptToExpandOneLimitAndOrderBy(client, unique_prop_val_2, edge_type_id); // #NoCommit + AttemptToExpandOneSimple(client, unique_prop_val_1, edge_type_id); AttemptToExpandOneWithWrongEdgeType(client, unique_prop_val_1, wrong_edge_type_id); AttemptToExpandOneWithSpecifiedSrcVertexProperties(client, unique_prop_val_1, edge_type_id); From b82e8748ad9bf2a0a96a59cf9b4f00eaf9f86ba1 Mon Sep 17 00:00:00 2001 From: jeremy <jeremy.bailleux@memgraph.io> Date: Mon, 24 Oct 2022 12:03:51 +0200 Subject: [PATCH 02/63] Attempt impl --- src/query/v2/requests.hpp | 2 +- src/storage/v3/request_helper.cpp | 20 +++--- src/storage/v3/request_helper.hpp | 3 +- src/storage/v3/shard_rsm.cpp | 105 +++++++++++++++++++++--------- tests/simulation/shard_rsm.cpp | 32 ++++----- 5 files changed, 103 insertions(+), 59 deletions(-) diff --git a/src/query/v2/requests.hpp b/src/query/v2/requests.hpp index 49ae346cb..d8d637037 100644 --- a/src/query/v2/requests.hpp +++ b/src/query/v2/requests.hpp @@ -404,7 +404,7 @@ struct ExpandOneRequest { std::vector<std::string> vertex_expressions; std::vector<std::string> edge_expressions; - std::optional<std::vector<OrderBy>> order_by; + std::vector<OrderBy> order_by; // Limit the edges or the vertices? std::optional<size_t> limit; std::vector<std::string> filters; diff --git a/src/storage/v3/request_helper.cpp b/src/storage/v3/request_helper.cpp index 7dab4c5c1..60633fbbd 100644 --- a/src/storage/v3/request_helper.cpp +++ b/src/storage/v3/request_helper.cpp @@ -42,36 +42,38 @@ std::vector<Element<VertexAccessor>>::const_iterator GetStartOrderedElementsIter return ordered_elements.end(); } -std::vector<EdgeAccessor> GetEdgesFromVertex(const VertexAccessor &vertex_accessor, - const msgs::EdgeDirection direction) { +std::array<std::vector<EdgeAccessor>, 2> GetEdgesFromVertex(const VertexAccessor &vertex_accessor, + const msgs::EdgeDirection direction) { + std::vector<EdgeAccessor> in_edges; + std::vector<EdgeAccessor> out_edges; + switch (direction) { case memgraph::msgs::EdgeDirection::IN: { auto edges = vertex_accessor.InEdges(View::OLD); if (edges.HasValue()) { - return edges.GetValue(); + in_edges = edges.GetValue(); } - return {}; } case memgraph::msgs::EdgeDirection::OUT: { auto edges = vertex_accessor.OutEdges(View::OLD); if (edges.HasValue()) { - return edges.GetValue(); + out_edges = edges.GetValue(); } - return {}; } case memgraph::msgs::EdgeDirection::BOTH: { auto maybe_in_edges = vertex_accessor.InEdges(View::OLD); auto maybe_out_edges = vertex_accessor.OutEdges(View::OLD); std::vector<EdgeAccessor> edges; if (maybe_in_edges.HasValue()) { - edges = maybe_in_edges.GetValue(); + in_edges = maybe_in_edges.GetValue(); } if (maybe_out_edges.HasValue()) { - edges.insert(edges.end(), maybe_out_edges.GetValue().begin(), maybe_out_edges.GetValue().end()); + out_edges = maybe_out_edges.GetValue(); } - return edges; } } + + return std::array<std::vector<EdgeAccessor>, 2>{in_edges, out_edges}; } } // namespace memgraph::storage::v3 diff --git a/src/storage/v3/request_helper.hpp b/src/storage/v3/request_helper.hpp index 6212c4a69..63758cf63 100644 --- a/src/storage/v3/request_helper.hpp +++ b/src/storage/v3/request_helper.hpp @@ -163,5 +163,6 @@ std::vector<Element<VertexAccessor>>::const_iterator GetStartOrderedElementsIter const std::vector<Element<VertexAccessor>> &ordered_elements, const std::vector<PropertyValue> &start_ids, View view); -std::vector<EdgeAccessor> GetEdgesFromVertex(const VertexAccessor &vertex_accessor, msgs::EdgeDirection direction); +std::array<std::vector<EdgeAccessor>, 2> GetEdgesFromVertex(const VertexAccessor &vertex_accessor, + msgs::EdgeDirection direction); } // namespace memgraph::storage::v3 diff --git a/src/storage/v3/shard_rsm.cpp b/src/storage/v3/shard_rsm.cpp index 97b275530..5f30692ec 100644 --- a/src/storage/v3/shard_rsm.cpp +++ b/src/storage/v3/shard_rsm.cpp @@ -354,6 +354,41 @@ std::optional<msgs::ExpandOneResultRow> GetExpandOneResult( return result_row; } +std::optional<msgs::ExpandOneResultRow> GetExpandOneResult2( + VertexAccessor v_acc, msgs::VertexId src_vertex, const msgs::ExpandOneRequest &req, + std::vector<EdgeAccessor> in_edge_accessors, std::vector<EdgeAccessor> out_edge_accessors, + const EdgeUniqunessFunction &maybe_filter_based_on_edge_uniquness, const EdgeFiller &edge_filler) { + /// Fill up source vertex + auto source_vertex = FillUpSourceVertex(v_acc, req, src_vertex); + if (!source_vertex) { + return std::nullopt; + } + + /// Fill up source vertex properties + auto src_vertex_properties = FillUpSourceVertexProperties(v_acc, req); + if (!src_vertex_properties) { + return std::nullopt; + } + + /// Fill up connecting edges + auto in_edges = maybe_filter_based_on_edge_uniquness(std::move(in_edge_accessors), msgs::EdgeDirection::IN); + auto out_edges = maybe_filter_based_on_edge_uniquness(std::move(out_edge_accessors), msgs::EdgeDirection::OUT); + + msgs::ExpandOneResultRow result_row; + result_row.src_vertex = std::move(*source_vertex); + result_row.src_vertex_properties = std::move(*src_vertex_properties); + static constexpr bool kInEdges = true; + static constexpr bool kOutEdges = false; + if (!in_edges.empty() && !FillEdges<kInEdges>(in_edges, req, result_row, edge_filler)) { + return std::nullopt; + } + if (!out_edges.empty() && !FillEdges<kOutEdges>(out_edges, req, result_row, edge_filler)) { + return std::nullopt; + } + + return result_row; +} + EdgeUniqunessFunction InitializeEdgeUniqunessFunction(bool only_unique_neighbor_rows) { // Functions to select connecting edges based on uniquness EdgeUniqunessFunction maybe_filter_based_on_edge_uniquness; @@ -862,57 +897,67 @@ msgs::ReadResponses ShardRsm::HandleRead(msgs::ExpandOneRequest &&req) { auto maybe_filter_based_on_edge_uniquness = InitializeEdgeUniqunessFunction(req.only_unique_neighbor_rows); auto edge_filler = InitializeEdgeFillerFunction(req); - //#NoCommit code below is duplicated, one needs to factor it once it's clear - if (req.order_by) { + //#NoCommit code below is duplicated, one needs to factor it once it's clear! + if (!req.order_by.empty()) { auto vertex_iterable = acc.Vertices(View::OLD); - const auto ordered_vertices = OrderByElements<VertexAccessor>(acc, dba, vertex_iterable, *req.order_by); - std::vector<std::pair<VertexAccessor, std::vector<Element<EdgeAccessor>>>> vertex_ordered_edges; - vertex_ordered_edges.reserve(req.src_vertices.size()); + const auto ordered_vertices = OrderByElements<VertexAccessor>(acc, dba, vertex_iterable, req.order_by); - for (auto &src_vertex : req.src_vertices) { + for (auto &ordered_vertice : ordered_vertices) { // Get Vertex acc - auto src_vertex_acc_opt = acc.FindVertex(ConvertPropertyVector((src_vertex.second)), View::NEW); - if (!src_vertex_acc_opt) { - action_successful = false; - spdlog::debug("Encountered an error while trying to obtain VertexAccessor. Transaction id: {}", - req.transaction_id.logical_id); - break; - } + auto src_vertex_acc = ordered_vertice.object_acc; if (!req.filters.empty()) { // NOTE - DbAccessor might get removed in the future. - auto dba = DbAccessor{&acc}; - const bool eval = FilterOnVertex(dba, src_vertex_acc_opt.value(), req.filters, expr::identifier_node_symbol); + const bool eval = FilterOnVertex(dba, src_vertex_acc, req.filters, expr::identifier_node_symbol); if (!eval) { continue; } } - auto result = GetExpandOneResult(acc, src_vertex, req, maybe_filter_based_on_edge_uniquness, edge_filler); - if (!result) { + auto [in_edge_accessors, out_edge_accessors] = GetEdgesFromVertex(src_vertex_acc, req.direction); + const auto in_ordered_edges = OrderByElements<EdgeAccessor>(acc, dba, in_edge_accessors, req.order_by); + const auto out_ordered_edges = OrderByElements<EdgeAccessor>(acc, dba, out_edge_accessors, req.order_by); + + std::vector<EdgeAccessor> in_edge_ordered_accessors; + std::transform(in_ordered_edges.begin(), in_ordered_edges.end(), std::back_inserter(in_edge_ordered_accessors), + [](auto &edge_element) { return edge_element.object_acc; }); + + std::vector<EdgeAccessor> out_edge_ordered_accessors; + std::transform(out_ordered_edges.begin(), out_ordered_edges.end(), std::back_inserter(out_edge_ordered_accessors), + [](auto &edge_element) { return edge_element.object_acc; }); + + auto label_id = src_vertex_acc.PrimaryLabel(View::NEW); + if (label_id.HasError()) { + action_successful = false; + break; + } + auto primary_key = src_vertex_acc.PrimaryKey(View::NEW); + if (primary_key.HasError()) { action_successful = false; break; } - results.emplace_back(result.value()); - if (batch_limit) { // #NoCommit can ebd one differently + msgs::VertexId src_vertice; + src_vertice.first = msgs::Label{.id = *label_id}; + src_vertice.second = conversions::ConvertValueVector(*primary_key); + auto maybe_result = + GetExpandOneResult2(src_vertex_acc, src_vertice, req, in_edge_ordered_accessors, out_edge_ordered_accessors, + maybe_filter_based_on_edge_uniquness, edge_filler); + + if (!maybe_result) { + action_successful = false; + break; + } + + results.emplace_back(maybe_result.value()); + + if (batch_limit) { // #NoCommit can be done differently --*batch_limit; if (batch_limit < 0) { break; } } } - // Now we have to construct response like this, and here we will care about - // limit: - // v1 e1 - // v1 e2 - // v1 e3 - // v2 e4 - // v2 e5 - // v2 e6 - // v2 e7 - // v3 e8 - // v3 e9 } else { for (auto &src_vertex : req.src_vertices) { // Get Vertex acc diff --git a/tests/simulation/shard_rsm.cpp b/tests/simulation/shard_rsm.cpp index 6c3ea7d60..8883e4952 100644 --- a/tests/simulation/shard_rsm.cpp +++ b/tests/simulation/shard_rsm.cpp @@ -531,7 +531,7 @@ void AttemptToExpandOneWithWrongEdgeType(ShardClient &client, uint64_t src_verte std::optional<std::vector<PropertyId>> edge_properties = {}; std::vector<std::string> expressions; - std::optional<std::vector<msgs::OrderBy>> order_by = {}; + std::vector<msgs::OrderBy> order_by = {}; std::optional<size_t> limit = {}; std::vector<std::string> filter = {}; @@ -586,7 +586,7 @@ void AttemptToExpandOneSimple(ShardClient &client, uint64_t src_vertex_val, Edge std::optional<std::vector<PropertyId>> edge_properties = {}; std::vector<std::string> expressions; - std::optional<std::vector<msgs::OrderBy>> order_by = {}; + std::vector<msgs::OrderBy> order_by = {}; std::optional<size_t> limit = {}; std::vector<std::string> filter = {}; @@ -642,7 +642,7 @@ void AttemptToExpandOneWithUniqueEdges(ShardClient &client, uint64_t src_vertex_ std::optional<std::vector<PropertyId>> edge_properties = {}; std::vector<std::string> expressions; - std::optional<std::vector<msgs::OrderBy>> order_by = {}; + std::vector<msgs::OrderBy> order_by = {}; std::optional<size_t> limit = {}; std::vector<std::string> filter = {}; @@ -698,9 +698,9 @@ void AttemptToExpandOneLimitAndOrderBy(ShardClient &client, uint64_t src_vertex_ // Edge properties to look for std::optional<std::vector<PropertyId>> edge_properties = {}; - std::vector<std::string> expressions; - std::optional<std::vector<msgs::OrderBy>> order_by = {}; - // std::optional<size_t> limit = 5; + std::vector<msgs::OrderBy> order_by = { + {msgs::Expression{"MG_SYMBOL_NODE.prop1"}, msgs::OrderingDirection::DESCENDING}}; + std::optional<size_t> limit = 3; // std::optional<msgs::Filter> filter = {}; msgs::ExpandOneRequest expand_one_req{}; @@ -708,9 +708,8 @@ void AttemptToExpandOneLimitAndOrderBy(ShardClient &client, uint64_t src_vertex_ expand_one_req.direction = edge_direction; expand_one_req.edge_properties = edge_properties; expand_one_req.edge_types = {edge_type}; - // expand_one_req.expressions = expressions; #NoCommit not existing? - // expand_one_req.filter = filter; - // expand_one_req.limit = limit; + // expand_one_req.filter = filter; #NoCommit later? + expand_one_req.limit = limit; expand_one_req.order_by = order_by; expand_one_req.src_vertex_properties = src_vertex_properties; expand_one_req.src_vertices = {src_vertex}; @@ -725,12 +724,9 @@ void AttemptToExpandOneLimitAndOrderBy(ShardClient &client, uint64_t src_vertex_ auto write_response_result = read_res.GetValue(); auto write_response = std::get<msgs::ExpandOneResponse>(write_response_result); MG_ASSERT(write_response.result.size() == 1); - // MG_ASSERT(write_response.result[0].edges_with_all_properties->size() == 10); // #NoCommit where does that come - // from? - // auto number_of_properties_on_edge = - // (std::get<std::map<PropertyId, msgs::Value>>(write_response.result[0].edges_with_all_properties.value()[0])) - // .size(); - // MG_ASSERT(number_of_properties_on_edge == 1); + MG_ASSERT(write_response.result[0].out_edges_with_all_properties.size() == 10); + auto number_of_properties_on_edge = write_response.result[0].out_edges_with_all_properties.size(); + MG_ASSERT(number_of_properties_on_edge == 1); break; } } @@ -756,7 +752,7 @@ void AttemptToExpandOneWithSpecifiedSrcVertexProperties(ShardClient &client, uin std::optional<std::vector<PropertyId>> edge_properties = {}; std::vector<std::string> expressions; - std::optional<std::vector<msgs::OrderBy>> order_by = {}; + std::vector<msgs::OrderBy> order_by = {}; std::optional<size_t> limit = {}; std::vector<std::string> filter = {}; @@ -816,7 +812,7 @@ void AttemptToExpandOneWithSpecifiedEdgeProperties(ShardClient &client, uint64_t std::optional<std::vector<PropertyId>> edge_properties = {specified_edge_prop}; std::vector<std::string> expressions; - std::optional<std::vector<msgs::OrderBy>> order_by = {}; + std::vector<msgs::OrderBy> order_by = {}; std::optional<size_t> limit = {}; std::vector<std::string> filter = {}; @@ -875,7 +871,7 @@ void AttemptToExpandOneWithFilters(ShardClient &client, uint64_t src_vertex_val, std::optional<std::vector<PropertyId>> edge_properties = {}; std::vector<std::string> expressions; - std::optional<std::vector<msgs::OrderBy>> order_by = {}; + std::vector<msgs::OrderBy> order_by = {}; std::optional<size_t> limit = {}; std::vector<std::string> filter = {}; From 386a0c568646478e3826870ae759bd7cadf08c53 Mon Sep 17 00:00:00 2001 From: jeremy <jeremy.bailleux@memgraph.io> Date: Mon, 24 Oct 2022 15:44:37 +0200 Subject: [PATCH 03/63] add comment --- tests/simulation/shard_rsm.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/simulation/shard_rsm.cpp b/tests/simulation/shard_rsm.cpp index 8883e4952..0fba6d74c 100644 --- a/tests/simulation/shard_rsm.cpp +++ b/tests/simulation/shard_rsm.cpp @@ -701,14 +701,17 @@ void AttemptToExpandOneLimitAndOrderBy(ShardClient &client, uint64_t src_vertex_ std::vector<msgs::OrderBy> order_by = { {msgs::Expression{"MG_SYMBOL_NODE.prop1"}, msgs::OrderingDirection::DESCENDING}}; std::optional<size_t> limit = 3; - // std::optional<msgs::Filter> filter = {}; + std::vector<std::string> filters = {}; + + // #NoCommit some filter? + // #NoCommit more orderBy? msgs::ExpandOneRequest expand_one_req{}; expand_one_req.direction = edge_direction; expand_one_req.edge_properties = edge_properties; expand_one_req.edge_types = {edge_type}; - // expand_one_req.filter = filter; #NoCommit later? + expand_one_req.filters = filters; expand_one_req.limit = limit; expand_one_req.order_by = order_by; expand_one_req.src_vertex_properties = src_vertex_properties; From 51e6802aa74f9e59a2f7941c039640ed883e7537 Mon Sep 17 00:00:00 2001 From: jeremy <jeremy.bailleux@memgraph.io> Date: Mon, 24 Oct 2022 15:44:57 +0200 Subject: [PATCH 04/63] Safeguard in case ComputeExpression is called without opt --- src/storage/v3/expr.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/storage/v3/expr.cpp b/src/storage/v3/expr.cpp index 1d6739433..eaff472bd 100644 --- a/src/storage/v3/expr.cpp +++ b/src/storage/v3/expr.cpp @@ -186,7 +186,7 @@ TypedValue ComputeExpression(DbAccessor &dba, const std::optional<memgraph::stor expr::SymbolGenerator symbol_generator(&symbol_table, identifiers); (std::any_cast<Expression *>(expr))->Accept(symbol_generator); - if (node_identifier.symbol_pos_ != -1) { + if (node_identifier.symbol_pos_ != -1 && v_acc.has_value()) { MG_ASSERT(std::find_if(symbol_table.table().begin(), symbol_table.table().end(), [&node_name](const std::pair<int32_t, Symbol> &position_symbol_pair) { return position_symbol_pair.second.name() == node_name; @@ -195,7 +195,7 @@ TypedValue ComputeExpression(DbAccessor &dba, const std::optional<memgraph::stor frame[symbol_table.at(node_identifier)] = *v_acc; } - if (edge_identifier.symbol_pos_ != -1) { + if (edge_identifier.symbol_pos_ != -1 && e_acc.has_value()) { MG_ASSERT(std::find_if(symbol_table.table().begin(), symbol_table.table().end(), [&edge_name](const std::pair<int32_t, Symbol> &position_symbol_pair) { return position_symbol_pair.second.name() == edge_name; From 8b9e7e2c65010e15012ec2d52cd939ec29c4c8fc Mon Sep 17 00:00:00 2001 From: jeremy <jeremy.bailleux@memgraph.io> Date: Mon, 24 Oct 2022 16:23:42 +0200 Subject: [PATCH 05/63] Correct behavior of batch limit (was size_t) --- src/storage/v3/shard_rsm.cpp | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/src/storage/v3/shard_rsm.cpp b/src/storage/v3/shard_rsm.cpp index 5f30692ec..7b64731d9 100644 --- a/src/storage/v3/shard_rsm.cpp +++ b/src/storage/v3/shard_rsm.cpp @@ -950,12 +950,8 @@ msgs::ReadResponses ShardRsm::HandleRead(msgs::ExpandOneRequest &&req) { } results.emplace_back(maybe_result.value()); - - if (batch_limit) { // #NoCommit can be done differently - --*batch_limit; - if (batch_limit < 0) { - break; - } + if (batch_limit.has_value() && results.size() >= batch_limit.value()) { + break; } } } else { @@ -985,11 +981,8 @@ msgs::ReadResponses ShardRsm::HandleRead(msgs::ExpandOneRequest &&req) { } results.emplace_back(result.value()); - if (batch_limit) { // #NoCommit can ebd one differently - --*batch_limit; - if (batch_limit < 0) { - break; - } + if (batch_limit.has_value() && results.size() >= batch_limit.value()) { + break; } } } From 33c9ccee663dfa0c98dd6f72e8755e7e02d39d29 Mon Sep 17 00:00:00 2001 From: jeremy <jeremy.bailleux@memgraph.io> Date: Mon, 24 Oct 2022 16:38:28 +0200 Subject: [PATCH 06/63] Adapt test --- tests/simulation/shard_rsm.cpp | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/tests/simulation/shard_rsm.cpp b/tests/simulation/shard_rsm.cpp index 0fba6d74c..0805ace17 100644 --- a/tests/simulation/shard_rsm.cpp +++ b/tests/simulation/shard_rsm.cpp @@ -701,10 +701,7 @@ void AttemptToExpandOneLimitAndOrderBy(ShardClient &client, uint64_t src_vertex_ std::vector<msgs::OrderBy> order_by = { {msgs::Expression{"MG_SYMBOL_NODE.prop1"}, msgs::OrderingDirection::DESCENDING}}; std::optional<size_t> limit = 3; - std::vector<std::string> filters = {}; - - // #NoCommit some filter? - // #NoCommit more orderBy? + std::vector<std::string> filters = {"MG_SYMBOL_NODE.prop1 != " + std::to_string(src_vertex_val)}; msgs::ExpandOneRequest expand_one_req{}; @@ -726,10 +723,23 @@ void AttemptToExpandOneLimitAndOrderBy(ShardClient &client, uint64_t src_vertex_ auto write_response_result = read_res.GetValue(); auto write_response = std::get<msgs::ExpandOneResponse>(write_response_result); - MG_ASSERT(write_response.result.size() == 1); - MG_ASSERT(write_response.result[0].out_edges_with_all_properties.size() == 10); - auto number_of_properties_on_edge = write_response.result[0].out_edges_with_all_properties.size(); - MG_ASSERT(number_of_properties_on_edge == 1); + + // We check that we do not have more results than the limit. Based on the data in the graph, we know that we should + // receive exactly limit responses. + MG_ASSERT(write_response.result.size() == limit); + + // We also check that the vertices are ordered by prop1 DESC + + std::is_sorted(write_response.result.cbegin(), write_response.result.cend(), + [](const auto &vertex, const auto &other_vertex) { + const auto primary_key = vertex.src_vertex.id.second; + const auto other_primary_key = other_vertex.src_vertex.id.second; + + MG_ASSERT(primary_key.size() == 1); + MG_ASSERT(other_primary_key.size() == 1); + return primary_key[0].int_v > other_primary_key[0].int_v; + }); + break; } } @@ -1090,9 +1100,9 @@ void TestExpandOneGraphOne(ShardClient &client) { MG_ASSERT(AttemptToAddEdgeWithProperties(client, unique_prop_val_2, unique_prop_val_3, edge_id, edge_prop_id, edge_prop_val, {edge_type_id})); }); - AttemptToExpandOneLimitAndOrderBy(client, unique_prop_val_2, edge_type_id); // #NoCommit AttemptToExpandOneSimple(client, unique_prop_val_1, edge_type_id); + AttemptToExpandOneLimitAndOrderBy(client, unique_prop_val_2, edge_type_id); AttemptToExpandOneWithWrongEdgeType(client, unique_prop_val_1, wrong_edge_type_id); AttemptToExpandOneWithSpecifiedSrcVertexProperties(client, unique_prop_val_1, edge_type_id); AttemptToExpandOneWithSpecifiedEdgeProperties(client, unique_prop_val_1, edge_type_id, edge_prop_id); From 862af552667bc5814600d3a9d2106fb1ed077b77 Mon Sep 17 00:00:00 2001 From: jeremy <jeremy.bailleux@memgraph.io> Date: Mon, 24 Oct 2022 16:39:38 +0200 Subject: [PATCH 07/63] Remove #NoCommit --- src/storage/v3/request_helper.hpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/storage/v3/request_helper.hpp b/src/storage/v3/request_helper.hpp index 63758cf63..86cff9bea 100644 --- a/src/storage/v3/request_helper.hpp +++ b/src/storage/v3/request_helper.hpp @@ -14,7 +14,7 @@ #include <vector> #include "ast/ast.hpp" -#include "pretty_print_ast_to_original_expression.hpp" // #NoCommit why like this? +#include "pretty_print_ast_to_original_expression.hpp" #include "query/v2/requests.hpp" #include "storage/v3/bindings/typed_value.hpp" #include "storage/v3/edge_accessor.hpp" @@ -112,7 +112,6 @@ struct Element { TObjectAccessor object_acc; }; -// #NoCommit in cpp template <ObjectAccessor TObjectAccessor, typename TIterable> std::vector<Element<TObjectAccessor>> OrderByElements(Shard::Accessor &acc, DbAccessor &dba, TIterable &iterable, std::vector<msgs::OrderBy> &order_bys) { From e901c1fdb78abfe25becb4ec428af3409e221ea2 Mon Sep 17 00:00:00 2001 From: jeremy <jeremy.bailleux@memgraph.io> Date: Tue, 25 Oct 2022 10:45:33 +0200 Subject: [PATCH 08/63] Refactor code --- src/storage/v3/shard_rsm.cpp | 231 +++++++++++++++++++---------------- 1 file changed, 126 insertions(+), 105 deletions(-) diff --git a/src/storage/v3/shard_rsm.cpp b/src/storage/v3/shard_rsm.cpp index 7b64731d9..cae2c93d7 100644 --- a/src/storage/v3/shard_rsm.cpp +++ b/src/storage/v3/shard_rsm.cpp @@ -354,7 +354,7 @@ std::optional<msgs::ExpandOneResultRow> GetExpandOneResult( return result_row; } -std::optional<msgs::ExpandOneResultRow> GetExpandOneResult2( +std::optional<msgs::ExpandOneResultRow> GetExpandOneResult( VertexAccessor v_acc, msgs::VertexId src_vertex, const msgs::ExpandOneRequest &req, std::vector<EdgeAccessor> in_edge_accessors, std::vector<EdgeAccessor> out_edge_accessors, const EdgeUniqunessFunction &maybe_filter_based_on_edge_uniquness, const EdgeFiller &edge_filler) { @@ -494,6 +494,129 @@ EdgeFiller InitializeEdgeFillerFunction(const msgs::ExpandOneRequest &req) { return edge_filler; } +msgs::ReadResponses HandleReadWithOrderBy(msgs::ExpandOneRequest &&req, Shard::Accessor &&acc) { + bool action_successful = true; + + std::vector<msgs::ExpandOneResultRow> results; + auto batch_limit = req.limit; + auto dba = DbAccessor{&acc}; + + auto maybe_filter_based_on_edge_uniquness = InitializeEdgeUniqunessFunction(req.only_unique_neighbor_rows); + auto edge_filler = InitializeEdgeFillerFunction(req); + + auto vertex_iterable = acc.Vertices(View::OLD); + const auto ordered_vertices = OrderByElements<VertexAccessor>(acc, dba, vertex_iterable, req.order_by); + + for (const auto &ordered_vertice : ordered_vertices) { + // Get Vertex acc + auto src_vertex_acc = ordered_vertice.object_acc; + + if (!req.filters.empty()) { + // NOTE - DbAccessor might get removed in the future. + const bool eval = FilterOnVertex(dba, src_vertex_acc, req.filters, expr::identifier_node_symbol); + if (!eval) { + continue; + } + } + + auto [in_edge_accessors, out_edge_accessors] = GetEdgesFromVertex(src_vertex_acc, req.direction); + const auto in_ordered_edges = OrderByElements<EdgeAccessor>(acc, dba, in_edge_accessors, req.order_by); + const auto out_ordered_edges = OrderByElements<EdgeAccessor>(acc, dba, out_edge_accessors, req.order_by); + + std::vector<EdgeAccessor> in_edge_ordered_accessors; + std::transform(in_ordered_edges.begin(), in_ordered_edges.end(), std::back_inserter(in_edge_ordered_accessors), + [](auto &edge_element) { return edge_element.object_acc; }); + + std::vector<EdgeAccessor> out_edge_ordered_accessors; + std::transform(out_ordered_edges.begin(), out_ordered_edges.end(), std::back_inserter(out_edge_ordered_accessors), + [](auto &edge_element) { return edge_element.object_acc; }); + + auto label_id = src_vertex_acc.PrimaryLabel(View::NEW); + if (label_id.HasError()) { + action_successful = false; + break; + } + auto primary_key = src_vertex_acc.PrimaryKey(View::NEW); + if (primary_key.HasError()) { + action_successful = false; + break; + } + + msgs::VertexId src_vertice(msgs::Label{.id = *label_id}, conversions::ConvertValueVector(*primary_key)); + auto maybe_result = + GetExpandOneResult(src_vertex_acc, src_vertice, req, in_edge_ordered_accessors, out_edge_ordered_accessors, + maybe_filter_based_on_edge_uniquness, edge_filler); + + if (!maybe_result) { + action_successful = false; + break; + } + + results.emplace_back(maybe_result.value()); + if (batch_limit.has_value() && results.size() >= batch_limit.value()) { + break; + } + } + + msgs::ExpandOneResponse resp{}; + resp.success = action_successful; + if (action_successful) { + resp.result = std::move(results); + } + + return resp; +} + +msgs::ReadResponses HandleReadWithoutOrderBy(msgs::ExpandOneRequest &&req, Shard::Accessor &&acc) { + bool action_successful = true; + + std::vector<msgs::ExpandOneResultRow> results; + auto batch_limit = req.limit; + auto dba = DbAccessor{&acc}; + + auto maybe_filter_based_on_edge_uniquness = InitializeEdgeUniqunessFunction(req.only_unique_neighbor_rows); + auto edge_filler = InitializeEdgeFillerFunction(req); + + for (auto &src_vertex : req.src_vertices) { + // Get Vertex acc + auto src_vertex_acc_opt = acc.FindVertex(ConvertPropertyVector((src_vertex.second)), View::NEW); + if (!src_vertex_acc_opt) { + action_successful = false; + spdlog::debug("Encountered an error while trying to obtain VertexAccessor. Transaction id: {}", + req.transaction_id.logical_id); + break; + } + + if (!req.filters.empty()) { + // NOTE - DbAccessor might get removed in the future. + auto dba = DbAccessor{&acc}; + const bool eval = FilterOnVertex(dba, src_vertex_acc_opt.value(), req.filters, expr::identifier_node_symbol); + if (!eval) { + continue; + } + } + auto result = GetExpandOneResult(acc, src_vertex, req, maybe_filter_based_on_edge_uniquness, edge_filler); + + if (!result) { + action_successful = false; + break; + } + + results.emplace_back(result.value()); + if (batch_limit.has_value() && results.size() >= batch_limit.value()) { + break; + } + } + + msgs::ExpandOneResponse resp{}; + resp.success = action_successful; + if (action_successful) { + resp.result = std::move(results); + } + + return resp; +} + }; // namespace msgs::WriteResponses ShardRsm::ApplyWrite(msgs::CreateVerticesRequest &&req) { auto acc = shard_->Access(req.transaction_id); @@ -887,113 +1010,11 @@ msgs::ReadResponses ShardRsm::HandleRead(msgs::ScanVerticesRequest &&req) { } msgs::ReadResponses ShardRsm::HandleRead(msgs::ExpandOneRequest &&req) { - auto acc = shard_->Access(req.transaction_id); - bool action_successful = true; - - std::vector<msgs::ExpandOneResultRow> results; - auto batch_limit = req.limit; - auto dba = DbAccessor{&acc}; - - auto maybe_filter_based_on_edge_uniquness = InitializeEdgeUniqunessFunction(req.only_unique_neighbor_rows); - auto edge_filler = InitializeEdgeFillerFunction(req); - - //#NoCommit code below is duplicated, one needs to factor it once it's clear! if (!req.order_by.empty()) { - auto vertex_iterable = acc.Vertices(View::OLD); - const auto ordered_vertices = OrderByElements<VertexAccessor>(acc, dba, vertex_iterable, req.order_by); - - for (auto &ordered_vertice : ordered_vertices) { - // Get Vertex acc - auto src_vertex_acc = ordered_vertice.object_acc; - - if (!req.filters.empty()) { - // NOTE - DbAccessor might get removed in the future. - const bool eval = FilterOnVertex(dba, src_vertex_acc, req.filters, expr::identifier_node_symbol); - if (!eval) { - continue; - } - } - - auto [in_edge_accessors, out_edge_accessors] = GetEdgesFromVertex(src_vertex_acc, req.direction); - const auto in_ordered_edges = OrderByElements<EdgeAccessor>(acc, dba, in_edge_accessors, req.order_by); - const auto out_ordered_edges = OrderByElements<EdgeAccessor>(acc, dba, out_edge_accessors, req.order_by); - - std::vector<EdgeAccessor> in_edge_ordered_accessors; - std::transform(in_ordered_edges.begin(), in_ordered_edges.end(), std::back_inserter(in_edge_ordered_accessors), - [](auto &edge_element) { return edge_element.object_acc; }); - - std::vector<EdgeAccessor> out_edge_ordered_accessors; - std::transform(out_ordered_edges.begin(), out_ordered_edges.end(), std::back_inserter(out_edge_ordered_accessors), - [](auto &edge_element) { return edge_element.object_acc; }); - - auto label_id = src_vertex_acc.PrimaryLabel(View::NEW); - if (label_id.HasError()) { - action_successful = false; - break; - } - auto primary_key = src_vertex_acc.PrimaryKey(View::NEW); - if (primary_key.HasError()) { - action_successful = false; - break; - } - - msgs::VertexId src_vertice; - src_vertice.first = msgs::Label{.id = *label_id}; - src_vertice.second = conversions::ConvertValueVector(*primary_key); - auto maybe_result = - GetExpandOneResult2(src_vertex_acc, src_vertice, req, in_edge_ordered_accessors, out_edge_ordered_accessors, - maybe_filter_based_on_edge_uniquness, edge_filler); - - if (!maybe_result) { - action_successful = false; - break; - } - - results.emplace_back(maybe_result.value()); - if (batch_limit.has_value() && results.size() >= batch_limit.value()) { - break; - } - } + return HandleReadWithOrderBy(std::move(req), shard_->Access(req.transaction_id)); } else { - for (auto &src_vertex : req.src_vertices) { - // Get Vertex acc - auto src_vertex_acc_opt = acc.FindVertex(ConvertPropertyVector((src_vertex.second)), View::NEW); - if (!src_vertex_acc_opt) { - action_successful = false; - spdlog::debug("Encountered an error while trying to obtain VertexAccessor. Transaction id: {}", - req.transaction_id.logical_id); - break; - } - - if (!req.filters.empty()) { - // NOTE - DbAccessor might get removed in the future. - auto dba = DbAccessor{&acc}; - const bool eval = FilterOnVertex(dba, src_vertex_acc_opt.value(), req.filters, expr::identifier_node_symbol); - if (!eval) { - continue; - } - } - auto result = GetExpandOneResult(acc, src_vertex, req, maybe_filter_based_on_edge_uniquness, edge_filler); - - if (!result) { - action_successful = false; - break; - } - - results.emplace_back(result.value()); - if (batch_limit.has_value() && results.size() >= batch_limit.value()) { - break; - } - } + return HandleReadWithoutOrderBy(std::move(req), shard_->Access(req.transaction_id)); } - - msgs::ExpandOneResponse resp{}; - resp.success = action_successful; - if (action_successful) { - resp.result = std::move(results); - } - - return resp; } msgs::WriteResponses ShardRsm::ApplyWrite(msgs::CommitRequest &&req) { From b4f68e7a60d92ccdfbe972d4fd9557b6fbab240e Mon Sep 17 00:00:00 2001 From: jeremy <jeremy.bailleux@memgraph.io> Date: Tue, 25 Oct 2022 10:56:16 +0200 Subject: [PATCH 09/63] remove includes --- src/storage/v3/shard_rsm.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/storage/v3/shard_rsm.cpp b/src/storage/v3/shard_rsm.cpp index cae2c93d7..054900e57 100644 --- a/src/storage/v3/shard_rsm.cpp +++ b/src/storage/v3/shard_rsm.cpp @@ -15,7 +15,6 @@ #include <unordered_set> #include <utility> -#include <unordered_map> #include "parser/opencypher/parser.hpp" #include "query/v2/requests.hpp" #include "storage/v3/bindings/ast/ast.hpp" From 18423ce34d74f7a32118092192336130667b1cf2 Mon Sep 17 00:00:00 2001 From: jeremy <jeremy.bailleux@memgraph.io> Date: Tue, 25 Oct 2022 11:01:04 +0200 Subject: [PATCH 10/63] remove includes --- src/storage/v3/shard_rsm.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/storage/v3/shard_rsm.cpp b/src/storage/v3/shard_rsm.cpp index 054900e57..1350b0e39 100644 --- a/src/storage/v3/shard_rsm.cpp +++ b/src/storage/v3/shard_rsm.cpp @@ -26,7 +26,6 @@ #include "storage/v3/bindings/symbol_generator.hpp" #include "storage/v3/bindings/symbol_table.hpp" #include "storage/v3/bindings/typed_value.hpp" -#include "storage/v3/edge_accessor.hpp" #include "storage/v3/expr.hpp" #include "storage/v3/id_types.hpp" #include "storage/v3/key_store.hpp" From c1d0fddaac1213deb7a4b1bf0cf8c3eab98815c5 Mon Sep 17 00:00:00 2001 From: jeremy <jeremy.bailleux@memgraph.io> Date: Tue, 25 Oct 2022 11:31:23 +0200 Subject: [PATCH 11/63] Remove unnecessary else --- src/storage/v3/shard_rsm.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/storage/v3/shard_rsm.cpp b/src/storage/v3/shard_rsm.cpp index 1350b0e39..b8dadbb7b 100644 --- a/src/storage/v3/shard_rsm.cpp +++ b/src/storage/v3/shard_rsm.cpp @@ -1010,9 +1010,9 @@ msgs::ReadResponses ShardRsm::HandleRead(msgs::ScanVerticesRequest &&req) { msgs::ReadResponses ShardRsm::HandleRead(msgs::ExpandOneRequest &&req) { if (!req.order_by.empty()) { return HandleReadWithOrderBy(std::move(req), shard_->Access(req.transaction_id)); - } else { + } else + return HandleReadWithoutOrderBy(std::move(req), shard_->Access(req.transaction_id)); - } } msgs::WriteResponses ShardRsm::ApplyWrite(msgs::CommitRequest &&req) { From cdab8828e42bb3083fc782ff0092c569b07a88fa Mon Sep 17 00:00:00 2001 From: jeremy <jeremy.bailleux@memgraph.io> Date: Tue, 25 Oct 2022 12:30:14 +0200 Subject: [PATCH 12/63] remove else --- src/storage/v3/shard_rsm.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/storage/v3/shard_rsm.cpp b/src/storage/v3/shard_rsm.cpp index b8dadbb7b..700433541 100644 --- a/src/storage/v3/shard_rsm.cpp +++ b/src/storage/v3/shard_rsm.cpp @@ -1010,9 +1010,9 @@ msgs::ReadResponses ShardRsm::HandleRead(msgs::ScanVerticesRequest &&req) { msgs::ReadResponses ShardRsm::HandleRead(msgs::ExpandOneRequest &&req) { if (!req.order_by.empty()) { return HandleReadWithOrderBy(std::move(req), shard_->Access(req.transaction_id)); - } else + } - return HandleReadWithoutOrderBy(std::move(req), shard_->Access(req.transaction_id)); + return HandleReadWithoutOrderBy(std::move(req), shard_->Access(req.transaction_id)); } msgs::WriteResponses ShardRsm::ApplyWrite(msgs::CommitRequest &&req) { From 79c2ae206fe3e62b89e84ffb3b2e11e3cc5b3b6d Mon Sep 17 00:00:00 2001 From: jeremy <jeremy.bailleux@memgraph.io> Date: Fri, 28 Oct 2022 11:24:41 +0200 Subject: [PATCH 13/63] Update FillEdges usage (for compilation) --- src/storage/v3/shard_rsm.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/storage/v3/shard_rsm.cpp b/src/storage/v3/shard_rsm.cpp index 6ebd0374e..69429493c 100644 --- a/src/storage/v3/shard_rsm.cpp +++ b/src/storage/v3/shard_rsm.cpp @@ -367,10 +367,10 @@ std::optional<msgs::ExpandOneResultRow> GetExpandOneResult( result_row.src_vertex_properties = std::move(*src_vertex_properties); static constexpr bool kInEdges = true; static constexpr bool kOutEdges = false; - if (!in_edges.empty() && !FillEdges<kInEdges>(in_edges, req, result_row, edge_filler)) { + if (!in_edges.empty() && !FillEdges<kInEdges>(in_edges, result_row, edge_filler)) { return std::nullopt; } - if (!out_edges.empty() && !FillEdges<kOutEdges>(out_edges, req, result_row, edge_filler)) { + if (!out_edges.empty() && !FillEdges<kOutEdges>(out_edges, result_row, edge_filler)) { return std::nullopt; } From d0b8b27c29b09025b71bd81bb00ed8e9548aa636 Mon Sep 17 00:00:00 2001 From: jeremy <jeremy.bailleux@memgraph.io> Date: Fri, 28 Oct 2022 12:52:37 +0200 Subject: [PATCH 14/63] Rename ordered->sorted --- src/storage/v3/shard_rsm.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/storage/v3/shard_rsm.cpp b/src/storage/v3/shard_rsm.cpp index 69429493c..b4078aaad 100644 --- a/src/storage/v3/shard_rsm.cpp +++ b/src/storage/v3/shard_rsm.cpp @@ -493,9 +493,9 @@ msgs::ReadResponses HandleReadWithOrderBy(msgs::ExpandOneRequest &&req, Shard::A auto edge_filler = InitializeEdgeFillerFunction(req); auto vertex_iterable = acc.Vertices(View::OLD); - const auto ordered_vertices = OrderByElements<VertexAccessor>(acc, dba, vertex_iterable, req.order_by); + const auto sorted_vertices = OrderByElements<VertexAccessor>(acc, dba, vertex_iterable, req.order_by); - for (const auto &ordered_vertice : ordered_vertices) { + for (const auto &ordered_vertice : sorted_vertices) { // Get Vertex acc auto src_vertex_acc = ordered_vertice.object_acc; From 009c1b40741bb98d645015b74200870eb6c793fe Mon Sep 17 00:00:00 2001 From: jeremy <jeremy.bailleux@memgraph.io> Date: Fri, 28 Oct 2022 12:55:23 +0200 Subject: [PATCH 15/63] Replace include --- src/storage/v3/request_helper.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/storage/v3/request_helper.hpp b/src/storage/v3/request_helper.hpp index 86cff9bea..c291e63d4 100644 --- a/src/storage/v3/request_helper.hpp +++ b/src/storage/v3/request_helper.hpp @@ -14,8 +14,8 @@ #include <vector> #include "ast/ast.hpp" -#include "pretty_print_ast_to_original_expression.hpp" #include "query/v2/requests.hpp" +#include "storage/v3/bindings/pretty_print_ast_to_original_expression.hpp" #include "storage/v3/bindings/typed_value.hpp" #include "storage/v3/edge_accessor.hpp" #include "storage/v3/expr.hpp" From 74181114c2d6b6951816184401e608bb92da2e8e Mon Sep 17 00:00:00 2001 From: jeremy <jeremy.bailleux@memgraph.io> Date: Fri, 28 Oct 2022 12:55:47 +0200 Subject: [PATCH 16/63] Remove un-necessary variable --- src/storage/v3/shard_rsm.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/storage/v3/shard_rsm.cpp b/src/storage/v3/shard_rsm.cpp index b4078aaad..105b3b3b4 100644 --- a/src/storage/v3/shard_rsm.cpp +++ b/src/storage/v3/shard_rsm.cpp @@ -577,7 +577,6 @@ msgs::ReadResponses HandleReadWithoutOrderBy(msgs::ExpandOneRequest &&req, Shard if (!req.filters.empty()) { // NOTE - DbAccessor might get removed in the future. - auto dba = DbAccessor{&acc}; const bool eval = FilterOnVertex(dba, src_vertex_acc_opt.value(), req.filters, expr::identifier_node_symbol); if (!eval) { continue; From 0d5ee49e19006bd82f70cf4a589e1dfa7ddd065e Mon Sep 17 00:00:00 2001 From: jeremy <jeremy.bailleux@memgraph.io> Date: Fri, 28 Oct 2022 12:55:56 +0200 Subject: [PATCH 17/63] Correct test expectation --- tests/simulation/shard_rsm.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/simulation/shard_rsm.cpp b/tests/simulation/shard_rsm.cpp index da4df9f34..ef15146af 100644 --- a/tests/simulation/shard_rsm.cpp +++ b/tests/simulation/shard_rsm.cpp @@ -726,7 +726,8 @@ void AttemptToExpandOneLimitAndOrderBy(ShardClient &client, uint64_t src_vertex_ // We check that we do not have more results than the limit. Based on the data in the graph, we know that we should // receive exactly limit responses. - MG_ASSERT(write_response.result.size() == limit); + auto expected_number_of_rows = std::min(expand_one_req.src_vertices.size(), limit); + MG_ASSERT(write_response.result.size() == expected_number_of_rows); // We also check that the vertices are ordered by prop1 DESC From 46388ad35c3a7a86bff0f536a720cb762e126cc7 Mon Sep 17 00:00:00 2001 From: jeremy <jeremy.bailleux@memgraph.io> Date: Fri, 28 Oct 2022 13:10:00 +0200 Subject: [PATCH 18/63] Correct compilation --- tests/simulation/shard_rsm.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/simulation/shard_rsm.cpp b/tests/simulation/shard_rsm.cpp index ef15146af..dac9c26b2 100644 --- a/tests/simulation/shard_rsm.cpp +++ b/tests/simulation/shard_rsm.cpp @@ -700,7 +700,7 @@ void AttemptToExpandOneLimitAndOrderBy(ShardClient &client, uint64_t src_vertex_ std::vector<msgs::OrderBy> order_by = { {msgs::Expression{"MG_SYMBOL_NODE.prop1"}, msgs::OrderingDirection::DESCENDING}}; - std::optional<size_t> limit = 3; + size_t limit = 3; std::vector<std::string> filters = {"MG_SYMBOL_NODE.prop1 != " + std::to_string(src_vertex_val)}; msgs::ExpandOneRequest expand_one_req{}; From be7aa55686d49dbe57433e2d1424c5c32e78496c Mon Sep 17 00:00:00 2001 From: jeremy <jeremy.bailleux@memgraph.io> Date: Fri, 28 Oct 2022 13:21:43 +0200 Subject: [PATCH 19/63] Add std::move --- src/storage/v3/shard_rsm.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/storage/v3/shard_rsm.cpp b/src/storage/v3/shard_rsm.cpp index 105b3b3b4..8727c0440 100644 --- a/src/storage/v3/shard_rsm.cpp +++ b/src/storage/v3/shard_rsm.cpp @@ -276,7 +276,7 @@ std::optional<std::array<std::vector<EdgeAccessor>, 2>> FillUpConnectingEdges( break; } } - return std::array<std::vector<EdgeAccessor>, 2>{in_edges, out_edges}; + return std::array<std::vector<EdgeAccessor>, 2>{std::move(in_edges), std::move(out_edges)}; } using AllEdgePropertyDataSructure = std::map<PropertyId, msgs::Value>; @@ -492,7 +492,7 @@ msgs::ReadResponses HandleReadWithOrderBy(msgs::ExpandOneRequest &&req, Shard::A auto maybe_filter_based_on_edge_uniquness = InitializeEdgeUniqunessFunction(req.only_unique_neighbor_rows); auto edge_filler = InitializeEdgeFillerFunction(req); - auto vertex_iterable = acc.Vertices(View::OLD); + auto vertex_iterable = acc.Vertices(View::OLD); // #NoCommit we get all vertices in the DB here, is it wanted? const auto sorted_vertices = OrderByElements<VertexAccessor>(acc, dba, vertex_iterable, req.order_by); for (const auto &ordered_vertice : sorted_vertices) { @@ -530,7 +530,8 @@ msgs::ReadResponses HandleReadWithOrderBy(msgs::ExpandOneRequest &&req, Shard::A break; } - msgs::VertexId src_vertice(msgs::Label{.id = *label_id}, conversions::ConvertValueVector(*primary_key)); + msgs::VertexId src_vertice(msgs::Label{.id = *label_id}, + conversions::ConvertValueVector(*primary_key)); // #NoCommit rename auto maybe_result = GetExpandOneResult(src_vertex_acc, src_vertice, req, in_edge_ordered_accessors, out_edge_ordered_accessors, maybe_filter_based_on_edge_uniquness, edge_filler); From 476e2670d5e24063e98035c735c3000bd9f81225 Mon Sep 17 00:00:00 2001 From: Jeremy B <97525434+42jeremy@users.noreply.github.com> Date: Fri, 28 Oct 2022 13:21:48 +0200 Subject: [PATCH 20/63] Update src/storage/v3/request_helper.cpp Co-authored-by: Jure Bajic <jure.bajic@memgraph.com> --- src/storage/v3/request_helper.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/storage/v3/request_helper.cpp b/src/storage/v3/request_helper.cpp index 60633fbbd..591d7b6d7 100644 --- a/src/storage/v3/request_helper.cpp +++ b/src/storage/v3/request_helper.cpp @@ -73,7 +73,7 @@ std::array<std::vector<EdgeAccessor>, 2> GetEdgesFromVertex(const VertexAccessor } } - return std::array<std::vector<EdgeAccessor>, 2>{in_edges, out_edges}; + return std::array<std::vector<EdgeAccessor>, 2>{std::move(in_edges), std::move(out_edges)}; } } // namespace memgraph::storage::v3 From 1c17692a260001eeeda0eb24048b761054803405 Mon Sep 17 00:00:00 2001 From: Jeremy B <97525434+42jeremy@users.noreply.github.com> Date: Fri, 28 Oct 2022 13:22:07 +0200 Subject: [PATCH 21/63] Update src/storage/v3/shard_rsm.cpp Co-authored-by: Jure Bajic <jure.bajic@memgraph.com> --- src/storage/v3/shard_rsm.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/storage/v3/shard_rsm.cpp b/src/storage/v3/shard_rsm.cpp index 69429493c..86ccbb4c9 100644 --- a/src/storage/v3/shard_rsm.cpp +++ b/src/storage/v3/shard_rsm.cpp @@ -513,7 +513,7 @@ msgs::ReadResponses HandleReadWithOrderBy(msgs::ExpandOneRequest &&req, Shard::A std::vector<EdgeAccessor> in_edge_ordered_accessors; std::transform(in_ordered_edges.begin(), in_ordered_edges.end(), std::back_inserter(in_edge_ordered_accessors), - [](auto &edge_element) { return edge_element.object_acc; }); + [](const auto &edge_element) { return edge_element.object_acc; }); std::vector<EdgeAccessor> out_edge_ordered_accessors; std::transform(out_ordered_edges.begin(), out_ordered_edges.end(), std::back_inserter(out_edge_ordered_accessors), From b6814b7a49905b3a9685d16ba0dbe9e9216e97ca Mon Sep 17 00:00:00 2001 From: Jeremy B <97525434+42jeremy@users.noreply.github.com> Date: Fri, 28 Oct 2022 13:22:49 +0200 Subject: [PATCH 22/63] Update src/storage/v3/shard_rsm.cpp Co-authored-by: Jure Bajic <jure.bajic@memgraph.com> --- src/storage/v3/shard_rsm.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/storage/v3/shard_rsm.cpp b/src/storage/v3/shard_rsm.cpp index 86ccbb4c9..f7b655a29 100644 --- a/src/storage/v3/shard_rsm.cpp +++ b/src/storage/v3/shard_rsm.cpp @@ -517,7 +517,7 @@ msgs::ReadResponses HandleReadWithOrderBy(msgs::ExpandOneRequest &&req, Shard::A std::vector<EdgeAccessor> out_edge_ordered_accessors; std::transform(out_ordered_edges.begin(), out_ordered_edges.end(), std::back_inserter(out_edge_ordered_accessors), - [](auto &edge_element) { return edge_element.object_acc; }); + [](const auto &edge_element) { return edge_element.object_acc; }); auto label_id = src_vertex_acc.PrimaryLabel(View::NEW); if (label_id.HasError()) { From e0f6c951c1b3050dabbc6979a8e1a2bc73adb53e Mon Sep 17 00:00:00 2001 From: jeremy <jeremy.bailleux@memgraph.io> Date: Fri, 28 Oct 2022 15:31:15 +0200 Subject: [PATCH 23/63] Add possibilty to orderByElement on vector<VertexAccessor> --- src/storage/v3/request_helper.hpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/storage/v3/request_helper.hpp b/src/storage/v3/request_helper.hpp index c291e63d4..e3c5b9d3f 100644 --- a/src/storage/v3/request_helper.hpp +++ b/src/storage/v3/request_helper.hpp @@ -138,7 +138,8 @@ std::vector<Element<TObjectAccessor>> OrderByElements(Shard::Accessor &acc, DbAc properties_order_by.reserve(order_bys.size()); for (const auto &order_by : order_bys) { - if constexpr (std::is_same_v<TIterable, VerticesIterable>) { + if constexpr (std::is_same_v<TIterable, VerticesIterable> || + std::is_same_v<TIterable, std::vector<VertexAccessor>>) { properties_order_by.push_back(ComputeExpression(dba, *it, std::nullopt, order_by.expression.expression, expr::identifier_node_symbol, expr::identifier_edge_symbol)); } else { From b2e9717ec3967adcfd49c37c7de4de40f321b04b Mon Sep 17 00:00:00 2001 From: jeremy <jeremy.bailleux@memgraph.io> Date: Fri, 28 Oct 2022 15:31:29 +0200 Subject: [PATCH 24/63] Factor HandleRead(msgs::ExpandOneRequest.. --- src/storage/v3/shard_rsm.cpp | 220 +++++++++++++++-------------------- 1 file changed, 94 insertions(+), 126 deletions(-) diff --git a/src/storage/v3/shard_rsm.cpp b/src/storage/v3/shard_rsm.cpp index 4c520858a..20808517d 100644 --- a/src/storage/v3/shard_rsm.cpp +++ b/src/storage/v3/shard_rsm.cpp @@ -482,129 +482,6 @@ EdgeFiller InitializeEdgeFillerFunction(const msgs::ExpandOneRequest &req) { return edge_filler; } -msgs::ReadResponses HandleReadWithOrderBy(msgs::ExpandOneRequest &&req, Shard::Accessor &&acc) { - bool action_successful = true; - - std::vector<msgs::ExpandOneResultRow> results; - auto batch_limit = req.limit; - auto dba = DbAccessor{&acc}; - - auto maybe_filter_based_on_edge_uniquness = InitializeEdgeUniqunessFunction(req.only_unique_neighbor_rows); - auto edge_filler = InitializeEdgeFillerFunction(req); - - auto vertex_iterable = acc.Vertices(View::OLD); // #NoCommit we get all vertices in the DB here, is it wanted? - const auto sorted_vertices = OrderByElements<VertexAccessor>(acc, dba, vertex_iterable, req.order_by); - - for (const auto &ordered_vertice : sorted_vertices) { - // Get Vertex acc - auto src_vertex_acc = ordered_vertice.object_acc; - - if (!req.filters.empty()) { - // NOTE - DbAccessor might get removed in the future. - const bool eval = FilterOnVertex(dba, src_vertex_acc, req.filters, expr::identifier_node_symbol); - if (!eval) { - continue; - } - } - - auto [in_edge_accessors, out_edge_accessors] = GetEdgesFromVertex(src_vertex_acc, req.direction); - const auto in_ordered_edges = OrderByElements<EdgeAccessor>(acc, dba, in_edge_accessors, req.order_by); - const auto out_ordered_edges = OrderByElements<EdgeAccessor>(acc, dba, out_edge_accessors, req.order_by); - - std::vector<EdgeAccessor> in_edge_ordered_accessors; - std::transform(in_ordered_edges.begin(), in_ordered_edges.end(), std::back_inserter(in_edge_ordered_accessors), - [](const auto &edge_element) { return edge_element.object_acc; }); - - std::vector<EdgeAccessor> out_edge_ordered_accessors; - std::transform(out_ordered_edges.begin(), out_ordered_edges.end(), std::back_inserter(out_edge_ordered_accessors), - [](const auto &edge_element) { return edge_element.object_acc; }); - - auto label_id = src_vertex_acc.PrimaryLabel(View::NEW); - if (label_id.HasError()) { - action_successful = false; - break; - } - auto primary_key = src_vertex_acc.PrimaryKey(View::NEW); - if (primary_key.HasError()) { - action_successful = false; - break; - } - - msgs::VertexId src_vertice(msgs::Label{.id = *label_id}, - conversions::ConvertValueVector(*primary_key)); // #NoCommit rename - auto maybe_result = - GetExpandOneResult(src_vertex_acc, src_vertice, req, in_edge_ordered_accessors, out_edge_ordered_accessors, - maybe_filter_based_on_edge_uniquness, edge_filler); - - if (!maybe_result) { - action_successful = false; - break; - } - - results.emplace_back(maybe_result.value()); - if (batch_limit.has_value() && results.size() >= batch_limit.value()) { - break; - } - } - - msgs::ExpandOneResponse resp{}; - resp.success = action_successful; - if (action_successful) { - resp.result = std::move(results); - } - - return resp; -} - -msgs::ReadResponses HandleReadWithoutOrderBy(msgs::ExpandOneRequest &&req, Shard::Accessor &&acc) { - bool action_successful = true; - - std::vector<msgs::ExpandOneResultRow> results; - auto batch_limit = req.limit; - auto dba = DbAccessor{&acc}; - - auto maybe_filter_based_on_edge_uniquness = InitializeEdgeUniqunessFunction(req.only_unique_neighbor_rows); - auto edge_filler = InitializeEdgeFillerFunction(req); - - for (auto &src_vertex : req.src_vertices) { - // Get Vertex acc - auto src_vertex_acc_opt = acc.FindVertex(ConvertPropertyVector((src_vertex.second)), View::NEW); - if (!src_vertex_acc_opt) { - action_successful = false; - spdlog::debug("Encountered an error while trying to obtain VertexAccessor. Transaction id: {}", - req.transaction_id.logical_id); - break; - } - - if (!req.filters.empty()) { - // NOTE - DbAccessor might get removed in the future. - const bool eval = FilterOnVertex(dba, src_vertex_acc_opt.value(), req.filters, expr::identifier_node_symbol); - if (!eval) { - continue; - } - } - auto result = GetExpandOneResult(acc, src_vertex, req, maybe_filter_based_on_edge_uniquness, edge_filler); - - if (!result) { - action_successful = false; - break; - } - - results.emplace_back(result.value()); - if (batch_limit.has_value() && results.size() >= batch_limit.value()) { - break; - } - } - - msgs::ExpandOneResponse resp{}; - resp.success = action_successful; - if (action_successful) { - resp.result = std::move(results); - } - - return resp; -} - }; // namespace msgs::WriteResponses ShardRsm::ApplyWrite(msgs::CreateVerticesRequest &&req) { auto acc = shard_->Access(req.transaction_id); @@ -984,11 +861,102 @@ msgs::ReadResponses ShardRsm::HandleRead(msgs::ScanVerticesRequest &&req) { } msgs::ReadResponses ShardRsm::HandleRead(msgs::ExpandOneRequest &&req) { - if (!req.order_by.empty()) { - return HandleReadWithOrderBy(std::move(req), shard_->Access(req.transaction_id)); + auto acc = shard_->Access(req.transaction_id); + bool action_successful = true; + + std::vector<msgs::ExpandOneResultRow> results; + auto batch_limit = req.limit; + auto dba = DbAccessor{&acc}; + + auto maybe_filter_based_on_edge_uniquness = InitializeEdgeUniqunessFunction(req.only_unique_neighbor_rows); + auto edge_filler = InitializeEdgeFillerFunction(req); + + std::vector<VertexAccessor> vertex_accessors; + vertex_accessors.reserve(req.src_vertices.size()); + for (auto &src_vertex : req.src_vertices) { + // Get Vertex acc + auto src_vertex_acc_opt = acc.FindVertex(ConvertPropertyVector((src_vertex.second)), View::NEW); + if (!src_vertex_acc_opt) { + action_successful = false; + spdlog::debug("Encountered an error while trying to obtain VertexAccessor. Transaction id: {}", + req.transaction_id.logical_id); + break; + } + if (!req.filters.empty()) { + // NOTE - DbAccessor might get removed in the future. + const bool eval = FilterOnVertex(dba, src_vertex_acc_opt.value(), req.filters, expr::identifier_node_symbol); + if (!eval) { + continue; + } + } + + vertex_accessors.emplace_back(src_vertex_acc_opt.value()); } - return HandleReadWithoutOrderBy(std::move(req), shard_->Access(req.transaction_id)); + if (!req.order_by.empty()) { + // #NoCommit can we do differently to avoid this? We need OrderByElements but currently + // #NoCommit it returns vector<Element>, so this workaround is here to avoid more duplication later + auto sorted_vertices = OrderByElements<VertexAccessor>(acc, dba, vertex_accessors, req.order_by); + vertex_accessors.clear(); + std::transform(sorted_vertices.begin(), sorted_vertices.end(), std::back_inserter(vertex_accessors), + [](auto &vertex) { return vertex.object_acc; }); + } + + for (const auto &src_vertex_acc : vertex_accessors) { + auto label_id = src_vertex_acc.PrimaryLabel(View::NEW); + if (label_id.HasError()) { + action_successful = false; + break; + } + + auto primary_key = src_vertex_acc.PrimaryKey(View::NEW); + if (primary_key.HasError()) { + action_successful = false; + break; + } + + msgs::VertexId src_vertice(msgs::Label{.id = *label_id}, + conversions::ConvertValueVector(*primary_key)); // #NoCommit rename + + std::optional<msgs::ExpandOneResultRow> maybe_result; + + if (req.order_by.empty()) { + maybe_result = GetExpandOneResult(acc, src_vertice, req, maybe_filter_based_on_edge_uniquness, edge_filler); + + } else { + auto [in_edge_accessors, out_edge_accessors] = GetEdgesFromVertex(src_vertex_acc, req.direction); + const auto in_ordered_edges = OrderByElements<EdgeAccessor>(acc, dba, in_edge_accessors, req.order_by); + const auto out_ordered_edges = OrderByElements<EdgeAccessor>(acc, dba, out_edge_accessors, req.order_by); + + std::vector<EdgeAccessor> in_edge_ordered_accessors; + std::transform(in_ordered_edges.begin(), in_ordered_edges.end(), std::back_inserter(in_edge_ordered_accessors), + [](const auto &edge_element) { return edge_element.object_acc; }); + + std::vector<EdgeAccessor> out_edge_ordered_accessors; + std::transform(out_ordered_edges.begin(), out_ordered_edges.end(), std::back_inserter(out_edge_ordered_accessors), + [](const auto &edge_element) { return edge_element.object_acc; }); + maybe_result = GetExpandOneResult(src_vertex_acc, src_vertice, req, in_edge_ordered_accessors, + out_edge_ordered_accessors, maybe_filter_based_on_edge_uniquness, edge_filler); + } + + if (!maybe_result) { + action_successful = false; + break; + } + + results.emplace_back(maybe_result.value()); + if (batch_limit.has_value() && results.size() >= batch_limit.value()) { + break; + } + } + + msgs::ExpandOneResponse resp{}; + resp.success = action_successful; + if (action_successful) { + resp.result = std::move(results); + } + + return resp; } msgs::WriteResponses ShardRsm::ApplyWrite(msgs::CommitRequest &&req) { From b2f3fab6935fe0e8fecf10a563e15da4f2078f4f Mon Sep 17 00:00:00 2001 From: jeremy <jeremy.bailleux@memgraph.io> Date: Tue, 8 Nov 2022 15:06:27 +0100 Subject: [PATCH 25/63] Remove comment --- src/storage/v3/shard_rsm.cpp | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/storage/v3/shard_rsm.cpp b/src/storage/v3/shard_rsm.cpp index 6725c0f23..9d1568bff 100644 --- a/src/storage/v3/shard_rsm.cpp +++ b/src/storage/v3/shard_rsm.cpp @@ -362,7 +362,8 @@ std::optional<msgs::ExpandOneResultRow> GetExpandOneResult( std::optional<msgs::ExpandOneResultRow> GetExpandOneResult( VertexAccessor v_acc, msgs::VertexId src_vertex, const msgs::ExpandOneRequest &req, std::vector<EdgeAccessor> in_edge_accessors, std::vector<EdgeAccessor> out_edge_accessors, - const EdgeUniqunessFunction &maybe_filter_based_on_edge_uniquness, const EdgeFiller &edge_filler) { + const EdgeUniqunessFunction &maybe_filter_based_on_edge_uniquness, const EdgeFiller &edge_filler, + const Schemas::Schema *schema) { /// Fill up source vertex auto source_vertex = FillUpSourceVertex(v_acc, req, src_vertex); if (!source_vertex) { @@ -370,7 +371,7 @@ std::optional<msgs::ExpandOneResultRow> GetExpandOneResult( } /// Fill up source vertex properties - auto src_vertex_properties = FillUpSourceVertexProperties(v_acc, req); + auto src_vertex_properties = FillUpSourceVertexProperties(v_acc, req, storage::v3::View::NEW, schema); if (!src_vertex_properties) { return std::nullopt; } @@ -932,13 +933,13 @@ msgs::ReadResponses ShardRsm::HandleRead(msgs::ExpandOneRequest &&req) { break; } - msgs::VertexId src_vertice(msgs::Label{.id = *label_id}, - conversions::ConvertValueVector(*primary_key)); // #NoCommit rename + msgs::VertexId src_vertice(msgs::Label{.id = *label_id}, conversions::ConvertValueVector(*primary_key)); std::optional<msgs::ExpandOneResultRow> maybe_result; if (req.order_by.empty()) { - maybe_result = GetExpandOneResult(acc, src_vertice, req, maybe_filter_based_on_edge_uniquness, edge_filler); + maybe_result = GetExpandOneResult(acc, src_vertice, req, maybe_filter_based_on_edge_uniquness, edge_filler, + shard_->GetSchema(shard_->PrimaryLabel())); } else { auto [in_edge_accessors, out_edge_accessors] = GetEdgesFromVertex(src_vertex_acc, req.direction); @@ -953,7 +954,8 @@ msgs::ReadResponses ShardRsm::HandleRead(msgs::ExpandOneRequest &&req) { std::transform(out_ordered_edges.begin(), out_ordered_edges.end(), std::back_inserter(out_edge_ordered_accessors), [](const auto &edge_element) { return edge_element.object_acc; }); maybe_result = GetExpandOneResult(src_vertex_acc, src_vertice, req, in_edge_ordered_accessors, - out_edge_ordered_accessors, maybe_filter_based_on_edge_uniquness, edge_filler); + out_edge_ordered_accessors, maybe_filter_based_on_edge_uniquness, edge_filler, + shard_->GetSchema(shard_->PrimaryLabel())); } if (!maybe_result) { From cad0e80d008c3040d8c99a535ce68429244e41eb Mon Sep 17 00:00:00 2001 From: jeremy <jeremy.bailleux@memgraph.io> Date: Tue, 8 Nov 2022 17:42:31 +0100 Subject: [PATCH 26/63] Update test --- tests/simulation/shard_rsm.cpp | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/tests/simulation/shard_rsm.cpp b/tests/simulation/shard_rsm.cpp index dac9c26b2..4c61f50b6 100644 --- a/tests/simulation/shard_rsm.cpp +++ b/tests/simulation/shard_rsm.cpp @@ -701,7 +701,7 @@ void AttemptToExpandOneLimitAndOrderBy(ShardClient &client, uint64_t src_vertex_ std::vector<msgs::OrderBy> order_by = { {msgs::Expression{"MG_SYMBOL_NODE.prop1"}, msgs::OrderingDirection::DESCENDING}}; size_t limit = 3; - std::vector<std::string> filters = {"MG_SYMBOL_NODE.prop1 != " + std::to_string(src_vertex_val)}; + std::vector<std::string> filters = {"MG_SYMBOL_NODE.prop1 != -1"}; msgs::ExpandOneRequest expand_one_req{}; @@ -731,16 +731,17 @@ void AttemptToExpandOneLimitAndOrderBy(ShardClient &client, uint64_t src_vertex_ // We also check that the vertices are ordered by prop1 DESC - std::is_sorted(write_response.result.cbegin(), write_response.result.cend(), - [](const auto &vertex, const auto &other_vertex) { - const auto primary_key = vertex.src_vertex.id.second; - const auto other_primary_key = other_vertex.src_vertex.id.second; + auto is_sorted = std::is_sorted(write_response.result.cbegin(), write_response.result.cend(), + [](const auto &vertex, const auto &other_vertex) { + const auto primary_key = vertex.src_vertex.id.second; + const auto other_primary_key = other_vertex.src_vertex.id.second; - MG_ASSERT(primary_key.size() == 1); - MG_ASSERT(other_primary_key.size() == 1); - return primary_key[0].int_v > other_primary_key[0].int_v; - }); + MG_ASSERT(primary_key.size() == 1); + MG_ASSERT(other_primary_key.size() == 1); + return primary_key[0].int_v > other_primary_key[0].int_v; + }); + MG_ASSERT(is_sorted); break; } } From 2087877df2edfe72f643ee099f6c9dd96c43a415 Mon Sep 17 00:00:00 2001 From: jeremy <jeremy.bailleux@memgraph.io> Date: Wed, 9 Nov 2022 08:48:51 +0100 Subject: [PATCH 27/63] Add more checks in test --- tests/simulation/shard_rsm.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/simulation/shard_rsm.cpp b/tests/simulation/shard_rsm.cpp index 4c61f50b6..b54f7a993 100644 --- a/tests/simulation/shard_rsm.cpp +++ b/tests/simulation/shard_rsm.cpp @@ -727,7 +727,12 @@ void AttemptToExpandOneLimitAndOrderBy(ShardClient &client, uint64_t src_vertex_ // We check that we do not have more results than the limit. Based on the data in the graph, we know that we should // receive exactly limit responses. auto expected_number_of_rows = std::min(expand_one_req.src_vertices.size(), limit); + MG_ASSERT(expected_number_of_rows == 1); // We are sending a single vertex to expand MG_ASSERT(write_response.result.size() == expected_number_of_rows); + const auto expected_number_of_edges = 10; // We know there are 10 out-going edges from V2->V3 + MG_ASSERT(write_response.result[0].out_edges_with_all_properties.size() == expected_number_of_edges); + MG_ASSERT(write_response.result[0] + .out_edges_with_specific_properties.empty()); // We are not asking for specific properties // We also check that the vertices are ordered by prop1 DESC From 2045f54577fbb2439dffc65d75a885764ea39405 Mon Sep 17 00:00:00 2001 From: jeremy <jeremy.bailleux@memgraph.io> Date: Wed, 9 Nov 2022 21:36:41 +0100 Subject: [PATCH 28/63] Correct merge issue --- src/storage/v3/shard_rsm.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/storage/v3/shard_rsm.cpp b/src/storage/v3/shard_rsm.cpp index 6623bc316..5f6cb5959 100644 --- a/src/storage/v3/shard_rsm.cpp +++ b/src/storage/v3/shard_rsm.cpp @@ -369,8 +369,10 @@ std::optional<msgs::ExpandOneResultRow> GetExpandOneResult( const EdgeUniqunessFunction &maybe_filter_based_on_edge_uniquness, const EdgeFiller &edge_filler, const Schemas::Schema *schema) { /// Fill up source vertex - auto source_vertex = FillUpSourceVertex(v_acc, req, src_vertex); - if (!source_vertex) { + msgs::Vertex source_vertex = {.id = src_vertex}; + if (const auto maybe_secondary_labels = FillUpSourceVertexSecondaryLabels(v_acc, req); maybe_secondary_labels) { + source_vertex.labels = *maybe_secondary_labels; + } else { return std::nullopt; } @@ -385,7 +387,7 @@ std::optional<msgs::ExpandOneResultRow> GetExpandOneResult( auto out_edges = maybe_filter_based_on_edge_uniquness(std::move(out_edge_accessors), msgs::EdgeDirection::OUT); msgs::ExpandOneResultRow result_row; - result_row.src_vertex = std::move(*source_vertex); + result_row.src_vertex = std::move(source_vertex); result_row.src_vertex_properties = std::move(*src_vertex_properties); static constexpr bool kInEdges = true; static constexpr bool kOutEdges = false; From 131d7f2a744d07c9c0d03bb7c57dd251284ec9f6 Mon Sep 17 00:00:00 2001 From: jeremy <jeremy.bailleux@memgraph.io> Date: Mon, 14 Nov 2022 18:21:03 +0100 Subject: [PATCH 29/63] OrderByElements: no longer templated over vertice/edge types. For edges, we always need to have access to the corresponding vertex_accessor (ex of sorting expr needing both : "vertex.map[edge]") ComputeExpression: made assert instead of if check --- src/storage/v3/expr.cpp | 6 ++- src/storage/v3/request_helper.hpp | 87 ++++++++++++++++++++----------- src/storage/v3/shard_rsm.cpp | 12 ++--- 3 files changed, 68 insertions(+), 37 deletions(-) diff --git a/src/storage/v3/expr.cpp b/src/storage/v3/expr.cpp index eaff472bd..53146f595 100644 --- a/src/storage/v3/expr.cpp +++ b/src/storage/v3/expr.cpp @@ -186,7 +186,8 @@ TypedValue ComputeExpression(DbAccessor &dba, const std::optional<memgraph::stor expr::SymbolGenerator symbol_generator(&symbol_table, identifiers); (std::any_cast<Expression *>(expr))->Accept(symbol_generator); - if (node_identifier.symbol_pos_ != -1 && v_acc.has_value()) { + if (node_identifier.symbol_pos_ != -1) { + MG_ASSERT(v_acc.has_value()); MG_ASSERT(std::find_if(symbol_table.table().begin(), symbol_table.table().end(), [&node_name](const std::pair<int32_t, Symbol> &position_symbol_pair) { return position_symbol_pair.second.name() == node_name; @@ -195,7 +196,8 @@ TypedValue ComputeExpression(DbAccessor &dba, const std::optional<memgraph::stor frame[symbol_table.at(node_identifier)] = *v_acc; } - if (edge_identifier.symbol_pos_ != -1 && e_acc.has_value()) { + if (edge_identifier.symbol_pos_ != -1) { + MG_ASSERT(e_acc.has_value()); MG_ASSERT(std::find_if(symbol_table.table().begin(), symbol_table.table().end(), [&edge_name](const std::pair<int32_t, Symbol> &position_symbol_pair) { return position_symbol_pair.second.name() == edge_name; diff --git a/src/storage/v3/request_helper.hpp b/src/storage/v3/request_helper.hpp index e3c5b9d3f..955059f26 100644 --- a/src/storage/v3/request_helper.hpp +++ b/src/storage/v3/request_helper.hpp @@ -112,44 +112,73 @@ struct Element { TObjectAccessor object_acc; }; -template <ObjectAccessor TObjectAccessor, typename TIterable> -std::vector<Element<TObjectAccessor>> OrderByElements(Shard::Accessor &acc, DbAccessor &dba, TIterable &iterable, - std::vector<msgs::OrderBy> &order_bys) { - std::vector<Element<TObjectAccessor>> ordered; - ordered.reserve(acc.ApproximateVertexCount()); +template <typename TIterable> +std::vector<Element<VertexAccessor>> OrderByVertices(Shard::Accessor &acc, DbAccessor &dba, TIterable &iterable, + std::vector<msgs::OrderBy> &order_bys) { + static_assert(std::is_same_v<TIterable, VerticesIterable> || std::is_same_v<TIterable, std::vector<VertexAccessor>>); + std::vector<Ordering> ordering; ordering.reserve(order_bys.size()); - for (const auto &order : order_bys) { - switch (order.direction) { - case memgraph::msgs::OrderingDirection::ASCENDING: { - ordering.push_back(Ordering::ASC); - break; - } - case memgraph::msgs::OrderingDirection::DESCENDING: { - ordering.push_back(Ordering::DESC); - break; - } + std::transform(order_bys.begin(), order_bys.end(), std::back_inserter(ordering), [](const auto &order_by) { + if (memgraph::msgs::OrderingDirection::ASCENDING == order_by.direction) { + return Ordering::ASC; } - } - auto compare_typed_values = TypedValueVectorCompare(ordering); - auto it = iterable.begin(); - for (; it != iterable.end(); ++it) { + MG_ASSERT(memgraph::msgs::OrderingDirection::DESCENDING == order_by.direction); + return Ordering::DESC; + }); + + std::vector<Element<VertexAccessor>> ordered; + ordered.reserve(acc.ApproximateVertexCount()); + for (auto it = iterable.begin(); it != iterable.end(); ++it) { std::vector<TypedValue> properties_order_by; properties_order_by.reserve(order_bys.size()); - for (const auto &order_by : order_bys) { - if constexpr (std::is_same_v<TIterable, VerticesIterable> || - std::is_same_v<TIterable, std::vector<VertexAccessor>>) { - properties_order_by.push_back(ComputeExpression(dba, *it, std::nullopt, order_by.expression.expression, - expr::identifier_node_symbol, expr::identifier_edge_symbol)); - } else { - properties_order_by.push_back(ComputeExpression(dba, std::nullopt, *it, order_by.expression.expression, - expr::identifier_node_symbol, expr::identifier_edge_symbol)); - } - } + std::transform(order_bys.begin(), order_bys.end(), std::back_inserter(properties_order_by), + [&dba, &it](const auto &order_by) { + return ComputeExpression(dba, *it, std::nullopt /*e_acc*/, order_by.expression.expression, + expr::identifier_node_symbol, expr::identifier_edge_symbol); + }); + ordered.push_back({std::move(properties_order_by), *it}); } + auto compare_typed_values = TypedValueVectorCompare(ordering); + std::sort(ordered.begin(), ordered.end(), [compare_typed_values](const auto &pair1, const auto &pair2) { + return compare_typed_values(pair1.properties_order_by, pair2.properties_order_by); + }); + return ordered; +} + +template <typename TIterable> +std::vector<Element<EdgeAccessor>> OrderByEdges(Shard::Accessor &acc, DbAccessor &dba, TIterable &iterable, + std::vector<msgs::OrderBy> &order_bys, + const VertexAccessor &vertex_acc) { + static_assert(std::is_same_v<TIterable, std::vector<EdgeAccessor>>); // Can be extended if needed + + std::vector<Ordering> ordering; + ordering.reserve(order_bys.size()); + std::transform(order_bys.begin(), order_bys.end(), std::back_inserter(ordering), [](const auto &order_by) { + if (memgraph::msgs::OrderingDirection::ASCENDING == order_by.direction) { + return Ordering::ASC; + } + MG_ASSERT(memgraph::msgs::OrderingDirection::DESCENDING == order_by.direction); + return Ordering::DESC; + }); + + std::vector<Element<EdgeAccessor>> ordered; + for (auto it = iterable.begin(); it != iterable.end(); ++it) { + std::vector<TypedValue> properties_order_by; + properties_order_by.reserve(order_bys.size()); + std::transform(order_bys.begin(), order_bys.end(), std::back_inserter(properties_order_by), + [&dba, &vertex_acc, &it](const auto &order_by) { + return ComputeExpression(dba, vertex_acc, *it, order_by.expression.expression, + expr::identifier_node_symbol, expr::identifier_edge_symbol); + }); + + ordered.push_back({std::move(properties_order_by), *it}); + } + + auto compare_typed_values = TypedValueVectorCompare(ordering); std::sort(ordered.begin(), ordered.end(), [compare_typed_values](const auto &pair1, const auto &pair2) { return compare_typed_values(pair1.properties_order_by, pair2.properties_order_by); }); diff --git a/src/storage/v3/shard_rsm.cpp b/src/storage/v3/shard_rsm.cpp index 5f6cb5959..55b726c79 100644 --- a/src/storage/v3/shard_rsm.cpp +++ b/src/storage/v3/shard_rsm.cpp @@ -875,7 +875,7 @@ msgs::ReadResponses ShardRsm::HandleRead(msgs::ScanVerticesRequest &&req) { uint64_t sample_counter{0}; auto vertex_iterable = acc.Vertices(view); if (!req.order_bys.empty()) { - const auto ordered = OrderByElements<VertexAccessor>(acc, dba, vertex_iterable, req.order_bys); + const auto ordered = OrderByVertices(acc, dba, vertex_iterable, req.order_bys); // we are traversing Elements auto it = GetStartOrderedElementsIterator(ordered, start_id, View(req.storage_view)); for (; it != ordered.end(); ++it) { @@ -956,9 +956,9 @@ msgs::ReadResponses ShardRsm::HandleRead(msgs::ExpandOneRequest &&req) { } if (!req.order_by.empty()) { - // #NoCommit can we do differently to avoid this? We need OrderByElements but currently - // #NoCommit it returns vector<Element>, so this workaround is here to avoid more duplication later - auto sorted_vertices = OrderByElements<VertexAccessor>(acc, dba, vertex_accessors, req.order_by); + // Can we do differently to avoid this? We need OrderByElements but currently it returns vector<Element>, so this + // workaround is here to avoid more duplication later + auto sorted_vertices = OrderByVertices(acc, dba, vertex_accessors, req.order_by); vertex_accessors.clear(); std::transform(sorted_vertices.begin(), sorted_vertices.end(), std::back_inserter(vertex_accessors), [](auto &vertex) { return vertex.object_acc; }); @@ -987,8 +987,8 @@ msgs::ReadResponses ShardRsm::HandleRead(msgs::ExpandOneRequest &&req) { } else { auto [in_edge_accessors, out_edge_accessors] = GetEdgesFromVertex(src_vertex_acc, req.direction); - const auto in_ordered_edges = OrderByElements<EdgeAccessor>(acc, dba, in_edge_accessors, req.order_by); - const auto out_ordered_edges = OrderByElements<EdgeAccessor>(acc, dba, out_edge_accessors, req.order_by); + const auto in_ordered_edges = OrderByEdges(acc, dba, in_edge_accessors, req.order_by, src_vertex_acc); + const auto out_ordered_edges = OrderByEdges(acc, dba, out_edge_accessors, req.order_by, src_vertex_acc); std::vector<EdgeAccessor> in_edge_ordered_accessors; std::transform(in_ordered_edges.begin(), in_ordered_edges.end(), std::back_inserter(in_edge_ordered_accessors), From cca4e97bcff287764d8d9646c6f7773f4a9b0c48 Mon Sep 17 00:00:00 2001 From: jeremy <jeremy.bailleux@memgraph.io> Date: Tue, 15 Nov 2022 13:37:43 +0100 Subject: [PATCH 30/63] Remove un-needed argument from OrderByEdges --- src/storage/v3/request_helper.hpp | 2 +- src/storage/v3/shard_rsm.cpp | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/storage/v3/request_helper.hpp b/src/storage/v3/request_helper.hpp index 955059f26..4d3018566 100644 --- a/src/storage/v3/request_helper.hpp +++ b/src/storage/v3/request_helper.hpp @@ -150,7 +150,7 @@ std::vector<Element<VertexAccessor>> OrderByVertices(Shard::Accessor &acc, DbAcc } template <typename TIterable> -std::vector<Element<EdgeAccessor>> OrderByEdges(Shard::Accessor &acc, DbAccessor &dba, TIterable &iterable, +std::vector<Element<EdgeAccessor>> OrderByEdges(DbAccessor &dba, TIterable &iterable, std::vector<msgs::OrderBy> &order_bys, const VertexAccessor &vertex_acc) { static_assert(std::is_same_v<TIterable, std::vector<EdgeAccessor>>); // Can be extended if needed diff --git a/src/storage/v3/shard_rsm.cpp b/src/storage/v3/shard_rsm.cpp index 55b726c79..2acdf9f3c 100644 --- a/src/storage/v3/shard_rsm.cpp +++ b/src/storage/v3/shard_rsm.cpp @@ -987,8 +987,8 @@ msgs::ReadResponses ShardRsm::HandleRead(msgs::ExpandOneRequest &&req) { } else { auto [in_edge_accessors, out_edge_accessors] = GetEdgesFromVertex(src_vertex_acc, req.direction); - const auto in_ordered_edges = OrderByEdges(acc, dba, in_edge_accessors, req.order_by, src_vertex_acc); - const auto out_ordered_edges = OrderByEdges(acc, dba, out_edge_accessors, req.order_by, src_vertex_acc); + const auto in_ordered_edges = OrderByEdges(dba, in_edge_accessors, req.order_by, src_vertex_acc); + const auto out_ordered_edges = OrderByEdges(dba, out_edge_accessors, req.order_by, src_vertex_acc); std::vector<EdgeAccessor> in_edge_ordered_accessors; std::transform(in_ordered_edges.begin(), in_ordered_edges.end(), std::back_inserter(in_edge_ordered_accessors), From bd11225d236b0fa16b579c41df8a303a40f6899a Mon Sep 17 00:00:00 2001 From: jeremy <jeremy.bailleux@memgraph.io> Date: Wed, 16 Nov 2022 14:14:35 +0100 Subject: [PATCH 31/63] Use ref instead of optional Use ref i.o. ptr Rename variable for clarity --- src/storage/v3/expr.cpp | 5 ++--- src/storage/v3/expr.hpp | 5 ++--- src/storage/v3/shard_rsm.cpp | 39 +++++++++++++++++++++--------------- 3 files changed, 27 insertions(+), 22 deletions(-) diff --git a/src/storage/v3/expr.cpp b/src/storage/v3/expr.cpp index 53146f595..8062a2662 100644 --- a/src/storage/v3/expr.cpp +++ b/src/storage/v3/expr.cpp @@ -165,7 +165,7 @@ std::any ParseExpression(const std::string &expr, memgraph::expr::AstStorage &st return visitor.visit(ast); } -TypedValue ComputeExpression(DbAccessor &dba, const std::optional<memgraph::storage::v3::VertexAccessor> &v_acc, +TypedValue ComputeExpression(DbAccessor &dba, const memgraph::storage::v3::VertexAccessor &v_acc, const std::optional<memgraph::storage::v3::EdgeAccessor> &e_acc, const std::string &expression, std::string_view node_name, std::string_view edge_name) { AstStorage storage; @@ -187,13 +187,12 @@ TypedValue ComputeExpression(DbAccessor &dba, const std::optional<memgraph::stor (std::any_cast<Expression *>(expr))->Accept(symbol_generator); if (node_identifier.symbol_pos_ != -1) { - MG_ASSERT(v_acc.has_value()); MG_ASSERT(std::find_if(symbol_table.table().begin(), symbol_table.table().end(), [&node_name](const std::pair<int32_t, Symbol> &position_symbol_pair) { return position_symbol_pair.second.name() == node_name; }) != symbol_table.table().end()); - frame[symbol_table.at(node_identifier)] = *v_acc; + frame[symbol_table.at(node_identifier)] = v_acc; } if (edge_identifier.symbol_pos_ != -1) { diff --git a/src/storage/v3/expr.hpp b/src/storage/v3/expr.hpp index 7eefdd71c..d344d36b2 100644 --- a/src/storage/v3/expr.hpp +++ b/src/storage/v3/expr.hpp @@ -50,8 +50,7 @@ auto Eval(TExpression *expr, EvaluationContext &ctx, AstStorage &storage, Expres std::any ParseExpression(const std::string &expr, AstStorage &storage); -TypedValue ComputeExpression(DbAccessor &dba, const std::optional<VertexAccessor> &v_acc, - const std::optional<EdgeAccessor> &e_acc, const std::string &expression, - std::string_view node_name, std::string_view edge_name); +TypedValue ComputeExpression(DbAccessor &dba, const VertexAccessor &v_acc, const std::optional<EdgeAccessor> &e_acc, + const std::string &expression, std::string_view node_name, std::string_view edge_name); } // namespace memgraph::storage::v3 diff --git a/src/storage/v3/shard_rsm.cpp b/src/storage/v3/shard_rsm.cpp index 2acdf9f3c..bf31e8ced 100644 --- a/src/storage/v3/shard_rsm.cpp +++ b/src/storage/v3/shard_rsm.cpp @@ -42,6 +42,7 @@ #include "storage/v3/vertex_accessor.hpp" #include "storage/v3/vertex_id.hpp" #include "storage/v3/view.hpp" +#include "utils/logging.hpp" namespace memgraph::storage::v3 { using msgs::Label; @@ -117,7 +118,7 @@ std::optional<std::map<PropertyId, Value>> CollectSpecificPropertiesFromAccessor } std::optional<std::map<PropertyId, Value>> PrimaryKeysFromAccessor(const VertexAccessor &acc, View view, - const Schemas::Schema *schema) { + const Schemas::Schema &schema) { std::map<PropertyId, Value> ret; auto props = acc.Properties(view); auto maybe_pk = acc.PrimaryKey(view); @@ -126,16 +127,16 @@ std::optional<std::map<PropertyId, Value>> PrimaryKeysFromAccessor(const VertexA return std::nullopt; } auto &pk = maybe_pk.GetValue(); - MG_ASSERT(schema->second.size() == pk.size(), "PrimaryKey size does not match schema!"); - for (size_t i{0}; i < schema->second.size(); ++i) { - ret.emplace(schema->second[i].property_id, FromPropertyValueToValue(std::move(pk[i]))); + MG_ASSERT(schema.second.size() == pk.size(), "PrimaryKey size does not match schema!"); + for (size_t i{0}; i < schema.second.size(); ++i) { + ret.emplace(schema.second[i].property_id, FromPropertyValueToValue(std::move(pk[i]))); } return ret; } std::optional<std::map<PropertyId, Value>> CollectAllPropertiesFromAccessor(const VertexAccessor &acc, View view, - const Schemas::Schema *schema) { + const Schemas::Schema &schema) { std::map<PropertyId, Value> ret; auto props = acc.Properties(view); if (props.HasError()) { @@ -204,7 +205,7 @@ std::optional<std::vector<msgs::Label>> FillUpSourceVertexSecondaryLabels(const std::optional<std::map<PropertyId, Value>> FillUpSourceVertexProperties(const std::optional<VertexAccessor> &v_acc, const msgs::ExpandOneRequest &req, storage::v3::View view, - const Schemas::Schema *schema) { + const Schemas::Schema &schema) { std::map<PropertyId, Value> src_vertex_properties; if (!req.src_vertex_properties) { @@ -321,7 +322,7 @@ bool FillEdges(const std::vector<EdgeAccessor> &edges, msgs::ExpandOneResultRow std::optional<msgs::ExpandOneResultRow> GetExpandOneResult( Shard::Accessor &acc, msgs::VertexId src_vertex, const msgs::ExpandOneRequest &req, const EdgeUniqunessFunction &maybe_filter_based_on_edge_uniquness, const EdgeFiller &edge_filler, - const Schemas::Schema *schema) { + const Schemas::Schema &schema) { /// Fill up source vertex const auto primary_key = ConvertPropertyVector(src_vertex.second); auto v_acc = acc.FindVertex(primary_key, View::NEW); @@ -367,7 +368,7 @@ std::optional<msgs::ExpandOneResultRow> GetExpandOneResult( VertexAccessor v_acc, msgs::VertexId src_vertex, const msgs::ExpandOneRequest &req, std::vector<EdgeAccessor> in_edge_accessors, std::vector<EdgeAccessor> out_edge_accessors, const EdgeUniqunessFunction &maybe_filter_based_on_edge_uniquness, const EdgeFiller &edge_filler, - const Schemas::Schema *schema) { + const Schemas::Schema &schema) { /// Fill up source vertex msgs::Vertex source_vertex = {.id = src_vertex}; if (const auto maybe_secondary_labels = FillUpSourceVertexSecondaryLabels(v_acc, req); maybe_secondary_labels) { @@ -856,7 +857,8 @@ msgs::ReadResponses ShardRsm::HandleRead(msgs::ScanVerticesRequest &&req) { found_props = CollectSpecificPropertiesFromAccessor(vertex, req.props_to_return.value(), view); } else { const auto *schema = shard_->GetSchema(shard_->PrimaryLabel()); - found_props = CollectAllPropertiesFromAccessor(vertex, view, schema); + MG_ASSERT(schema); + found_props = CollectAllPropertiesFromAccessor(vertex, view, *schema); } // TODO(gvolfing) -VERIFY- @@ -958,9 +960,10 @@ msgs::ReadResponses ShardRsm::HandleRead(msgs::ExpandOneRequest &&req) { if (!req.order_by.empty()) { // Can we do differently to avoid this? We need OrderByElements but currently it returns vector<Element>, so this // workaround is here to avoid more duplication later - auto sorted_vertices = OrderByVertices(acc, dba, vertex_accessors, req.order_by); + auto local_sorted_vertices = OrderByVertices( + acc, dba, vertex_accessors, req.order_by); // #NoCommit see whether we can avoid the extra std::transform vertex_accessors.clear(); - std::transform(sorted_vertices.begin(), sorted_vertices.end(), std::back_inserter(vertex_accessors), + std::transform(local_sorted_vertices.begin(), local_sorted_vertices.end(), std::back_inserter(vertex_accessors), [](auto &vertex) { return vertex.object_acc; }); } @@ -982,8 +985,10 @@ msgs::ReadResponses ShardRsm::HandleRead(msgs::ExpandOneRequest &&req) { std::optional<msgs::ExpandOneResultRow> maybe_result; if (req.order_by.empty()) { - maybe_result = GetExpandOneResult(acc, src_vertice, req, maybe_filter_based_on_edge_uniquness, edge_filler, - shard_->GetSchema(shard_->PrimaryLabel())); + auto schema = shard_->GetSchema(shard_->PrimaryLabel()); + MG_ASSERT(schema); + maybe_result = + GetExpandOneResult(acc, src_vertice, req, maybe_filter_based_on_edge_uniquness, edge_filler, *schema); } else { auto [in_edge_accessors, out_edge_accessors] = GetEdgesFromVertex(src_vertex_acc, req.direction); @@ -997,9 +1002,11 @@ msgs::ReadResponses ShardRsm::HandleRead(msgs::ExpandOneRequest &&req) { std::vector<EdgeAccessor> out_edge_ordered_accessors; std::transform(out_ordered_edges.begin(), out_ordered_edges.end(), std::back_inserter(out_edge_ordered_accessors), [](const auto &edge_element) { return edge_element.object_acc; }); - maybe_result = GetExpandOneResult(src_vertex_acc, src_vertice, req, in_edge_ordered_accessors, - out_edge_ordered_accessors, maybe_filter_based_on_edge_uniquness, edge_filler, - shard_->GetSchema(shard_->PrimaryLabel())); + auto schema = shard_->GetSchema(shard_->PrimaryLabel()); + MG_ASSERT(schema); + maybe_result = + GetExpandOneResult(src_vertex_acc, src_vertice, req, in_edge_ordered_accessors, out_edge_ordered_accessors, + maybe_filter_based_on_edge_uniquness, edge_filler, *schema); } if (!maybe_result) { From b3ef0ccd7138968671cf3f6bc278b3072c6d26da Mon Sep 17 00:00:00 2001 From: jeremy <jeremy.bailleux@memgraph.io> Date: Wed, 16 Nov 2022 18:50:22 +0100 Subject: [PATCH 32/63] Moving function from shard_rsm to helper files --- src/storage/v3/request_helper.cpp | 436 +++++++++++++++++++++++++ src/storage/v3/request_helper.hpp | 41 +++ src/storage/v3/shard_rsm.cpp | 457 +-------------------------- src/storage/v3/value_conversions.hpp | 23 ++ 4 files changed, 503 insertions(+), 454 deletions(-) diff --git a/src/storage/v3/request_helper.cpp b/src/storage/v3/request_helper.cpp index 591d7b6d7..b5c7ea2dd 100644 --- a/src/storage/v3/request_helper.cpp +++ b/src/storage/v3/request_helper.cpp @@ -16,8 +16,444 @@ #include "pretty_print_ast_to_original_expression.hpp" #include "storage/v3/bindings/db_accessor.hpp" #include "storage/v3/expr.hpp" +#include "storage/v3/value_conversions.hpp" namespace memgraph::storage::v3 { +using msgs::Label; +using msgs::PropertyId; + +using conversions::ConvertPropertyMap; +using conversions::ConvertPropertyVector; +using conversions::ConvertValueVector; +using conversions::FromPropertyValueToValue; +using conversions::ToMsgsVertexId; +using conversions::ToPropertyValue; + +namespace { +namespace msgs = msgs; + +using AllEdgePropertyDataSructure = std::map<PropertyId, msgs::Value>; +using SpecificEdgePropertyDataSructure = std::vector<msgs::Value>; + +using AllEdgeProperties = std::tuple<msgs::VertexId, msgs::Gid, AllEdgePropertyDataSructure>; +using SpecificEdgeProperties = std::tuple<msgs::VertexId, msgs::Gid, SpecificEdgePropertyDataSructure>; + +using SpecificEdgePropertiesVector = std::vector<SpecificEdgeProperties>; +using AllEdgePropertiesVector = std::vector<AllEdgeProperties>; + +struct VertexIdCmpr { + bool operator()(const storage::v3::VertexId *lhs, const storage::v3::VertexId *rhs) const { return *lhs < *rhs; } +}; + +std::optional<std::map<PropertyId, Value>> PrimaryKeysFromAccessor(const VertexAccessor &acc, View view, + const Schemas::Schema &schema) { + std::map<PropertyId, Value> ret; + auto props = acc.Properties(view); + auto maybe_pk = acc.PrimaryKey(view); + if (maybe_pk.HasError()) { + spdlog::debug("Encountered an error while trying to get vertex primary key."); + return std::nullopt; + } + auto &pk = maybe_pk.GetValue(); + MG_ASSERT(schema.second.size() == pk.size(), "PrimaryKey size does not match schema!"); + for (size_t i{0}; i < schema.second.size(); ++i) { + ret.emplace(schema.second[i].property_id, FromPropertyValueToValue(std::move(pk[i]))); + } + + return ret; +} + +struct LocalError {}; + +std::optional<std::vector<msgs::Label>> FillUpSourceVertexSecondaryLabels(const std::optional<VertexAccessor> &v_acc, + const msgs::ExpandOneRequest &req) { + auto secondary_labels = v_acc->Labels(View::NEW); + if (secondary_labels.HasError()) { + spdlog::debug("Encountered an error while trying to get the secondary labels of a vertex. Transaction id: {}", + req.transaction_id.logical_id); + return std::nullopt; + } + + auto &sec_labels = secondary_labels.GetValue(); + std::vector<msgs::Label> msgs_secondary_labels; + msgs_secondary_labels.reserve(sec_labels.size()); + + std::transform(sec_labels.begin(), sec_labels.end(), std::back_inserter(msgs_secondary_labels), + [](auto label_id) { return msgs::Label{.id = label_id}; }); + + return msgs_secondary_labels; +} + +std::optional<std::map<PropertyId, Value>> FillUpSourceVertexProperties(const std::optional<VertexAccessor> &v_acc, + const msgs::ExpandOneRequest &req, + storage::v3::View view, + const Schemas::Schema &schema) { + std::map<PropertyId, Value> src_vertex_properties; + + if (!req.src_vertex_properties) { + auto props = v_acc->Properties(View::NEW); + if (props.HasError()) { + spdlog::debug("Encountered an error while trying to access vertex properties. Transaction id: {}", + req.transaction_id.logical_id); + return std::nullopt; + } + + for (auto &[key, val] : props.GetValue()) { + src_vertex_properties.insert(std::make_pair(key, FromPropertyValueToValue(std::move(val)))); + } + auto pks = PrimaryKeysFromAccessor(*v_acc, view, schema); + if (pks) { + src_vertex_properties.merge(*pks); + } + + } else if (req.src_vertex_properties.value().empty()) { + // NOOP + } else { + for (const auto &prop : req.src_vertex_properties.value()) { + auto prop_val = v_acc->GetProperty(prop, View::OLD); + if (prop_val.HasError()) { + spdlog::debug("Encountered an error while trying to access vertex properties. Transaction id: {}", + req.transaction_id.logical_id); + return std::nullopt; + } + src_vertex_properties.insert(std::make_pair(prop, FromPropertyValueToValue(std::move(prop_val.GetValue())))); + } + } + + return src_vertex_properties; +} + +std::optional<std::array<std::vector<EdgeAccessor>, 2>> FillUpConnectingEdges( + const std::optional<VertexAccessor> &v_acc, const msgs::ExpandOneRequest &req, + const EdgeUniqunessFunction &maybe_filter_based_on_edge_uniquness) { + std::vector<EdgeTypeId> edge_types{}; + edge_types.reserve(req.edge_types.size()); + std::transform(req.edge_types.begin(), req.edge_types.end(), std::back_inserter(edge_types), + [](const msgs::EdgeType &edge_type) { return edge_type.id; }); + + std::vector<EdgeAccessor> in_edges; + std::vector<EdgeAccessor> out_edges; + + switch (req.direction) { + case msgs::EdgeDirection::OUT: { + auto out_edges_result = v_acc->OutEdges(View::NEW, edge_types); + if (out_edges_result.HasError()) { + spdlog::debug("Encountered an error while trying to get out-going EdgeAccessors. Transaction id: {}", + req.transaction_id.logical_id); + return std::nullopt; + } + out_edges = + maybe_filter_based_on_edge_uniquness(std::move(out_edges_result.GetValue()), msgs::EdgeDirection::OUT); + break; + } + case msgs::EdgeDirection::IN: { + auto in_edges_result = v_acc->InEdges(View::NEW, edge_types); + if (in_edges_result.HasError()) { + spdlog::debug( + "Encountered an error while trying to get in-going EdgeAccessors. Transaction id: {}"[req.transaction_id + .logical_id]); + return std::nullopt; + } + in_edges = maybe_filter_based_on_edge_uniquness(std::move(in_edges_result.GetValue()), msgs::EdgeDirection::IN); + break; + } + case msgs::EdgeDirection::BOTH: { + auto in_edges_result = v_acc->InEdges(View::NEW, edge_types); + if (in_edges_result.HasError()) { + spdlog::debug("Encountered an error while trying to get in-going EdgeAccessors. Transaction id: {}", + req.transaction_id.logical_id); + return std::nullopt; + } + in_edges = maybe_filter_based_on_edge_uniquness(std::move(in_edges_result.GetValue()), msgs::EdgeDirection::IN); + auto out_edges_result = v_acc->OutEdges(View::NEW, edge_types); + if (out_edges_result.HasError()) { + spdlog::debug("Encountered an error while trying to get out-going EdgeAccessors. Transaction id: {}", + req.transaction_id.logical_id); + return std::nullopt; + } + out_edges = + maybe_filter_based_on_edge_uniquness(std::move(out_edges_result.GetValue()), msgs::EdgeDirection::OUT); + break; + } + } + return std::array<std::vector<EdgeAccessor>, 2>{std::move(in_edges), std::move(out_edges)}; +} + +using AllEdgePropertyDataSructure = std::map<PropertyId, msgs::Value>; +using SpecificEdgePropertyDataSructure = std::vector<msgs::Value>; + +using AllEdgeProperties = std::tuple<msgs::VertexId, msgs::Gid, AllEdgePropertyDataSructure>; +using SpecificEdgeProperties = std::tuple<msgs::VertexId, msgs::Gid, SpecificEdgePropertyDataSructure>; + +using SpecificEdgePropertiesVector = std::vector<SpecificEdgeProperties>; +using AllEdgePropertiesVector = std::vector<AllEdgeProperties>; + +using EdgeFiller = std::function<bool(const EdgeAccessor &edge, bool is_in_edge, msgs::ExpandOneResultRow &result_row)>; + +template <bool are_in_edges> +bool FillEdges(const std::vector<EdgeAccessor> &edges, msgs::ExpandOneResultRow &row, const EdgeFiller &edge_filler) { + for (const auto &edge : edges) { + if (!edge_filler(edge, are_in_edges, row)) { + return false; + } + } + + return true; +} + +}; // namespace + +std::optional<std::map<PropertyId, Value>> CollectSpecificPropertiesFromAccessor(const VertexAccessor &acc, + const std::vector<PropertyId> &props, + View view) { + std::map<PropertyId, Value> ret; + + for (const auto &prop : props) { + auto result = acc.GetProperty(prop, view); + if (result.HasError()) { + spdlog::debug("Encountered an Error while trying to get a vertex property."); + return std::nullopt; + } + auto &value = result.GetValue(); + ret.emplace(std::make_pair(prop, FromPropertyValueToValue(std::move(value)))); + } + + return ret; +} + +std::vector<TypedValue> EvaluateVertexExpressions(DbAccessor &dba, const VertexAccessor &v_acc, + const std::vector<std::string> &expressions, + std::string_view node_name) { + std::vector<TypedValue> evaluated_expressions; + evaluated_expressions.reserve(expressions.size()); + + std::transform(expressions.begin(), expressions.end(), std::back_inserter(evaluated_expressions), + [&dba, &v_acc, &node_name](const auto &expression) { + return ComputeExpression(dba, v_acc, std::nullopt, expression, node_name, ""); + }); + + return evaluated_expressions; +} + +std::optional<std::map<PropertyId, Value>> CollectAllPropertiesFromAccessor(const VertexAccessor &acc, View view, + const Schemas::Schema &schema) { + std::map<PropertyId, Value> ret; + auto props = acc.Properties(view); + if (props.HasError()) { + spdlog::debug("Encountered an error while trying to get vertex properties."); + return std::nullopt; + } + + auto &properties = props.GetValue(); + std::transform(properties.begin(), properties.end(), std::inserter(ret, ret.begin()), + [](std::pair<const PropertyId, PropertyValue> &pair) { + return std::make_pair(pair.first, FromPropertyValueToValue(std::move(pair.second))); + }); + properties.clear(); + + auto pks = PrimaryKeysFromAccessor(acc, view, schema); + if (pks) { + ret.merge(*pks); + } + + return ret; +} + +EdgeUniqunessFunction InitializeEdgeUniqunessFunction(bool only_unique_neighbor_rows) { + // Functions to select connecting edges based on uniquness + EdgeUniqunessFunction maybe_filter_based_on_edge_uniquness; + + if (only_unique_neighbor_rows) { + maybe_filter_based_on_edge_uniquness = [](EdgeAccessors &&edges, + msgs::EdgeDirection edge_direction) -> EdgeAccessors { + std::function<bool(std::set<const storage::v3::VertexId *, VertexIdCmpr> &, const storage::v3::EdgeAccessor &)> + is_edge_unique; + switch (edge_direction) { + case msgs::EdgeDirection::OUT: { + is_edge_unique = [](std::set<const storage::v3::VertexId *, VertexIdCmpr> &other_vertex_set, + const storage::v3::EdgeAccessor &edge_acc) { + auto [it, insertion_happened] = other_vertex_set.insert(&edge_acc.ToVertex()); + return insertion_happened; + }; + break; + } + case msgs::EdgeDirection::IN: { + is_edge_unique = [](std::set<const storage::v3::VertexId *, VertexIdCmpr> &other_vertex_set, + const storage::v3::EdgeAccessor &edge_acc) { + auto [it, insertion_happened] = other_vertex_set.insert(&edge_acc.FromVertex()); + return insertion_happened; + }; + break; + } + case msgs::EdgeDirection::BOTH: + MG_ASSERT(false, "This is should never happen, msgs::EdgeDirection::BOTH should not be passed here."); + } + + EdgeAccessors ret; + std::set<const storage::v3::VertexId *, VertexIdCmpr> other_vertex_set; + + for (const auto &edge : edges) { + if (is_edge_unique(other_vertex_set, edge)) { + ret.emplace_back(edge); + } + } + + return ret; + }; + } else { + maybe_filter_based_on_edge_uniquness = + [](EdgeAccessors &&edges, msgs::EdgeDirection /*edge_direction*/) -> EdgeAccessors { return std::move(edges); }; + } + + return maybe_filter_based_on_edge_uniquness; +} + +EdgeFiller InitializeEdgeFillerFunction(const msgs::ExpandOneRequest &req) { + EdgeFiller edge_filler; + + if (!req.edge_properties) { + edge_filler = [transaction_id = req.transaction_id.logical_id](const EdgeAccessor &edge, const bool is_in_edge, + msgs::ExpandOneResultRow &result_row) -> bool { + auto properties_results = edge.Properties(View::NEW); + if (properties_results.HasError()) { + spdlog::debug("Encountered an error while trying to get edge properties. Transaction id: {}", transaction_id); + return false; + } + + std::map<PropertyId, msgs::Value> value_properties; + for (auto &[prop_key, prop_val] : properties_results.GetValue()) { + value_properties.insert(std::make_pair(prop_key, FromPropertyValueToValue(std::move(prop_val)))); + } + using EdgeWithAllProperties = msgs::ExpandOneResultRow::EdgeWithAllProperties; + EdgeWithAllProperties edges{ToMsgsVertexId(edge.FromVertex()), msgs::EdgeType{edge.EdgeType()}, + edge.Gid().AsUint(), std::move(value_properties)}; + if (is_in_edge) { + result_row.in_edges_with_all_properties.push_back(std::move(edges)); + } else { + result_row.out_edges_with_all_properties.push_back(std::move(edges)); + } + return true; + }; + } else { + // TODO(gvolfing) - do we want to set the action_successful here? + edge_filler = [&req](const EdgeAccessor &edge, const bool is_in_edge, + msgs::ExpandOneResultRow &result_row) -> bool { + std::vector<msgs::Value> value_properties; + value_properties.reserve(req.edge_properties.value().size()); + for (const auto &edge_prop : req.edge_properties.value()) { + auto property_result = edge.GetProperty(edge_prop, View::NEW); + if (property_result.HasError()) { + spdlog::debug("Encountered an error while trying to get edge properties. Transaction id: {}", + req.transaction_id.logical_id); + return false; + } + value_properties.emplace_back(FromPropertyValueToValue(std::move(property_result.GetValue()))); + } + using EdgeWithSpecificProperties = msgs::ExpandOneResultRow::EdgeWithSpecificProperties; + EdgeWithSpecificProperties edges{ToMsgsVertexId(edge.FromVertex()), msgs::EdgeType{edge.EdgeType()}, + edge.Gid().AsUint(), std::move(value_properties)}; + if (is_in_edge) { + result_row.in_edges_with_specific_properties.push_back(std::move(edges)); + } else { + result_row.out_edges_with_specific_properties.push_back(std::move(edges)); + } + return true; + }; + } + + return edge_filler; +} + +bool FilterOnVertex(DbAccessor &dba, const storage::v3::VertexAccessor &v_acc, const std::vector<std::string> &filters, + const std::string_view node_name) { + return std::ranges::all_of(filters, [&node_name, &dba, &v_acc](const auto &filter_expr) { + auto res = ComputeExpression(dba, v_acc, std::nullopt, filter_expr, node_name, ""); + return res.IsBool() && res.ValueBool(); + }); +} + +std::optional<msgs::ExpandOneResultRow> GetExpandOneResult( + Shard::Accessor &acc, msgs::VertexId src_vertex, const msgs::ExpandOneRequest &req, + const EdgeUniqunessFunction &maybe_filter_based_on_edge_uniquness, const EdgeFiller &edge_filler, + const Schemas::Schema &schema) { + /// Fill up source vertex + const auto primary_key = ConvertPropertyVector(src_vertex.second); + auto v_acc = acc.FindVertex(primary_key, View::NEW); + + msgs::Vertex source_vertex = {.id = src_vertex}; + if (const auto maybe_secondary_labels = FillUpSourceVertexSecondaryLabels(v_acc, req); maybe_secondary_labels) { + source_vertex.labels = *maybe_secondary_labels; + } else { + return std::nullopt; + } + + std::optional<std::map<PropertyId, Value>> src_vertex_properties; + src_vertex_properties = FillUpSourceVertexProperties(v_acc, req, storage::v3::View::NEW, schema); + + if (!src_vertex_properties) { + return std::nullopt; + } + + /// Fill up connecting edges + auto fill_up_connecting_edges = FillUpConnectingEdges(v_acc, req, maybe_filter_based_on_edge_uniquness); + if (!fill_up_connecting_edges) { + return std::nullopt; + } + + auto [in_edges, out_edges] = fill_up_connecting_edges.value(); + + msgs::ExpandOneResultRow result_row; + result_row.src_vertex = std::move(source_vertex); + result_row.src_vertex_properties = std::move(*src_vertex_properties); + static constexpr bool kInEdges = true; + static constexpr bool kOutEdges = false; + if (!in_edges.empty() && !FillEdges<kInEdges>(in_edges, result_row, edge_filler)) { + return std::nullopt; + } + if (!out_edges.empty() && !FillEdges<kOutEdges>(out_edges, result_row, edge_filler)) { + return std::nullopt; + } + + return result_row; +} + +std::optional<msgs::ExpandOneResultRow> GetExpandOneResult( + VertexAccessor v_acc, msgs::VertexId src_vertex, const msgs::ExpandOneRequest &req, + std::vector<EdgeAccessor> in_edge_accessors, std::vector<EdgeAccessor> out_edge_accessors, + const EdgeUniqunessFunction &maybe_filter_based_on_edge_uniquness, const EdgeFiller &edge_filler, + const Schemas::Schema &schema) { + /// Fill up source vertex + msgs::Vertex source_vertex = {.id = src_vertex}; + if (const auto maybe_secondary_labels = FillUpSourceVertexSecondaryLabels(v_acc, req); maybe_secondary_labels) { + source_vertex.labels = *maybe_secondary_labels; + } else { + return std::nullopt; + } + + /// Fill up source vertex properties + auto src_vertex_properties = FillUpSourceVertexProperties(v_acc, req, storage::v3::View::NEW, schema); + if (!src_vertex_properties) { + return std::nullopt; + } + + /// Fill up connecting edges + auto in_edges = maybe_filter_based_on_edge_uniquness(std::move(in_edge_accessors), msgs::EdgeDirection::IN); + auto out_edges = maybe_filter_based_on_edge_uniquness(std::move(out_edge_accessors), msgs::EdgeDirection::OUT); + + msgs::ExpandOneResultRow result_row; + result_row.src_vertex = std::move(source_vertex); + result_row.src_vertex_properties = std::move(*src_vertex_properties); + static constexpr bool kInEdges = true; + static constexpr bool kOutEdges = false; + if (!in_edges.empty() && !FillEdges<kInEdges>(in_edges, result_row, edge_filler)) { + return std::nullopt; + } + if (!out_edges.empty() && !FillEdges<kOutEdges>(out_edges, result_row, edge_filler)) { + return std::nullopt; + } + + return result_row; +} VerticesIterable::Iterator GetStartVertexIterator(VerticesIterable &vertex_iterable, const std::vector<PropertyValue> &start_ids, const View view) { diff --git a/src/storage/v3/request_helper.hpp b/src/storage/v3/request_helper.hpp index 4d3018566..035cc1ae5 100644 --- a/src/storage/v3/request_helper.hpp +++ b/src/storage/v3/request_helper.hpp @@ -22,6 +22,11 @@ #include "storage/v3/shard.hpp" #include "storage/v3/vertex_accessor.hpp" namespace memgraph::storage::v3 { +using EdgeAccessors = std::vector<storage::v3::EdgeAccessor>; +using EdgeUniqunessFunction = std::function<EdgeAccessors(EdgeAccessors &&, msgs::EdgeDirection)>; +using EdgeFiller = std::function<bool(const EdgeAccessor &edge, bool is_in_edge, msgs::ExpandOneResultRow &result_row)>; +using msgs::Value; + template <typename T> concept ObjectAccessor = std::is_same_v<T, VertexAccessor> || std::is_same_v<T, EdgeAccessor>; @@ -194,4 +199,40 @@ std::vector<Element<VertexAccessor>>::const_iterator GetStartOrderedElementsIter std::array<std::vector<EdgeAccessor>, 2> GetEdgesFromVertex(const VertexAccessor &vertex_accessor, msgs::EdgeDirection direction); + +bool FilterOnVertex(DbAccessor &dba, const storage::v3::VertexAccessor &v_acc, const std::vector<std::string> &filters, + const std::string_view node_name); + +std::vector<TypedValue> EvaluateVertexExpressions(DbAccessor &dba, const VertexAccessor &v_acc, + const std::vector<std::string> &expressions, + std::string_view node_name); + +std::optional<std::map<PropertyId, Value>> CollectSpecificPropertiesFromAccessor(const VertexAccessor &acc, + const std::vector<PropertyId> &props, + View view); + +std::optional<std::map<PropertyId, Value>> CollectAllPropertiesFromAccessor(const VertexAccessor &acc, View view, + const Schemas::Schema &schema); + +EdgeUniqunessFunction InitializeEdgeUniqunessFunction(bool only_unique_neighbor_rows); + +EdgeFiller InitializeEdgeFillerFunction(const msgs::ExpandOneRequest &req); + +std::optional<std::map<PropertyId, Value>> CollectSpecificPropertiesFromAccessor(const VertexAccessor &acc, + const std::vector<PropertyId> &props, + View view); + +bool FilterOnVertex(DbAccessor &dba, const storage::v3::VertexAccessor &v_acc, const std::vector<std::string> &filters, + const std::string_view node_name); + +std::optional<msgs::ExpandOneResultRow> GetExpandOneResult( + Shard::Accessor &acc, msgs::VertexId src_vertex, const msgs::ExpandOneRequest &req, + const EdgeUniqunessFunction &maybe_filter_based_on_edge_uniquness, const EdgeFiller &edge_filler, + const Schemas::Schema &schema); + +std::optional<msgs::ExpandOneResultRow> GetExpandOneResult( + VertexAccessor v_acc, msgs::VertexId src_vertex, const msgs::ExpandOneRequest &req, + std::vector<EdgeAccessor> in_edge_accessors, std::vector<EdgeAccessor> out_edge_accessors, + const EdgeUniqunessFunction &maybe_filter_based_on_edge_uniquness, const EdgeFiller &edge_filler, + const Schemas::Schema &schema); } // namespace memgraph::storage::v3 diff --git a/src/storage/v3/shard_rsm.cpp b/src/storage/v3/shard_rsm.cpp index bf31e8ced..841b64e44 100644 --- a/src/storage/v3/shard_rsm.cpp +++ b/src/storage/v3/shard_rsm.cpp @@ -45,469 +45,18 @@ #include "utils/logging.hpp" namespace memgraph::storage::v3 { -using msgs::Label; +using msgs::Label; // #NoCommit not needed? using msgs::PropertyId; using msgs::Value; +using conversions::ConvertPropertyMap; using conversions::ConvertPropertyVector; using conversions::ConvertValueVector; +using conversions::FromMap; using conversions::FromPropertyValueToValue; using conversions::ToMsgsVertexId; using conversions::ToPropertyValue; -namespace { -namespace msgs = msgs; - -using AllEdgePropertyDataSructure = std::map<PropertyId, msgs::Value>; -using SpecificEdgePropertyDataSructure = std::vector<msgs::Value>; - -using AllEdgeProperties = std::tuple<msgs::VertexId, msgs::Gid, AllEdgePropertyDataSructure>; -using SpecificEdgeProperties = std::tuple<msgs::VertexId, msgs::Gid, SpecificEdgePropertyDataSructure>; - -using SpecificEdgePropertiesVector = std::vector<SpecificEdgeProperties>; -using AllEdgePropertiesVector = std::vector<AllEdgeProperties>; - -using EdgeAccessors = std::vector<storage::v3::EdgeAccessor>; - -using EdgeFiller = std::function<bool(const EdgeAccessor &edge, bool is_in_edge, msgs::ExpandOneResultRow &result_row)>; -using EdgeUniqunessFunction = std::function<EdgeAccessors(EdgeAccessors &&, msgs::EdgeDirection)>; - -struct VertexIdCmpr { - bool operator()(const storage::v3::VertexId *lhs, const storage::v3::VertexId *rhs) const { return *lhs < *rhs; } -}; - -std::vector<std::pair<PropertyId, PropertyValue>> ConvertPropertyMap( - std::vector<std::pair<PropertyId, Value>> &&properties) { - std::vector<std::pair<PropertyId, PropertyValue>> ret; - ret.reserve(properties.size()); - - std::transform(std::make_move_iterator(properties.begin()), std::make_move_iterator(properties.end()), - std::back_inserter(ret), [](std::pair<PropertyId, Value> &&property) { - return std::make_pair(property.first, ToPropertyValue(std::move(property.second))); - }); - - return ret; -} - -std::vector<std::pair<PropertyId, Value>> FromMap(const std::map<PropertyId, Value> &properties) { - std::vector<std::pair<PropertyId, Value>> ret; - ret.reserve(properties.size()); - - std::transform(properties.begin(), properties.end(), std::back_inserter(ret), - [](const auto &property) { return std::make_pair(property.first, property.second); }); - - return ret; -} - -std::optional<std::map<PropertyId, Value>> CollectSpecificPropertiesFromAccessor(const VertexAccessor &acc, - const std::vector<PropertyId> &props, - View view) { - std::map<PropertyId, Value> ret; - - for (const auto &prop : props) { - auto result = acc.GetProperty(prop, view); - if (result.HasError()) { - spdlog::debug("Encountered an Error while trying to get a vertex property."); - return std::nullopt; - } - auto &value = result.GetValue(); - ret.emplace(std::make_pair(prop, FromPropertyValueToValue(std::move(value)))); - } - - return ret; -} - -std::optional<std::map<PropertyId, Value>> PrimaryKeysFromAccessor(const VertexAccessor &acc, View view, - const Schemas::Schema &schema) { - std::map<PropertyId, Value> ret; - auto props = acc.Properties(view); - auto maybe_pk = acc.PrimaryKey(view); - if (maybe_pk.HasError()) { - spdlog::debug("Encountered an error while trying to get vertex primary key."); - return std::nullopt; - } - auto &pk = maybe_pk.GetValue(); - MG_ASSERT(schema.second.size() == pk.size(), "PrimaryKey size does not match schema!"); - for (size_t i{0}; i < schema.second.size(); ++i) { - ret.emplace(schema.second[i].property_id, FromPropertyValueToValue(std::move(pk[i]))); - } - - return ret; -} - -std::optional<std::map<PropertyId, Value>> CollectAllPropertiesFromAccessor(const VertexAccessor &acc, View view, - const Schemas::Schema &schema) { - std::map<PropertyId, Value> ret; - auto props = acc.Properties(view); - if (props.HasError()) { - spdlog::debug("Encountered an error while trying to get vertex properties."); - return std::nullopt; - } - - auto &properties = props.GetValue(); - std::transform(properties.begin(), properties.end(), std::inserter(ret, ret.begin()), - [](std::pair<const PropertyId, PropertyValue> &pair) { - return std::make_pair(pair.first, FromPropertyValueToValue(std::move(pair.second))); - }); - properties.clear(); - - auto pks = PrimaryKeysFromAccessor(acc, view, schema); - if (pks) { - ret.merge(*pks); - } - - return ret; -} - -bool FilterOnVertex(DbAccessor &dba, const storage::v3::VertexAccessor &v_acc, const std::vector<std::string> &filters, - const std::string_view node_name) { - return std::ranges::all_of(filters, [&node_name, &dba, &v_acc](const auto &filter_expr) { - auto res = ComputeExpression(dba, v_acc, std::nullopt, filter_expr, node_name, ""); - return res.IsBool() && res.ValueBool(); - }); -} - -std::vector<TypedValue> EvaluateVertexExpressions(DbAccessor &dba, const VertexAccessor &v_acc, - const std::vector<std::string> &expressions, - std::string_view node_name) { - std::vector<TypedValue> evaluated_expressions; - evaluated_expressions.reserve(expressions.size()); - - std::transform(expressions.begin(), expressions.end(), std::back_inserter(evaluated_expressions), - [&dba, &v_acc, &node_name](const auto &expression) { - return ComputeExpression(dba, v_acc, std::nullopt, expression, node_name, ""); - }); - - return evaluated_expressions; -} - -struct LocalError {}; - -std::optional<std::vector<msgs::Label>> FillUpSourceVertexSecondaryLabels(const std::optional<VertexAccessor> &v_acc, - const msgs::ExpandOneRequest &req) { - auto secondary_labels = v_acc->Labels(View::NEW); - if (secondary_labels.HasError()) { - spdlog::debug("Encountered an error while trying to get the secondary labels of a vertex. Transaction id: {}", - req.transaction_id.logical_id); - return std::nullopt; - } - - auto &sec_labels = secondary_labels.GetValue(); - std::vector<msgs::Label> msgs_secondary_labels; - msgs_secondary_labels.reserve(sec_labels.size()); - - std::transform(sec_labels.begin(), sec_labels.end(), std::back_inserter(msgs_secondary_labels), - [](auto label_id) { return msgs::Label{.id = label_id}; }); - - return msgs_secondary_labels; -} - -std::optional<std::map<PropertyId, Value>> FillUpSourceVertexProperties(const std::optional<VertexAccessor> &v_acc, - const msgs::ExpandOneRequest &req, - storage::v3::View view, - const Schemas::Schema &schema) { - std::map<PropertyId, Value> src_vertex_properties; - - if (!req.src_vertex_properties) { - auto props = v_acc->Properties(View::NEW); - if (props.HasError()) { - spdlog::debug("Encountered an error while trying to access vertex properties. Transaction id: {}", - req.transaction_id.logical_id); - return std::nullopt; - } - - for (auto &[key, val] : props.GetValue()) { - src_vertex_properties.insert(std::make_pair(key, FromPropertyValueToValue(std::move(val)))); - } - auto pks = PrimaryKeysFromAccessor(*v_acc, view, schema); - if (pks) { - src_vertex_properties.merge(*pks); - } - - } else if (req.src_vertex_properties.value().empty()) { - // NOOP - } else { - for (const auto &prop : req.src_vertex_properties.value()) { - auto prop_val = v_acc->GetProperty(prop, View::OLD); - if (prop_val.HasError()) { - spdlog::debug("Encountered an error while trying to access vertex properties. Transaction id: {}", - req.transaction_id.logical_id); - return std::nullopt; - } - src_vertex_properties.insert(std::make_pair(prop, FromPropertyValueToValue(std::move(prop_val.GetValue())))); - } - } - - return src_vertex_properties; -} - -std::optional<std::array<std::vector<EdgeAccessor>, 2>> FillUpConnectingEdges( - const std::optional<VertexAccessor> &v_acc, const msgs::ExpandOneRequest &req, - const EdgeUniqunessFunction &maybe_filter_based_on_edge_uniquness) { - std::vector<EdgeTypeId> edge_types{}; - edge_types.reserve(req.edge_types.size()); - std::transform(req.edge_types.begin(), req.edge_types.end(), std::back_inserter(edge_types), - [](const msgs::EdgeType &edge_type) { return edge_type.id; }); - - std::vector<EdgeAccessor> in_edges; - std::vector<EdgeAccessor> out_edges; - - switch (req.direction) { - case msgs::EdgeDirection::OUT: { - auto out_edges_result = v_acc->OutEdges(View::NEW, edge_types); - if (out_edges_result.HasError()) { - spdlog::debug("Encountered an error while trying to get out-going EdgeAccessors. Transaction id: {}", - req.transaction_id.logical_id); - return std::nullopt; - } - out_edges = - maybe_filter_based_on_edge_uniquness(std::move(out_edges_result.GetValue()), msgs::EdgeDirection::OUT); - break; - } - case msgs::EdgeDirection::IN: { - auto in_edges_result = v_acc->InEdges(View::NEW, edge_types); - if (in_edges_result.HasError()) { - spdlog::debug( - "Encountered an error while trying to get in-going EdgeAccessors. Transaction id: {}"[req.transaction_id - .logical_id]); - return std::nullopt; - } - in_edges = maybe_filter_based_on_edge_uniquness(std::move(in_edges_result.GetValue()), msgs::EdgeDirection::IN); - break; - } - case msgs::EdgeDirection::BOTH: { - auto in_edges_result = v_acc->InEdges(View::NEW, edge_types); - if (in_edges_result.HasError()) { - spdlog::debug("Encountered an error while trying to get in-going EdgeAccessors. Transaction id: {}", - req.transaction_id.logical_id); - return std::nullopt; - } - in_edges = maybe_filter_based_on_edge_uniquness(std::move(in_edges_result.GetValue()), msgs::EdgeDirection::IN); - auto out_edges_result = v_acc->OutEdges(View::NEW, edge_types); - if (out_edges_result.HasError()) { - spdlog::debug("Encountered an error while trying to get out-going EdgeAccessors. Transaction id: {}", - req.transaction_id.logical_id); - return std::nullopt; - } - out_edges = - maybe_filter_based_on_edge_uniquness(std::move(out_edges_result.GetValue()), msgs::EdgeDirection::OUT); - break; - } - } - return std::array<std::vector<EdgeAccessor>, 2>{std::move(in_edges), std::move(out_edges)}; -} - -using AllEdgePropertyDataSructure = std::map<PropertyId, msgs::Value>; -using SpecificEdgePropertyDataSructure = std::vector<msgs::Value>; - -using AllEdgeProperties = std::tuple<msgs::VertexId, msgs::Gid, AllEdgePropertyDataSructure>; -using SpecificEdgeProperties = std::tuple<msgs::VertexId, msgs::Gid, SpecificEdgePropertyDataSructure>; - -using SpecificEdgePropertiesVector = std::vector<SpecificEdgeProperties>; -using AllEdgePropertiesVector = std::vector<AllEdgeProperties>; - -using EdgeFiller = std::function<bool(const EdgeAccessor &edge, bool is_in_edge, msgs::ExpandOneResultRow &result_row)>; - -template <bool are_in_edges> -bool FillEdges(const std::vector<EdgeAccessor> &edges, msgs::ExpandOneResultRow &row, const EdgeFiller &edge_filler) { - for (const auto &edge : edges) { - if (!edge_filler(edge, are_in_edges, row)) { - return false; - } - } - - return true; -} - -std::optional<msgs::ExpandOneResultRow> GetExpandOneResult( - Shard::Accessor &acc, msgs::VertexId src_vertex, const msgs::ExpandOneRequest &req, - const EdgeUniqunessFunction &maybe_filter_based_on_edge_uniquness, const EdgeFiller &edge_filler, - const Schemas::Schema &schema) { - /// Fill up source vertex - const auto primary_key = ConvertPropertyVector(src_vertex.second); - auto v_acc = acc.FindVertex(primary_key, View::NEW); - - msgs::Vertex source_vertex = {.id = src_vertex}; - if (const auto maybe_secondary_labels = FillUpSourceVertexSecondaryLabels(v_acc, req); maybe_secondary_labels) { - source_vertex.labels = *maybe_secondary_labels; - } else { - return std::nullopt; - } - - std::optional<std::map<PropertyId, Value>> src_vertex_properties; - src_vertex_properties = FillUpSourceVertexProperties(v_acc, req, storage::v3::View::NEW, schema); - - if (!src_vertex_properties) { - return std::nullopt; - } - - /// Fill up connecting edges - auto fill_up_connecting_edges = FillUpConnectingEdges(v_acc, req, maybe_filter_based_on_edge_uniquness); - if (!fill_up_connecting_edges) { - return std::nullopt; - } - - auto [in_edges, out_edges] = fill_up_connecting_edges.value(); - - msgs::ExpandOneResultRow result_row; - result_row.src_vertex = std::move(source_vertex); - result_row.src_vertex_properties = std::move(*src_vertex_properties); - static constexpr bool kInEdges = true; - static constexpr bool kOutEdges = false; - if (!in_edges.empty() && !FillEdges<kInEdges>(in_edges, result_row, edge_filler)) { - return std::nullopt; - } - if (!out_edges.empty() && !FillEdges<kOutEdges>(out_edges, result_row, edge_filler)) { - return std::nullopt; - } - - return result_row; -} - -std::optional<msgs::ExpandOneResultRow> GetExpandOneResult( - VertexAccessor v_acc, msgs::VertexId src_vertex, const msgs::ExpandOneRequest &req, - std::vector<EdgeAccessor> in_edge_accessors, std::vector<EdgeAccessor> out_edge_accessors, - const EdgeUniqunessFunction &maybe_filter_based_on_edge_uniquness, const EdgeFiller &edge_filler, - const Schemas::Schema &schema) { - /// Fill up source vertex - msgs::Vertex source_vertex = {.id = src_vertex}; - if (const auto maybe_secondary_labels = FillUpSourceVertexSecondaryLabels(v_acc, req); maybe_secondary_labels) { - source_vertex.labels = *maybe_secondary_labels; - } else { - return std::nullopt; - } - - /// Fill up source vertex properties - auto src_vertex_properties = FillUpSourceVertexProperties(v_acc, req, storage::v3::View::NEW, schema); - if (!src_vertex_properties) { - return std::nullopt; - } - - /// Fill up connecting edges - auto in_edges = maybe_filter_based_on_edge_uniquness(std::move(in_edge_accessors), msgs::EdgeDirection::IN); - auto out_edges = maybe_filter_based_on_edge_uniquness(std::move(out_edge_accessors), msgs::EdgeDirection::OUT); - - msgs::ExpandOneResultRow result_row; - result_row.src_vertex = std::move(source_vertex); - result_row.src_vertex_properties = std::move(*src_vertex_properties); - static constexpr bool kInEdges = true; - static constexpr bool kOutEdges = false; - if (!in_edges.empty() && !FillEdges<kInEdges>(in_edges, result_row, edge_filler)) { - return std::nullopt; - } - if (!out_edges.empty() && !FillEdges<kOutEdges>(out_edges, result_row, edge_filler)) { - return std::nullopt; - } - - return result_row; -} - -EdgeUniqunessFunction InitializeEdgeUniqunessFunction(bool only_unique_neighbor_rows) { - // Functions to select connecting edges based on uniquness - EdgeUniqunessFunction maybe_filter_based_on_edge_uniquness; - - if (only_unique_neighbor_rows) { - maybe_filter_based_on_edge_uniquness = [](EdgeAccessors &&edges, - msgs::EdgeDirection edge_direction) -> EdgeAccessors { - std::function<bool(std::set<const storage::v3::VertexId *, VertexIdCmpr> &, const storage::v3::EdgeAccessor &)> - is_edge_unique; - switch (edge_direction) { - case msgs::EdgeDirection::OUT: { - is_edge_unique = [](std::set<const storage::v3::VertexId *, VertexIdCmpr> &other_vertex_set, - const storage::v3::EdgeAccessor &edge_acc) { - auto [it, insertion_happened] = other_vertex_set.insert(&edge_acc.ToVertex()); - return insertion_happened; - }; - break; - } - case msgs::EdgeDirection::IN: { - is_edge_unique = [](std::set<const storage::v3::VertexId *, VertexIdCmpr> &other_vertex_set, - const storage::v3::EdgeAccessor &edge_acc) { - auto [it, insertion_happened] = other_vertex_set.insert(&edge_acc.FromVertex()); - return insertion_happened; - }; - break; - } - case msgs::EdgeDirection::BOTH: - MG_ASSERT(false, "This is should never happen, msgs::EdgeDirection::BOTH should not be passed here."); - } - - EdgeAccessors ret; - std::set<const storage::v3::VertexId *, VertexIdCmpr> other_vertex_set; - - for (const auto &edge : edges) { - if (is_edge_unique(other_vertex_set, edge)) { - ret.emplace_back(edge); - } - } - - return ret; - }; - } else { - maybe_filter_based_on_edge_uniquness = - [](EdgeAccessors &&edges, msgs::EdgeDirection /*edge_direction*/) -> EdgeAccessors { return std::move(edges); }; - } - - return maybe_filter_based_on_edge_uniquness; -} - -EdgeFiller InitializeEdgeFillerFunction(const msgs::ExpandOneRequest &req) { - EdgeFiller edge_filler; - - if (!req.edge_properties) { - edge_filler = [transaction_id = req.transaction_id.logical_id](const EdgeAccessor &edge, const bool is_in_edge, - msgs::ExpandOneResultRow &result_row) -> bool { - auto properties_results = edge.Properties(View::NEW); - if (properties_results.HasError()) { - spdlog::debug("Encountered an error while trying to get edge properties. Transaction id: {}", transaction_id); - return false; - } - - std::map<PropertyId, msgs::Value> value_properties; - for (auto &[prop_key, prop_val] : properties_results.GetValue()) { - value_properties.insert(std::make_pair(prop_key, FromPropertyValueToValue(std::move(prop_val)))); - } - using EdgeWithAllProperties = msgs::ExpandOneResultRow::EdgeWithAllProperties; - EdgeWithAllProperties edges{ToMsgsVertexId(edge.FromVertex()), msgs::EdgeType{edge.EdgeType()}, - edge.Gid().AsUint(), std::move(value_properties)}; - if (is_in_edge) { - result_row.in_edges_with_all_properties.push_back(std::move(edges)); - } else { - result_row.out_edges_with_all_properties.push_back(std::move(edges)); - } - return true; - }; - } else { - // TODO(gvolfing) - do we want to set the action_successful here? - edge_filler = [&req](const EdgeAccessor &edge, const bool is_in_edge, - msgs::ExpandOneResultRow &result_row) -> bool { - std::vector<msgs::Value> value_properties; - value_properties.reserve(req.edge_properties.value().size()); - for (const auto &edge_prop : req.edge_properties.value()) { - auto property_result = edge.GetProperty(edge_prop, View::NEW); - if (property_result.HasError()) { - spdlog::debug("Encountered an error while trying to get edge properties. Transaction id: {}", - req.transaction_id.logical_id); - return false; - } - value_properties.emplace_back(FromPropertyValueToValue(std::move(property_result.GetValue()))); - } - using EdgeWithSpecificProperties = msgs::ExpandOneResultRow::EdgeWithSpecificProperties; - EdgeWithSpecificProperties edges{ToMsgsVertexId(edge.FromVertex()), msgs::EdgeType{edge.EdgeType()}, - edge.Gid().AsUint(), std::move(value_properties)}; - if (is_in_edge) { - result_row.in_edges_with_specific_properties.push_back(std::move(edges)); - } else { - result_row.out_edges_with_specific_properties.push_back(std::move(edges)); - } - return true; - }; - } - - return edge_filler; -} - -}; // namespace msgs::WriteResponses ShardRsm::ApplyWrite(msgs::CreateVerticesRequest &&req) { auto acc = shard_->Access(req.transaction_id); diff --git a/src/storage/v3/value_conversions.hpp b/src/storage/v3/value_conversions.hpp index 05fd1394b..80b6f02b9 100644 --- a/src/storage/v3/value_conversions.hpp +++ b/src/storage/v3/value_conversions.hpp @@ -129,4 +129,27 @@ inline std::vector<Value> ConvertValueVector(const std::vector<v3::PropertyValue inline msgs::VertexId ToMsgsVertexId(const v3::VertexId &vertex_id) { return {msgs::Label{vertex_id.primary_label}, ConvertValueVector(vertex_id.primary_key)}; } + +inline std::vector<std::pair<v3::PropertyId, v3::PropertyValue>> ConvertPropertyMap( + std::vector<std::pair<v3::PropertyId, Value>> &&properties) { + std::vector<std::pair<v3::PropertyId, v3::PropertyValue>> ret; + ret.reserve(properties.size()); + + std::transform(std::make_move_iterator(properties.begin()), std::make_move_iterator(properties.end()), + std::back_inserter(ret), [](std::pair<v3::PropertyId, Value> &&property) { + return std::make_pair(property.first, ToPropertyValue(std::move(property.second))); + }); + + return ret; +} + +inline std::vector<std::pair<PropertyId, Value>> FromMap(const std::map<PropertyId, Value> &properties) { + std::vector<std::pair<PropertyId, Value>> ret; + ret.reserve(properties.size()); + + std::transform(properties.begin(), properties.end(), std::back_inserter(ret), + [](const auto &property) { return std::make_pair(property.first, property.second); }); + + return ret; +} } // namespace memgraph::storage::conversions From c4e22ffde3606b92a26cbeb351d04fca8f649b38 Mon Sep 17 00:00:00 2001 From: jeremy <jeremy.bailleux@memgraph.io> Date: Wed, 16 Nov 2022 18:51:57 +0100 Subject: [PATCH 33/63] Remove unnecessary tag --- src/storage/v3/shard_rsm.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/storage/v3/shard_rsm.cpp b/src/storage/v3/shard_rsm.cpp index 841b64e44..5bb925fa8 100644 --- a/src/storage/v3/shard_rsm.cpp +++ b/src/storage/v3/shard_rsm.cpp @@ -45,7 +45,7 @@ #include "utils/logging.hpp" namespace memgraph::storage::v3 { -using msgs::Label; // #NoCommit not needed? +using msgs::Label; using msgs::PropertyId; using msgs::Value; From a17a6aea5a880ba144ea51d345e6080165269eca Mon Sep 17 00:00:00 2001 From: jeremy <jeremy.bailleux@memgraph.io> Date: Thu, 17 Nov 2022 12:27:12 +0100 Subject: [PATCH 34/63] rename variable vertice->vertex --- src/storage/v3/shard_rsm.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/storage/v3/shard_rsm.cpp b/src/storage/v3/shard_rsm.cpp index 5bb925fa8..88aa028e6 100644 --- a/src/storage/v3/shard_rsm.cpp +++ b/src/storage/v3/shard_rsm.cpp @@ -510,7 +510,7 @@ msgs::ReadResponses ShardRsm::HandleRead(msgs::ExpandOneRequest &&req) { // Can we do differently to avoid this? We need OrderByElements but currently it returns vector<Element>, so this // workaround is here to avoid more duplication later auto local_sorted_vertices = OrderByVertices( - acc, dba, vertex_accessors, req.order_by); // #NoCommit see whether we can avoid the extra std::transform + acc, dba, vertex_accessors, req.order_by); // #NoCommit see whether we can avoid the extra std::transform vertex_accessors.clear(); std::transform(local_sorted_vertices.begin(), local_sorted_vertices.end(), std::back_inserter(vertex_accessors), [](auto &vertex) { return vertex.object_acc; }); @@ -529,7 +529,7 @@ msgs::ReadResponses ShardRsm::HandleRead(msgs::ExpandOneRequest &&req) { break; } - msgs::VertexId src_vertice(msgs::Label{.id = *label_id}, conversions::ConvertValueVector(*primary_key)); + msgs::VertexId src_vertex(msgs::Label{.id = *label_id}, conversions::ConvertValueVector(*primary_key)); std::optional<msgs::ExpandOneResultRow> maybe_result; @@ -537,7 +537,7 @@ msgs::ReadResponses ShardRsm::HandleRead(msgs::ExpandOneRequest &&req) { auto schema = shard_->GetSchema(shard_->PrimaryLabel()); MG_ASSERT(schema); maybe_result = - GetExpandOneResult(acc, src_vertice, req, maybe_filter_based_on_edge_uniquness, edge_filler, *schema); + GetExpandOneResult(acc, src_vertex, req, maybe_filter_based_on_edge_uniquness, edge_filler, *schema); } else { auto [in_edge_accessors, out_edge_accessors] = GetEdgesFromVertex(src_vertex_acc, req.direction); @@ -554,7 +554,7 @@ msgs::ReadResponses ShardRsm::HandleRead(msgs::ExpandOneRequest &&req) { auto schema = shard_->GetSchema(shard_->PrimaryLabel()); MG_ASSERT(schema); maybe_result = - GetExpandOneResult(src_vertex_acc, src_vertice, req, in_edge_ordered_accessors, out_edge_ordered_accessors, + GetExpandOneResult(src_vertex_acc, src_vertex, req, in_edge_ordered_accessors, out_edge_ordered_accessors, maybe_filter_based_on_edge_uniquness, edge_filler, *schema); } From 68e51e73bacf66bb386dfb3edf66502a3933becd Mon Sep 17 00:00:00 2001 From: Jeremy B <97525434+42jeremy@users.noreply.github.com> Date: Thu, 17 Nov 2022 12:42:36 +0100 Subject: [PATCH 35/63] Update src/storage/v3/shard_rsm.cpp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: János Benjamin Antal <antaljanosbenjamin@users.noreply.github.com> --- src/storage/v3/shard_rsm.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/storage/v3/shard_rsm.cpp b/src/storage/v3/shard_rsm.cpp index 5bb925fa8..ac6e7ae66 100644 --- a/src/storage/v3/shard_rsm.cpp +++ b/src/storage/v3/shard_rsm.cpp @@ -563,7 +563,7 @@ msgs::ReadResponses ShardRsm::HandleRead(msgs::ExpandOneRequest &&req) { break; } - results.emplace_back(maybe_result.value()); + results.emplace_back(std::move(maybe_result.value())); if (batch_limit.has_value() && results.size() >= batch_limit.value()) { break; } From 2f554912717b77c9cd41550011064a44ed0ac6a0 Mon Sep 17 00:00:00 2001 From: jeremy <jeremy.bailleux@memgraph.io> Date: Thu, 17 Nov 2022 12:45:15 +0100 Subject: [PATCH 36/63] use std::SameAsAnyOf i.o. is_sale_v --- src/storage/v3/request_helper.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/storage/v3/request_helper.hpp b/src/storage/v3/request_helper.hpp index 035cc1ae5..9359bd52f 100644 --- a/src/storage/v3/request_helper.hpp +++ b/src/storage/v3/request_helper.hpp @@ -28,7 +28,7 @@ using EdgeFiller = std::function<bool(const EdgeAccessor &edge, bool is_in_edge, using msgs::Value; template <typename T> -concept ObjectAccessor = std::is_same_v<T, VertexAccessor> || std::is_same_v<T, EdgeAccessor>; +concept ObjectAccessor = utils::SameAsAnyOf<T, VertexAccessor, EdgeAccessor>; inline bool TypedValueCompare(const TypedValue &a, const TypedValue &b) { // in ordering null comes after everything else From 38b0b308cedf9d9b80afb20842b54fbdf8e40cf6 Mon Sep 17 00:00:00 2001 From: jeremy <jeremy.bailleux@memgraph.io> Date: Thu, 17 Nov 2022 12:58:28 +0100 Subject: [PATCH 37/63] Remove unnecessary reserve --- src/storage/v3/request_helper.hpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/storage/v3/request_helper.hpp b/src/storage/v3/request_helper.hpp index 9359bd52f..309902645 100644 --- a/src/storage/v3/request_helper.hpp +++ b/src/storage/v3/request_helper.hpp @@ -133,7 +133,6 @@ std::vector<Element<VertexAccessor>> OrderByVertices(Shard::Accessor &acc, DbAcc }); std::vector<Element<VertexAccessor>> ordered; - ordered.reserve(acc.ApproximateVertexCount()); for (auto it = iterable.begin(); it != iterable.end(); ++it) { std::vector<TypedValue> properties_order_by; properties_order_by.reserve(order_bys.size()); From 77ab07d99100b568fb00db32d6e19bcc654407d7 Mon Sep 17 00:00:00 2001 From: Jeremy B <97525434+42jeremy@users.noreply.github.com> Date: Thu, 17 Nov 2022 12:58:39 +0100 Subject: [PATCH 38/63] Update src/storage/v3/request_helper.hpp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: János Benjamin Antal <antaljanosbenjamin@users.noreply.github.com> --- src/storage/v3/request_helper.hpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/storage/v3/request_helper.hpp b/src/storage/v3/request_helper.hpp index 035cc1ae5..bc622cc73 100644 --- a/src/storage/v3/request_helper.hpp +++ b/src/storage/v3/request_helper.hpp @@ -117,10 +117,11 @@ struct Element { TObjectAccessor object_acc; }; -template <typename TIterable> +template <typename T> +concept IDontKnowAGoodNameForThis = utils::SameAsAnyOf<T, VerticesIterable, std::vector<VertexAccessor>> +template <IDontKnowAGoodNameForThis TIterable> std::vector<Element<VertexAccessor>> OrderByVertices(Shard::Accessor &acc, DbAccessor &dba, TIterable &iterable, std::vector<msgs::OrderBy> &order_bys) { - static_assert(std::is_same_v<TIterable, VerticesIterable> || std::is_same_v<TIterable, std::vector<VertexAccessor>>); std::vector<Ordering> ordering; ordering.reserve(order_bys.size()); From 6e44c2295d289080267eb37a3fe6ec157c4e4261 Mon Sep 17 00:00:00 2001 From: jeremy <jeremy.bailleux@memgraph.io> Date: Thu, 17 Nov 2022 13:30:39 +0100 Subject: [PATCH 39/63] Remove template from OrderByEdges + move to impl file --- src/storage/v3/request_helper.cpp | 33 +++++++++++++++++++++++++ src/storage/v3/request_helper.hpp | 40 ++++--------------------------- 2 files changed, 37 insertions(+), 36 deletions(-) diff --git a/src/storage/v3/request_helper.cpp b/src/storage/v3/request_helper.cpp index b5c7ea2dd..c59cadf9c 100644 --- a/src/storage/v3/request_helper.cpp +++ b/src/storage/v3/request_helper.cpp @@ -512,4 +512,37 @@ std::array<std::vector<EdgeAccessor>, 2> GetEdgesFromVertex(const VertexAccessor return std::array<std::vector<EdgeAccessor>, 2>{std::move(in_edges), std::move(out_edges)}; } +std::vector<Element<EdgeAccessor>> OrderByEdges(DbAccessor &dba, std::vector<EdgeAccessor> &iterable, + std::vector<msgs::OrderBy> &order_bys, + const VertexAccessor &vertex_acc) { + std::vector<Ordering> ordering; + ordering.reserve(order_bys.size()); + std::transform(order_bys.begin(), order_bys.end(), std::back_inserter(ordering), [](const auto &order_by) { + if (memgraph::msgs::OrderingDirection::ASCENDING == order_by.direction) { + return Ordering::ASC; + } + MG_ASSERT(memgraph::msgs::OrderingDirection::DESCENDING == order_by.direction); + return Ordering::DESC; + }); + + std::vector<Element<EdgeAccessor>> ordered; + for (auto it = iterable.begin(); it != iterable.end(); ++it) { + std::vector<TypedValue> properties_order_by; + properties_order_by.reserve(order_bys.size()); + std::transform(order_bys.begin(), order_bys.end(), std::back_inserter(properties_order_by), + [&dba, &vertex_acc, &it](const auto &order_by) { + return ComputeExpression(dba, vertex_acc, *it, order_by.expression.expression, + expr::identifier_node_symbol, expr::identifier_edge_symbol); + }); + + ordered.push_back({std::move(properties_order_by), *it}); + } + + auto compare_typed_values = TypedValueVectorCompare(ordering); + std::sort(ordered.begin(), ordered.end(), [compare_typed_values](const auto &pair1, const auto &pair2) { + return compare_typed_values(pair1.properties_order_by, pair2.properties_order_by); + }); + return ordered; +} + } // namespace memgraph::storage::v3 diff --git a/src/storage/v3/request_helper.hpp b/src/storage/v3/request_helper.hpp index 5a065c3c9..37cc0e7ff 100644 --- a/src/storage/v3/request_helper.hpp +++ b/src/storage/v3/request_helper.hpp @@ -118,8 +118,8 @@ struct Element { }; template <typename T> -concept IDontKnowAGoodNameForThis = utils::SameAsAnyOf<T, VerticesIterable, std::vector<VertexAccessor>> -template <IDontKnowAGoodNameForThis TIterable> +concept VerticesIt = utils::SameAsAnyOf<T, VerticesIterable, std::vector<VertexAccessor>>; +template <VerticesIt TIterable> std::vector<Element<VertexAccessor>> OrderByVertices(Shard::Accessor &acc, DbAccessor &dba, TIterable &iterable, std::vector<msgs::OrderBy> &order_bys) { std::vector<Ordering> ordering; @@ -153,41 +153,9 @@ std::vector<Element<VertexAccessor>> OrderByVertices(Shard::Accessor &acc, DbAcc return ordered; } -template <typename TIterable> -std::vector<Element<EdgeAccessor>> OrderByEdges(DbAccessor &dba, TIterable &iterable, +std::vector<Element<EdgeAccessor>> OrderByEdges(DbAccessor &dba, std::vector<EdgeAccessor> &iterable, std::vector<msgs::OrderBy> &order_bys, - const VertexAccessor &vertex_acc) { - static_assert(std::is_same_v<TIterable, std::vector<EdgeAccessor>>); // Can be extended if needed - - std::vector<Ordering> ordering; - ordering.reserve(order_bys.size()); - std::transform(order_bys.begin(), order_bys.end(), std::back_inserter(ordering), [](const auto &order_by) { - if (memgraph::msgs::OrderingDirection::ASCENDING == order_by.direction) { - return Ordering::ASC; - } - MG_ASSERT(memgraph::msgs::OrderingDirection::DESCENDING == order_by.direction); - return Ordering::DESC; - }); - - std::vector<Element<EdgeAccessor>> ordered; - for (auto it = iterable.begin(); it != iterable.end(); ++it) { - std::vector<TypedValue> properties_order_by; - properties_order_by.reserve(order_bys.size()); - std::transform(order_bys.begin(), order_bys.end(), std::back_inserter(properties_order_by), - [&dba, &vertex_acc, &it](const auto &order_by) { - return ComputeExpression(dba, vertex_acc, *it, order_by.expression.expression, - expr::identifier_node_symbol, expr::identifier_edge_symbol); - }); - - ordered.push_back({std::move(properties_order_by), *it}); - } - - auto compare_typed_values = TypedValueVectorCompare(ordering); - std::sort(ordered.begin(), ordered.end(), [compare_typed_values](const auto &pair1, const auto &pair2) { - return compare_typed_values(pair1.properties_order_by, pair2.properties_order_by); - }); - return ordered; -} + const VertexAccessor &vertex_acc); VerticesIterable::Iterator GetStartVertexIterator(VerticesIterable &vertex_iterable, const std::vector<PropertyValue> &start_ids, View view); From 49652d0a61d54a8ac60b8904f358459173c06e09 Mon Sep 17 00:00:00 2001 From: Jeremy B <97525434+42jeremy@users.noreply.github.com> Date: Thu, 17 Nov 2022 13:32:48 +0100 Subject: [PATCH 40/63] Update src/storage/v3/shard_rsm.cpp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: János Benjamin Antal <antaljanosbenjamin@users.noreply.github.com> --- src/storage/v3/shard_rsm.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/storage/v3/shard_rsm.cpp b/src/storage/v3/shard_rsm.cpp index ac6e7ae66..5b2afeda2 100644 --- a/src/storage/v3/shard_rsm.cpp +++ b/src/storage/v3/shard_rsm.cpp @@ -478,7 +478,7 @@ msgs::ReadResponses ShardRsm::HandleRead(msgs::ExpandOneRequest &&req) { bool action_successful = true; std::vector<msgs::ExpandOneResultRow> results; - auto batch_limit = req.limit; + const auto batch_limit = req.limit; auto dba = DbAccessor{&acc}; auto maybe_filter_based_on_edge_uniquness = InitializeEdgeUniqunessFunction(req.only_unique_neighbor_rows); From a499bf6dfdc33da69cc35b5b980a3e6034955a17 Mon Sep 17 00:00:00 2001 From: jeremy <jeremy.bailleux@memgraph.io> Date: Thu, 17 Nov 2022 13:33:11 +0100 Subject: [PATCH 41/63] Rename variable --- src/storage/v3/request_helper.cpp | 8 ++++---- src/storage/v3/request_helper.hpp | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/storage/v3/request_helper.cpp b/src/storage/v3/request_helper.cpp index c59cadf9c..0222f38ad 100644 --- a/src/storage/v3/request_helper.cpp +++ b/src/storage/v3/request_helper.cpp @@ -456,10 +456,10 @@ std::optional<msgs::ExpandOneResultRow> GetExpandOneResult( } VerticesIterable::Iterator GetStartVertexIterator(VerticesIterable &vertex_iterable, - const std::vector<PropertyValue> &start_ids, const View view) { + const std::vector<PropertyValue> &primary_key, const View view) { auto it = vertex_iterable.begin(); while (it != vertex_iterable.end()) { - if (const auto &vertex = *it; start_ids <= vertex.PrimaryKey(view).GetValue()) { + if (const auto &vertex = *it; primary_key <= vertex.PrimaryKey(view).GetValue()) { break; } ++it; @@ -468,10 +468,10 @@ VerticesIterable::Iterator GetStartVertexIterator(VerticesIterable &vertex_itera } std::vector<Element<VertexAccessor>>::const_iterator GetStartOrderedElementsIterator( - const std::vector<Element<VertexAccessor>> &ordered_elements, const std::vector<PropertyValue> &start_ids, + const std::vector<Element<VertexAccessor>> &ordered_elements, const std::vector<PropertyValue> &primary_key, const View view) { for (auto it = ordered_elements.begin(); it != ordered_elements.end(); ++it) { - if (const auto &vertex = it->object_acc; start_ids <= vertex.PrimaryKey(view).GetValue()) { + if (const auto &vertex = it->object_acc; primary_key <= vertex.PrimaryKey(view).GetValue()) { return it; } } diff --git a/src/storage/v3/request_helper.hpp b/src/storage/v3/request_helper.hpp index 37cc0e7ff..71bef4a20 100644 --- a/src/storage/v3/request_helper.hpp +++ b/src/storage/v3/request_helper.hpp @@ -158,10 +158,10 @@ std::vector<Element<EdgeAccessor>> OrderByEdges(DbAccessor &dba, std::vector<Edg const VertexAccessor &vertex_acc); VerticesIterable::Iterator GetStartVertexIterator(VerticesIterable &vertex_iterable, - const std::vector<PropertyValue> &start_ids, View view); + const std::vector<PropertyValue> &primary_key, View view); std::vector<Element<VertexAccessor>>::const_iterator GetStartOrderedElementsIterator( - const std::vector<Element<VertexAccessor>> &ordered_elements, const std::vector<PropertyValue> &start_ids, + const std::vector<Element<VertexAccessor>> &ordered_elements, const std::vector<PropertyValue> &primary_key, View view); std::array<std::vector<EdgeAccessor>, 2> GetEdgesFromVertex(const VertexAccessor &vertex_accessor, From 5f88e7557158f07102dd0a12963df0ca0f0725ab Mon Sep 17 00:00:00 2001 From: jeremy <jeremy.bailleux@memgraph.io> Date: Thu, 17 Nov 2022 14:10:49 +0100 Subject: [PATCH 42/63] Remove double declaration --- src/storage/v3/request_helper.hpp | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/storage/v3/request_helper.hpp b/src/storage/v3/request_helper.hpp index 71bef4a20..94220f214 100644 --- a/src/storage/v3/request_helper.hpp +++ b/src/storage/v3/request_helper.hpp @@ -185,13 +185,6 @@ EdgeUniqunessFunction InitializeEdgeUniqunessFunction(bool only_unique_neighbor_ EdgeFiller InitializeEdgeFillerFunction(const msgs::ExpandOneRequest &req); -std::optional<std::map<PropertyId, Value>> CollectSpecificPropertiesFromAccessor(const VertexAccessor &acc, - const std::vector<PropertyId> &props, - View view); - -bool FilterOnVertex(DbAccessor &dba, const storage::v3::VertexAccessor &v_acc, const std::vector<std::string> &filters, - const std::string_view node_name); - std::optional<msgs::ExpandOneResultRow> GetExpandOneResult( Shard::Accessor &acc, msgs::VertexId src_vertex, const msgs::ExpandOneRequest &req, const EdgeUniqunessFunction &maybe_filter_based_on_edge_uniquness, const EdgeFiller &edge_filler, From fe03f5b206f601a66c675bfed0b11914de658cfd Mon Sep 17 00:00:00 2001 From: jeremy <jeremy.bailleux@memgraph.io> Date: Thu, 17 Nov 2022 14:11:25 +0100 Subject: [PATCH 43/63] Update include to full path add auto To variable declaration --- src/storage/v3/request_helper.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/storage/v3/request_helper.cpp b/src/storage/v3/request_helper.cpp index 0222f38ad..f9cbf8886 100644 --- a/src/storage/v3/request_helper.cpp +++ b/src/storage/v3/request_helper.cpp @@ -13,8 +13,8 @@ #include <vector> -#include "pretty_print_ast_to_original_expression.hpp" #include "storage/v3/bindings/db_accessor.hpp" +#include "storage/v3/bindings/pretty_print_ast_to_original_expression.hpp" #include "storage/v3/expr.hpp" #include "storage/v3/value_conversions.hpp" @@ -387,8 +387,7 @@ std::optional<msgs::ExpandOneResultRow> GetExpandOneResult( return std::nullopt; } - std::optional<std::map<PropertyId, Value>> src_vertex_properties; - src_vertex_properties = FillUpSourceVertexProperties(v_acc, req, storage::v3::View::NEW, schema); + auto src_vertex_properties = FillUpSourceVertexProperties(v_acc, req, storage::v3::View::NEW, schema); if (!src_vertex_properties) { return std::nullopt; From 3840c148460e0073cb1a7220ac0ea9aeade70ca7 Mon Sep 17 00:00:00 2001 From: jeremy <jeremy.bailleux@memgraph.io> Date: Thu, 17 Nov 2022 14:33:08 +0100 Subject: [PATCH 44/63] Remove nocommit comment --- src/storage/v3/shard_rsm.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/storage/v3/shard_rsm.cpp b/src/storage/v3/shard_rsm.cpp index 9f739695e..2cf265205 100644 --- a/src/storage/v3/shard_rsm.cpp +++ b/src/storage/v3/shard_rsm.cpp @@ -509,8 +509,7 @@ msgs::ReadResponses ShardRsm::HandleRead(msgs::ExpandOneRequest &&req) { if (!req.order_by.empty()) { // Can we do differently to avoid this? We need OrderByElements but currently it returns vector<Element>, so this // workaround is here to avoid more duplication later - auto local_sorted_vertices = OrderByVertices( - acc, dba, vertex_accessors, req.order_by); // #NoCommit see whether we can avoid the extra std::transform + auto local_sorted_vertices = OrderByVertices(acc, dba, vertex_accessors, req.order_by); vertex_accessors.clear(); std::transform(local_sorted_vertices.begin(), local_sorted_vertices.end(), std::back_inserter(vertex_accessors), [](auto &vertex) { return vertex.object_acc; }); From 1b0db5289d6ffc75b6f74dc7e327fee0b38eba66 Mon Sep 17 00:00:00 2001 From: jeremy <jeremy.bailleux@memgraph.io> Date: Mon, 21 Nov 2022 10:41:08 +0100 Subject: [PATCH 45/63] OrderByVertices only keeps OrderBy expression which corresponds to Vertices --- src/storage/v3/request_helper.hpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/storage/v3/request_helper.hpp b/src/storage/v3/request_helper.hpp index 94220f214..08094e8ba 100644 --- a/src/storage/v3/request_helper.hpp +++ b/src/storage/v3/request_helper.hpp @@ -121,7 +121,14 @@ template <typename T> concept VerticesIt = utils::SameAsAnyOf<T, VerticesIterable, std::vector<VertexAccessor>>; template <VerticesIt TIterable> std::vector<Element<VertexAccessor>> OrderByVertices(Shard::Accessor &acc, DbAccessor &dba, TIterable &iterable, - std::vector<msgs::OrderBy> &order_bys) { + std::vector<msgs::OrderBy> &original_order_bys) { + auto order_bys = original_order_bys; + auto it_to_remove = std::remove_if(order_bys.begin(), order_bys.end(), [](const auto &order_by) { + // We only want to keep OrderBys not impliying edges-ordering + return std::string::npos != order_by.expression.expression.find(expr::identifier_edge_symbol); + }); + order_bys.erase(it_to_remove, order_bys.end()); + std::vector<Ordering> ordering; ordering.reserve(order_bys.size()); std::transform(order_bys.begin(), order_bys.end(), std::back_inserter(ordering), [](const auto &order_by) { From 86e5b44c1c89a3f0a0606f6d682d7125bea4b48d Mon Sep 17 00:00:00 2001 From: jeremy <jeremy.bailleux@memgraph.io> Date: Mon, 21 Nov 2022 10:42:15 +0100 Subject: [PATCH 46/63] Remove Shard::Accessor (unsued) --- src/storage/v3/request_helper.hpp | 2 +- src/storage/v3/shard_rsm.cpp | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/storage/v3/request_helper.hpp b/src/storage/v3/request_helper.hpp index 08094e8ba..44d012f1d 100644 --- a/src/storage/v3/request_helper.hpp +++ b/src/storage/v3/request_helper.hpp @@ -120,7 +120,7 @@ struct Element { template <typename T> concept VerticesIt = utils::SameAsAnyOf<T, VerticesIterable, std::vector<VertexAccessor>>; template <VerticesIt TIterable> -std::vector<Element<VertexAccessor>> OrderByVertices(Shard::Accessor &acc, DbAccessor &dba, TIterable &iterable, +std::vector<Element<VertexAccessor>> OrderByVertices(DbAccessor &dba, TIterable &iterable, std::vector<msgs::OrderBy> &original_order_bys) { auto order_bys = original_order_bys; auto it_to_remove = std::remove_if(order_bys.begin(), order_bys.end(), [](const auto &order_by) { diff --git a/src/storage/v3/shard_rsm.cpp b/src/storage/v3/shard_rsm.cpp index 2cf265205..eaf760253 100644 --- a/src/storage/v3/shard_rsm.cpp +++ b/src/storage/v3/shard_rsm.cpp @@ -426,7 +426,7 @@ msgs::ReadResponses ShardRsm::HandleRead(msgs::ScanVerticesRequest &&req) { uint64_t sample_counter{0}; auto vertex_iterable = acc.Vertices(view); if (!req.order_bys.empty()) { - const auto ordered = OrderByVertices(acc, dba, vertex_iterable, req.order_bys); + const auto ordered = OrderByVertices(dba, vertex_iterable, req.order_bys); // we are traversing Elements auto it = GetStartOrderedElementsIterator(ordered, start_id, View(req.storage_view)); for (; it != ordered.end(); ++it) { @@ -509,7 +509,7 @@ msgs::ReadResponses ShardRsm::HandleRead(msgs::ExpandOneRequest &&req) { if (!req.order_by.empty()) { // Can we do differently to avoid this? We need OrderByElements but currently it returns vector<Element>, so this // workaround is here to avoid more duplication later - auto local_sorted_vertices = OrderByVertices(acc, dba, vertex_accessors, req.order_by); + auto local_sorted_vertices = OrderByVertices(dba, vertex_accessors, req.order_by); vertex_accessors.clear(); std::transform(local_sorted_vertices.begin(), local_sorted_vertices.end(), std::back_inserter(vertex_accessors), [](auto &vertex) { return vertex.object_acc; }); From 2fc1aeb087d83a3f8a6125f691a1f671f83d4ec2 Mon Sep 17 00:00:00 2001 From: jeremy <jeremy.bailleux@memgraph.io> Date: Mon, 21 Nov 2022 10:59:16 +0100 Subject: [PATCH 47/63] Remove unneeded using statements --- src/storage/v3/request_helper.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/storage/v3/request_helper.cpp b/src/storage/v3/request_helper.cpp index f9cbf8886..ab863afdb 100644 --- a/src/storage/v3/request_helper.cpp +++ b/src/storage/v3/request_helper.cpp @@ -22,12 +22,9 @@ namespace memgraph::storage::v3 { using msgs::Label; using msgs::PropertyId; -using conversions::ConvertPropertyMap; using conversions::ConvertPropertyVector; -using conversions::ConvertValueVector; using conversions::FromPropertyValueToValue; using conversions::ToMsgsVertexId; -using conversions::ToPropertyValue; namespace { namespace msgs = msgs; From b2050d55ce4842c3b87a4e2491c5220ad9f2b8da Mon Sep 17 00:00:00 2001 From: jeremy <jeremy.bailleux@memgraph.io> Date: Mon, 21 Nov 2022 10:59:38 +0100 Subject: [PATCH 48/63] Add const --- src/storage/v3/shard_rsm.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/storage/v3/shard_rsm.cpp b/src/storage/v3/shard_rsm.cpp index eaf760253..9c281b51f 100644 --- a/src/storage/v3/shard_rsm.cpp +++ b/src/storage/v3/shard_rsm.cpp @@ -550,7 +550,7 @@ msgs::ReadResponses ShardRsm::HandleRead(msgs::ExpandOneRequest &&req) { std::vector<EdgeAccessor> out_edge_ordered_accessors; std::transform(out_ordered_edges.begin(), out_ordered_edges.end(), std::back_inserter(out_edge_ordered_accessors), [](const auto &edge_element) { return edge_element.object_acc; }); - auto schema = shard_->GetSchema(shard_->PrimaryLabel()); + const auto schema = shard_->GetSchema(shard_->PrimaryLabel()); MG_ASSERT(schema); maybe_result = GetExpandOneResult(src_vertex_acc, src_vertex, req, in_edge_ordered_accessors, out_edge_ordered_accessors, From 4eb673c7b92e351bfc2c9418d7052899c19845ec Mon Sep 17 00:00:00 2001 From: jeremy <jeremy.bailleux@memgraph.io> Date: Mon, 21 Nov 2022 11:31:22 +0100 Subject: [PATCH 49/63] Add const to variable --- src/storage/v3/shard_rsm.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/storage/v3/shard_rsm.cpp b/src/storage/v3/shard_rsm.cpp index 9c281b51f..767fefed9 100644 --- a/src/storage/v3/shard_rsm.cpp +++ b/src/storage/v3/shard_rsm.cpp @@ -533,7 +533,7 @@ msgs::ReadResponses ShardRsm::HandleRead(msgs::ExpandOneRequest &&req) { std::optional<msgs::ExpandOneResultRow> maybe_result; if (req.order_by.empty()) { - auto schema = shard_->GetSchema(shard_->PrimaryLabel()); + const auto schema = shard_->GetSchema(shard_->PrimaryLabel()); MG_ASSERT(schema); maybe_result = GetExpandOneResult(acc, src_vertex, req, maybe_filter_based_on_edge_uniquness, edge_filler, *schema); From 1a67dec302ed85a8e330c2c21f69eea1328e9a3c Mon Sep 17 00:00:00 2001 From: jeremy <jeremy.bailleux@memgraph.io> Date: Mon, 21 Nov 2022 11:45:35 +0100 Subject: [PATCH 50/63] Update test to use OrderBy and Limit on Expand --- tests/simulation/shard_rsm.cpp | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/tests/simulation/shard_rsm.cpp b/tests/simulation/shard_rsm.cpp index b54f7a993..c1ef1079d 100644 --- a/tests/simulation/shard_rsm.cpp +++ b/tests/simulation/shard_rsm.cpp @@ -680,10 +680,12 @@ void AttemptToExpandOneWithUniqueEdges(ShardClient &client, uint64_t src_vertex_ } } -void AttemptToExpandOneLimitAndOrderBy(ShardClient &client, uint64_t src_vertex_val, EdgeTypeId edge_type_id) { +void AttemptToExpandOneLimitAndOrderBy(ShardClient &client, uint64_t src_vertex_val, uint64_t other_src_vertex_val, + EdgeTypeId edge_type_id) { // Source vertex msgs::Label label = {.id = get_primary_label()}; auto src_vertex = std::make_pair(label, GetPrimaryKey(src_vertex_val)); + auto other_src_vertex = std::make_pair(label, GetPrimaryKey(other_src_vertex_val)); // Edge type auto edge_type = msgs::EdgeType{}; @@ -699,8 +701,9 @@ void AttemptToExpandOneLimitAndOrderBy(ShardClient &client, uint64_t src_vertex_ std::optional<std::vector<PropertyId>> edge_properties = {}; std::vector<msgs::OrderBy> order_by = { - {msgs::Expression{"MG_SYMBOL_NODE.prop1"}, msgs::OrderingDirection::DESCENDING}}; - size_t limit = 3; + {msgs::Expression{"MG_SYMBOL_NODE.prop1"}, msgs::OrderingDirection::ASCENDING}, + {msgs::Expression{"MG_SYMBOL_EDGE.prop4"}, msgs::OrderingDirection::DESCENDING}}; + size_t limit = 1; std::vector<std::string> filters = {"MG_SYMBOL_NODE.prop1 != -1"}; msgs::ExpandOneRequest expand_one_req{}; @@ -712,7 +715,7 @@ void AttemptToExpandOneLimitAndOrderBy(ShardClient &client, uint64_t src_vertex_ expand_one_req.limit = limit; expand_one_req.order_by = order_by; expand_one_req.src_vertex_properties = src_vertex_properties; - expand_one_req.src_vertices = {src_vertex}; + expand_one_req.src_vertices = {src_vertex, other_src_vertex}; expand_one_req.transaction_id.logical_id = GetTransactionId(); while (true) { @@ -727,9 +730,14 @@ void AttemptToExpandOneLimitAndOrderBy(ShardClient &client, uint64_t src_vertex_ // We check that we do not have more results than the limit. Based on the data in the graph, we know that we should // receive exactly limit responses. auto expected_number_of_rows = std::min(expand_one_req.src_vertices.size(), limit); - MG_ASSERT(expected_number_of_rows == 1); // We are sending a single vertex to expand + MG_ASSERT(expected_number_of_rows == 1); MG_ASSERT(write_response.result.size() == expected_number_of_rows); - const auto expected_number_of_edges = 10; // We know there are 10 out-going edges from V2->V3 + + // We know there are 1 out-going edges from V1->V2 + // We know there are 10 out-going edges from V2->V3 + // Since we sort on prop1 and limit 1, we will have a single response + // with two edges corresponding to V1->V2 and V1->V3 + const auto expected_number_of_edges = 2; MG_ASSERT(write_response.result[0].out_edges_with_all_properties.size() == expected_number_of_edges); MG_ASSERT(write_response.result[0] .out_edges_with_specific_properties.empty()); // We are not asking for specific properties @@ -1109,7 +1117,7 @@ void TestExpandOneGraphOne(ShardClient &client) { }); AttemptToExpandOneSimple(client, unique_prop_val_1, edge_type_id); - AttemptToExpandOneLimitAndOrderBy(client, unique_prop_val_2, edge_type_id); + AttemptToExpandOneLimitAndOrderBy(client, unique_prop_val_1, unique_prop_val_2, edge_type_id); AttemptToExpandOneWithWrongEdgeType(client, unique_prop_val_1, wrong_edge_type_id); AttemptToExpandOneWithSpecifiedSrcVertexProperties(client, unique_prop_val_1, edge_type_id); AttemptToExpandOneWithSpecifiedEdgeProperties(client, unique_prop_val_1, edge_type_id, edge_prop_id); From e9e42a0614d05cfaecfd9bebf0cf282af683af9b Mon Sep 17 00:00:00 2001 From: jeremy <jeremy.bailleux@memgraph.io> Date: Mon, 21 Nov 2022 11:50:56 +0100 Subject: [PATCH 51/63] add * token to variable declaration --- src/storage/v3/shard_rsm.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/storage/v3/shard_rsm.cpp b/src/storage/v3/shard_rsm.cpp index 767fefed9..d3cf8e4c3 100644 --- a/src/storage/v3/shard_rsm.cpp +++ b/src/storage/v3/shard_rsm.cpp @@ -533,7 +533,7 @@ msgs::ReadResponses ShardRsm::HandleRead(msgs::ExpandOneRequest &&req) { std::optional<msgs::ExpandOneResultRow> maybe_result; if (req.order_by.empty()) { - const auto schema = shard_->GetSchema(shard_->PrimaryLabel()); + const auto *schema = shard_->GetSchema(shard_->PrimaryLabel()); MG_ASSERT(schema); maybe_result = GetExpandOneResult(acc, src_vertex, req, maybe_filter_based_on_edge_uniquness, edge_filler, *schema); @@ -550,7 +550,7 @@ msgs::ReadResponses ShardRsm::HandleRead(msgs::ExpandOneRequest &&req) { std::vector<EdgeAccessor> out_edge_ordered_accessors; std::transform(out_ordered_edges.begin(), out_ordered_edges.end(), std::back_inserter(out_edge_ordered_accessors), [](const auto &edge_element) { return edge_element.object_acc; }); - const auto schema = shard_->GetSchema(shard_->PrimaryLabel()); + const auto *schema = shard_->GetSchema(shard_->PrimaryLabel()); MG_ASSERT(schema); maybe_result = GetExpandOneResult(src_vertex_acc, src_vertex, req, in_edge_ordered_accessors, out_edge_ordered_accessors, From ce8bc522d048d478d045146b93ae7ce318bee524 Mon Sep 17 00:00:00 2001 From: jeremy <jeremy.bailleux@memgraph.io> Date: Mon, 21 Nov 2022 12:23:04 +0100 Subject: [PATCH 52/63] Clang warning --- src/storage/v3/request_helper.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/storage/v3/request_helper.hpp b/src/storage/v3/request_helper.hpp index 44d012f1d..5591ac11a 100644 --- a/src/storage/v3/request_helper.hpp +++ b/src/storage/v3/request_helper.hpp @@ -175,7 +175,7 @@ std::array<std::vector<EdgeAccessor>, 2> GetEdgesFromVertex(const VertexAccessor msgs::EdgeDirection direction); bool FilterOnVertex(DbAccessor &dba, const storage::v3::VertexAccessor &v_acc, const std::vector<std::string> &filters, - const std::string_view node_name); + std::string_view node_name); std::vector<TypedValue> EvaluateVertexExpressions(DbAccessor &dba, const VertexAccessor &v_acc, const std::vector<std::string> &expressions, From 1101c2444c5bc8ec0140892d71eff5c885e11f12 Mon Sep 17 00:00:00 2001 From: jeremy <jeremy.bailleux@memgraph.io> Date: Tue, 22 Nov 2022 14:01:16 +0100 Subject: [PATCH 53/63] Make ConvertPropertyMap expect ref and not rvalue --- src/storage/v3/shard_rsm.cpp | 2 +- src/storage/v3/value_conversions.hpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/storage/v3/shard_rsm.cpp b/src/storage/v3/shard_rsm.cpp index d3cf8e4c3..97e5af703 100644 --- a/src/storage/v3/shard_rsm.cpp +++ b/src/storage/v3/shard_rsm.cpp @@ -66,7 +66,7 @@ msgs::WriteResponses ShardRsm::ApplyWrite(msgs::CreateVerticesRequest &&req) { /// TODO(gvolfing) Consider other methods than converting. Change either /// the way that the property map is stored in the messages, or the /// signature of CreateVertexAndValidate. - auto converted_property_map = ConvertPropertyMap(std::move(new_vertex.properties)); + auto converted_property_map = ConvertPropertyMap(new_vertex.properties); // TODO(gvolfing) make sure if this conversion is actually needed. std::vector<LabelId> converted_label_ids; diff --git a/src/storage/v3/value_conversions.hpp b/src/storage/v3/value_conversions.hpp index 80b6f02b9..53374e1ed 100644 --- a/src/storage/v3/value_conversions.hpp +++ b/src/storage/v3/value_conversions.hpp @@ -131,7 +131,7 @@ inline msgs::VertexId ToMsgsVertexId(const v3::VertexId &vertex_id) { } inline std::vector<std::pair<v3::PropertyId, v3::PropertyValue>> ConvertPropertyMap( - std::vector<std::pair<v3::PropertyId, Value>> &&properties) { + std::vector<std::pair<v3::PropertyId, Value>> &properties) { std::vector<std::pair<v3::PropertyId, v3::PropertyValue>> ret; ret.reserve(properties.size()); From a6f3937692d5a13506abf5d7fa45ca85003ac2cc Mon Sep 17 00:00:00 2001 From: Jeremy B <97525434+42jeremy@users.noreply.github.com> Date: Tue, 22 Nov 2022 14:10:02 +0100 Subject: [PATCH 54/63] Update src/storage/v3/request_helper.hpp Co-authored-by: Jure Bajic <jure.bajic@memgraph.com> --- src/storage/v3/request_helper.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/storage/v3/request_helper.hpp b/src/storage/v3/request_helper.hpp index 5591ac11a..3735d1e29 100644 --- a/src/storage/v3/request_helper.hpp +++ b/src/storage/v3/request_helper.hpp @@ -13,7 +13,7 @@ #include <vector> -#include "ast/ast.hpp" +#include "storage/v3/bindings/ast/ast.hpp" #include "query/v2/requests.hpp" #include "storage/v3/bindings/pretty_print_ast_to_original_expression.hpp" #include "storage/v3/bindings/typed_value.hpp" From bbbd722eebf87f841dff594babc43ec69bcef68d Mon Sep 17 00:00:00 2001 From: Jeremy B <97525434+42jeremy@users.noreply.github.com> Date: Tue, 22 Nov 2022 14:11:29 +0100 Subject: [PATCH 55/63] Update src/storage/v3/request_helper.hpp Co-authored-by: Jure Bajic <jure.bajic@memgraph.com> --- src/storage/v3/request_helper.hpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/storage/v3/request_helper.hpp b/src/storage/v3/request_helper.hpp index 3735d1e29..2ac8e05ae 100644 --- a/src/storage/v3/request_helper.hpp +++ b/src/storage/v3/request_helper.hpp @@ -119,6 +119,7 @@ struct Element { template <typename T> concept VerticesIt = utils::SameAsAnyOf<T, VerticesIterable, std::vector<VertexAccessor>>; + template <VerticesIt TIterable> std::vector<Element<VertexAccessor>> OrderByVertices(DbAccessor &dba, TIterable &iterable, std::vector<msgs::OrderBy> &original_order_bys) { From c0cb53e156c649ddeba00718dc912a2f740a35f6 Mon Sep 17 00:00:00 2001 From: jeremy <jeremy.bailleux@memgraph.io> Date: Tue, 22 Nov 2022 14:20:22 +0100 Subject: [PATCH 56/63] Replace if by switch --- src/storage/v3/request_helper.hpp | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/storage/v3/request_helper.hpp b/src/storage/v3/request_helper.hpp index 2ac8e05ae..2f469ae5b 100644 --- a/src/storage/v3/request_helper.hpp +++ b/src/storage/v3/request_helper.hpp @@ -13,8 +13,8 @@ #include <vector> -#include "storage/v3/bindings/ast/ast.hpp" #include "query/v2/requests.hpp" +#include "storage/v3/bindings/ast/ast.hpp" #include "storage/v3/bindings/pretty_print_ast_to_original_expression.hpp" #include "storage/v3/bindings/typed_value.hpp" #include "storage/v3/edge_accessor.hpp" @@ -133,11 +133,14 @@ std::vector<Element<VertexAccessor>> OrderByVertices(DbAccessor &dba, TIterable std::vector<Ordering> ordering; ordering.reserve(order_bys.size()); std::transform(order_bys.begin(), order_bys.end(), std::back_inserter(ordering), [](const auto &order_by) { - if (memgraph::msgs::OrderingDirection::ASCENDING == order_by.direction) { - return Ordering::ASC; + switch (order_by.direction) { + case memgraph::msgs::OrderingDirection::ASCENDING: + return Ordering::ASC; + case memgraph::msgs::OrderingDirection::DESCENDING: + return Ordering::DESC; + default: + LOG_FATAL("Unknown ordering direction"); } - MG_ASSERT(memgraph::msgs::OrderingDirection::DESCENDING == order_by.direction); - return Ordering::DESC; }); std::vector<Element<VertexAccessor>> ordered; From 662fa2e6d2f4a6dd60ec695e035cd49674481c2c Mon Sep 17 00:00:00 2001 From: jeremy <jeremy.bailleux@memgraph.io> Date: Tue, 22 Nov 2022 14:22:19 +0100 Subject: [PATCH 57/63] Remove uneeded using statement --- src/storage/v3/request_helper.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/storage/v3/request_helper.cpp b/src/storage/v3/request_helper.cpp index ab863afdb..9c0a3f21a 100644 --- a/src/storage/v3/request_helper.cpp +++ b/src/storage/v3/request_helper.cpp @@ -27,7 +27,6 @@ using conversions::FromPropertyValueToValue; using conversions::ToMsgsVertexId; namespace { -namespace msgs = msgs; using AllEdgePropertyDataSructure = std::map<PropertyId, msgs::Value>; using SpecificEdgePropertyDataSructure = std::vector<msgs::Value>; From 307cce9e2174e7331a6c57f7f2f6920b7bdda30e Mon Sep 17 00:00:00 2001 From: jeremy <jeremy.bailleux@memgraph.io> Date: Tue, 22 Nov 2022 14:23:24 +0100 Subject: [PATCH 58/63] Remove unused struct --- src/storage/v3/request_helper.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/storage/v3/request_helper.cpp b/src/storage/v3/request_helper.cpp index 9c0a3f21a..24c1f431f 100644 --- a/src/storage/v3/request_helper.cpp +++ b/src/storage/v3/request_helper.cpp @@ -59,8 +59,6 @@ std::optional<std::map<PropertyId, Value>> PrimaryKeysFromAccessor(const VertexA return ret; } -struct LocalError {}; - std::optional<std::vector<msgs::Label>> FillUpSourceVertexSecondaryLabels(const std::optional<VertexAccessor> &v_acc, const msgs::ExpandOneRequest &req) { auto secondary_labels = v_acc->Labels(View::NEW); From 6801d6ff09979c092b7aca1281b76ba80ac26137 Mon Sep 17 00:00:00 2001 From: jeremy <jeremy.bailleux@memgraph.io> Date: Tue, 22 Nov 2022 14:49:54 +0100 Subject: [PATCH 59/63] Remove duplicate using statement --- src/storage/v3/request_helper.cpp | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/src/storage/v3/request_helper.cpp b/src/storage/v3/request_helper.cpp index 24c1f431f..73e6f86e5 100644 --- a/src/storage/v3/request_helper.cpp +++ b/src/storage/v3/request_helper.cpp @@ -173,17 +173,6 @@ std::optional<std::array<std::vector<EdgeAccessor>, 2>> FillUpConnectingEdges( return std::array<std::vector<EdgeAccessor>, 2>{std::move(in_edges), std::move(out_edges)}; } -using AllEdgePropertyDataSructure = std::map<PropertyId, msgs::Value>; -using SpecificEdgePropertyDataSructure = std::vector<msgs::Value>; - -using AllEdgeProperties = std::tuple<msgs::VertexId, msgs::Gid, AllEdgePropertyDataSructure>; -using SpecificEdgeProperties = std::tuple<msgs::VertexId, msgs::Gid, SpecificEdgePropertyDataSructure>; - -using SpecificEdgePropertiesVector = std::vector<SpecificEdgeProperties>; -using AllEdgePropertiesVector = std::vector<AllEdgeProperties>; - -using EdgeFiller = std::function<bool(const EdgeAccessor &edge, bool is_in_edge, msgs::ExpandOneResultRow &result_row)>; - template <bool are_in_edges> bool FillEdges(const std::vector<EdgeAccessor> &edges, msgs::ExpandOneResultRow &row, const EdgeFiller &edge_filler) { for (const auto &edge : edges) { From 3a171376d717f1fa9664bbdb6e6c7b2e5fabe643 Mon Sep 17 00:00:00 2001 From: jeremy <jeremy.bailleux@memgraph.io> Date: Tue, 22 Nov 2022 16:47:25 +0100 Subject: [PATCH 60/63] OrderBy in Expand has two members to differ vertices Vs edges --- src/query/v2/requests.hpp | 4 ++- src/storage/v3/request_helper.cpp | 10 +++---- src/storage/v3/request_helper.hpp | 36 +++++++++++-------------- src/storage/v3/shard_rsm.cpp | 10 +++---- tests/simulation/shard_rsm.cpp | 45 ++++++++++++++++++++----------- 5 files changed, 58 insertions(+), 47 deletions(-) diff --git a/src/query/v2/requests.hpp b/src/query/v2/requests.hpp index 971977246..888265bb4 100644 --- a/src/query/v2/requests.hpp +++ b/src/query/v2/requests.hpp @@ -403,7 +403,9 @@ struct ExpandOneRequest { std::vector<std::string> vertex_expressions; std::vector<std::string> edge_expressions; - std::vector<OrderBy> order_by; + std::vector<OrderBy> order_by_vertices; + std::vector<OrderBy> order_by_edges; + // Limit the edges or the vertices? std::optional<size_t> limit; std::vector<std::string> filters; diff --git a/src/storage/v3/request_helper.cpp b/src/storage/v3/request_helper.cpp index 73e6f86e5..cca506088 100644 --- a/src/storage/v3/request_helper.cpp +++ b/src/storage/v3/request_helper.cpp @@ -495,11 +495,11 @@ std::array<std::vector<EdgeAccessor>, 2> GetEdgesFromVertex(const VertexAccessor } std::vector<Element<EdgeAccessor>> OrderByEdges(DbAccessor &dba, std::vector<EdgeAccessor> &iterable, - std::vector<msgs::OrderBy> &order_bys, + std::vector<msgs::OrderBy> &order_by_edges, const VertexAccessor &vertex_acc) { std::vector<Ordering> ordering; - ordering.reserve(order_bys.size()); - std::transform(order_bys.begin(), order_bys.end(), std::back_inserter(ordering), [](const auto &order_by) { + ordering.reserve(order_by_edges.size()); + std::transform(order_by_edges.begin(), order_by_edges.end(), std::back_inserter(ordering), [](const auto &order_by) { if (memgraph::msgs::OrderingDirection::ASCENDING == order_by.direction) { return Ordering::ASC; } @@ -510,8 +510,8 @@ std::vector<Element<EdgeAccessor>> OrderByEdges(DbAccessor &dba, std::vector<Edg std::vector<Element<EdgeAccessor>> ordered; for (auto it = iterable.begin(); it != iterable.end(); ++it) { std::vector<TypedValue> properties_order_by; - properties_order_by.reserve(order_bys.size()); - std::transform(order_bys.begin(), order_bys.end(), std::back_inserter(properties_order_by), + properties_order_by.reserve(order_by_edges.size()); + std::transform(order_by_edges.begin(), order_by_edges.end(), std::back_inserter(properties_order_by), [&dba, &vertex_acc, &it](const auto &order_by) { return ComputeExpression(dba, vertex_acc, *it, order_by.expression.expression, expr::identifier_node_symbol, expr::identifier_edge_symbol); diff --git a/src/storage/v3/request_helper.hpp b/src/storage/v3/request_helper.hpp index 2f469ae5b..b43b2d24a 100644 --- a/src/storage/v3/request_helper.hpp +++ b/src/storage/v3/request_helper.hpp @@ -122,33 +122,27 @@ concept VerticesIt = utils::SameAsAnyOf<T, VerticesIterable, std::vector<VertexA template <VerticesIt TIterable> std::vector<Element<VertexAccessor>> OrderByVertices(DbAccessor &dba, TIterable &iterable, - std::vector<msgs::OrderBy> &original_order_bys) { - auto order_bys = original_order_bys; - auto it_to_remove = std::remove_if(order_bys.begin(), order_bys.end(), [](const auto &order_by) { - // We only want to keep OrderBys not impliying edges-ordering - return std::string::npos != order_by.expression.expression.find(expr::identifier_edge_symbol); - }); - order_bys.erase(it_to_remove, order_bys.end()); - + std::vector<msgs::OrderBy> &order_by_vertices) { std::vector<Ordering> ordering; - ordering.reserve(order_bys.size()); - std::transform(order_bys.begin(), order_bys.end(), std::back_inserter(ordering), [](const auto &order_by) { - switch (order_by.direction) { - case memgraph::msgs::OrderingDirection::ASCENDING: - return Ordering::ASC; - case memgraph::msgs::OrderingDirection::DESCENDING: - return Ordering::DESC; - default: - LOG_FATAL("Unknown ordering direction"); - } - }); + ordering.reserve(order_by_vertices.size()); + std::transform(order_by_vertices.begin(), order_by_vertices.end(), std::back_inserter(ordering), + [](const auto &order_by) { + switch (order_by.direction) { + case memgraph::msgs::OrderingDirection::ASCENDING: + return Ordering::ASC; + case memgraph::msgs::OrderingDirection::DESCENDING: + return Ordering::DESC; + default: + LOG_FATAL("Unknown ordering direction"); + } + }); std::vector<Element<VertexAccessor>> ordered; for (auto it = iterable.begin(); it != iterable.end(); ++it) { std::vector<TypedValue> properties_order_by; - properties_order_by.reserve(order_bys.size()); + properties_order_by.reserve(order_by_vertices.size()); - std::transform(order_bys.begin(), order_bys.end(), std::back_inserter(properties_order_by), + std::transform(order_by_vertices.begin(), order_by_vertices.end(), std::back_inserter(properties_order_by), [&dba, &it](const auto &order_by) { return ComputeExpression(dba, *it, std::nullopt /*e_acc*/, order_by.expression.expression, expr::identifier_node_symbol, expr::identifier_edge_symbol); diff --git a/src/storage/v3/shard_rsm.cpp b/src/storage/v3/shard_rsm.cpp index 97e5af703..2d536fe42 100644 --- a/src/storage/v3/shard_rsm.cpp +++ b/src/storage/v3/shard_rsm.cpp @@ -506,10 +506,10 @@ msgs::ReadResponses ShardRsm::HandleRead(msgs::ExpandOneRequest &&req) { vertex_accessors.emplace_back(src_vertex_acc_opt.value()); } - if (!req.order_by.empty()) { + if (!req.order_by_vertices.empty()) { // Can we do differently to avoid this? We need OrderByElements but currently it returns vector<Element>, so this // workaround is here to avoid more duplication later - auto local_sorted_vertices = OrderByVertices(dba, vertex_accessors, req.order_by); + auto local_sorted_vertices = OrderByVertices(dba, vertex_accessors, req.order_by_vertices); vertex_accessors.clear(); std::transform(local_sorted_vertices.begin(), local_sorted_vertices.end(), std::back_inserter(vertex_accessors), [](auto &vertex) { return vertex.object_acc; }); @@ -532,7 +532,7 @@ msgs::ReadResponses ShardRsm::HandleRead(msgs::ExpandOneRequest &&req) { std::optional<msgs::ExpandOneResultRow> maybe_result; - if (req.order_by.empty()) { + if (req.order_by_vertices.empty()) { const auto *schema = shard_->GetSchema(shard_->PrimaryLabel()); MG_ASSERT(schema); maybe_result = @@ -540,8 +540,8 @@ msgs::ReadResponses ShardRsm::HandleRead(msgs::ExpandOneRequest &&req) { } else { auto [in_edge_accessors, out_edge_accessors] = GetEdgesFromVertex(src_vertex_acc, req.direction); - const auto in_ordered_edges = OrderByEdges(dba, in_edge_accessors, req.order_by, src_vertex_acc); - const auto out_ordered_edges = OrderByEdges(dba, out_edge_accessors, req.order_by, src_vertex_acc); + const auto in_ordered_edges = OrderByEdges(dba, in_edge_accessors, req.order_by_edges, src_vertex_acc); + const auto out_ordered_edges = OrderByEdges(dba, out_edge_accessors, req.order_by_edges, src_vertex_acc); std::vector<EdgeAccessor> in_edge_ordered_accessors; std::transform(in_ordered_edges.begin(), in_ordered_edges.end(), std::back_inserter(in_edge_ordered_accessors), diff --git a/tests/simulation/shard_rsm.cpp b/tests/simulation/shard_rsm.cpp index c1ef1079d..c668a43a2 100644 --- a/tests/simulation/shard_rsm.cpp +++ b/tests/simulation/shard_rsm.cpp @@ -531,7 +531,8 @@ void AttemptToExpandOneWithWrongEdgeType(ShardClient &client, uint64_t src_verte std::optional<std::vector<PropertyId>> edge_properties = {}; std::vector<std::string> expressions; - std::vector<msgs::OrderBy> order_by = {}; + std::vector<msgs::OrderBy> order_by_vertices = {}; + std::vector<msgs::OrderBy> order_by_edges = {}; std::optional<size_t> limit = {}; std::vector<std::string> filter = {}; @@ -543,7 +544,8 @@ void AttemptToExpandOneWithWrongEdgeType(ShardClient &client, uint64_t src_verte expand_one_req.vertex_expressions = expressions; expand_one_req.filters = filter; expand_one_req.limit = limit; - expand_one_req.order_by = order_by; + expand_one_req.order_by_vertices = order_by_vertices; + expand_one_req.order_by_edges = order_by_edges; expand_one_req.src_vertex_properties = src_vertex_properties; expand_one_req.src_vertices = {src_vertex}; expand_one_req.transaction_id.logical_id = GetTransactionId(); @@ -586,7 +588,8 @@ void AttemptToExpandOneSimple(ShardClient &client, uint64_t src_vertex_val, Edge std::optional<std::vector<PropertyId>> edge_properties = {}; std::vector<std::string> expressions; - std::vector<msgs::OrderBy> order_by = {}; + std::vector<msgs::OrderBy> order_by_vertices = {}; + std::vector<msgs::OrderBy> order_by_edges = {}; std::optional<size_t> limit = {}; std::vector<std::string> filter = {}; @@ -598,7 +601,8 @@ void AttemptToExpandOneSimple(ShardClient &client, uint64_t src_vertex_val, Edge expand_one_req.vertex_expressions = expressions; expand_one_req.filters = filter; expand_one_req.limit = limit; - expand_one_req.order_by = order_by; + expand_one_req.order_by_vertices = order_by_vertices; + expand_one_req.order_by_edges = order_by_edges; expand_one_req.src_vertex_properties = src_vertex_properties; expand_one_req.src_vertices = {src_vertex}; expand_one_req.transaction_id.logical_id = GetTransactionId(); @@ -642,7 +646,8 @@ void AttemptToExpandOneWithUniqueEdges(ShardClient &client, uint64_t src_vertex_ std::optional<std::vector<PropertyId>> edge_properties = {}; std::vector<std::string> expressions; - std::vector<msgs::OrderBy> order_by = {}; + std::vector<msgs::OrderBy> order_by_vertices = {}; + std::vector<msgs::OrderBy> order_by_edges = {}; std::optional<size_t> limit = {}; std::vector<std::string> filter = {}; @@ -654,7 +659,8 @@ void AttemptToExpandOneWithUniqueEdges(ShardClient &client, uint64_t src_vertex_ expand_one_req.vertex_expressions = expressions; expand_one_req.filters = filter; expand_one_req.limit = limit; - expand_one_req.order_by = order_by; + expand_one_req.order_by_vertices = order_by_vertices; + expand_one_req.order_by_edges = order_by_edges; expand_one_req.src_vertex_properties = src_vertex_properties; expand_one_req.src_vertices = {src_vertex}; expand_one_req.only_unique_neighbor_rows = true; @@ -700,9 +706,11 @@ void AttemptToExpandOneLimitAndOrderBy(ShardClient &client, uint64_t src_vertex_ // Edge properties to look for std::optional<std::vector<PropertyId>> edge_properties = {}; - std::vector<msgs::OrderBy> order_by = { - {msgs::Expression{"MG_SYMBOL_NODE.prop1"}, msgs::OrderingDirection::ASCENDING}, + std::vector<msgs::OrderBy> order_by_vertices = { + {msgs::Expression{"MG_SYMBOL_NODE.prop1"}, msgs::OrderingDirection::ASCENDING}}; + std::vector<msgs::OrderBy> order_by_edges = { {msgs::Expression{"MG_SYMBOL_EDGE.prop4"}, msgs::OrderingDirection::DESCENDING}}; + size_t limit = 1; std::vector<std::string> filters = {"MG_SYMBOL_NODE.prop1 != -1"}; @@ -713,7 +721,8 @@ void AttemptToExpandOneLimitAndOrderBy(ShardClient &client, uint64_t src_vertex_ expand_one_req.edge_types = {edge_type}; expand_one_req.filters = filters; expand_one_req.limit = limit; - expand_one_req.order_by = order_by; + expand_one_req.order_by_vertices = order_by_vertices; + expand_one_req.order_by_edges = order_by_edges; expand_one_req.src_vertex_properties = src_vertex_properties; expand_one_req.src_vertices = {src_vertex, other_src_vertex}; expand_one_req.transaction_id.logical_id = GetTransactionId(); @@ -780,7 +789,8 @@ void AttemptToExpandOneWithSpecifiedSrcVertexProperties(ShardClient &client, uin std::optional<std::vector<PropertyId>> edge_properties = {}; std::vector<std::string> expressions; - std::vector<msgs::OrderBy> order_by = {}; + std::vector<msgs::OrderBy> order_by_vertices = {}; + std::vector<msgs::OrderBy> order_by_edges = {}; std::optional<size_t> limit = {}; std::vector<std::string> filter = {}; @@ -792,7 +802,8 @@ void AttemptToExpandOneWithSpecifiedSrcVertexProperties(ShardClient &client, uin expand_one_req.vertex_expressions = expressions; expand_one_req.filters = filter; expand_one_req.limit = limit; - expand_one_req.order_by = order_by; + expand_one_req.order_by_vertices = order_by_vertices; + expand_one_req.order_by_edges = order_by_edges; expand_one_req.src_vertex_properties = src_vertex_properties; expand_one_req.src_vertices = {src_vertex}; expand_one_req.transaction_id.logical_id = GetTransactionId(); @@ -840,7 +851,8 @@ void AttemptToExpandOneWithSpecifiedEdgeProperties(ShardClient &client, uint64_t std::optional<std::vector<PropertyId>> edge_properties = {specified_edge_prop}; std::vector<std::string> expressions; - std::vector<msgs::OrderBy> order_by = {}; + std::vector<msgs::OrderBy> order_by_vertices = {}; + std::vector<msgs::OrderBy> order_by_edges = {}; std::optional<size_t> limit = {}; std::vector<std::string> filter = {}; @@ -852,7 +864,8 @@ void AttemptToExpandOneWithSpecifiedEdgeProperties(ShardClient &client, uint64_t expand_one_req.vertex_expressions = expressions; expand_one_req.filters = filter; expand_one_req.limit = limit; - expand_one_req.order_by = order_by; + expand_one_req.order_by_vertices = order_by_vertices; + expand_one_req.order_by_edges = order_by_edges; expand_one_req.src_vertex_properties = src_vertex_properties; expand_one_req.src_vertices = {src_vertex}; expand_one_req.transaction_id.logical_id = GetTransactionId(); @@ -899,7 +912,8 @@ void AttemptToExpandOneWithFilters(ShardClient &client, uint64_t src_vertex_val, std::optional<std::vector<PropertyId>> edge_properties = {}; std::vector<std::string> expressions; - std::vector<msgs::OrderBy> order_by = {}; + std::vector<msgs::OrderBy> order_by_vertices = {}; + std::vector<msgs::OrderBy> order_by_edges = {}; std::optional<size_t> limit = {}; std::vector<std::string> filter = {}; @@ -911,7 +925,8 @@ void AttemptToExpandOneWithFilters(ShardClient &client, uint64_t src_vertex_val, expand_one_req.vertex_expressions = expressions; expand_one_req.filters = {filter_expr1}; expand_one_req.limit = limit; - expand_one_req.order_by = order_by; + expand_one_req.order_by_vertices = order_by_vertices; + expand_one_req.order_by_edges = order_by_edges; expand_one_req.src_vertex_properties = src_vertex_properties; expand_one_req.src_vertices = {src_vertex}; expand_one_req.transaction_id.logical_id = GetTransactionId(); From 85034ddcbeed4300de620c9ad17c2c25572d3e4c Mon Sep 17 00:00:00 2001 From: jeremy <jeremy.bailleux@memgraph.io> Date: Tue, 22 Nov 2022 17:29:02 +0100 Subject: [PATCH 61/63] Rename variable un function def --- src/storage/v3/request_helper.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/storage/v3/request_helper.hpp b/src/storage/v3/request_helper.hpp index b43b2d24a..ebf4f1d0e 100644 --- a/src/storage/v3/request_helper.hpp +++ b/src/storage/v3/request_helper.hpp @@ -159,7 +159,7 @@ std::vector<Element<VertexAccessor>> OrderByVertices(DbAccessor &dba, TIterable } std::vector<Element<EdgeAccessor>> OrderByEdges(DbAccessor &dba, std::vector<EdgeAccessor> &iterable, - std::vector<msgs::OrderBy> &order_bys, + std::vector<msgs::OrderBy> &order_by_edges, const VertexAccessor &vertex_acc); VerticesIterable::Iterator GetStartVertexIterator(VerticesIterable &vertex_iterable, From 56e2ad4546e1adf4fdb3c6fd635f7f531cd7fa33 Mon Sep 17 00:00:00 2001 From: Jeremy B <97525434+42jeremy@users.noreply.github.com> Date: Wed, 23 Nov 2022 13:24:35 +0100 Subject: [PATCH 62/63] Update src/storage/v3/shard_rsm.cpp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: János Benjamin Antal <antaljanosbenjamin@users.noreply.github.com> --- src/storage/v3/shard_rsm.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/storage/v3/shard_rsm.cpp b/src/storage/v3/shard_rsm.cpp index 26fba10cf..7e90b11bd 100644 --- a/src/storage/v3/shard_rsm.cpp +++ b/src/storage/v3/shard_rsm.cpp @@ -531,7 +531,7 @@ msgs::ReadResponses ShardRsm::HandleRead(msgs::ExpandOneRequest &&req) { std::optional<msgs::ExpandOneResultRow> maybe_result; - if (req.order_by_vertices.empty()) { + if (req.order_by_edges.empty()) { const auto *schema = shard_->GetSchema(shard_->PrimaryLabel()); MG_ASSERT(schema); maybe_result = From 6d86801be0bd4e7f964d2ad318bc87d455c0cba7 Mon Sep 17 00:00:00 2001 From: jeremy <jeremy.bailleux@memgraph.io> Date: Wed, 23 Nov 2022 13:54:54 +0100 Subject: [PATCH 63/63] Extract logic to convert ORderingDirection to Ordering --- src/storage/v3/request_helper.cpp | 9 ++------- src/storage/v3/request_helper.hpp | 22 ++++++++++++---------- 2 files changed, 14 insertions(+), 17 deletions(-) diff --git a/src/storage/v3/request_helper.cpp b/src/storage/v3/request_helper.cpp index cca506088..bfc217b81 100644 --- a/src/storage/v3/request_helper.cpp +++ b/src/storage/v3/request_helper.cpp @@ -499,13 +499,8 @@ std::vector<Element<EdgeAccessor>> OrderByEdges(DbAccessor &dba, std::vector<Edg const VertexAccessor &vertex_acc) { std::vector<Ordering> ordering; ordering.reserve(order_by_edges.size()); - std::transform(order_by_edges.begin(), order_by_edges.end(), std::back_inserter(ordering), [](const auto &order_by) { - if (memgraph::msgs::OrderingDirection::ASCENDING == order_by.direction) { - return Ordering::ASC; - } - MG_ASSERT(memgraph::msgs::OrderingDirection::DESCENDING == order_by.direction); - return Ordering::DESC; - }); + std::transform(order_by_edges.begin(), order_by_edges.end(), std::back_inserter(ordering), + [](const auto &order_by) { return ConvertMsgsOrderByToOrdering(order_by.direction); }); std::vector<Element<EdgeAccessor>> ordered; for (auto it = iterable.begin(); it != iterable.end(); ++it) { diff --git a/src/storage/v3/request_helper.hpp b/src/storage/v3/request_helper.hpp index ebf4f1d0e..dce555d88 100644 --- a/src/storage/v3/request_helper.hpp +++ b/src/storage/v3/request_helper.hpp @@ -84,6 +84,17 @@ inline bool TypedValueCompare(const TypedValue &a, const TypedValue &b) { } } +inline Ordering ConvertMsgsOrderByToOrdering(msgs::OrderingDirection ordering) { + switch (ordering) { + case memgraph::msgs::OrderingDirection::ASCENDING: + return memgraph::storage::v3::Ordering::ASC; + case memgraph::msgs::OrderingDirection::DESCENDING: + return memgraph::storage::v3::Ordering::DESC; + default: + LOG_FATAL("Unknown ordering direction"); + } +} + class TypedValueVectorCompare final { public: explicit TypedValueVectorCompare(const std::vector<Ordering> &ordering) : ordering_(ordering) {} @@ -126,16 +137,7 @@ std::vector<Element<VertexAccessor>> OrderByVertices(DbAccessor &dba, TIterable std::vector<Ordering> ordering; ordering.reserve(order_by_vertices.size()); std::transform(order_by_vertices.begin(), order_by_vertices.end(), std::back_inserter(ordering), - [](const auto &order_by) { - switch (order_by.direction) { - case memgraph::msgs::OrderingDirection::ASCENDING: - return Ordering::ASC; - case memgraph::msgs::OrderingDirection::DESCENDING: - return Ordering::DESC; - default: - LOG_FATAL("Unknown ordering direction"); - } - }); + [](const auto &order_by) { return ConvertMsgsOrderByToOrdering(order_by.direction); }); std::vector<Element<VertexAccessor>> ordered; for (auto it = iterable.begin(); it != iterable.end(); ++it) {