Ah, yea, I see what you mean. It's actually replaceStep() which is buggy,
not the strategy. The fix is easy, we just need to change the order of the
2 statements in replaceStep():
public static <S, E> void replaceStep(final Step<S, E> removeStep,
final Step<S, E> insertStep, final Traversal.Admin<?, ?> traversal) {
final int i; traversal.removeStep(i = stepIndex(removeStep,
traversal)); traversal.addStep(i, insertStep);}
Cheers,
Daniel
On Mon, Oct 30, 2017 at 7:43 AM, pieter gmail <pieter.mar...@gmail.com>
wrote:
> 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').it
>> erate().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('nam
>> e').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').it
>> erate().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('nam
>> e').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
>>>
>>>
>
