From 08c6a09c774a01d18ee078ff7a859fa2de8133f1 Mon Sep 17 00:00:00 2001 From: antoniofilipovic Date: Sun, 7 Aug 2022 16:26:45 +0200 Subject: [PATCH] refactor graph storing as set of edges and vertices and refactor serialization --- .../bolt/v1/encoder/base_encoder.hpp | 14 ----- src/communication/bolt/v1/value.cpp | 19 ------- src/communication/bolt/v1/value.hpp | 52 +------------------ src/glue/communication.cpp | 23 ++++---- src/glue/communication.hpp | 5 +- src/query/graph.hpp | 9 ++-- 6 files changed, 21 insertions(+), 101 deletions(-) diff --git a/src/communication/bolt/v1/encoder/base_encoder.hpp b/src/communication/bolt/v1/encoder/base_encoder.hpp index 27ab7930d..064cd0f95 100644 --- a/src/communication/bolt/v1/encoder/base_encoder.hpp +++ b/src/communication/bolt/v1/encoder/base_encoder.hpp @@ -181,17 +181,6 @@ class BaseEncoder { for (auto &i : path.indices) WriteInt(i); } - void WriteGraph(const Graph &graph) { - WriteRAW(utils::UnderlyingCast(Marker::TinyStruct) + 3); - WriteRAW(utils::UnderlyingCast(Signature::Path)); // todo fico, fix - WriteTypeSize(graph.vertices.size(), MarkerList); - for (auto &v : graph.vertices) WriteVertex(v); - WriteTypeSize(graph.edges.size(), MarkerList); - for (auto &e : graph.edges) WriteEdge(e); - WriteTypeSize(graph.indices.size(), MarkerList); - for (auto &i : graph.indices) WriteInt(i); - } - void WriteDate(const utils::Date &date) { WriteRAW(utils::UnderlyingCast(Marker::TinyStruct1)); WriteRAW(utils::UnderlyingCast(Signature::Date)); @@ -270,9 +259,6 @@ class BaseEncoder { case Value::Type::Duration: WriteDuration(value.ValueDuration()); break; - case Value::Type::Graph: - WriteGraph(value.ValueGraph()); - break; } } diff --git a/src/communication/bolt/v1/value.cpp b/src/communication/bolt/v1/value.cpp index d64cf70fb..a370f78d5 100644 --- a/src/communication/bolt/v1/value.cpp +++ b/src/communication/bolt/v1/value.cpp @@ -54,7 +54,6 @@ DEF_GETTER_BY_REF(Date, utils::Date, date_v) DEF_GETTER_BY_REF(LocalTime, utils::LocalTime, local_time_v) DEF_GETTER_BY_REF(LocalDateTime, utils::LocalDateTime, local_date_time_v) DEF_GETTER_BY_REF(Duration, utils::Duration, duration_v) -DEF_GETTER_BY_REF(Graph, Graph, graph_v) #undef DEF_GETTER_BY_REF @@ -104,8 +103,6 @@ Value::Value(const Value &other) : type_(other.type_) { case Type::Duration: new (&duration_v) utils::Duration(other.duration_v); return; - case Type::Graph: - throw ValueException(); } } @@ -160,9 +157,6 @@ Value &Value::operator=(const Value &other) { case Type::Duration: new (&duration_v) utils::Duration(other.duration_v); return *this; - case Type::Graph: - new (&graph_v) Graph(other.graph_v); - return *this; } } return *this; @@ -214,9 +208,6 @@ Value::Value(Value &&other) noexcept : type_(other.type_) { case Type::Duration: new (&duration_v) utils::Duration(other.duration_v); break; - case Type::Graph: - new (&graph_v) Graph(std::move(other.graph_v)); - break; } // reset the type of other @@ -275,9 +266,6 @@ Value &Value::operator=(Value &&other) noexcept { case Type::Duration: new (&duration_v) utils::Duration(other.duration_v); break; - case Type::Graph: - new (&graph_v) Graph(std::move(other.graph_v)); - break; } // reset the type of other @@ -336,9 +324,6 @@ Value::~Value() { case Type::Duration: duration_v.~Duration(); return; - case Type::Graph: - graph_v.~Graph(); - return; } } @@ -439,8 +424,6 @@ std::ostream &operator<<(std::ostream &os, const Value &value) { return os << value.ValueLocalDateTime(); case Value::Type::Duration: return os << value.ValueDuration(); - case Value::Type::Graph: - throw ValueException("Not supported for Graph"); } } @@ -476,8 +459,6 @@ std::ostream &operator<<(std::ostream &os, const Value::Type type) { return os << "local_date_time"; case Value::Type::Duration: return os << "duration"; - case Value::Type::Graph: - throw ValueException("error"); } } } // namespace memgraph::communication::bolt diff --git a/src/communication/bolt/v1/value.hpp b/src/communication/bolt/v1/value.hpp index ebee13d4d..6eae77645 100644 --- a/src/communication/bolt/v1/value.hpp +++ b/src/communication/bolt/v1/value.hpp @@ -125,50 +125,6 @@ struct Path { std::vector indices; }; -/** - * Structure used when reading a Graph with the decoder. - * The decoder writes data into this structure. - */ -struct Graph { - Graph() {} - - Graph(const std::vector &vertices, const std::vector &edges) { - // Helper function. Looks for the given element in the collection. If found, - // puts its index into `indices`. Otherwise emplaces the given element - // into the collection and puts that index into `indices`. A multiplier is - // added to switch between positive and negative indices (that define edge - // direction). - auto add_element = [this](auto &collection, const auto &element, int multiplier, int offset) { - auto found = - std::find_if(collection.begin(), collection.end(), [&](const auto &e) { return e.id == element.id; }); - indices.emplace_back(multiplier * (std::distance(collection.begin(), found) + offset)); - if (found == collection.end()) collection.push_back(element); - }; - - this->vertices.reserve(vertices.size()); - this->edges.reserve(edges.size()); - this->vertices.emplace_back(vertices[0]); - for (uint i = 0; i < edges.size(); i++) { - const auto &e = edges[i]; - const auto &v = vertices[i + 1]; - UnboundedEdge unbounded_edge{e.id, e.type, e.properties}; - add_element(this->edges, unbounded_edge, e.to == v.id ? 1 : -1, 1); - add_element(this->vertices, v, 1, 0); - } - } - - /** Unique vertices in the path. */ - std::vector vertices; - /** Unique edges in the path. */ - std::vector edges; - /** - * Indices that map path positions to vertices/edges. - * Positive indices for left-to-right directionality and negative for - * right-to-left. - */ - std::vector indices; -}; - /** Value represents supported values in the Bolt protocol. */ class Value { public: @@ -191,8 +147,7 @@ class Value { Date, LocalTime, LocalDateTime, - Duration, - Graph + Duration }; // constructors for primitive types @@ -212,7 +167,6 @@ class Value { Value(const Edge &value) : type_(Type::Edge) { new (&edge_v) Edge(value); } Value(const UnboundedEdge &value) : type_(Type::UnboundedEdge) { new (&unbounded_edge_v) UnboundedEdge(value); } Value(const Path &value) : type_(Type::Path) { new (&path_v) Path(value); } - Value(const Graph &value) : type_(Type::Graph) { new (&graph_v) Graph(value); } Value(const utils::Date &date) : type_(Type::Date) { new (&date_v) utils::Date(date); } Value(const utils::LocalTime &time) : type_(Type::LocalTime) { new (&local_time_v) utils::LocalTime(time); } @@ -232,7 +186,6 @@ class Value { new (&unbounded_edge_v) UnboundedEdge(std::move(value)); } Value(Path &&value) noexcept : type_(Type::Path) { new (&path_v) Path(std::move(value)); } - Value(Graph &&value) noexcept : type_(Type::Graph) { new (&graph_v) Graph(std::move(value)); } Value &operator=(const Value &other); Value &operator=(Value &&other) noexcept; @@ -268,7 +221,6 @@ class Value { DECL_GETTER_BY_REFERENCE(LocalTime, utils::LocalTime) DECL_GETTER_BY_REFERENCE(LocalDateTime, utils::LocalDateTime) DECL_GETTER_BY_REFERENCE(Duration, utils::Duration) - DECL_GETTER_BY_REFERENCE(Graph, Graph) #undef DECL_GETTER_BY_REFERNCE #define TYPE_CHECKER(type) \ @@ -288,7 +240,6 @@ class Value { TYPE_CHECKER(LocalTime) TYPE_CHECKER(LocalDateTime) TYPE_CHECKER(Duration) - TYPE_CHECKER(Graph) #undef TYPE_CHECKER friend std::ostream &operator<<(std::ostream &os, const Value &value); @@ -312,7 +263,6 @@ class Value { utils::LocalTime local_time_v; utils::LocalDateTime local_date_time_v; utils::Duration duration_v; - Graph graph_v; }; }; /** diff --git a/src/glue/communication.cpp b/src/glue/communication.cpp index f357d8b4c..10850323b 100644 --- a/src/glue/communication.cpp +++ b/src/glue/communication.cpp @@ -60,8 +60,6 @@ query::TypedValue ToTypedValue(const Value &value) { return query::TypedValue(value.ValueLocalDateTime()); case Value::Type::Duration: return query::TypedValue(value.ValueDuration()); - case Value::Type::Graph: - throw communication::bolt::ValueException("Unsupported conversion from Value to TypedValue"); } } @@ -189,23 +187,28 @@ storage::Result ToBoltPath(const query::Path &path, c return communication::bolt::Path(vertices, edges); } -storage::Result ToBoltGraph(const query::Graph &graph, const storage::Storage &db, - storage::View view) { - std::vector vertices; +storage::Result> ToBoltGraph(const query::Graph &graph, const storage::Storage &db, + storage::View view) { + std::map map; + std::vector vertices; vertices.reserve(graph.vertices().size()); for (const auto &v : graph.vertices()) { auto maybe_vertex = ToBoltVertex(v, db, view); if (maybe_vertex.HasError()) return maybe_vertex.GetError(); - vertices.emplace_back(std::move(*maybe_vertex)); + vertices.emplace_back(Value(std::move(*maybe_vertex))); } - std::vector edges; + map.emplace(std::make_pair("nodes", Value(vertices))); + + std::vector edges; edges.reserve(graph.edges().size()); for (const auto &e : graph.edges()) { auto maybe_edge = ToBoltEdge(e, db, view); if (maybe_edge.HasError()) return maybe_edge.GetError(); - edges.emplace_back(std::move(*maybe_edge)); + edges.emplace_back(Value(std::move(*maybe_edge))); } - return communication::bolt::Graph(vertices, edges); + map.emplace(std::make_pair("edges", Value(edges))); + + return std::move(map); } storage::PropertyValue ToPropertyValue(const Value &value) { @@ -248,8 +251,6 @@ storage::PropertyValue ToPropertyValue(const Value &value) { case Value::Type::Duration: return storage::PropertyValue( storage::TemporalData(storage::TemporalType::Duration, value.ValueDuration().microseconds)); - case Value::Type::Graph: - throw communication::bolt::ValueException("Unsupported conversion from Value to PropertyValue"); } } diff --git a/src/glue/communication.hpp b/src/glue/communication.hpp index 9b1ec670b..e40d1ed4a 100644 --- a/src/glue/communication.hpp +++ b/src/glue/communication.hpp @@ -56,8 +56,9 @@ storage::Result ToBoltPath(const query::Path &path, c /// @param storage::View for ToBoltVertex and ToBoltEdge. /// /// @throw std::bad_alloc -storage::Result ToBoltGraph(const query::Graph &graph, const storage::Storage &db, - storage::View view); +storage::Result> ToBoltGraph(const query::Graph &graph, + const storage::Storage &db, + storage::View view); /// @param query::TypedValue for converting to communication::bolt::Value. /// @param storage::Storage for ToBoltVertex and ToBoltEdge. diff --git a/src/query/graph.hpp b/src/query/graph.hpp index 3547034eb..d974211e0 100644 --- a/src/query/graph.hpp +++ b/src/query/graph.hpp @@ -18,6 +18,7 @@ #include "query/path.hpp" #include "utils/logging.hpp" #include "utils/memory.hpp" +#include "utils/pmr/unordered_set.hpp" #include "utils/pmr/vector.hpp" namespace memgraph::query { @@ -62,8 +63,8 @@ class Graph { const auto path_vertices_ = path.vertices(); const auto path_edges_ = path.edges(); std::for_each(path_vertices_.begin(), path_vertices_.end(), - [this](const VertexAccessor v) { vertices_.push_back(v); }); - std::for_each(path_edges_.begin(), path_edges_.end(), [this](const EdgeAccessor e) { edges_.push_back(e); }); + [this](const VertexAccessor v) { vertices_.insert(v); }); + std::for_each(path_edges_.begin(), path_edges_.end(), [this](const EdgeAccessor e) { edges_.insert(e); }); } std::vector OutEdges(query::VertexAccessor vertex_accessor) { @@ -93,9 +94,9 @@ class Graph { private: // Contains all the vertices in the Graph. - utils::pmr::vector vertices_; + utils::pmr::unordered_set vertices_; // Contains all the edges in the Graph - utils::pmr::vector edges_; + utils::pmr::unordered_set edges_; }; } // namespace memgraph::query