[GitHub] tinkerpop issue #838: TINKERPOP-1822: Change default RepeatStep to DFS
Github user krlohnes commented on the issue: https://github.com/apache/tinkerpop/pull/838 @spmallette Yeah, I'm still giving this some thought. Just rebased. ---
[GitHub] tinkerpop issue #838: TINKERPOP-1822: Change default RepeatStep to DFS
Github user spmallette commented on the issue: https://github.com/apache/tinkerpop/pull/838 @krlohnes not sure if you're still thinking about this one, but if you are and you get a chance, could you please rebase your branch to freshen things up a bit? ---
[GitHub] tinkerpop pull request #838: TINKERPOP-1822: Change default RepeatStep to DF...
Github user spmallette commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/838#discussion_r201976894
--- Diff:
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/branch/RepeatStep.java
---
@@ -273,11 +300,40 @@ public RepeatEndStep(final Traversal.Admin traversal)
{
super(traversal);
}
+final LinkedList> stashedStarts = new
LinkedList<>();
--- End diff --
I had to remove `RepeatUnrollStrategy` because it adds barriers
occasionally as do other strategies - that really needs to be fixed as
something separate.
https://issues.apache.org/jira/browse/TINKERPOP-2004
I think that if you had a specific use case in mind when you started doing
this it would be cool if you did a performance test on that and shared your
results if possible.
I also think that the queries in my tests weren't really presenting
scenarios where someone would want to do DFS. I'm imaging that the only time
DFS will be used is when the user is knowledgeable and has advanced
understanding of their data to know that DFS will out perform the default. I
assume that your specific use case was falling into that scenario. As i said,
it would be nice to see that in action.
So, that said, it would be great to see DFS perform more quickly for that
case I presented for the JFRs, but that may not be an explicit requirement. It
may be more important to simply demonstrate that DFS has a use case where it
can shine.
I didn't study the JFRs for too long as the performance struck me as a
point of discussion first. If you have any thoughts to share on those
specifically, that would also be cool. Thanks again!
---
[GitHub] tinkerpop pull request #838: TINKERPOP-1822: Change default RepeatStep to DF...
Github user krlohnes commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/838#discussion_r201906049
--- Diff:
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/branch/RepeatStep.java
---
@@ -273,11 +300,40 @@ public RepeatEndStep(final Traversal.Admin traversal)
{
super(traversal);
}
+final LinkedList> stashedStarts = new
LinkedList<>();
--- End diff --
Looking at this a bit more, it looks like I misread some of the test query
results I had, and the new commit doesn't work to make the repeat step depth
first, so ignore that last comment. I'm still working on trying to figure out a
new approach.
---
[GitHub] tinkerpop pull request #838: TINKERPOP-1822: Change default RepeatStep to DF...
Github user krlohnes commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/838#discussion_r201900734
--- Diff:
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/branch/RepeatStep.java
---
@@ -273,11 +300,40 @@ public RepeatEndStep(final Traversal.Admin traversal)
{
super(traversal);
}
+final LinkedList> stashedStarts = new
LinkedList<>();
--- End diff --
@spmallette Those results are definitely...interesting to say the least. I
think the tests themselves are reasonable, though, as a comment, I'm not
typically using a repeat that's going to be able to utilize the
`RepeatUnrollStrategy`. At least not for what I originally started
investigating this for.
That said, I took a step back and worked on performance between the BFS and
DFS, and have gotten them much closer. On my local machine that BFS test was
returning 889 from the counter. With the latest commit I added, DFS is
returning 758. That's obviously not coming close to the default "let the
strategies do their thing" performance, but it's significantly better than the
ops counts being in the teens for DFS. Given that I was expecting slightly
degraded performance with DFS, I think this is in a much better place
performance wise.
---
[GitHub] tinkerpop pull request #838: TINKERPOP-1822: Change default RepeatStep to DF...
Github user spmallette commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/838#discussion_r199549370
--- Diff:
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/branch/RepeatStep.java
---
@@ -273,11 +300,40 @@ public RepeatEndStep(final Traversal.Admin traversal)
{
super(traversal);
}
+final LinkedList> stashedStarts = new
LinkedList<>();
--- End diff --
added some JFRs to the JIRA and a performance comparison:
https://issues.apache.org/jira/browse/TINKERPOP-1822?focusedCommentId=16530124&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-16530124
The results have left me with questions
---
[GitHub] tinkerpop pull request #838: TINKERPOP-1822: Change default RepeatStep to DF...
Github user spmallette commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/838#discussion_r195710841
--- Diff:
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/branch/RepeatStep.java
---
@@ -273,11 +300,40 @@ public RepeatEndStep(final Traversal.Admin traversal)
{
super(traversal);
}
+final LinkedList> stashedStarts = new
LinkedList<>();
--- End diff --
> Any thoughts on what might be some decent data/traversals for the
JFRs/microbenchmarks around this?
Maybe just start with the Grateful Dead dataset? I think it might be
sufficiently complex to yield a good test of the different approaches we have
now. If not, maybe we need to generate something artificial.
Personally, I'd love to see a JFR that executes the same traversal with
each of the three configurations that we now have with a `Thread.sleep()`
between them so that we can easily distinguish when one traversal stops and the
next starts. Not sure what the traversal (or traversals) needs to be - I guess
I'd just like to easily compare what happens from a processing/memory
perspective with each of the configurations we've talked about and then true
that up with the expectations that we have regarding each configuration that we
have.
> As far as OLAP goes, what are the expectations there?
I was just curious if it all the tests still pass there or not. I'd assume
so given that you didn't make changes there, but I just wanted to be sure.
---
[GitHub] tinkerpop pull request #838: TINKERPOP-1822: Change default RepeatStep to DF...
Github user krlohnes commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/838#discussion_r195496700
--- Diff:
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/branch/RepeatStep.java
---
@@ -273,11 +300,40 @@ public RepeatEndStep(final Traversal.Admin traversal)
{
super(traversal);
}
+final LinkedList> stashedStarts = new
LinkedList<>();
--- End diff --
@spmallette Any thoughts on what might be some decent data/traversals for
the JFRs/microbenchmarks around this?
As far as OLAP goes, what are the expectations there? I haven't made any
code changes to the computer algorithm yet since I'm not terribly familiar with
that side of TinkerPop, so I think things should still be working working as
before. I guess the question here is code changes or documentation changes when
it comes to OLAP?
I'm hopefully going to spend some time this weekend or the next around this.
---
[GitHub] tinkerpop pull request #838: TINKERPOP-1822: Change default RepeatStep to DF...
Github user spmallette commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/838#discussion_r193389532
--- Diff:
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/branch/RepeatStep.java
---
@@ -273,11 +300,40 @@ public RepeatEndStep(final Traversal.Admin traversal)
{
super(traversal);
}
+final LinkedList> stashedStarts = new
LinkedList<>();
--- End diff --
An `ArrayList` sounds reasonable to me as far as the right `List`
implementation for how `stashedStarts` is being used. When I'd posed the
question I was more thinking about the more general increased memory
requirements for doing DFS in this fashion as we basically have to accumulate
what could be a fairly large list in memory in order to perform this operation.
I suppose that we do such things in `fold()`-ing steps but the user is
explicitly aware of their choice to do that when they use such a step. In the
case of `stashedStarts` the memory requirement for choosing DFS is somewhat
hidden as it's not clear from the Gremlin they've written that an internal list
is being built. Perhaps that's simply a side-effect of allowing this to work in
the way that it does (as you alluded to in your comment).
I'm still thinking that DFS will be something that users will invoke in
specific use cases where they will be more aware of the consequences of what it
is that they are doing. If that is the case, then this would perhaps just be
one more consequence of making that choice to consider.
I'd be curious to see some JFRs around the different modes of execution
that we now have. Perhaps some microbenchmarks are in order too. And then the
fun partdoes OLAP still work without any change? :smile:
---
[GitHub] tinkerpop pull request #838: TINKERPOP-1822: Change default RepeatStep to DF...
Github user krlohnes commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/838#discussion_r192728027
--- Diff:
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/branch/RepeatStep.java
---
@@ -273,11 +300,40 @@ public RepeatEndStep(final Traversal.Admin traversal)
{
super(traversal);
}
+final LinkedList> stashedStarts = new
LinkedList<>();
--- End diff --
Sorry for the delay, things have gotten fairly busy for me. I'd given a
little bit of thought around this, but not a whole lot. I don't think this
needs to be a linked list, it could likely an ArrayList or an ArrayDeque and
the entries would take up less memory. ArrayDeque may be preferable for large
graphs since it'll resize less frequently than an ArrayList, but it will
consume more memory than an ArrayList by default.
However, I think having a `stashedStarts` here is necessary from the view
of "one piece of code serving 2 algorithms" If this were to be completely
rewritten as DFS (I don't know how to do that at this point, but for the sake
of argument), and there was a desire to use the same code to utilize BFS,
there'd likely be a need to have stashed starts to achieve BFS.
That's about as far as I've gotten in thinking about this for now.
---
[GitHub] tinkerpop pull request #838: TINKERPOP-1822: Change default RepeatStep to DF...
Github user krlohnes commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/838#discussion_r191572290
--- Diff:
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/branch/RepeatStep.java
---
@@ -273,11 +300,40 @@ public RepeatEndStep(final Traversal.Admin traversal)
{
super(traversal);
}
+final LinkedList> stashedStarts = new
LinkedList<>();
+
+private Traverser.Admin nextStart(RepeatStep repeatStep,
boolean useDfs) {
+if (!useDfs) {
+return this.starts.next();
+} else {
+if (this.starts.hasNext()) {
+return this.starts.next();
+} else {
+return this.stashedStarts.pop();
+}
+}
+}
+
+@Override
+public boolean hasNext() {
+return super.hasNext() || !this.stashedStarts.isEmpty();
+}
+
@Override
protected Iterator> standardAlgorithm() throws
NoSuchElementException {
final RepeatStep repeatStep = (RepeatStep)
this.getTraversal().getParent();
+final List steps = repeatStep.repeatTraversal.getSteps();
+final Step stepBeforeRepeatEndStep = steps.get(steps.size() -
2);
+final boolean useDfs = !(stepBeforeRepeatEndStep instanceof
Barrier);
while (true) {
-final Traverser.Admin start = this.starts.next();
+final Traverser.Admin start = nextStart(repeatStep,
useDfs);
+if (useDfs) {
+final List> localStarts = new
ArrayList<>();
--- End diff --
No, this could be done with an index and writing directly to stashed
starts, definitely. will fix.
---
[GitHub] tinkerpop pull request #838: TINKERPOP-1822: Change default RepeatStep to DF...
Github user spmallette commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/838#discussion_r191529207
--- Diff:
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/branch/RepeatStep.java
---
@@ -273,11 +300,40 @@ public RepeatEndStep(final Traversal.Admin traversal)
{
super(traversal);
}
+final LinkedList> stashedStarts = new
LinkedList<>();
--- End diff --
Have you given any thought to the memory requirements of `stashedStarts`?
Seems like that approach could be really intensive for a large graph (i.e. a
traversal that touches a lot of data)? any thoughts?
---
[GitHub] tinkerpop pull request #838: TINKERPOP-1822: Change default RepeatStep to DF...
Github user spmallette commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/838#discussion_r191525536
--- Diff:
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/branch/RepeatStep.java
---
@@ -273,11 +300,40 @@ public RepeatEndStep(final Traversal.Admin traversal)
{
super(traversal);
}
+final LinkedList> stashedStarts = new
LinkedList<>();
+
+private Traverser.Admin nextStart(RepeatStep repeatStep,
boolean useDfs) {
+if (!useDfs) {
+return this.starts.next();
+} else {
+if (this.starts.hasNext()) {
+return this.starts.next();
+} else {
+return this.stashedStarts.pop();
+}
+}
+}
+
+@Override
+public boolean hasNext() {
+return super.hasNext() || !this.stashedStarts.isEmpty();
+}
+
@Override
protected Iterator> standardAlgorithm() throws
NoSuchElementException {
final RepeatStep repeatStep = (RepeatStep)
this.getTraversal().getParent();
+final List steps = repeatStep.repeatTraversal.getSteps();
+final Step stepBeforeRepeatEndStep = steps.get(steps.size() -
2);
+final boolean useDfs = !(stepBeforeRepeatEndStep instanceof
Barrier);
while (true) {
-final Traverser.Admin start = this.starts.next();
+final Traverser.Admin start = nextStart(repeatStep,
useDfs);
+if (useDfs) {
+final List> localStarts = new
ArrayList<>();
--- End diff --
any reason you need to allocate a `List` here? could you not write directly
to `stashedStarts`?
---
[jira] [Closed] (TINKERPOP-1973) Emit predicate ignored after final loop of a RepeatStep with times or until
[
https://issues.apache.org/jira/browse/TINKERPOP-1973?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Ted Wilmes closed TINKERPOP-1973.
-
Resolution: Won't Fix
Closing as it is the expected behavior. Thanks for the examples [~dkuppitz]
> Emit predicate ignored after final loop of a RepeatStep with times or until
> ---
>
> Key: TINKERPOP-1973
> URL: https://issues.apache.org/jira/browse/TINKERPOP-1973
> Project: TinkerPop
> Issue Type: Bug
> Components: process
>Affects Versions: 3.3.3, 3.2.9
>Reporter: Ted Wilmes
>Assignee: Ted Wilmes
>Priority: Major
>
> When combined with a {{times}} or {{until}}, the {{emit}} predicate is not
> applied to the output of the last loop of the {{RepeatStep}}.
> {code}
> gremlin> g = TinkerFactory.createModern().traversal()
> ==>graphtraversalsource[tinkergraph[vertices:6 edges:6], standard]
> gremlin> g.V().repeat(out()).emit(has('name', 'lop')).values('name')
> ==>lop
> ==>lop
> ==>lop
> ==>lop
> gremlin> g.V().repeat(out()).times(2).emit(has('name', 'lop')).values('name')
> ==>lop
> ==>ripple
> ==>lop
> ==>lop
> ==>lop
> gremlin>
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (TINKERPOP-1973) Emit predicate ignored after final loop of a RepeatStep with times or until
[
https://issues.apache.org/jira/browse/TINKERPOP-1973?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16484214#comment-16484214
]
Daniel Kuppitz commented on TINKERPOP-1973:
---
{quote}the key would be to include the same predicate after the emit{quote}
Right, that's the most common pattern.
{quote}the loop count is no longer available after the traverser has exited the
repeat{quote}
True, but there are several techniques to make it available. The easiest would
be to store the loop counter in the traverser's sack:
{noformat}
g.V()...repeat(...sack(assign).by(loops()))...filter(sack().is(lt(3)))...
{noformat}
If the sack is needed for other things, you can still emit projected values
(containing the actual element and the loop counter).
{noformat}
gremlin>
g.V(1).repeat(optional(select("a")).out().project("a","b").by().by(loops())).emit()
==>[a:v[3],b:0]
==>[a:v[2],b:0]
==>[a:v[4],b:0]
==>[a:v[5],b:1]
==>[a:v[3],b:1]
gremlin>
g.V(1).project("a","b").by().by(constant(-1)).emit().repeat(select("a").out().project("a","b").by().by(loops()))
==>[a:v[1],b:-1]
==>[a:v[3],b:0]
==>[a:v[2],b:0]
==>[a:v[4],b:0]
==>[a:v[5],b:1]
==>[a:v[3],b:1]
{noformat}
> Emit predicate ignored after final loop of a RepeatStep with times or until
> ---
>
> Key: TINKERPOP-1973
> URL: https://issues.apache.org/jira/browse/TINKERPOP-1973
> Project: TinkerPop
> Issue Type: Bug
> Components: process
>Affects Versions: 3.3.3, 3.2.9
>Reporter: Ted Wilmes
>Assignee: Ted Wilmes
>Priority: Major
>
> When combined with a {{times}} or {{until}}, the {{emit}} predicate is not
> applied to the output of the last loop of the {{RepeatStep}}.
> {code}
> gremlin> g = TinkerFactory.createModern().traversal()
> ==>graphtraversalsource[tinkergraph[vertices:6 edges:6], standard]
> gremlin> g.V().repeat(out()).emit(has('name', 'lop')).values('name')
> ==>lop
> ==>lop
> ==>lop
> ==>lop
> gremlin> g.V().repeat(out()).times(2).emit(has('name', 'lop')).values('name')
> ==>lop
> ==>ripple
> ==>lop
> ==>lop
> ==>lop
> gremlin>
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (TINKERPOP-1973) Emit predicate ignored after final loop of a RepeatStep with times or until
[
https://issues.apache.org/jira/browse/TINKERPOP-1973?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16484040#comment-16484040
]
Ted Wilmes commented on TINKERPOP-1973:
---
Ok, that make sense in the context of the injector distinction. So for my
contrived example, the key would be to include the same predicate after the
emit. That'll work for most cases I think as long as you aren't relying on the
loop counter as part of your emit logic since (I think) the loop count is no
longer available after the traverser has exited the repeat. The reason this
came up was I was writing a traversal that traversed up a tree from the leaves
a set number of levels, storing the intermediate results in a subgraph. For a
smaller subset of those levels, It needed to execute another repeat traversal
so I was using an {{emit(loops().is(lt(3))).repeat()}}. The issue was that
repeat was executed for levels less than 3 and then the last level exiting the
repeat.
> Emit predicate ignored after final loop of a RepeatStep with times or until
> ---
>
> Key: TINKERPOP-1973
> URL: https://issues.apache.org/jira/browse/TINKERPOP-1973
> Project: TinkerPop
> Issue Type: Bug
> Components: process
>Affects Versions: 3.3.3, 3.2.9
>Reporter: Ted Wilmes
>Assignee: Ted Wilmes
>Priority: Major
>
> When combined with a {{times}} or {{until}}, the {{emit}} predicate is not
> applied to the output of the last loop of the {{RepeatStep}}.
> {code}
> gremlin> g = TinkerFactory.createModern().traversal()
> ==>graphtraversalsource[tinkergraph[vertices:6 edges:6], standard]
> gremlin> g.V().repeat(out()).emit(has('name', 'lop')).values('name')
> ==>lop
> ==>lop
> ==>lop
> ==>lop
> gremlin> g.V().repeat(out()).times(2).emit(has('name', 'lop')).values('name')
> ==>lop
> ==>ripple
> ==>lop
> ==>lop
> ==>lop
> gremlin>
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (TINKERPOP-1973) Emit predicate ignored after final loop of a RepeatStep with times or until
[
https://issues.apache.org/jira/browse/TINKERPOP-1973?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16483090#comment-16483090
]
Daniel Kuppitz commented on TINKERPOP-1973:
---
This is the expected behavior. {{emit()}} is only for intermediate vertices.
You see {{ripple}} because it's the end of the repetition.
{noformat}
gremlin> g.V().repeat(out()).times(2).emit(has('name', 'lop')).path().by('name')
==>[marko,lop]
==>[marko,josh,ripple]
==>[marko,josh,lop]
==>[josh,lop]
==>[peter,lop]
{noformat}
{{emit())) should not be seen as a filter, but rather as an injector.
> Emit predicate ignored after final loop of a RepeatStep with times or until
> ---
>
> Key: TINKERPOP-1973
> URL: https://issues.apache.org/jira/browse/TINKERPOP-1973
> Project: TinkerPop
> Issue Type: Bug
> Components: process
>Affects Versions: 3.3.3, 3.2.9
>Reporter: Ted Wilmes
>Assignee: Ted Wilmes
>Priority: Major
>
> When combined with a {{times}} or {{until}}, the {{emit}} predicate is not
> applied to the output of the last loop of the {{RepeatStep}}.
> {code}
> gremlin> g = TinkerFactory.createModern().traversal()
> ==>graphtraversalsource[tinkergraph[vertices:6 edges:6], standard]
> gremlin> g.V().repeat(out()).emit(has('name', 'lop')).values('name')
> ==>lop
> ==>lop
> ==>lop
> ==>lop
> gremlin> g.V().repeat(out()).times(2).emit(has('name', 'lop')).values('name')
> ==>lop
> ==>ripple
> ==>lop
> ==>lop
> ==>lop
> gremlin>
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (TINKERPOP-1973) Emit predicate ignored after final loop of a RepeatStep with times or until
[
https://issues.apache.org/jira/browse/TINKERPOP-1973?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16482859#comment-16482859
]
Ted Wilmes commented on TINKERPOP-1973:
---
[~dkuppitz] this behavior seemed like a bug to me but I wanted to double check
with you to see if you thought there was a purpose for it.
> Emit predicate ignored after final loop of a RepeatStep with times or until
> ---
>
> Key: TINKERPOP-1973
> URL: https://issues.apache.org/jira/browse/TINKERPOP-1973
> Project: TinkerPop
> Issue Type: Bug
> Components: process
>Affects Versions: 3.3.3, 3.2.9
>Reporter: Ted Wilmes
>Assignee: Ted Wilmes
>Priority: Major
>
> When combined with a {{times}} or {{until}}, the {{emit}} predicate is not
> applied to the output of the last loop of the {{RepeatStep}}.
> {code}
> gremlin> g = TinkerFactory.createModern().traversal()
> ==>graphtraversalsource[tinkergraph[vertices:6 edges:6], standard]
> gremlin> g.V().repeat(out()).emit(has('name', 'lop')).values('name')
> ==>lop
> ==>lop
> ==>lop
> ==>lop
> gremlin> g.V().repeat(out()).times(2).emit(has('name', 'lop')).values('name')
> ==>lop
> ==>ripple
> ==>lop
> ==>lop
> ==>lop
> gremlin>
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Updated] (TINKERPOP-1973) Emit predicate ignored after final loop of a RepeatStep with times or until
[
https://issues.apache.org/jira/browse/TINKERPOP-1973?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Ted Wilmes updated TINKERPOP-1973:
--
Description:
When combined with a {{times}} or {{until}}, the {{emit}} predicate is not
applied to the output of the last loop of the {{RepeatStep}}.
{code}
gremlin> g = TinkerFactory.createModern().traversal()
==>graphtraversalsource[tinkergraph[vertices:6 edges:6], standard]
gremlin> g.V().repeat(out()).emit(has('name', 'lop')).values('name')
==>lop
==>lop
==>lop
==>lop
gremlin> g.V().repeat(out()).times(2).emit(has('name', 'lop')).values('name')
==>lop
==>ripple
==>lop
==>lop
==>lop
gremlin>
{code}
was:
When combined with a `times` or `until`, the `emit` predicate is not applied to
the output of the last loop of the `RepeatStep`.
{code:groovy}
gremlin> g = TinkerFactory.createModern().traversal()
==>graphtraversalsource[tinkergraph[vertices:6 edges:6], standard]
gremlin> g.V().repeat(out()).emit(has('name', 'lop')).values('name')
==>lop
==>lop
==>lop
==>lop
gremlin> g.V().repeat(out()).times(2).emit(has('name', 'lop')).values('name')
==>lop
==>ripple
==>lop
==>lop
==>lop
gremlin>
{code}
> Emit predicate ignored after final loop of a RepeatStep with times or until
> ---
>
> Key: TINKERPOP-1973
> URL: https://issues.apache.org/jira/browse/TINKERPOP-1973
> Project: TinkerPop
> Issue Type: Bug
> Components: process
>Affects Versions: 3.3.3, 3.2.9
> Reporter: Ted Wilmes
>Assignee: Ted Wilmes
>Priority: Major
>
> When combined with a {{times}} or {{until}}, the {{emit}} predicate is not
> applied to the output of the last loop of the {{RepeatStep}}.
> {code}
> gremlin> g = TinkerFactory.createModern().traversal()
> ==>graphtraversalsource[tinkergraph[vertices:6 edges:6], standard]
> gremlin> g.V().repeat(out()).emit(has('name', 'lop')).values('name')
> ==>lop
> ==>lop
> ==>lop
> ==>lop
> gremlin> g.V().repeat(out()).times(2).emit(has('name', 'lop')).values('name')
> ==>lop
> ==>ripple
> ==>lop
> ==>lop
> ==>lop
> gremlin>
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Updated] (TINKERPOP-1973) Emit predicate ignored after final loop of a RepeatStep with times or until
[
https://issues.apache.org/jira/browse/TINKERPOP-1973?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Ted Wilmes updated TINKERPOP-1973:
--
Description:
When combined with a `times` or `until`, the `emit` predicate is not applied to
the output of the last loop of the `RepeatStep`.
{code:groovy}
gremlin> g = TinkerFactory.createModern().traversal()
==>graphtraversalsource[tinkergraph[vertices:6 edges:6], standard]
gremlin> g.V().repeat(out()).emit(has('name', 'lop')).values('name')
==>lop
==>lop
==>lop
==>lop
gremlin> g.V().repeat(out()).times(2).emit(has('name', 'lop')).values('name')
==>lop
==>ripple
==>lop
==>lop
==>lop
gremlin>
{code}
was:
When combined with a `times` or `until`, the `emit` predicate is not applied to
the output of the last loop of the `RepeatStep`.
```
gremlin> g = TinkerFactory.createModern().traversal()
==>graphtraversalsource[tinkergraph[vertices:6 edges:6], standard]
gremlin> g.V().repeat(out()).emit(has('name', 'lop')).values('name')
==>lop
==>lop
==>lop
==>lop
gremlin> g.V().repeat(out()).times(2).emit(has('name', 'lop')).values('name')
==>lop
==>ripple
==>lop
==>lop
==>lop
gremlin>
```
> Emit predicate ignored after final loop of a RepeatStep with times or until
> ---
>
> Key: TINKERPOP-1973
> URL: https://issues.apache.org/jira/browse/TINKERPOP-1973
> Project: TinkerPop
> Issue Type: Bug
> Components: process
>Affects Versions: 3.3.3, 3.2.9
>Reporter: Ted Wilmes
>Assignee: Ted Wilmes
>Priority: Major
>
> When combined with a `times` or `until`, the `emit` predicate is not applied
> to the output of the last loop of the `RepeatStep`.
> {code:groovy}
> gremlin> g = TinkerFactory.createModern().traversal()
> ==>graphtraversalsource[tinkergraph[vertices:6 edges:6], standard]
> gremlin> g.V().repeat(out()).emit(has('name', 'lop')).values('name')
> ==>lop
> ==>lop
> ==>lop
> ==>lop
> gremlin> g.V().repeat(out()).times(2).emit(has('name', 'lop')).values('name')
> ==>lop
> ==>ripple
> ==>lop
> ==>lop
> ==>lop
> gremlin>
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Created] (TINKERPOP-1973) Emit predicate ignored after final loop of a RepeatStep with times or until
Ted Wilmes created TINKERPOP-1973:
-
Summary: Emit predicate ignored after final loop of a RepeatStep
with times or until
Key: TINKERPOP-1973
URL: https://issues.apache.org/jira/browse/TINKERPOP-1973
Project: TinkerPop
Issue Type: Bug
Components: process
Affects Versions: 3.2.9, 3.3.3
Reporter: Ted Wilmes
Assignee: Ted Wilmes
When combined with a `times` or `until`, the `emit` predicate is not applied to
the output of the last loop of the `RepeatStep`.
```
gremlin> g = TinkerFactory.createModern().traversal()
==>graphtraversalsource[tinkergraph[vertices:6 edges:6], standard]
gremlin> g.V().repeat(out()).emit(has('name', 'lop')).values('name')
==>lop
==>lop
==>lop
==>lop
gremlin> g.V().repeat(out()).times(2).emit(has('name', 'lop')).values('name')
==>lop
==>ripple
==>lop
==>lop
==>lop
gremlin>
```
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
Re: RepeatStep
Hello,
times(0) means "go through the loop 0 times." However, given that its a post
times(0), then it would go through once.
I just updated the RepeatUnrollStrategy to only do unrolling for times > 0 to
keep things consistent with previous behavior — though times(0) is a really
weird case.
Thanks,
Marko.
http://markorodriguez.com
> On Jul 16, 2016, at 11:40 AM, pieter-gmail wrote:
>
> More investigation, shows that the new RepeatUnrollStrategy completely
> removes the RepeatStep because of the times 0 logic.
>
> For the global case the leaves behind the GraphStep and the HasStep thus
> returning a1.
>
> For the LocalStep case the LocalSteps are now empty (after the
> RepeatSteps have been removed) throwing a FastNoSuchElementException and
> thus returning no elements.
>
> However I think we first need to understand what the expected semantics
> of a before times(0) is.
> I have a feeling that my previous passing tests were just getting lucky?
>
> Thanks
> Pieter
>
>
> On 16/07/2016 18:37, pieter-gmail wrote:
>> Hi,
>>
>> I am running the following tests on 3.2.1-SNAPSHOT
>>
>>@Test
>>public void testRepeat() {
>>final TinkerGraph g = TinkerGraph.open();
>>Vertex a1 = g.addVertex(T.label, "A", "name", "a1");
>>Vertex b1 = g.addVertex(T.label, "B", "name", "b1");
>>Vertex b2 = g.addVertex(T.label, "B", "name", "b2");
>>Vertex b3 = g.addVertex(T.label, "B", "name", "b3");
>>a1.addEdge("ab", b1);
>>a1.addEdge("ab", b2);
>>a1.addEdge("ab", b3);
>>Vertex c1 = g.addVertex(T.label, "C", "name", "c1");
>>Vertex c2 = g.addVertex(T.label, "C", "name", "c2");
>>Vertex c3 = g.addVertex(T.label, "C", "name", "c3");
>>b1.addEdge("bc", c1);
>>b1.addEdge("bc", c2);
>>b1.addEdge("bc", c3);
>>
>>//this passes
>>List vertices =
>> g.traversal().V().hasLabel("A").times(0).repeat(out("ab").out("bc")).toList();
>>assertEquals(1, vertices.size());
>>assertTrue(vertices.contains(a1));
>>
>>//this fails
>>vertices =
>> g.traversal().V().hasLabel("A").local(__.times(0).repeat(out("ab").out("bc"))).toList();
>>assertEquals(1, vertices.size());
>>assertTrue(vertices.contains(a1));
>>}
>>
>> Previously on 3.2.0-incubating the test passed.
>>
>> Is this a bug or a new interpretation of the while do zero times logic?
>>
>> Thanks
>> Pieter
>>
>>
>>
>
Re: RepeatStep
More investigation, shows that the new RepeatUnrollStrategy completely
removes the RepeatStep because of the times 0 logic.
For the global case the leaves behind the GraphStep and the HasStep thus
returning a1.
For the LocalStep case the LocalSteps are now empty (after the
RepeatSteps have been removed) throwing a FastNoSuchElementException and
thus returning no elements.
However I think we first need to understand what the expected semantics
of a before times(0) is.
I have a feeling that my previous passing tests were just getting lucky?
Thanks
Pieter
On 16/07/2016 18:37, pieter-gmail wrote:
> Hi,
>
> I am running the following tests on 3.2.1-SNAPSHOT
>
> @Test
> public void testRepeat() {
> final TinkerGraph g = TinkerGraph.open();
> Vertex a1 = g.addVertex(T.label, "A", "name", "a1");
> Vertex b1 = g.addVertex(T.label, "B", "name", "b1");
> Vertex b2 = g.addVertex(T.label, "B", "name", "b2");
> Vertex b3 = g.addVertex(T.label, "B", "name", "b3");
> a1.addEdge("ab", b1);
> a1.addEdge("ab", b2);
> a1.addEdge("ab", b3);
> Vertex c1 = g.addVertex(T.label, "C", "name", "c1");
> Vertex c2 = g.addVertex(T.label, "C", "name", "c2");
> Vertex c3 = g.addVertex(T.label, "C", "name", "c3");
> b1.addEdge("bc", c1);
> b1.addEdge("bc", c2);
> b1.addEdge("bc", c3);
>
> //this passes
> List vertices =
> g.traversal().V().hasLabel("A").times(0).repeat(out("ab").out("bc")).toList();
> assertEquals(1, vertices.size());
> assertTrue(vertices.contains(a1));
>
> //this fails
> vertices =
> g.traversal().V().hasLabel("A").local(__.times(0).repeat(out("ab").out("bc"))).toList();
> assertEquals(1, vertices.size());
> assertTrue(vertices.contains(a1));
> }
>
> Previously on 3.2.0-incubating the test passed.
>
> Is this a bug or a new interpretation of the while do zero times logic?
>
> Thanks
> Pieter
>
>
>
Re: RepeatStep
Hi,
I am running the following tests on 3.2.1-SNAPSHOT
@Test
public void testRepeat() {
final TinkerGraph g = TinkerGraph.open();
Vertex a1 = g.addVertex(T.label, "A", "name", "a1");
Vertex b1 = g.addVertex(T.label, "B", "name", "b1");
Vertex b2 = g.addVertex(T.label, "B", "name", "b2");
Vertex b3 = g.addVertex(T.label, "B", "name", "b3");
a1.addEdge("ab", b1);
a1.addEdge("ab", b2);
a1.addEdge("ab", b3);
Vertex c1 = g.addVertex(T.label, "C", "name", "c1");
Vertex c2 = g.addVertex(T.label, "C", "name", "c2");
Vertex c3 = g.addVertex(T.label, "C", "name", "c3");
b1.addEdge("bc", c1);
b1.addEdge("bc", c2);
b1.addEdge("bc", c3);
//this passes
List vertices =
g.traversal().V().hasLabel("A").times(0).repeat(out("ab").out("bc")).toList();
assertEquals(1, vertices.size());
assertTrue(vertices.contains(a1));
//this fails
vertices =
g.traversal().V().hasLabel("A").local(__.times(0).repeat(out("ab").out("bc"))).toList();
assertEquals(1, vertices.size());
assertTrue(vertices.contains(a1));
}
Previously on 3.2.0-incubating the test passed.
Is this a bug or a new interpretation of the while do zero times logic?
Thanks
Pieter
