davebshow / goblin

A Python 3.5 rewrite of the TinkerPop 3 OGM Goblin
Other
93 stars 21 forks source link

Unexpected results when updating property #99

Open mbbfyb opened 6 years ago

mbbfyb commented 6 years ago

I am receiving some unexpected results updating a property on a vertex, and wanted to make sure it was not user error.

Here is the output of my python shell

>>> import goblin
>>> import asyncio
>>> from goblin import Goblin
>>>
>>> class Person(goblin.Vertex):
...     car = goblin.Property(goblin.String)
...
>>> async def store_element(app, vertex1):
...     session = await app.session()
...     session.add(vertex1)
...     await session.flush()
...
>>> async def close(app):
...     await app.close()
...
>>> loop = asyncio.get_event_loop()
>>> app = loop.run_until_complete(Goblin.open(loop, hosts=["localhost"]))
>>> app.register(Person)
>>> p1 = Person()
>>> p1.car = "honda"
>>> loop.run_until_complete(store_element(app, p1))
>>> print(p1.car)
honda
>>> p1.car = "toyota"
>>> loop.run_until_complete(store_element(app, p1))
>>> print(p1.car)
['toyota', 'honda']
>>> loop.run_until_complete(close(app))
>>> loop.close()

Notice that instead of overwriting the element, it is appended like an array. I see this behavior using both the regular Property class and VertexProperty class, in which I would expect the behavior to just overwrite the field value. In the gremlin console the particular vertex ends up having 2 fields named car.

gremlin> g.V().has('car').properties()
==>vp[car->toyota]
==>vp[car->honda]

Is this to be expected? I am using Amazon Neptune with the following packages

aiogremlin==3.2.4
goblin==2.0.0
gremlinpython==3.3.1

My suspicion is that somewhere within this function there is something going wrong when doing either the drop iterator or the add iterator that is causing the issue.

davebshow commented 6 years ago

I expect this is due to the lack of meta/multi-card props in Neptune and/or the version of Goblin you are using. I am pretty sure this behaves as expected with TinkerGraph and JanusGraph with the newest Goblin.

I haven't had much time to work on these libraries lately and I don't have access to Neptune. I would love to get Goblin integration going for this provider but I may need some community support here...

mbbfyb commented 6 years ago

Sure. I will see if I can make any progress on it, and report back if I do. Was just confirming the desired behavior here before I started to mess with things. The goal of that function is to drop() all properties off of the vertex, and then add them back in? Instead of doing an inline update?

aolieman commented 6 years ago

This does not seem Neptune-specific as I've started getting this incorrect behavior after a DSE upgrade as well. I'm guessing it involves a bug in TinkerPop though, because the gremlin bytecode that is created by goblin/gremlinpython looks good, e.g. when updating properties of an existing vertex:

[
 ['addV', 'some_vertex_label'],
 ['property', binding[k0=canonical_name], binding[v0=new name]],
 ['property', binding[k1=names], binding[v1=third name]],
 ['property', <Cardinality.list_: 1>, binding[k2=names], binding[v2=fourth name]], 
 ['property', <Cardinality.list_: 1>, binding[k3=names], binding[v3=fifth name]]
]

This should let the addProperty step behave as described in the TP docs: http://tinkerpop.apache.org/docs/current/reference/#vertex-properties

The result however is "concat" behavior, rather than overwrite:

{'id': '0c5b6974-ac51-4b78-955c-859eec3b3544', 
 'canonical_name': 'new name', 
 'names': ['first name', 'second name', 'third name', 'fourth name', 'fifth name']
}

The DSE update I did leads me to believe that the addProperty behavior changed somewhere between TP 3.2.x and 3.2.7, but I am still investigating this.

@mbbfyb which version of TinkerPop is used by the Neptune version you are using for these tests?

davebshow commented 6 years ago

I will look into this with more detail when I get a spare moment.

mbbfyb commented 6 years ago

@aolieman According to the FAQs it looks like it supports Tinkerpop 3.3

Amazon Neptune’s Gremlin server will support clients that are compatible with Apache TinkerPop version 3.3 using both Websocket and REST connections.