The-Don-Himself / gremlin-ogm

A PHP Object Graph Mapper for Tinkerpop 3+ compatible Graph Databases (JanusGraph, Neo4j, etc.) that allows you to persist data and run gremlin queries.
MIT License
17 stars 2 forks source link

Gremlin addVertex() vs. addV() #3

Closed juhasev closed 6 years ago

juhasev commented 6 years ago

This might not be a bug but rather some kind of configuration issue. When I execute the following traversal:

        $command = $traversalBuilder
            ->g()
            ->addV("'user'")
            ->property("'name'", "'Juha Vehnia'")
            ->property("'joined'", "'2018-04-10 10:10:00'")
            ->next()
            ->getTraversal();

You actually get command:

g.addVertex('user').property('name', 'Juha Vehnia').property('joined', '2018-04-10 10:10:00').next()

Which throws an error in JanusGraph

groovy.lang.MissingMethodException: No signature of method: org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource.addVertex()

However when I change g.addVertex() to g.addV() everything works as expected. Aren't these two equivalent or aliased steps which means both should work just fine?

The-Don-Himself commented 6 years ago

Yes its not a bug as they are different.

addVertex() operations are done on the graph object such as

            graph = TinkerGraph.open();
            graph.addVertex(T.label, "person", T.id, 1, "name", "marko", "age", 29);

Whereas addV() operations are done on the traversal object such as

            g=graph.traversal()
            g.addVertex('user').property('name', 'Juha Vehnia').property('joined', '2018-04-10 10:10:00')
The-Don-Himself commented 6 years ago

For further reference

http://docs.janusgraph.org/latest/schema.html http://docs.janusgraph.org/latest/gremlin.html

and

https://tinkerpop.apache.org/docs/current/reference/#addvertex-step

juhasev commented 6 years ago

Ok makes sense. So why would addV() magically turn into addVertex() though?

        $command = $traversalBuilder
            ->g()
            ->addV("'user'")
            ->property("'name'", "'Juha Vehnia'")
            ->next()
            ->getTraversal();

         echo $command;

And you get

g.addVertex('user').property('name', 'Juha Vehnia').property('joined', '2018-04-10 10:10:00').next()
The-Don-Himself commented 6 years ago

You know, I totally forgot I did that, while the meanings of addV() and addVertex() are different, the implementation is definitely off, my apologies for closing. I'll reopen :) I believe, at the time, I must have been tired of confusing the two so aliased it since almost always one will be using the traversal, let me look into it and remember the exact reasons I did it. Again a great spot @juhasev

The-Don-Himself commented 6 years ago

I have locate the error,

https://github.com/The-Don-Himself/gremlin-ogm/blob/0db8c65a80fb3d1768121dbb7935a6af25a0c641/src/Traversal/TraversalBuilder.php#L251

should be

$addV = new AddVStep($args);

just as it is with

https://github.com/The-Don-Himself/gremlin-ogm/blob/0db8c65a80fb3d1768121dbb7935a6af25a0c641/src/Traversal/TraversalBuilder.php#L159

and

https://github.com/The-Don-Himself/gremlin-ogm/blob/0db8c65a80fb3d1768121dbb7935a6af25a0c641/src/Traversal/TraversalBuilder.php#L176

.

A fix is on the way. Thanks for spotting it.

juhasev commented 6 years ago

Awesome! Currently there is no way to add new vertices using traversal which is what you probably do all the time.

The-Don-Himself commented 6 years ago

Actually there is, I almost always mutate the graph using the GraphSerializer Because it handles all the goodies for you, DateTime to Java's timestamp (i.e SdateTime->getTimestamp() * 1000), it handles complex GeoShape and more. For your specific example, try

$graph_serializer = new GraphSerializer();

$user_array = array(
  'name' => 'Juha Vehnia'
);

// if user fetched from DB or in object form
// $user_array = $graph_serializer->toArray($user);

$vertex_label = 'user';

$gremlin_command = $graph_serializer->toVertex(
    array(
        $vertex_label => $user_array
    )
);

The irony is that GraphSerializer uses addV() internally hence why I never spotted this. https://github.com/The-Don-Himself/gremlin-ogm/blob/0db8c65a80fb3d1768121dbb7935a6af25a0c641/src/Serializer/GraphSerializer.php#L495-L514

juhasev commented 6 years ago

Nice. I will check it out. You already merged the fix so closing this issue for now. Thanks for quick response!