Re: CountStrategy and TraversalHelper.replaceStep

[pieter gmail]

Created a jira  
for this.

I tested your fix and all tests are passing again. Sqlg and TinkerGraph.

Thanks
Pieter

On 30/10/2017 18:01, Daniel Kuppitz wrote:

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  void replaceStep(final Step removeStep,
final Step 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 
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 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 
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 traversal1 = graph.traversal()
  .V(convertToVertexId(graph, "marko"))
  .repeat(__.out())
  .until(__.not(__.outE()))
  .values("name");
  checkResults(Arrays.asList("lop", "lop", "ripple", "vadas"),
traversal1);

  List vertexSteps = TraversalHelper.getStepsOfAssi
gnableClassRecursively(VertexStep.class, 

Re: CountStrategy and TraversalHelper.replaceStep

[pieter gmail]

Ok great, I'll test a bit with the order changed.

Thanks,
Pieter

On 30/10/2017 18:01, Daniel Kuppitz wrote:

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  void replaceStep(final Step removeStep,
final Step 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 
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 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 
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 traversal1 = graph.traversal()
  .V(convertToVertexId(graph, "marko"))
  .repeat(__.out())
  .until(__.not(__.outE()))
  .values("name");
  checkResults(Arrays.asList("lop", "lop", "ripple", "vadas"),
traversal1);

  List vertexSteps = TraversalHelper.getStepsOfAssi
gnableClassRecursively(VertexStep.class, traversal1.asAdmin());
  VertexStep vertexStep = vertexSteps.stream().filter(s ->

Re: CountStrategy and TraversalHelper.replaceStep

[Daniel Kuppitz]

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  void replaceStep(final Step removeStep,
final Step 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 
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 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 
>> 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 traversal1 = graph.traversal()
>>>  .V(convertToVertexId(graph, "marko"))
>>>  .repeat(__.out())
>>>  .until(__.not(__.outE()))
>>>  .values("name");
>>>  checkResults(Arrays.asList("lop", "lop", "ripple", "vadas"),
>>> traversal1);
>>>
>>>  

Re: CountStrategy and TraversalHelper.replaceStep

[pieter gmail]

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 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 
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 traversal1 = graph.traversal()
 .V(convertToVertexId(graph, "marko"))
 .repeat(__.out())
 .until(__.not(__.outE()))
 .values("name");
 checkResults(Arrays.asList("lop", "lop", "ripple", "vadas"),
traversal1);

 List 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 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 

Re: CountStrategy and TraversalHelper.replaceStep

[Daniel Kuppitz]

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 
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 traversal1 = graph.traversal()
> .V(convertToVertexId(graph, "marko"))
> .repeat(__.out())
> .until(__.not(__.outE()))
> .values("name");
> checkResults(Arrays.asList("lop", "lop", "ripple", "vadas"),
> traversal1);
>
> List 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 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
>

CountStrategy and TraversalHelper.replaceStep

[pieter gmail]

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 traversal1 = graph.traversal()
    .V(convertToVertexId(graph, "marko"))
    .repeat(__.out())
    .until(__.not(__.outE()))
    .values("name");
    checkResults(Arrays.asList("lop", "lop", "ripple", "vadas"), 
traversal1);


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


    final Traversal 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.getStepsOfAssignableClassRecursively(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