From 929e9433ea322f1aec93d3e4bff5cbf2c3b78e20 Mon Sep 17 00:00:00 2001 From: antoniofilipovic Date: Sat, 20 Aug 2022 20:26:04 +0200 Subject: [PATCH] fix most clang tidy errors --- src/query/db_accessor.cpp | 21 ++---- src/query/db_accessor.hpp | 90 ++++------------------- src/query/graph.cpp | 20 ++--- src/query/graph.hpp | 6 +- src/query/plan/operator.cpp | 6 +- src/query/procedure/mg_procedure_impl.cpp | 61 +++++++-------- src/query/procedure/mg_procedure_impl.hpp | 17 ++--- 7 files changed, 71 insertions(+), 150 deletions(-) diff --git a/src/query/db_accessor.cpp b/src/query/db_accessor.cpp index 8f591306d..66c6ecefe 100644 --- a/src/query/db_accessor.cpp +++ b/src/query/db_accessor.cpp @@ -9,8 +9,6 @@ // by the Apache License, Version 2.0, included in the file // licenses/APL.txt. -#pragma once - #include "query/db_accessor.hpp" #include "query/graph.hpp" @@ -73,7 +71,7 @@ storage::Result SubgraphDbAccessor::InsertEdge(SubgraphVertexAcces } storage::Result>>> -SubgraphDbAccessor::DetachRemoveVertex(VertexAccessor *vertex_accessor) { +SubgraphDbAccessor::DetachRemoveVertex(VertexAccessor *) { throw std::logic_error{"Such operation not possible on subgraph"}; } @@ -93,14 +91,9 @@ SubgraphVertexAccessor SubgraphDbAccessor::InsertVertex() { return SubgraphVertexAccessor(vertex, this->getGraph()); } -VerticesIterable SubgraphDbAccessor::Vertices(storage::View view) { - // todo antoniofilipovic change to get vertices from subgraph - return VerticesIterable(graph_->vertices()); - // return db_accessor_->Vertices(view); -} +VerticesIterable SubgraphDbAccessor::Vertices(storage::View) { return VerticesIterable(graph_->vertices()); } std::optional SubgraphDbAccessor::FindVertex(storage::Gid gid, storage::View view) { - // todo antoniofilipovic change to return SubgraphVertexAccessor && add check that vertex exists in subgraph std::optional maybe_vertex = db_accessor_->FindVertex(gid, view); if (maybe_vertex && this->graph_->ContainsVertex(*maybe_vertex)) { return *maybe_vertex; @@ -110,11 +103,9 @@ std::optional SubgraphDbAccessor::FindVertex(storage::Gid gid, s query::Graph *SubgraphDbAccessor::getGraph() { return graph_; } -VertexAccessor SubgraphVertexAccessor::GetVertexAccessor() { return impl_; } - -auto SubgraphVertexAccessor::OutEdges(storage::View view) -> decltype(impl_.OutEdges(view)) const { - // todo antoniofilipovic add filtering here +VertexAccessor SubgraphVertexAccessor::GetVertexAccessor() const { return impl_; } +auto SubgraphVertexAccessor::OutEdges(storage::View view) const -> decltype(impl_.OutEdges(view)) { auto maybe_edges = impl_.impl_.OutEdges(view, {}); if (maybe_edges.HasError()) return maybe_edges.GetError(); auto edges = std::move(*maybe_edges); @@ -136,9 +127,7 @@ auto SubgraphVertexAccessor::OutEdges(storage::View view) -> decltype(impl_.OutE return iter::imap(VertexAccessor::MakeEdgeAccessor, std::move(filteredOutEdges)); } -auto SubgraphVertexAccessor::InEdges(storage::View view) -> decltype(impl_.OutEdges(view)) const { - // todo antoniofilipovic add filtering here - +auto SubgraphVertexAccessor::InEdges(storage::View view) const -> decltype(impl_.InEdges(view)) { auto maybe_edges = impl_.impl_.InEdges(view, {}); if (maybe_edges.HasError()) return maybe_edges.GetError(); auto edges = std::move(*maybe_edges); diff --git a/src/query/db_accessor.hpp b/src/query/db_accessor.hpp index d134d7e90..18e2eec9f 100644 --- a/src/query/db_accessor.hpp +++ b/src/query/db_accessor.hpp @@ -200,9 +200,9 @@ class SubgraphVertexAccessor final { return impl_ == v.impl_; } - auto InEdges(storage::View view) -> decltype(impl_.OutEdges(view)) const; + auto InEdges(storage::View view) const -> decltype(impl_.OutEdges(view)); - auto OutEdges(storage::View view) -> decltype(impl_.OutEdges(view)) const; + auto OutEdges(storage::View view) const -> decltype(impl_.OutEdges(view)); auto Labels(storage::View view) const { return impl_.Labels(view); } @@ -225,7 +225,7 @@ class SubgraphVertexAccessor final { storage::Result SetProperty(storage::PropertyId key, const storage::PropertyValue &value) { return impl_.SetProperty(key, value); } - VertexAccessor GetVertexAccessor(); + VertexAccessor GetVertexAccessor() const; }; } // namespace memgraph::query @@ -246,12 +246,9 @@ struct hash { namespace memgraph::query { class VerticesIterable final { - enum class Type { VERTEX, SUBGRAPH_VERTEX }; - std::variant, std::equal_to, utils::Allocator>> iterable_; - Type type_; public: class Iterator final { @@ -259,29 +256,18 @@ class VerticesIterable final { std::unordered_set, std::equal_to, utils::Allocator>::iterator> it_; - Type type_; public: - explicit Iterator(storage::VerticesIterable::Iterator it, Type type_) : it_(it), type_(type_) {} + explicit Iterator(storage::VerticesIterable::Iterator it) : it_(it) {} explicit Iterator(std::unordered_set, std::equal_to, - utils::Allocator>::iterator it, - Type type_) - : it_(it), type_(type_) {} + utils::Allocator>::iterator it) + : it_(it) {} VertexAccessor operator*() const { return std::visit(memgraph::utils::Overloaded{[](auto it_) { return VertexAccessor(*it_); }}, it_); - // switch (type_) { - // case Type::VERTEX: { - // return VertexAccessor(*it_); - // } - // case Type::SUBGRAPH_VERTEX: { - // return SubgraphVertexAccessor(VertexAccessor(*it_)); - // } - // } } Iterator &operator++() { - //++it_; std::visit(memgraph::utils::Overloaded{[this](auto it_) { this->it_ = ++it_; }}, it_); return *this; } @@ -291,22 +277,19 @@ class VerticesIterable final { bool operator!=(const Iterator &other) const { return !(other == *this); } }; - explicit VerticesIterable(storage::VerticesIterable iterable) : iterable_(std::move(iterable)), type_(Type::VERTEX) {} + explicit VerticesIterable(storage::VerticesIterable iterable) : iterable_(std::move(iterable)) {} explicit VerticesIterable(std::unordered_set, std::equal_to, utils::Allocator> vertices) - : iterable_(vertices), type_(Type::SUBGRAPH_VERTEX) {} + : iterable_(vertices) {} Iterator begin() { - // return Iterator(iterable_.begin(), type_); - return std::visit( - memgraph::utils::Overloaded{[this](auto &iterable_) { return Iterator(iterable_.begin(), type_); }}, iterable_); + return std::visit(memgraph::utils::Overloaded{[](auto &iterable_) { return Iterator(iterable_.begin()); }}, + iterable_); } Iterator end() { - // return Iterator(iterable_.end(), type_); - - return std::visit(memgraph::utils::Overloaded{[this](auto &iterable_) { return Iterator(iterable_.end(), type_); }}, + return std::visit(memgraph::utils::Overloaded{[](auto &iterable_) { return Iterator(iterable_.end()); }}, iterable_); } }; @@ -463,13 +446,12 @@ class SubgraphDbAccessor final { explicit SubgraphDbAccessor(DbAccessor *db_accessor, Graph *graph); static SubgraphDbAccessor *MakeSubgraphDbAccessor(DbAccessor *db_accessor, Graph *graph); - storage::PropertyId SubgraphDBNameToProperty(const std::string_view name); - storage::PropertyId NameToProperty(const std::string_view name); + storage::PropertyId NameToProperty(std::string_view name); - storage::LabelId NameToLabel(const std::string_view name); + storage::LabelId NameToLabel(std::string_view name); - storage::EdgeTypeId NameToEdgeType(const std::string_view name); + storage::EdgeTypeId NameToEdgeType(std::string_view name); const std::string &PropertyToName(storage::PropertyId prop) const; @@ -496,48 +478,4 @@ class SubgraphDbAccessor final { Graph *getGraph(); }; -// class SubgraphEdgeAccessor final { -// public: -// query::EdgeAccessor impl_; - -// public: -// explicit SubgraphEdgeAccessor(query::EdgeAccessor impl) : impl_(std::move(impl)) {} - -// bool IsVisible(storage::View view) const { return impl_.IsVisible(view); } - -// storage::EdgeTypeId EdgeType() const { return impl_.EdgeType(); } - -// auto Properties(storage::View view) const { return impl_.Properties(view); } - -// storage::Result GetProperty(storage::View view, storage::PropertyId key) const { -// return impl_.GetProperty(key, view); -// } - -// storage::Result SetProperty(storage::PropertyId key, const storage::PropertyValue &value) { -// return impl_.SetProperty(key, value); -// } - -// storage::Result RemoveProperty(storage::PropertyId key) { -// return SetProperty(key, storage::PropertyValue()); -// } - -// storage::Result> ClearProperties() { -// return impl_.ClearProperties(); -// } - -// VertexAccessor To() const; //SubgraphVertexAccessor - -// VertexAccessor From() const; //SubgraphVertexAccessor - -// bool IsCycle() const; - -// int64_t CypherId() const { return impl_.Gid().AsInt(); } - -// storage::Gid Gid() const noexcept { return impl_.Gid(); } - -// bool operator==(const EdgeAccessor &e) const noexcept { return impl_ == e.impl_; } - -// bool operator!=(const EdgeAccessor &e) const noexcept { return !(*this == e); } -// }; - } // namespace memgraph::query diff --git a/src/query/graph.cpp b/src/query/graph.cpp index 5f5701b19..7e6947493 100644 --- a/src/query/graph.cpp +++ b/src/query/graph.cpp @@ -25,8 +25,8 @@ Graph::Graph(Graph &&other, utils::MemoryResource *memory) : vertices_(std::move(other.vertices_), memory), edges_(std::move(other.edges_), memory) {} void Graph::Expand(const Path &path) { - const auto path_vertices_ = path.vertices(); - const auto path_edges_ = path.edges(); + 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_.insert(v); }); std::for_each(path_edges_.begin(), path_edges_.end(), [this](const EdgeAccessor e) { edges_.insert(e); }); } @@ -61,21 +61,23 @@ std::optional Graph::RemoveEdge(const EdgeAccessor &edge) { return edge; } -std::vector Graph::OutEdges(query::VertexAccessor vertex_accessor) { - std::vector out_edges; - for (auto it = edges_.begin(); it != edges_.end(); ++it) { - if (it->From() == vertex_accessor) { - out_edges.emplace_back(*it); +std::vector Graph::OutEdges(VertexAccessor vertex_accessor) { + std::vector out_edges; + for (const auto &edge : edges_) { + if (edge.From() == vertex_accessor) { + out_edges.emplace_back(edge); } } return out_edges; } +/** Copy assign other, utils::MemoryResource of `this` is used */ +Graph &Graph::operator=(const Graph &) = default; + /** Move assign other, utils::MemoryResource of `this` is used. */ Graph &Graph::operator=(Graph &&) = default; -/** Returns the number of expansions (edges) in this path. */ -auto Graph::size() const { return edges_.size(); } +Graph::~Graph() = default; utils::pmr::unordered_set &Graph::vertices() { return vertices_; } utils::pmr::unordered_set &Graph::edges() { return edges_; } diff --git a/src/query/graph.hpp b/src/query/graph.hpp index 8c2c0bc7b..4fb6d230e 100644 --- a/src/query/graph.hpp +++ b/src/query/graph.hpp @@ -65,11 +65,13 @@ class Graph final { std::vector OutEdges(VertexAccessor vertex_accessor); + /** Copy assign other, utils::MemoryResource of `this` is used */ + Graph &operator=(const Graph &); + /** Move assign other, utils::MemoryResource of `this` is used. */ Graph &operator=(Graph &&); - /** Returns the number of expansions (edges) in this path. */ - auto size() const; + ~Graph(); utils::pmr::unordered_set &vertices(); utils::pmr::unordered_set &edges(); diff --git a/src/query/plan/operator.cpp b/src/query/plan/operator.cpp index 589e438dd..5f6f964b5 100644 --- a/src/query/plan/operator.cpp +++ b/src/query/plan/operator.cpp @@ -2918,7 +2918,7 @@ class AggregateCursor : public Cursor { case TypedValue::Type::Path: return; default: - throw QueryRuntimeException("Only path values allowed in PROJECT and PROJECT_TRANSITIVE aggregations."); + throw QueryRuntimeException("Only path values allowed in PROJECT aggregation."); } } }; @@ -3735,11 +3735,9 @@ void CallCustomProcedure(const std::string_view fully_qualified_procedure_name, } if (!args_list.empty() && args_list.front().type() == TypedValue::Type::Graph) { - // TypedValue subgraph_typed = TypedValue(args_list.front(), args_list.front().ValueGraph().GetMemoryResource()); - query::Graph *subgraph = + auto *subgraph = new query::Graph(std::move(args_list.front().ValueGraph()), args_list.front().ValueGraph().GetMemoryResource()); args_list.erase(args_list.begin()); - // query::Graph *subgraph = new query::Graph(std::move(graph), args_list.front().ValueGraph().GetMemoryResource()) graph.impl = query::SubgraphDbAccessor::MakeSubgraphDbAccessor(std::get(graph.impl), subgraph); } diff --git a/src/query/procedure/mg_procedure_impl.cpp b/src/query/procedure/mg_procedure_impl.cpp index 2787c3c1f..0fc8b0595 100644 --- a/src/query/procedure/mg_procedure_impl.cpp +++ b/src/query/procedure/mg_procedure_impl.cpp @@ -462,14 +462,12 @@ mgp_value::mgp_value(const memgraph::query::TypedValue &tv, mgp_graph *graph, me case MGP_VALUE_TYPE_VERTEX: { memgraph::utils::Allocator allocator(m); vertex_v = std::visit( - memgraph::utils::Overloaded{[&](memgraph::query::DbAccessor *impl) { - return allocator.new_object(tv.ValueVertex(), graph); - }, - [&](memgraph::query::SubgraphDbAccessor *impl) { - return allocator.new_object( - memgraph::query::SubgraphVertexAccessor(tv.ValueVertex(), impl->getGraph()), - graph); - }}, + memgraph::utils::Overloaded{ + [&](memgraph::query::DbAccessor *) { return allocator.new_object(tv.ValueVertex(), graph); }, + [&](memgraph::query::SubgraphDbAccessor *impl) { + return allocator.new_object( + memgraph::query::SubgraphVertexAccessor(tv.ValueVertex(), impl->getGraph()), graph); + }}, graph->impl); break; @@ -479,7 +477,7 @@ mgp_value::mgp_value(const memgraph::query::TypedValue &tv, mgp_graph *graph, me edge_v = std::visit( memgraph::utils::Overloaded{ - [&tv, graph, &allocator](memgraph::query::DbAccessor *db_impl) { + [&tv, graph, &allocator](memgraph::query::DbAccessor *) { return allocator.new_object(tv.ValueEdge(), graph); }, [&tv, graph, &allocator](memgraph::query::SubgraphDbAccessor *db_impl) { @@ -497,12 +495,10 @@ mgp_value::mgp_value(const memgraph::query::TypedValue &tv, mgp_graph *graph, me // released. mgp_path tmp_path(m); tmp_path.vertices.reserve(tv.ValuePath().vertices().size()); - // todo(antoniofilipovic) check if this works correctly -> test valuepath created as parameter for (const auto &v : tv.ValuePath().vertices()) { - // tmp_path.vertices.emplace_back(v, graph); std::visit( memgraph::utils::Overloaded{ - [&v, graph, &tmp_path](memgraph::query::DbAccessor *impl) { tmp_path.vertices.emplace_back(v, graph); }, + [&v, graph, &tmp_path](memgraph::query::DbAccessor *) { tmp_path.vertices.emplace_back(v, graph); }, [&v, graph, &tmp_path](memgraph::query::SubgraphDbAccessor *impl) { tmp_path.vertices.emplace_back(memgraph::query::SubgraphVertexAccessor(v, impl->getGraph()), graph); }}, @@ -510,15 +506,14 @@ mgp_value::mgp_value(const memgraph::query::TypedValue &tv, mgp_graph *graph, me } tmp_path.edges.reserve(tv.ValuePath().edges().size()); for (const auto &e : tv.ValuePath().edges()) { - std::visit( - memgraph::utils::Overloaded{ - [&e, graph, &tmp_path](memgraph::query::DbAccessor *db_impl) { tmp_path.edges.emplace_back(e, graph); }, - [&e, graph, &tmp_path](memgraph::query::SubgraphDbAccessor *db_impl) { - tmp_path.edges.emplace_back(e, memgraph::query::SubgraphVertexAccessor(e.From(), db_impl->getGraph()), - memgraph::query::SubgraphVertexAccessor(e.To(), db_impl->getGraph()), - graph); - }}, - graph->impl); + std::visit(memgraph::utils::Overloaded{ + [&e, graph, &tmp_path](memgraph::query::DbAccessor *) { tmp_path.edges.emplace_back(e, graph); }, + [&e, graph, &tmp_path](memgraph::query::SubgraphDbAccessor *db_impl) { + tmp_path.edges.emplace_back( + e, memgraph::query::SubgraphVertexAccessor(e.From(), db_impl->getGraph()), + memgraph::query::SubgraphVertexAccessor(e.To(), db_impl->getGraph()), graph); + }}, + graph->impl); } memgraph::utils::Allocator allocator(m); path_v = allocator.new_object(std::move(tmp_path)); @@ -845,9 +840,7 @@ mgp_value::~mgp_value() noexcept { DeleteValueMember(this); } mgp_edge *mgp_edge::Copy(const mgp_edge &edge, mgp_memory &memory) { return std::visit( memgraph::utils::Overloaded{ - [&](memgraph::query::DbAccessor *db_impl) { - return NewRawMgpObject(&memory, edge.impl, edge.from.graph); - }, + [&](memgraph::query::DbAccessor *) { return NewRawMgpObject(&memory, edge.impl, edge.from.graph); }, [&](memgraph::query::SubgraphDbAccessor *db_impl) { return NewRawMgpObject( &memory, edge.impl, memgraph::query::SubgraphVertexAccessor(edge.impl.From(), db_impl->getGraph()), @@ -1979,7 +1972,7 @@ mgp_error mgp_vertex_iter_in_edges(mgp_vertex *v, mgp_memory *memory, mgp_edges_ it->in_it.emplace(it->in->begin()); if (*it->in_it != it->in->end()) { std::visit(memgraph::utils::Overloaded{ - [&](memgraph::query::DbAccessor *impl) { + [&](memgraph::query::DbAccessor *) { it->current_e.emplace(**it->in_it, (**it->in_it).From(), (**it->in_it).To(), v->graph, it->GetMemoryResource()); }, @@ -2028,7 +2021,7 @@ mgp_error mgp_vertex_iter_out_edges(mgp_vertex *v, mgp_memory *memory, mgp_edges it->out_it.emplace(it->out->begin()); if (*it->out_it != it->out->end()) { std::visit(memgraph::utils::Overloaded{ - [&](memgraph::query::DbAccessor *impl) { + [&](memgraph::query::DbAccessor *) { it->current_e.emplace(**it->out_it, (**it->out_it).From(), (**it->out_it).To(), v->graph, it->GetMemoryResource()); }, @@ -2078,7 +2071,7 @@ mgp_error mgp_edges_iterator_next(mgp_edges_iterator *it, mgp_edge **result) { return nullptr; } std::visit(memgraph::utils::Overloaded{ - [&](memgraph::query::DbAccessor *impl) { + [&](memgraph::query::DbAccessor *) { it->current_e.emplace(**impl_it, (**impl_it).From(), (**impl_it).To(), it->source_vertex.graph, it->GetMemoryResource()); }, @@ -2260,7 +2253,7 @@ mgp_error mgp_graph_get_vertex_by_id(mgp_graph *graph, mgp_vertex_id id, mgp_mem graph->impl); if (maybe_vertex) { return std::visit(memgraph::utils::Overloaded{ - [memory, graph, maybe_vertex](memgraph::query::DbAccessor *impl) { + [memory, graph, maybe_vertex](memgraph::query::DbAccessor *) { return NewRawMgpObject(memory, *maybe_vertex, graph); }, [memory, graph, maybe_vertex](memgraph::query::SubgraphDbAccessor *impl) { @@ -2286,10 +2279,10 @@ mgp_error mgp_graph_create_vertex(struct mgp_graph *graph, mgp_memory *memory, m if (!MgpGraphIsMutable(*graph)) { throw ImmutableObjectException{"Cannot create a vertex in an immutable graph!"}; } - auto vertex = std::visit(memgraph::utils::Overloaded{[=](auto *impl) { - return NewRawMgpObject(memory, impl->InsertVertex(), graph); - }}, - graph->impl); + auto *vertex = std::visit(memgraph::utils::Overloaded{[=](auto *impl) { + return NewRawMgpObject(memory, impl->InsertVertex(), graph); + }}, + graph->impl); auto &ctx = graph->ctx; ctx->execution_stats[memgraph::query::ExecutionStats::Key::CREATED_NODES] += 1; @@ -2466,7 +2459,7 @@ mgp_error mgp_graph_create_edge(mgp_graph *graph, mgp_vertex *from, mgp_vertex * } return std::visit( memgraph::utils::Overloaded{ - [memory, edge, from](memgraph::query::DbAccessor *db_impl) { + [memory, edge, from](memgraph::query::DbAccessor *) { return NewRawMgpObject(memory->impl, edge.GetValue(), from->graph); }, [memory, edge, from](memgraph::query::SubgraphDbAccessor *db_impl) { @@ -2548,7 +2541,7 @@ mgp_error mgp_vertices_iterator_next(mgp_vertices_iterator *it, mgp_vertex **res return nullptr; } memgraph::utils::OnScopeExit clean_up([it] { it->current_v = std::nullopt; }); - std::visit(memgraph::utils::Overloaded{[it](memgraph::query::DbAccessor *impl) { + std::visit(memgraph::utils::Overloaded{[it](memgraph::query::DbAccessor *) { it->current_v.emplace(*it->current_it, it->graph, it->GetMemoryResource()); ; diff --git a/src/query/procedure/mg_procedure_impl.hpp b/src/query/procedure/mg_procedure_impl.hpp index 7dea54605..9f95164d0 100644 --- a/src/query/procedure/mg_procedure_impl.hpp +++ b/src/query/procedure/mg_procedure_impl.hpp @@ -764,15 +764,14 @@ struct mgp_vertices_iterator { graph->impl)), current_it(vertices.begin()) { if (current_it != vertices.end()) { - std::visit(memgraph::utils::Overloaded{[this, graph, memory](memgraph::query::DbAccessor *impl) { - current_v.emplace(*current_it, graph, memory); - }, - [this, graph, memory](memgraph::query::SubgraphDbAccessor *impl) { - current_v.emplace(memgraph::query::SubgraphVertexAccessor( - *current_it, impl->getGraph()), - graph, memory); - }}, - graph->impl); + std::visit( + memgraph::utils::Overloaded{ + [this, graph, memory](memgraph::query::DbAccessor *) { current_v.emplace(*current_it, graph, memory); }, + [this, graph, memory](memgraph::query::SubgraphDbAccessor *impl) { + current_v.emplace(memgraph::query::SubgraphVertexAccessor(*current_it, impl->getGraph()), graph, + memory); + }}, + graph->impl); } }