From 74d9dd0b0a145eefb1497544fd3c03852bef48a5 Mon Sep 17 00:00:00 2001
From: Teon Banek <teon.banek@memgraph.io>
Date: Fri, 28 Feb 2020 09:44:39 +0100
Subject: [PATCH] Implement mgp.Path
Reviewers: ipaljak, mferencevic
Reviewed By: ipaljak, mferencevic
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D2700
---
include/mgp.py | 119 ++++++++++--
src/py/py.hpp | 1 +
src/query/procedure/py_module.cpp | 232 ++++++++++++++++++++++-
src/query/procedure/py_module.hpp | 2 +
tests/unit/query_procedure_py_module.cpp | 73 ++++++-
5 files changed, 406 insertions(+), 21 deletions(-)
diff --git a/include/mgp.py b/include/mgp.py
index 8ce6d5232..982ae329f 100644
--- a/include/mgp.py
+++ b/include/mgp.py
@@ -128,9 +128,16 @@ class Edge:
def __init__(self, edge):
if not isinstance(edge, _mgp.Edge):
- raise TypeError("Expected '_mgp.Edge', got '{}'".fmt(type(edge)))
+ raise TypeError("Expected '_mgp.Edge', got '{}'".format(type(edge)))
self._edge = edge
+ def __deepcopy__(self, memo):
+ # This is the same as the shallow copy, because we want to share the
+ # underlying C struct. Besides, it doesn't make much sense to actually
+ # copy _mgp.Edge as that is actually a reference to a graph element
+ # and not a proper value.
+ return Edge(self._edge)
+
def is_valid(self) -> bool:
'''Return True if `self` is in valid context and may be used.'''
return self._edge.is_valid()
@@ -192,9 +199,16 @@ class Vertex:
def __init__(self, vertex):
if not isinstance(vertex, _mgp.Vertex):
- raise TypeError("Expected '_mgp.Vertex', got '{}'".fmt(type(vertex)))
+ raise TypeError("Expected '_mgp.Vertex', got '{}'".format(type(vertex)))
self._vertex = vertex
+ def __deepcopy__(self, memo):
+ # This is the same as the shallow copy, because we want to share the
+ # underlying C struct. Besides, it doesn't make much sense to actually
+ # copy _mgp.Vertex as that is actually a reference to a graph element
+ # and not a proper value.
+ return Vertex(self._vertex)
+
def is_valid(self) -> bool:
'''Return True if `self` is in valid context and may be used'''
return self._vertex.is_valid()
@@ -254,15 +268,60 @@ class Vertex:
return self._vertex == other._vertex
+class InvalidPathError(Exception):
+ '''Signals using a Path instance not part of the procedure context.'''
+ pass
+
+
class Path:
'''Path containing Vertex and Edge instances.'''
+ __slots__ = ('_path', '_vertices', '_edges')
- def __init__(self, starting_vertex: Vertex):
+ def __init__(self, starting_vertex_or_path: typing.Union[_mgp.Path, Vertex]):
'''Initialize with a starting Vertex.
Raise InvalidVertexError if passed in Vertex is invalid.
'''
- pass
+ # We cache calls to `vertices` and `edges`, so as to avoid needless
+ # allocations at the C level.
+ self._vertices = None
+ self._edges = None
+ # Accepting _mgp.Path is just for internal usage.
+ if isinstance(starting_vertex_or_path, _mgp.Path):
+ self._path = starting_vertex_or_path
+ elif isinstance(starting_vertex_or_path, Vertex):
+ vertex = starting_vertex_or_path._vertex
+ if not vertex.is_valid():
+ raise InvalidVertexError()
+ self._path = _mgp.Path.make_with_start(vertex)
+ else:
+ raise TypeError("Expected '_mgp.Vertex' or '_mgp.Path', got '{}'"
+ .format(type(starting_vertex_or_path)))
+
+ def __copy__(self):
+ if not self.is_valid():
+ raise InvalidPathError()
+ assert len(self.vertices) >= 1
+ path = Path(self.vertices[0])
+ for e in self.edges:
+ path.expand(e)
+ return path
+
+ def __deepcopy__(self, memo):
+ try:
+ return Path(memo[id(self._path)])
+ except KeyError:
+ pass
+ # This is the same as the shallow copy, as the underlying C API should
+ # not support deepcopy. Besides, it doesn't make much sense to actually
+ # copy Edge and Vertex types as they are actually references to graph
+ # elements and not proper values.
+ path = self.__copy__()
+ memo[id(self._path)] = path._path
+ return path
+
+ def is_valid(self) -> bool:
+ return self._path.is_valid()
def expand(self, edge: Edge):
'''Append an edge continuing from the last vertex on the path.
@@ -273,18 +332,44 @@ class Path:
Raise ValueError if the current last vertex in the path is not part of
the given edge.
Raise InvalidEdgeError if passed in edge is invalid.
+ Raise InvalidPathError if using an invalid Path instance.
'''
- pass
+ if not isinstance(edge, Edge):
+ raise TypeError("Expected '_mgp.Edge', got '{}'".format(type(edge)))
+ if not self.is_valid():
+ raise InvalidPathError()
+ if not edge.is_valid():
+ raise InvalidEdgeError()
+ self._path.expand(edge._edge)
+ # Invalidate our cached tuples
+ self._vertices = None
+ self._edges = None
@property
def vertices(self) -> typing.Tuple[Vertex, ...]:
- '''Vertices ordered from the start to the end of the path.'''
- pass
+ '''Vertices ordered from the start to the end of the path.
+
+ Raise InvalidPathError if using an invalid Path instance.'''
+ if not self.is_valid():
+ raise InvalidPathError()
+ if self._vertices is None:
+ num_vertices = self._path.size() + 1
+ self._vertices = tuple(Vertex(self._path.vertex_at(i))
+ for i in range(num_vertices))
+ return self._vertices
@property
def edges(self) -> typing.Tuple[Edge, ...]:
- '''Edges ordered from the start to the end of the path.'''
- pass
+ '''Edges ordered from the start to the end of the path.
+
+ Raise InvalidPathError if using an invalid Path instance.'''
+ if not self.is_valid():
+ raise InvalidPathError()
+ if self._edges is None:
+ num_edges = self._path.size()
+ self._edges = tuple(Edge(self._path.edge_at(i))
+ for i in range(num_edges))
+ return self._edges
class Record:
@@ -297,7 +382,7 @@ class Record:
class InvalidProcCtxError(Exception):
- '''Signals using an ProcCtx instance outside of the registered procedure.'''
+ '''Signals using a ProcCtx instance outside of the registered procedure.'''
pass
@@ -307,9 +392,15 @@ class Vertices:
def __init__(self, graph):
if not isinstance(graph, _mgp.Graph):
- raise TypeError("Expected '_mgp.Graph', got '{}'".fmt(type(graph)))
+ raise TypeError("Expected '_mgp.Graph', got '{}'".format(type(graph)))
self._graph = graph
+ def __deepcopy__(self, memo):
+ # This is the same as the shallow copy, because we want to share the
+ # underlying C struct. Besides, it doesn't make much sense to actually
+ # copy _mgp.Graph as that always references the whole graph state.
+ return Vertices(self._graph)
+
def is_valid(self) -> bool:
'''Return True if `self` is in valid context and may be used.'''
return self._graph.is_valid()
@@ -336,6 +427,12 @@ class Graph:
raise TypeError("Expected '_mgp.Graph', got '{}'".format(type(graph)))
self._graph = graph
+ def __deepcopy__(self, memo):
+ # This is the same as the shallow copy, because we want to share the
+ # underlying C struct. Besides, it doesn't make much sense to actually
+ # copy _mgp.Graph as that always references the whole graph state.
+ return Graph(self._graph)
+
def is_valid(self) -> bool:
'''Return True if `self` is in valid context and may be used.'''
return self._graph.is_valid()
diff --git a/src/py/py.hpp b/src/py/py.hpp
index 8460fe5a1..d88631575 100644
--- a/src/py/py.hpp
+++ b/src/py/py.hpp
@@ -43,6 +43,7 @@ class Object final {
public:
Object() = default;
+ Object(std::nullptr_t) {}
explicit Object(PyObject *ptr) noexcept : ptr_(ptr) {}
~Object() noexcept { Py_XDECREF(ptr_); }
diff --git a/src/query/procedure/py_module.cpp b/src/query/procedure/py_module.cpp
index a52819632..f635721c4 100644
--- a/src/query/procedure/py_module.cpp
+++ b/src/query/procedure/py_module.cpp
@@ -1,11 +1,24 @@
#include "query/procedure/py_module.hpp"
+#include <sstream>
#include <stdexcept>
+#include <string>
#include "query/procedure/mg_procedure_impl.hpp"
namespace query::procedure {
+// Set this as a __reduce__ special method on our types to prevent `pickle` and
+// `copy` module operations on our types.
+PyObject *DisallowPickleAndCopy(PyObject *self, PyObject *Py_UNUSED(ignored)) {
+ auto *type = Py_TYPE(self);
+ std::stringstream ss;
+ ss << "cannot pickle nor copy '" << type->tp_name << "' object";
+ const auto &msg = ss.str();
+ PyErr_SetString(PyExc_TypeError, msg.c_str());
+ return nullptr;
+}
+
// Definitions of types wrapping C API types
//
// These should all be in the private `_mgp` Python module, which will be used
@@ -68,6 +81,8 @@ PyObject *PyVerticesIteratorNext(PyVerticesIterator *self,
}
static PyMethodDef PyVerticesIteratorMethods[] = {
+ {"__reduce__", reinterpret_cast<PyCFunction>(DisallowPickleAndCopy),
+ METH_NOARGS, "__reduce__ is not supported"},
{"get", reinterpret_cast<PyCFunction>(PyVerticesIteratorGet), METH_NOARGS,
"Get the current vertex pointed to by the iterator or return None."},
{"next", reinterpret_cast<PyCFunction>(PyVerticesIteratorNext), METH_NOARGS,
@@ -126,6 +141,8 @@ PyObject *PyEdgesIteratorNext(PyEdgesIterator *self,
}
static PyMethodDef PyEdgesIteratorMethods[] = {
+ {"__reduce__", reinterpret_cast<PyCFunction>(DisallowPickleAndCopy),
+ METH_NOARGS, "__reduce__ is not supported"},
{"get", reinterpret_cast<PyCFunction>(PyEdgesIteratorGet), METH_NOARGS,
"Get the current edge pointed to by the iterator or return None."},
{"next", reinterpret_cast<PyCFunction>(PyEdgesIteratorNext), METH_NOARGS,
@@ -189,6 +206,8 @@ PyObject *PyGraphIterVertices(PyGraph *self, PyObject *Py_UNUSED(ignored)) {
}
static PyMethodDef PyGraphMethods[] = {
+ {"__reduce__", reinterpret_cast<PyCFunction>(DisallowPickleAndCopy),
+ METH_NOARGS, "__reduce__ is not supported"},
{"is_valid", reinterpret_cast<PyCFunction>(PyGraphIsValid), METH_NOARGS,
"Return True if Graph is in valid context and may be used."},
{"get_vertex_by_id", reinterpret_cast<PyCFunction>(PyGraphGetVertexById),
@@ -337,6 +356,8 @@ PyObject *PyQueryProcAddDeprecatedResult(PyQueryProc *self, PyObject *args) {
}
static PyMethodDef PyQueryProcMethods[] = {
+ {"__reduce__", reinterpret_cast<PyCFunction>(DisallowPickleAndCopy),
+ METH_NOARGS, "__reduce__ is not supported"},
{"add_arg", reinterpret_cast<PyCFunction>(PyQueryProcAddArg), METH_VARARGS,
"Add a required argument to a procedure."},
{"add_opt_arg", reinterpret_cast<PyCFunction>(PyQueryProcAddOptArg),
@@ -399,6 +420,8 @@ PyObject *PyQueryModuleAddReadProcedure(PyQueryModule *self, PyObject *cb) {
}
static PyMethodDef PyQueryModuleMethods[] = {
+ {"__reduce__", reinterpret_cast<PyCFunction>(DisallowPickleAndCopy),
+ METH_NOARGS, "__reduce__ is not supported"},
{"add_read_procedure",
reinterpret_cast<PyCFunction>(PyQueryModuleAddReadProcedure), METH_O,
"Register a read-only procedure with this module."},
@@ -566,10 +589,12 @@ void PyEdgeDealloc(PyEdge *self) {
}
PyObject *PyEdgeIsValid(PyEdge *self, PyObject *Py_UNUSED(ignored)) {
- return PyBool_FromLong(!!self->py_graph->graph);
+ return PyBool_FromLong(self->py_graph && self->py_graph->graph);
}
static PyMethodDef PyEdgeMethods[] = {
+ {"__reduce__", reinterpret_cast<PyCFunction>(DisallowPickleAndCopy),
+ METH_NOARGS, "__reduce__ is not supported"},
{"is_valid", reinterpret_cast<PyCFunction>(PyEdgeIsValid), METH_NOARGS,
"Return True if Edge is in valid context and may be used."},
{"get_type_name", reinterpret_cast<PyCFunction>(PyEdgeGetTypeName),
@@ -578,7 +603,8 @@ static PyMethodDef PyEdgeMethods[] = {
METH_NOARGS, "Return the edge's source vertex."},
{"to_vertex", reinterpret_cast<PyCFunction>(PyEdgeToVertex), METH_NOARGS,
"Return the edge's destination vertex."},
- {nullptr}};
+ {nullptr},
+};
PyObject *PyEdgeRichCompare(PyObject *self, PyObject *other, int op);
@@ -652,7 +678,7 @@ void PyVertexDealloc(PyVertex *self) {
}
PyObject *PyVertexIsValid(PyVertex *self, PyObject *Py_UNUSED(ignored)) {
- return PyBool_FromLong(!!self->py_graph->graph);
+ return PyBool_FromLong(self->py_graph && self->py_graph->graph);
}
PyObject *PyVertexGetId(PyVertex *self, PyObject *Py_UNUSED(ignored)) {
@@ -742,6 +768,8 @@ PyObject *PyVertexIterOutEdges(PyVertex *self, PyObject *Py_UNUSED(ignored)) {
}
static PyMethodDef PyVertexMethods[] = {
+ {"__reduce__", reinterpret_cast<PyCFunction>(DisallowPickleAndCopy),
+ METH_NOARGS, "__reduce__ is not supported"},
{"is_valid", reinterpret_cast<PyCFunction>(PyVertexIsValid), METH_NOARGS,
"Return True if Vertex is in valid context and may be used."},
{"get_id", reinterpret_cast<PyCFunction>(PyVertexGetId), METH_NOARGS,
@@ -754,7 +782,8 @@ static PyMethodDef PyVertexMethods[] = {
METH_NOARGS, "Return _mgp.EdgesIterator for in edges"},
{"iter_out_edges", reinterpret_cast<PyCFunction>(PyVertexIterOutEdges),
METH_NOARGS, "Return _mgp.EdgesIterator for out edges"},
- {nullptr}};
+ {nullptr},
+};
PyObject *PyVertexRichCompare(PyObject *self, PyObject *other, int op);
@@ -805,6 +834,170 @@ PyObject *PyVertexRichCompare(PyObject *self, PyObject *other, int op) {
return PyBool_FromLong(mgp_vertex_equal(v1->vertex, v2->vertex));
}
+struct PyPath {
+ PyObject_HEAD
+ mgp_path *path;
+ PyGraph *py_graph;
+};
+
+void PyPathDealloc(PyPath *self) {
+ CHECK(self->path);
+ CHECK(self->py_graph);
+ // Avoid invoking `mgp_path_destroy` if we are not in valid execution
+ // context. The query execution should free all memory used during
+ // execution, so we may cause a double free issue.
+ if (self->py_graph->graph) mgp_path_destroy(self->path);
+ Py_DECREF(self->py_graph);
+ Py_TYPE(self)->tp_free(self);
+}
+
+PyObject *PyPathIsValid(PyPath *self, PyObject *Py_UNUSED(ignored)) {
+ return PyBool_FromLong(self->py_graph && self->py_graph->graph);
+}
+
+PyObject *PyPathMakeWithStart(PyTypeObject *type, PyObject *vertex);
+
+PyObject *PyPathExpand(PyPath *self, PyObject *edge) {
+ CHECK(self->path);
+ CHECK(self->py_graph);
+ CHECK(self->py_graph->graph);
+ if (Py_TYPE(edge) != &PyEdgeType) {
+ PyErr_SetString(PyExc_TypeError, "Expected a _mgp.Edge.");
+ return nullptr;
+ }
+ auto *py_edge = reinterpret_cast<PyEdge *>(edge);
+ const auto *to = mgp_edge_get_to(py_edge->edge);
+ const auto *from = mgp_edge_get_from(py_edge->edge);
+ const auto *last_vertex =
+ mgp_path_vertex_at(self->path, mgp_path_size(self->path));
+ if (!mgp_vertex_equal(last_vertex, to) &&
+ !mgp_vertex_equal(last_vertex, from)) {
+ PyErr_SetString(PyExc_ValueError,
+ "Edge is not a continuation of the path.");
+ return nullptr;
+ }
+ if (!mgp_path_expand(self->path, py_edge->edge)) {
+ PyErr_SetString(PyExc_MemoryError, "Unable to expand mgp_path.");
+ return nullptr;
+ }
+ Py_RETURN_NONE;
+}
+
+PyObject *PyPathSize(PyPath *self, PyObject *Py_UNUSED(ignored)) {
+ CHECK(self->path);
+ CHECK(self->py_graph);
+ CHECK(self->py_graph->graph);
+ return PyLong_FromSize_t(mgp_path_size(self->path));
+}
+
+PyObject *PyPathVertexAt(PyPath *self, PyObject *args) {
+ CHECK(self->path);
+ CHECK(self->py_graph);
+ CHECK(self->py_graph->graph);
+ static_assert(std::numeric_limits<Py_ssize_t>::max() <=
+ std::numeric_limits<size_t>::max());
+ Py_ssize_t i;
+ if (!PyArg_ParseTuple(args, "n", &i)) return nullptr;
+ const auto *vertex = mgp_path_vertex_at(self->path, i);
+ if (!vertex) {
+ PyErr_SetString(PyExc_IndexError, "Index is out of range.");
+ return nullptr;
+ }
+ return MakePyVertex(*vertex, self->py_graph);
+}
+
+PyObject *PyPathEdgeAt(PyPath *self, PyObject *args) {
+ CHECK(self->path);
+ CHECK(self->py_graph);
+ CHECK(self->py_graph->graph);
+ static_assert(std::numeric_limits<Py_ssize_t>::max() <=
+ std::numeric_limits<size_t>::max());
+ Py_ssize_t i;
+ if (!PyArg_ParseTuple(args, "n", &i)) return nullptr;
+ const auto *edge = mgp_path_edge_at(self->path, i);
+ if (!edge) {
+ PyErr_SetString(PyExc_IndexError, "Index is out of range.");
+ return nullptr;
+ }
+ return MakePyEdge(*edge, self->py_graph);
+}
+
+static PyMethodDef PyPathMethods[] = {
+ {"__reduce__", reinterpret_cast<PyCFunction>(DisallowPickleAndCopy),
+ METH_NOARGS, "__reduce__ is not supported"},
+ {"is_valid", reinterpret_cast<PyCFunction>(PyPathIsValid), METH_NOARGS,
+ "Return True if Path is in valid context and may be used."},
+ {"make_with_start", reinterpret_cast<PyCFunction>(PyPathMakeWithStart),
+ METH_O | METH_CLASS, "Create a path with a starting vertex."},
+ {"expand", reinterpret_cast<PyCFunction>(PyPathExpand), METH_O,
+ "Append an edge continuing from the last vertex on the path."},
+ {"size", reinterpret_cast<PyCFunction>(PyPathSize), METH_NOARGS,
+ "Return the number of edges in a mgp_path."},
+ {"vertex_at", reinterpret_cast<PyCFunction>(PyPathVertexAt), METH_VARARGS,
+ "Return the vertex from a path at given index."},
+ {"edge_at", reinterpret_cast<PyCFunction>(PyPathEdgeAt), METH_VARARGS,
+ "Return the edge from a path at given index."},
+ {nullptr},
+};
+
+static PyTypeObject PyPathType = {
+ PyVarObject_HEAD_INIT(nullptr, 0).tp_name = "_mgp.Path",
+ .tp_doc = "Wraps struct mgp_path.",
+ .tp_basicsize = sizeof(PyPath),
+ .tp_flags = Py_TPFLAGS_DEFAULT,
+ .tp_new = PyType_GenericNew,
+ .tp_methods = PyPathMethods,
+ .tp_dealloc = reinterpret_cast<destructor>(PyPathDealloc),
+};
+
+PyObject *MakePyPath(mgp_path *path, PyGraph *py_graph) {
+ CHECK(path);
+ CHECK(py_graph->graph && py_graph->memory);
+ CHECK(path->GetMemoryResource() == py_graph->memory->impl);
+ auto *py_path = PyObject_New(PyPath, &PyPathType);
+ if (!py_path) return nullptr;
+ py_path->path = path;
+ py_path->py_graph = py_graph;
+ Py_INCREF(py_graph);
+ return PyObject_Init(reinterpret_cast<PyObject *>(py_path), &PyPathType);
+}
+
+PyObject *MakePyPath(const mgp_path &path, PyGraph *py_graph) {
+ CHECK(py_graph);
+ CHECK(py_graph->graph && py_graph->memory);
+ auto *path_copy = mgp_path_copy(&path, py_graph->memory);
+ if (!path_copy) {
+ PyErr_SetString(PyExc_MemoryError, "Unable to allocate mgp_path.");
+ return nullptr;
+ }
+ auto *py_path = MakePyPath(path_copy, py_graph);
+ if (!py_path) mgp_path_destroy(path_copy);
+ return py_path;
+}
+
+PyObject *PyPathMakeWithStart(PyTypeObject *type, PyObject *vertex) {
+ if (type != &PyPathType) {
+ PyErr_SetString(PyExc_TypeError,
+ "Expected '<class _mgp.Path>' as the first argument.");
+ return nullptr;
+ }
+ if (Py_TYPE(vertex) != &PyVertexType) {
+ PyErr_SetString(PyExc_TypeError,
+ "Expected a '_mgp.Vertex' as the second argument.");
+ return nullptr;
+ }
+ auto *py_vertex = reinterpret_cast<PyVertex *>(vertex);
+ auto *path =
+ mgp_path_make_with_start(py_vertex->vertex, py_vertex->py_graph->memory);
+ if (!path) {
+ PyErr_SetString(PyExc_MemoryError, "Unable to allocate mgp_path.");
+ return nullptr;
+ }
+ auto *py_path = MakePyPath(path, py_vertex->py_graph);
+ if (!py_path) mgp_path_destroy(path);
+ return py_path;
+}
+
PyObject *PyInitMgpModule() {
PyObject *mgp = PyModule_Create(&PyMgpModule);
if (!mgp) return nullptr;
@@ -829,6 +1022,7 @@ PyObject *PyInitMgpModule() {
if (!register_type(&PyQueryProcType, "Proc")) return nullptr;
if (!register_type(&PyQueryModuleType, "Module")) return nullptr;
if (!register_type(&PyVertexType, "Vertex")) return nullptr;
+ if (!register_type(&PyPathType, "Path")) return nullptr;
if (!register_type(&PyCypherTypeType, "Type")) return nullptr;
Py_INCREF(Py_None);
if (PyModule_AddObject(mgp, "_MODULE", Py_None) < 0) {
@@ -874,6 +1068,14 @@ py::Object ReloadPyModule(PyObject *py_module, mgp_module *module_def) {
});
}
+py::Object MgpValueToPyObject(const mgp_value &value, PyObject *py_graph) {
+ if (Py_TYPE(py_graph) != &PyGraphType) {
+ PyErr_SetString(PyExc_TypeError, "Expected a _mgp.Graph.");
+ return nullptr;
+ }
+ return MgpValueToPyObject(value, reinterpret_cast<PyGraph *>(py_graph));
+}
+
py::Object MgpValueToPyObject(const mgp_value &value, PyGraph *py_graph) {
switch (mgp_value_get_type(&value)) {
case MGP_VALUE_TYPE_NULL:
@@ -919,8 +1121,10 @@ py::Object MgpValueToPyObject(const mgp_value &value, PyGraph *py_graph) {
const auto *e = mgp_value_get_edge(&value);
return py::Object(reinterpret_cast<PyObject *>(MakePyEdge(*e, py_graph)));
}
- case MGP_VALUE_TYPE_PATH:
- throw utils::NotYetImplemented("MgpValueToPyObject");
+ case MGP_VALUE_TYPE_PATH: {
+ const auto *p = mgp_value_get_path(&value);
+ return py::Object(reinterpret_cast<PyObject *>(MakePyPath(*p, py_graph)));
+ }
}
}
@@ -1037,10 +1241,20 @@ mgp_value *PyObjectToMgpValue(PyObject *o, mgp_memory *memory) {
mgp_edge_destroy(e);
throw std::bad_alloc();
}
- } else {
- // TODO: Check for Vertex and Path. Throw std::invalid_argument for
- // everything else.
+ } else if (Py_TYPE(o) == &PyPathType) {
+ auto *p = mgp_path_copy(reinterpret_cast<PyPath *>(o)->path, memory);
+ if (!p) {
+ throw std::bad_alloc();
+ }
+ mgp_v = mgp_value_make_path(p);
+ if (!mgp_v) {
+ mgp_path_destroy(p);
+ throw std::bad_alloc();
+ }
+ } else if (Py_TYPE(o) == &PyVertexType) {
throw utils::NotYetImplemented("PyObjectToMgpValue");
+ } else {
+ throw std::invalid_argument("Unsupported PyObject conversion");
}
if (!mgp_v) {
diff --git a/src/query/procedure/py_module.hpp b/src/query/procedure/py_module.hpp
index ff67b4f20..bd9e53130 100644
--- a/src/query/procedure/py_module.hpp
+++ b/src/query/procedure/py_module.hpp
@@ -20,6 +20,8 @@ struct PyGraph;
/// `py::Object` instance and set the appropriate Python exception.
py::Object MgpValueToPyObject(const mgp_value &value, PyGraph *py_graph);
+py::Object MgpValueToPyObject(const mgp_value &value, PyObject *py_graph);
+
/// Convert a Python object into `mgp_value`, constructing it using the given
/// `mgp_memory` allocator.
///
diff --git a/tests/unit/query_procedure_py_module.cpp b/tests/unit/query_procedure_py_module.cpp
index b49378300..0d933aac9 100644
--- a/tests/unit/query_procedure_py_module.cpp
+++ b/tests/unit/query_procedure_py_module.cpp
@@ -57,7 +57,78 @@ TEST(PyModule, MgpValueToPyObject) {
ASSERT_TRUE(PyUnicode_Check(py_str));
EXPECT_EQ(std::string(PyUnicode_AsUTF8(py_str)), "some text");
}
- // TODO: Vertex, Edge and Path values
+}
+
+// Our _mgp types should not support (by default) pickling and copying.
+static void AssertPickleAndCopyAreNotSupported(PyObject *py_obj) {
+ py::Object pickle_mod(PyImport_ImportModule("pickle"));
+ ASSERT_TRUE(pickle_mod);
+ ASSERT_FALSE(py::FetchError());
+ py::Object dumps_res(pickle_mod.CallMethod("dumps", py_obj));
+ ASSERT_FALSE(dumps_res);
+ ASSERT_TRUE(py::FetchError());
+ py::Object copy_mod(PyImport_ImportModule("copy"));
+ ASSERT_TRUE(pickle_mod);
+ ASSERT_FALSE(py::FetchError());
+ py::Object copy_res(pickle_mod.CallMethod("copy", py_obj));
+ ASSERT_FALSE(copy_res);
+ ASSERT_TRUE(py::FetchError());
+ // We should have cleared the error state.
+ ASSERT_FALSE(py::FetchError());
+ py::Object deepcopy_res(pickle_mod.CallMethod("deepcopy", py_obj));
+ ASSERT_FALSE(deepcopy_res);
+ ASSERT_TRUE(py::FetchError());
+}
+
+// TODO: Test Vertex and Edge values
+TEST(PyModule, PyPath) {
+ storage::Storage db;
+ {
+ auto dba = db.Access();
+ auto v1 = dba.CreateVertex();
+ auto v2 = dba.CreateVertex();
+ ASSERT_TRUE(
+ dba.CreateEdge(&v1, &v2, dba.NameToEdgeType("type")).HasValue());
+ ASSERT_FALSE(dba.Commit().HasError());
+ }
+ auto storage_dba = db.Access();
+ query::DbAccessor dba(&storage_dba);
+ mgp_memory memory{utils::NewDeleteResource()};
+ mgp_graph graph{&dba, storage::View::OLD};
+ auto *start_v = mgp_graph_get_vertex_by_id(&graph, mgp_vertex_id{0}, &memory);
+ ASSERT_TRUE(start_v);
+ auto *path = mgp_path_make_with_start(start_v, &memory);
+ ASSERT_TRUE(path);
+ auto *edges_it = mgp_vertex_iter_out_edges(start_v, &memory);
+ ASSERT_TRUE(edges_it);
+ for (const auto *edge = mgp_edges_iterator_get(edges_it); edge;
+ edge = mgp_edges_iterator_next(edges_it)) {
+ ASSERT_TRUE(mgp_path_expand(path, edge));
+ }
+ ASSERT_EQ(mgp_path_size(path), 1);
+ mgp_edges_iterator_destroy(edges_it);
+ mgp_vertex_destroy(start_v);
+ auto *path_value = mgp_value_make_path(path);
+ ASSERT_TRUE(path_value);
+ auto gil = py::EnsureGIL();
+ py::Object py_graph(query::procedure::MakePyGraph(&graph, &memory));
+ ASSERT_TRUE(py_graph);
+ // We have setup the C structs, so create convert to PyObject.
+ py::Object py_path_value(
+ query::procedure::MgpValueToPyObject(*path_value, py_graph));
+ ASSERT_TRUE(py_path_value);
+ AssertPickleAndCopyAreNotSupported(py_path_value);
+ // Convert back to C struct and check equality.
+ auto *new_path_value =
+ query::procedure::PyObjectToMgpValue(py_path_value, &memory);
+ ASSERT_TRUE(new_path_value);
+ ASSERT_NE(new_path_value, path_value); // Pointer compare.
+ ASSERT_TRUE(mgp_value_is_path(new_path_value));
+ ASSERT_TRUE(mgp_path_equal(mgp_value_get_path(path_value),
+ mgp_value_get_path(new_path_value)));
+ mgp_value_destroy(new_path_value);
+ mgp_value_destroy(path_value);
+ ASSERT_FALSE(dba.Commit().HasError());
}
TEST(PyModule, PyObjectToMgpValue) {