pietermartin / sqlg

TinkerPop graph over sql
MIT License
246 stars 51 forks source link

Incorrect results from traversal when long IDs happen to be the same #174

Closed ktschmidt closed 7 years ago

ktschmidt commented 7 years ago

I just ran into what appears to be a strange bug. The scenario is as follows.

I create three different labeled vertices, and being the first created they all get a long ID of 1 within their vertex type:

gremlin> a = graph.addVertex(T.label, 'A') ==>v[public.A:::1] gremlin> b = graph.addVertex(T.label, 'B') ==>v[public.B:::1] gremlin> c = graph.addVertex(T.label, 'C') ==>v[public.C:::1]

I then create an edge between two of them:

gremlin> a.addEdge('related', b)

And doing the following traversal results in the expected response:

gremlin> g.V(a.id()).outE('related').as('e').inV().hasId(b.id()).select('e') ==>e[public.related:::1][public.A:::1-related->public.B:::1]

But oddly, changing the hasId to the third vertex still returns the same thing:

gremlin> g.V(a.id()).outE('related').as('e').inV().hasId(c.id()).select('e') ==>e[public.related:::1][public.A:::1-related->public.B:::1]

Doing the same with a TinkerGraph works as expected, the last traversal doesn't return anything. But of course, the vertex IDs are unique.

If I create another B vertex that gets a long ID of 2, then traversing with it does not return anything as expected..

So it appears the hasId is not using the schema/table in its check and instead just the long ID which results in incorrect traversals if the long IDs are "just right".

pietermartin commented 7 years ago

I'll have a look, there is specific hasId logic which probably drops the label somehow?

ktschmidt commented 7 years ago

Yeah, that is my guess. I started looking around the code and debugging my scenario and it did appear that only the getId method on RecordId was called in some cases and it didn't get the schema table.

But looking at the generated SQL, it isn't just the ID but whatever logic is used to determine the tables to join as the SQL is only joining with the label B table even when I specify c.id() in the hasId.

On Mon, Mar 20, 2017 at 11:25 AM, pietermartin notifications@github.com wrote:

I'll have a look, there is specific hasId logic which probably drops the label somehow?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/pietermartin/sqlg/issues/174#issuecomment-287853452, or mute the thread https://github.com/notifications/unsubscribe-auth/ABHUDhUbOQ6mwgSdOnyPeHCnSi-3o6t7ks5rnsSkgaJpZM4MizYh .

JPMoresmau commented 7 years ago

We already do some checks but not sufficiently. I've added a test case and managed to fix it, I just need to verify I haven't broken anything.

ktschmidt commented 7 years ago

Thanks. Happy to test things out whenever ready.

On Sat, Mar 25, 2017 at 9:07 AM, JP Moresmau notifications@github.com wrote:

We already do some checks but not sufficiently. I've added a test case and managed to fix it, I just need to verify I haven't broken anything.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/pietermartin/sqlg/issues/174#issuecomment-289221242, or mute the thread https://github.com/notifications/unsubscribe-auth/ABHUDhBnG-J8PbR-t5IweQnfwPM8faEGks5rpTvJgaJpZM4MizYh .