Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unexpected results when updating property #99

Open
mbbfyb opened this issue Feb 18, 2018 · 5 comments
Open

Unexpected results when updating property #99

mbbfyb opened this issue Feb 18, 2018 · 5 comments

Comments

@mbbfyb
Copy link

mbbfyb commented Feb 18, 2018

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
Copy link
Owner

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
Copy link
Author

mbbfyb commented Feb 18, 2018

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
Copy link

aolieman commented Mar 19, 2018

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
Copy link
Owner

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

@mbbfyb
Copy link
Author

mbbfyb commented Mar 21, 2018

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants