From c55183bd974d91fe938bc069183ee89db89c5146 Mon Sep 17 00:00:00 2001
From: antoniofilipovic <filipovicantonio1998@gmail.com>
Date: Fri, 19 Aug 2022 12:58:37 +0200
Subject: [PATCH] fix edges bug and paths bug and add tests
---
src/query/db_accessor.cpp | 3 -
src/query/db_accessor.hpp | 1 -
src/query/procedure/mg_procedure_impl.cpp | 74 +++++++----
src/query/procedure/mg_procedure_impl.hpp | 80 ++++++-----
tests/e2e/write_procedures/CMakeLists.txt | 1 +
tests/e2e/write_procedures/procedures/read.py | 36 ++++-
tests/e2e/write_procedures/read_subgraph.py | 124 ++++++++++++++++++
7 files changed, 254 insertions(+), 65 deletions(-)
create mode 100644 tests/e2e/write_procedures/read_subgraph.py
diff --git a/src/query/db_accessor.cpp b/src/query/db_accessor.cpp
index 5268b3cd2..8f591306d 100644
--- a/src/query/db_accessor.cpp
+++ b/src/query/db_accessor.cpp
@@ -111,9 +111,6 @@ std::optional<VertexAccessor> SubgraphDbAccessor::FindVertex(storage::Gid gid, s
query::Graph *SubgraphDbAccessor::getGraph() { return graph_; }
VertexAccessor SubgraphVertexAccessor::GetVertexAccessor() { return impl_; }
-static SubgraphVertexAccessor MakeSubgraphVertexAccessor(query::VertexAccessor impl, query::Graph *graph_) {
- return SubgraphVertexAccessor(impl, graph_);
-}
auto SubgraphVertexAccessor::OutEdges(storage::View view) -> decltype(impl_.OutEdges(view)) const {
// todo antoniofilipovic add filtering here
diff --git a/src/query/db_accessor.hpp b/src/query/db_accessor.hpp
index abb8619e7..d134d7e90 100644
--- a/src/query/db_accessor.hpp
+++ b/src/query/db_accessor.hpp
@@ -193,7 +193,6 @@ class SubgraphVertexAccessor final {
query::VertexAccessor impl_;
query::Graph *graph_;
- static SubgraphVertexAccessor MakeSubgraphVertexAccessor(query::VertexAccessor impl, query::Graph *graph_);
explicit SubgraphVertexAccessor(query::VertexAccessor impl, query::Graph *graph_) : impl_(impl), graph_(graph_) {}
bool operator==(const SubgraphVertexAccessor &v) const noexcept {
diff --git a/src/query/procedure/mg_procedure_impl.cpp b/src/query/procedure/mg_procedure_impl.cpp
index 6a0038760..2787c3c1f 100644
--- a/src/query/procedure/mg_procedure_impl.cpp
+++ b/src/query/procedure/mg_procedure_impl.cpp
@@ -476,8 +476,18 @@ mgp_value::mgp_value(const memgraph::query::TypedValue &tv, mgp_graph *graph, me
}
case MGP_VALUE_TYPE_EDGE: {
memgraph::utils::Allocator<mgp_edge> allocator(m);
- // todo(antoniofilipovic) fix
- // edge_v = allocator.new_object<mgp_edge>(tv.ValueEdge(), graph);
+
+ edge_v = std::visit(
+ memgraph::utils::Overloaded{
+ [&tv, graph, &allocator](memgraph::query::DbAccessor *db_impl) {
+ return allocator.new_object<mgp_edge>(tv.ValueEdge(), graph);
+ },
+ [&tv, graph, &allocator](memgraph::query::SubgraphDbAccessor *db_impl) {
+ return allocator.new_object<mgp_edge>(
+ tv.ValueEdge(), memgraph::query::SubgraphVertexAccessor(tv.ValueEdge().From(), db_impl->getGraph()),
+ memgraph::query::SubgraphVertexAccessor(tv.ValueEdge().To(), db_impl->getGraph()), graph);
+ }},
+ graph->impl);
break;
}
case MGP_VALUE_TYPE_PATH: {
@@ -487,13 +497,28 @@ 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);
+ // 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::SubgraphDbAccessor *impl) {
+ tmp_path.vertices.emplace_back(memgraph::query::SubgraphVertexAccessor(v, impl->getGraph()), graph);
+ }},
+ graph->impl);
}
tmp_path.edges.reserve(tv.ValuePath().edges().size());
for (const auto &e : tv.ValuePath().edges()) {
- // todo(antoniofilipovic) fix this
- // tmp_path.edges.emplace_back(e, graph);
+ 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);
}
memgraph::utils::Allocator<mgp_path> allocator(m);
path_v = allocator.new_object<mgp_path>(std::move(tmp_path));
@@ -818,19 +843,15 @@ mgp_value::mgp_value(mgp_value &&other, memgraph::utils::MemoryResource *m) : ty
mgp_value::~mgp_value() noexcept { DeleteValueMember(this); }
mgp_edge *mgp_edge::Copy(const mgp_edge &edge, mgp_memory &memory) {
- // return NewRawMgpObject<mgp_edge>(&memory, edge.impl, edge.from.graph);
-
return std::visit(
memgraph::utils::Overloaded{
[&](memgraph::query::DbAccessor *db_impl) {
return NewRawMgpObject<mgp_edge>(&memory, edge.impl, edge.from.graph);
- // return NewRawMgpObject<mgp_edge>(&memory, edge.impl, edge.impl.From(), edge.impl.To(), edge.to.graph,
- // memory.impl);
},
[&](memgraph::query::SubgraphDbAccessor *db_impl) {
- const auto &from_v = memgraph::query::SubgraphVertexAccessor(edge.impl.From(), db_impl->getGraph());
- const auto &to_v = memgraph::query::SubgraphVertexAccessor(edge.impl.To(), db_impl->getGraph());
- return NewRawMgpObject<mgp_edge>(&memory, edge.impl, from_v, to_v, edge.to.graph, memory.impl);
+ return NewRawMgpObject<mgp_edge>(
+ &memory, edge.impl, memgraph::query::SubgraphVertexAccessor(edge.impl.From(), db_impl->getGraph()),
+ memgraph::query::SubgraphVertexAccessor(edge.impl.To(), db_impl->getGraph()), edge.to.graph);
}},
edge.to.graph->impl);
}
@@ -2443,23 +2464,18 @@ mgp_error mgp_graph_create_edge(mgp_graph *graph, mgp_vertex *from, mgp_vertex *
if (ctx->trigger_context_collector) {
ctx->trigger_context_collector->RegisterCreatedObject(*edge);
}
- // check what does this method call
- // todo(antoniofilipovic)
- // return NewRawMgpObject<mgp_edge>(memory, edge.GetValue(), from->graph);
- return std::visit(memgraph::utils::Overloaded{
- [memory, edge, from](memgraph::query::DbAccessor *db_impl) {
- return NewRawMgpObject<mgp_edge>(memory->impl, edge.GetValue(), from->graph);
- },
- [memory, edge, from](memgraph::query::SubgraphDbAccessor *db_impl) {
- const auto &v_from =
- memgraph::query::SubgraphVertexAccessor::MakeSubgraphVertexAccessor(
- edge.GetValue().From(), db_impl->getGraph());
- const auto &v_to = memgraph::query::SubgraphVertexAccessor::MakeSubgraphVertexAccessor(
- edge.GetValue().To(), db_impl->getGraph());
- return NewRawMgpObject<mgp_edge>(memory->impl, edge.GetValue(), v_from, v_to,
- from->graph, memory->impl);
- }},
- graph->impl);
+ return std::visit(
+ memgraph::utils::Overloaded{
+ [memory, edge, from](memgraph::query::DbAccessor *db_impl) {
+ return NewRawMgpObject<mgp_edge>(memory->impl, edge.GetValue(), from->graph);
+ },
+ [memory, edge, from](memgraph::query::SubgraphDbAccessor *db_impl) {
+ const auto &v_from =
+ memgraph::query::SubgraphVertexAccessor(edge.GetValue().From(), db_impl->getGraph());
+ const auto &v_to = memgraph::query::SubgraphVertexAccessor(edge.GetValue().To(), db_impl->getGraph());
+ return NewRawMgpObject<mgp_edge>(memory->impl, edge.GetValue(), v_from, v_to, from->graph);
+ }},
+ graph->impl);
},
result);
}
diff --git a/src/query/procedure/mg_procedure_impl.hpp b/src/query/procedure/mg_procedure_impl.hpp
index f26ed98b4..7dea54605 100644
--- a/src/query/procedure/mg_procedure_impl.hpp
+++ b/src/query/procedure/mg_procedure_impl.hpp
@@ -448,10 +448,30 @@ struct mgp_vertex {
: memory(memory), impl(v), graph(graph) {}
mgp_vertex(const mgp_vertex &other, memgraph::utils::MemoryResource *memory) noexcept
- : memory(memory), impl(other.impl), graph(other.graph) {}
+ : memory(memory), impl(other.impl), graph(other.graph) {
+ std::visit(memgraph::utils::Overloaded{
+ [](memgraph::query::VertexAccessor) { std::cout << "VertexAccessor" << std::endl; },
+ [](memgraph::query::SubgraphVertexAccessor) { std::cout << "SubgraphVertexAccessor" << std::endl; }},
+ other.impl);
+
+ std::visit(memgraph::utils::Overloaded{
+ [](memgraph::query::VertexAccessor) { std::cout << "VertexAccessor" << std::endl; },
+ [](memgraph::query::SubgraphVertexAccessor) { std::cout << "SubgraphVertexAccessor" << std::endl; }},
+ this->impl);
+ }
mgp_vertex(mgp_vertex &&other, memgraph::utils::MemoryResource *memory) noexcept
- : memory(memory), impl(other.impl), graph(other.graph) {}
+ : memory(memory), impl(other.impl), graph(other.graph) {
+ std::visit(memgraph::utils::Overloaded{
+ [](memgraph::query::VertexAccessor) { std::cout << "VertexAccessor" << std::endl; },
+ [](memgraph::query::SubgraphVertexAccessor) { std::cout << "SubgraphVertexAccessor" << std::endl; }},
+ other.impl);
+
+ std::visit(memgraph::utils::Overloaded{
+ [](memgraph::query::VertexAccessor) { std::cout << "VertexAccessor" << std::endl; },
+ [](memgraph::query::SubgraphVertexAccessor) { std::cout << "SubgraphVertexAccessor" << std::endl; }},
+ this->impl);
+ }
mgp_vertex(mgp_vertex &&other) noexcept : memory(other.memory), impl(other.impl), graph(other.graph) {}
@@ -497,6 +517,34 @@ struct mgp_vertex {
mgp_graph *graph;
};
+struct mgp_graph {
+ std::variant<memgraph::query::DbAccessor *, memgraph::query::SubgraphDbAccessor *> impl;
+ memgraph::storage::View view;
+ // TODO: Merge `mgp_graph` and `mgp_memory` into a single `mgp_context`. The
+ // `ctx` field is out of place here.
+ memgraph::query::ExecutionContext *ctx;
+
+ // memgraph:::query::Graph *subraph;
+
+ static mgp_graph WritableGraph(memgraph::query::DbAccessor &acc, memgraph::storage::View view,
+ memgraph::query::ExecutionContext &ctx) {
+ return mgp_graph{&acc, view, &ctx};
+ }
+
+ static mgp_graph NonWritableGraph(memgraph::query::DbAccessor &acc, memgraph::storage::View view) {
+ return mgp_graph{&acc, view, nullptr};
+ }
+
+ static mgp_graph WritableGraph(memgraph::query::SubgraphDbAccessor &acc, memgraph::storage::View view,
+ memgraph::query::ExecutionContext &ctx) {
+ return mgp_graph{&acc, view, &ctx};
+ }
+
+ static mgp_graph NonWritableGraph(memgraph::query::SubgraphDbAccessor &acc, memgraph::storage::View view) {
+ return mgp_graph{&acc, view, nullptr};
+ }
+};
+
struct mgp_edge {
/// Allocator type so that STL containers are aware that we need one.
/// We don't actually need this, but it simplifies the C API, because we store
@@ -609,34 +657,6 @@ struct mgp_func_result {
std::optional<memgraph::utils::pmr::string> error_msg;
};
-struct mgp_graph {
- std::variant<memgraph::query::DbAccessor *, memgraph::query::SubgraphDbAccessor *> impl;
- memgraph::storage::View view;
- // TODO: Merge `mgp_graph` and `mgp_memory` into a single `mgp_context`. The
- // `ctx` field is out of place here.
- memgraph::query::ExecutionContext *ctx;
-
- // memgraph:::query::Graph *subraph;
-
- static mgp_graph WritableGraph(memgraph::query::DbAccessor &acc, memgraph::storage::View view,
- memgraph::query::ExecutionContext &ctx) {
- return mgp_graph{&acc, view, &ctx};
- }
-
- static mgp_graph NonWritableGraph(memgraph::query::DbAccessor &acc, memgraph::storage::View view) {
- return mgp_graph{&acc, view, nullptr};
- }
-
- static mgp_graph WritableGraph(memgraph::query::SubgraphDbAccessor &acc, memgraph::storage::View view,
- memgraph::query::ExecutionContext &ctx) {
- return mgp_graph{&acc, view, &ctx};
- }
-
- static mgp_graph NonWritableGraph(memgraph::query::SubgraphDbAccessor &acc, memgraph::storage::View view) {
- return mgp_graph{&acc, view, nullptr};
- }
-};
-
// Prevents user to use ExecutionContext in writable callables
struct mgp_func_context {
memgraph::query::DbAccessor *impl;
diff --git a/tests/e2e/write_procedures/CMakeLists.txt b/tests/e2e/write_procedures/CMakeLists.txt
index 72b640271..27a9a73e2 100644
--- a/tests/e2e/write_procedures/CMakeLists.txt
+++ b/tests/e2e/write_procedures/CMakeLists.txt
@@ -5,5 +5,6 @@ endfunction()
copy_write_procedures_e2e_python_files(common.py)
copy_write_procedures_e2e_python_files(conftest.py)
copy_write_procedures_e2e_python_files(simple_write.py)
+copy_write_procedures_e2e_python_files(read_subgraph.py)
add_subdirectory(procedures)
diff --git a/tests/e2e/write_procedures/procedures/read.py b/tests/e2e/write_procedures/procedures/read.py
index 3f791a37a..5f503721d 100644
--- a/tests/e2e/write_procedures/procedures/read.py
+++ b/tests/e2e/write_procedures/procedures/read.py
@@ -13,11 +13,43 @@ import mgp
@mgp.read_proc
-def underlying_graph_is_mutable(ctx: mgp.ProcCtx,
- object: mgp.Any) -> mgp.Record(mutable=bool):
+def underlying_graph_is_mutable(ctx: mgp.ProcCtx, object: mgp.Any) -> mgp.Record(mutable=bool):
return mgp.Record(mutable=object.underlying_graph_is_mutable())
@mgp.read_proc
def graph_is_mutable(ctx: mgp.ProcCtx) -> mgp.Record(mutable=bool):
return mgp.Record(mutable=ctx.graph.is_mutable())
+
+
+@mgp.read_proc
+def subgraph_get_vertices(ctx: mgp.ProcCtx) -> mgp.Record(node=mgp.Vertex):
+ return [mgp.Record(node=vertex) for vertex in ctx.graph.vertices]
+
+
+@mgp.read_proc
+def subgraph_get_out_edges(ctx: mgp.ProcCtx, vertex: mgp.Vertex) -> mgp.Record(edge=mgp.Edge):
+ return [mgp.Record(edge=edge) for edge in vertex.out_edges]
+
+
+@mgp.read_proc
+def subgraph_get_in_edges(ctx: mgp.ProcCtx, vertex: mgp.Vertex) -> mgp.Record(edge=mgp.Edge):
+ return [mgp.Record(edge=edge) for edge in vertex.in_edges]
+
+
+@mgp.read_proc
+def subgraph_get_2_hop_edges(ctx: mgp.ProcCtx, vertex: mgp.Vertex) -> mgp.Record(edge=mgp.Edge):
+ out_edges = vertex.out_edges
+ records = []
+ for edge in out_edges:
+ vertex = edge.to_vertex
+ properties = vertex.properties
+ print(properties)
+ records.extend([mgp.Record(edge=edge) for edge in edge.to_vertex.out_edges])
+ return records
+
+
+@mgp.read_proc
+def subgraph_get_out_edges_vertex_id(ctx: mgp.ProcCtx, vertex: mgp.Vertex) -> mgp.Record(edge=mgp.Edge):
+ vertex = ctx.graph.get_vertex_by_id(vertex.id)
+ return [mgp.Record(edge=edge) for edge in vertex.out_edges]
diff --git a/tests/e2e/write_procedures/read_subgraph.py b/tests/e2e/write_procedures/read_subgraph.py
new file mode 100644
index 000000000..cd1f22939
--- /dev/null
+++ b/tests/e2e/write_procedures/read_subgraph.py
@@ -0,0 +1,124 @@
+# Copyright 2022 Memgraph Ltd.
+#
+# Use of this software is governed by the Business Source License
+# included in the file licenses/BSL.txt; by using this file, you agree to be bound by the terms of the Business Source
+# License, and you may not use this file except in compliance with the Business Source License.
+#
+# As of the Change Date specified in that file, in accordance with
+# the Business Source License, use of this software will be governed
+# by the Apache License, Version 2.0, included in the file
+# licenses/APL.txt.
+
+import typing
+import mgclient
+import sys
+import pytest
+from common import execute_and_fetch_all, has_n_result_row
+
+
+def create_subgraph(cursor):
+ execute_and_fetch_all(cursor, "CREATE (n:Person {id: 1});")
+ execute_and_fetch_all(cursor, "CREATE (n:Person {id: 2});")
+ execute_and_fetch_all(cursor, "CREATE (n:Person {id: 3});")
+ execute_and_fetch_all(cursor, "CREATE (n:Person {id: 4});")
+ execute_and_fetch_all(cursor, "CREATE (n:Team {id: 5});")
+ execute_and_fetch_all(cursor, "CREATE (n:Team {id: 6});")
+ execute_and_fetch_all(cursor, "MATCH (p:Person {id: 1}) MATCH (t:Team {id:5}) CREATE (p)-[:SUPPORTS]->(t);")
+ execute_and_fetch_all(cursor, "MATCH (p:Person {id: 1}) MATCH (t:Team {id:6}) CREATE (p)-[:SUPPORTS]->(t);")
+ execute_and_fetch_all(cursor, "MATCH (p:Person {id: 2}) MATCH (t:Team {id:6}) CREATE (p)-[:SUPPORTS]->(t);")
+ execute_and_fetch_all(cursor, "MATCH (p1:Person {id: 1}) MATCH (p2:Person {id:2}) CREATE (p1)-[:KNOWS]->(p2);")
+ execute_and_fetch_all(cursor, "MATCH (t1:Team {id: 5}) MATCH (t2:Team {id:6}) CREATE (t1)-[:IS_RIVAL_TO]->(t2);")
+ execute_and_fetch_all(cursor, "MATCH (p1:Person {id: 3}) MATCH (p2:Person {id:4}) CREATE (p1)-[:KNOWS]->(p2);")
+
+
+def test_get_vertices(connection):
+ cursor = connection.cursor()
+ execute_and_fetch_all(cursor, "MATCH (n) DETACH DELETE n;")
+ create_subgraph(cursor)
+
+ assert has_n_result_row(cursor, "MATCH (n) RETURN n", 6)
+ results = execute_and_fetch_all(
+ cursor,
+ f"MATCH p=(n:Person)-[:SUPPORTS]->(m:Team) WITH project(p) AS graph CALL read.subgraph_get_vertices(graph) YIELD node RETURN node;",
+ )
+ assert len(results) == 4
+
+ execute_and_fetch_all(
+ cursor,
+ f"MATCH (n) DETACH DELETE n;",
+ )
+
+
+def test_get_out_edges(connection):
+ cursor = connection.cursor()
+ execute_and_fetch_all(cursor, "MATCH (n) DETACH DELETE n;")
+ create_subgraph(cursor)
+
+ assert has_n_result_row(cursor, "MATCH (n) RETURN n", 6)
+ results = execute_and_fetch_all(
+ cursor,
+ f"MATCH p=(n:Person)-[:SUPPORTS]->(m:Team) WITH project(p) AS graph MATCH (n1:Person {{id:1}}) CALL read.subgraph_get_out_edges(graph, n1) YIELD edge RETURN edge;",
+ )
+ assert len(results) == 2
+ execute_and_fetch_all(
+ cursor,
+ f"MATCH (n) DETACH DELETE n;",
+ )
+
+
+def test_get_in_edges(connection):
+ cursor = connection.cursor()
+ execute_and_fetch_all(cursor, "MATCH (n) DETACH DELETE n;")
+ create_subgraph(cursor)
+
+ assert has_n_result_row(cursor, "MATCH (n) RETURN n", 6)
+ results = execute_and_fetch_all(
+ cursor,
+ f"MATCH p=(n:Person)-[:SUPPORTS]->(m:Team) WITH project(p) AS graph MATCH (t1:Team {{id:6}}) CALL read.subgraph_get_in_edges(graph, t1) YIELD edge RETURN edge;",
+ )
+
+ assert len(results) == 2
+
+ execute_and_fetch_all(
+ cursor,
+ f"MATCH (n) DETACH DELETE n;",
+ )
+
+
+def test_get_2_hop_edges(connection):
+ cursor = connection.cursor()
+ execute_and_fetch_all(cursor, "MATCH (n) DETACH DELETE n;")
+ create_subgraph(cursor)
+
+ assert has_n_result_row(cursor, "MATCH (n) RETURN n", 6)
+ results = execute_and_fetch_all(
+ cursor,
+ f"MATCH p=(n:Person)-[:SUPPORTS]->(m:Team) WITH project(p) AS graph MATCH (n1:Person {{id:1}}) CALL read.subgraph_get_2_hop_edges(graph, n1) YIELD edge RETURN edge;",
+ )
+ assert len(results) == 0
+ execute_and_fetch_all(
+ cursor,
+ f"MATCH (n) DETACH DELETE n;",
+ )
+
+
+def test_get_out_edges_vertex_id(connection):
+ cursor = connection.cursor()
+ execute_and_fetch_all(cursor, "MATCH (n) DETACH DELETE n;")
+ create_subgraph(cursor=cursor)
+
+ assert has_n_result_row(cursor, "MATCH (n) RETURN n", 6)
+ results = execute_and_fetch_all(
+ cursor,
+ f"MATCH p=(n:Person)-[:SUPPORTS]->(m:Team) WITH project(p) AS graph MATCH (n1:Person {{id:1}}) CALL read.subgraph_get_out_edges_vertex_id(graph, n1) YIELD edge RETURN edge;",
+ )
+ assert len(results) == 2
+
+ execute_and_fetch_all(
+ cursor,
+ f"MATCH (n) DETACH DELETE n;",
+ )
+
+
+if __name__ == "__main__":
+ sys.exit(pytest.main([__file__, "-rA"]))