I am on the latest 3.3.1-SNAPSHOT, just pulled master again.

The actual traversals and results are correct. However after the CountStrategy the VertexStep(OUT,edge) previousStep pointer (AbstractStep.previousStep) is incorrect. I should be EmptyStep but infact points to the newly inserted NotStep.

toString() on a traversal does not show the previous/nextStep so you can not see what I am talking about in the console. This is not breaking anything in TinkerGraph but breaks stuff in Sqlg.

In CountStrategy if I change line 153 to then my problems go away and TinkerGraph also works as expected.

//TraversalHelper.replaceStep(prev, new NotStep<>(traversal, inner), traversal);
NotStep notStep = new NotStep<>(traversal, inner);
TraversalHelper.replaceStep(prev, notStep, traversal);
List<Traversal.Admin<?, ?>> notStepTraversal = notStep.getLocalChildren();
Traversal.Admin<?, ?> notStepTraversalFirstStep = notStepTraversal.get(0);
//The first step is pointing to the NotStep, it should point to an EmptyStep
notStepTraversalFirstStep.getSteps().get(0).setPreviousStep(EmptyStep.instance());

So I suppose the question is, is it correct for the previousStep of the first step of a local traversal to point to the parent step and not an EmptyStep?

The TraversalHelper.replaceStep always makes the first step of the traversal point to the newly inserted step. If the traversal however is a local traversal then the root step should be an EmptyStep.

Hope it makes some sense.

Thanks
Pieter



On 30/10/2017 15:28, Daniel Kuppitz wrote:
I don't see any issues. Which version are you talking about?

*gremlin> Gremlin.version()*
*==>3.2.7-SNAPSHOT*
gremlin> g = TinkerFactory.createModern().traversal()
==>graphtraversalsource[tinkergraph[vertices:6 edges:6], standard]
gremlin> g.V(1).repeat(out()).until(__.not(outE())).values('name')
==>lop
==>vadas
==>ripple
==>lop
gremlin> g.V(1).repeat(out()).until(outE().count().is(0)).values('name')
==>lop
==>vadas
==>ripple
==>lop
gremlin>
g.V(1).repeat(out()).until(__.not(outE())).values('name').iterate().toString()
==>[TinkerGraphStep(vertex,[1]), RepeatStep([VertexStep(OUT,vertex),
RepeatEndStep],until([NotStep([VertexStep(OUT,edge)])]),emit(false)),
PropertiesStep([name],value)]
gremlin>
g.V(1).repeat(out()).until(outE().count().is(0)).values('name').iterate().toString()
==>[TinkerGraphStep(vertex,[1]), RepeatStep([VertexStep(OUT,vertex),
RepeatEndStep],until([NotStep([VertexStep(OUT,edge)])]),emit(false)),
PropertiesStep([name],value)]


*gremlin> Gremlin.version()*
*==>3.3.1-SNAPSHOT*
gremlin> g = TinkerFactory.createModern().traversal()
==>graphtraversalsource[tinkergraph[vertices:6 edges:6], standard]
gremlin> g.V(1).repeat(out()).until(__.not(outE())).values('name')
==>lop
==>vadas
==>ripple
==>lop
gremlin> g.V(1).repeat(out()).until(outE().count().is(0)).values('name')
==>lop
==>vadas
==>ripple
==>lop
gremlin>
g.V(1).repeat(out()).until(__.not(outE())).values('name').iterate().toString()
==>[TinkerGraphStep(vertex,[1]), RepeatStep([VertexStep(OUT,vertex),
RepeatEndStep],until([NotStep([VertexStep(OUT,edge)])]),emit(false)),
PropertiesStep([name],value)]
gremlin>
g.V(1).repeat(out()).until(outE().count().is(0)).values('name').iterate().toString()
==>[TinkerGraphStep(vertex,[1]), RepeatStep([VertexStep(OUT,vertex),
RepeatEndStep],until([NotStep([VertexStep(OUT,edge)])]),emit(false)),
PropertiesStep([name],value)]


Cheers,
Daniel


On Mon, Oct 30, 2017 at 1:53 AM, pieter gmail <pieter.mar...@gmail.com>
wrote:

Hi,

Whilst optimizing the NotStep I came across what looks to me like a bug in
TraversalHelper.replaceStep or perhaps rather in CountStrategy.

The test below shows the bug.

     @Test
     public void g_VX1X_repeatXoutX_untilXoutE_count_isX0XX_name() {
         Graph graph = TinkerFactory.createModern();
         final Traversal<Vertex, String> traversal1 = graph.traversal()
                 .V(convertToVertexId(graph, "marko"))
                 .repeat(__.out())
                 .until(__.not(__.outE()))
                 .values("name");
         checkResults(Arrays.asList("lop", "lop", "ripple", "vadas"),
traversal1);

         List<VertexStep> vertexSteps = TraversalHelper.getStepsOfAssi
gnableClassRecursively(VertexStep.class, traversal1.asAdmin());
         VertexStep vertexStep = vertexSteps.stream().filter(s ->
s.returnsEdge()).findAny().get();
         Assert.assertEquals(EmptyStep.instance(),
vertexStep.getPreviousStep());

         final Traversal<Vertex, String> traversal2 = graph.traversal()
                 .V(convertToVertexId(graph, "marko"))
                 .repeat(__.out())
                 .until(__.outE().count().is(0))
                 .values("name");
         checkResults(Arrays.asList("lop", "lop", "ripple", "vadas"),
traversal2);

         vertexSteps = TraversalHelper.getStepsOfAssi
gnableClassRecursively(VertexStep.class, traversal2.asAdmin());
         vertexStep = vertexSteps.stream().filter(s ->
s.returnsEdge()).findAny().get();

         //This fails because the vertexStep's previous step is the
NotStepwhen it should be an EmptyStep.
         Assert.assertEquals(EmptyStep.instance(),
vertexStep.getPreviousStep());
     }

traversal1 and traversal2 should be the same as the CountStrategy will
replace the __.outE().count().is(0) with __.not(__.outE())
The CountStrategy does what its suppose to do however then it calls
TraversalHelper.replaceStep(prev, new NotStep<>(traversal, inner),
traversal); the traversal's VertexStep gets its previousStep set to the
NotStep. This is because of the way TraversalHelper.replaceStep is
implemented.

I am not sure whether the fix should be in replaceStep or rather in
CountStrategy.

Thanks
Pieter


Reply via email to