[ 
https://issues.apache.org/jira/browse/TINKERPOP-2854?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17679118#comment-17679118
 ] 

Stephen Mallette commented on TINKERPOP-2854:
---------------------------------------------

As best I understand that cypher-for-gremlin library, it can take cypher and 
produce either a Gremlin string or Gremlin bytecode. The Gremlin string it 
produces is fine, but the bytecode it produces adds steps you can't account 
for. 

>From what I can see, you aren't really getting extra steps, but you are 
>getting a change in {{constant()}} value. The {{ChooseStep([ConstantStep(3)}} 
>is flipping to  {{ChooseStep([ConstantStep(OUT)}} which is really like calling 
>{{constant(Direction.OUT)}}. I can't think of what code in TinkerPop would 
>cause that.

Please correct me if I'm not following the problem.

> Why does it add Constant(IN) and OUT in the CHOOSE step in 3.6.1?
> can you at least provide some guidance about constructing a test case into 
> Gremlin to reproduce the situation in Gremlin and debug it by myself?

I have no idea how cypher-for-gremlin constructs bytecode, but they could do it 
in one of several fashions that I can think of:

1. They could do what sparql-gremlin does and directly constructs a 
[GraphTraversal|https://github.com/apache/tinkerpop/blob/3.6.1/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.java]
 from a  
[GraphTraversalSource|https://github.com/apache/tinkerpop/blob/3.6.1/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversalSource.java].
 Once the {{GraphTraversal}} is fully constructed from the translation they 
would call 
[getBytecode()|https://github.com/apache/tinkerpop/blob/3.6.1/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/DefaultTraversal.java#L103-L105]
 and return that. 
2. They could manually construct that 
[Bytecode|https://github.com/apache/tinkerpop/blob/3.6.1/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Bytecode.java]
 object as they translate. 
3. They could have taken a really indirect and unlikely approach and took the 
{{String}} Gremlin they constructed, passed that through our ANTLR grammar or 
maybe the {{GremlinGroovyScriptEngine}} to produce a {{GraphTraversal}} object 
and then got the {{Bytecode}} from that. 
4. There is a runaway [TraversalStrategy 
|https://tinkerpop.apache.org/docs/current/reference/#traversalstrategy]. I 
can't think of any in TinkerPop that would do this. Maybe cypher-for-gremlin 
has some installed that are not transforming something properly? 

I can't think of any other ways to create a {{Bytecode}} object but maybe they 
are more creative that me. I think 4 was pretty creative :)

Of those possibilities I enumerated, I think (3) is the most likely way this 
could possibly be TinkerPop's issue because they would be using our libraries 
to do the {{Bytecode}} construction from a {{String}}. That said, I believe you 
stated that the {{String}} created by cypher-for-gremlin works fine, which 
means it passes through the same infrastructure I mentioned in (3) to success, 
so I'd say that one is somewhat unlikely.  

For (1) I'd say you'd have a problem in cypher-for-gremlin. You will note in 
{{GraphTraversal}} and {{GraphTraversalSource}} how calls to each of the 
Gremlin steps like {{out()}} and {{in()}} adds that step to the {{Bytecode}} 
object to create it. I'd say that's tested quite rigorously and there shouldn't 
be any calls that are specifically adding something like like {{Constant(IN)}} 
inadvertently. Something would have to call {{constant(Direction.IN)}} for that 
to happen.

For (2) I'd say you'd also have a problem in cypher-for-gremlin. This one seems 
more likely that something could be messed up in their creation of {{Bytecode} 
in the sense that you can add whatever you want to the {{Bytecode}} object 
(even mistakes). This approach seems like the method most prone to error and 
breaks on upgrades like this one.

For (4), I can't say I can think of any of our {{TraversalStrategy}} 
implementations that would change {{constant(3)}} to 
{{constant(Direction.OUT}}. You could probably rule this one out pretty quickly 
by seeing if cypher-for-gremlin has an {{TraversalStrategy}} implementations.

> If you don't want to dive into this, 

We didn't mean to sound dismissive of your problem. fwiw, I did glance at the 
cypher-to-gremlin library but it is more complex than i have time to dig into 
unfortunately. i'm further hampered by not knowing scala well which they seem 
to use for the core of translation. I hope my explanation of how i view the 
problem from the TinkerPop perspective was helpful. I think for us to help 
you'd have to help us understand how they construct the {{Bytecode}} object so 
that we know what part of our libraries we should be looking at for a problem.

As an aside, I'd recommend passing your {{Bytecode}} objects through the 
[GraphSONMapper|https://github.com/apache/tinkerpop/blob/3.6.1/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONMapper.java]
 to get a nice clean JSON representation of the {{Bytecode}}. The 
{{toString()}} representation is really hard to read and often insufficient for 
debugging. If you care to share that JSON here for each of the versions, it 
might be helpful.

> Generated Traversal differs from v3.4.13 to v3.6.1
> --------------------------------------------------
>
>                 Key: TINKERPOP-2854
>                 URL: https://issues.apache.org/jira/browse/TINKERPOP-2854
>             Project: TinkerPop
>          Issue Type: Bug
>    Affects Versions: 3.6.1
>            Reporter: Andrea Santurbano
>            Priority: Major
>
> Hi everybody,
> I'm playing with Gremlin and OpenCypher query transpiler by using the 
> following bytecode query:
> {code:java}
> [[], [inject(  cypher.start), map([[], [project(  GENERATED1,   GENERATED2,   
> GENERATED3), by([[], [constant(1)]]), by([[], [constant(2)]]), by([[], 
> [constant(3)]]), select(values)]]), project(r), by([[], [project(  
> GENERATED5,   GENERATED6,   GENERATED7), by([[], [identity()]]), by([[], 
> [choose([[], [constant(binding[from=3])]], [[], [constant(binding[from=3])]], 
> [[], [constant(  cypher.null)]])]]), by([[], [choose([[], 
> [constant(binding[to=1])]], [[], [constant(binding[to=1])]], [[], [constant(  
> cypher.null)]])]]), select(values), 
> map(lambda[cypherListSlice().apply(it)])]])]]{code}
> Using Gremlin 3.4.13 produces the following traversal:
>  
>  
> {code:java}
> [InjectStep([  cypher.start]), TraversalMapStep([ProjectStep([  GENERATED1,   
> GENERATED2,   GENERATED3],[[ConstantStep(1)], [ConstantStep(2)], 
> [ConstantStep(3)]]), TraversalMapStep(values)]), 
> ProjectStep([r],[[ProjectStep([  GENERATED5,   GENERATED6,   
> GENERATED7],[[IdentityStep], [ChooseStep([ConstantStep(3), 
> HasNextStep],[[(eq(true)), [ConstantStep(3), EndStep]], [(eq(false)), 
> [ConstantStep(  cypher.null), EndStep]]])], [ChooseStep([ConstantStep(1), 
> HasNextStep],[[(eq(true)), [ConstantStep(1), EndStep]], [(eq(false)), 
> [ConstantStep(  cypher.null), EndStep]]])]]), TraversalMapStep(values), 
> LambdaMapStep(lambda)]])]{code}
> while 3.6.1 creates this one (we only upgraded the library and changed some 
> package name references):
>  
>  
> {code:java}
> [InjectStep([  cypher.start]), TraversalMapStep([ProjectStep([  GENERATED1,   
> GENERATED2,   GENERATED3],[[ConstantStep(1)], [ConstantStep(2)], 
> [ConstantStep(3)]]), TraversalMapStep(values)]), 
> ProjectStep([r],[[ProjectStep([  GENERATED5,   GENERATED6,   
> GENERATED7],[[IdentityStep], [ChooseStep([ConstantStep(OUT), 
> HasNextStep],[[(eq(true)), [ConstantStep(OUT), EndStep]], [(eq(false)), 
> [ConstantStep(  cypher.null), EndStep]]])], [ChooseStep([ConstantStep(IN), 
> HasNextStep],[[(eq(true)), [ConstantStep(IN), EndStep]], [(eq(false)), 
> [ConstantStep(  cypher.null), EndStep]]])]]), TraversalMapStep(values), 
> LambdaMapStep(lambda)]])]{code}
> Indeed the one in 3.6.1 is the wrong one because it adds some extra steps 
> like {*}ConstantStep(IN){*}.
>  
>  
>  



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to