[jira] [Commented] (TINKERPOP-1506) Optional/Coalesce should not allow sideEffect traversals.
[ https://issues.apache.org/jira/browse/TINKERPOP-1506?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15631157#comment-15631157 ] ASF GitHub Bot commented on TINKERPOP-1506: --- Github user spmallette commented on the issue: https://github.com/apache/tinkerpop/pull/471 All tests pass with `docker/build.sh -t -n -i` VOTE +1 > Optional/Coalesce should not allow sideEffect traversals. > - > > Key: TINKERPOP-1506 > URL: https://issues.apache.org/jira/browse/TINKERPOP-1506 > Project: TinkerPop > Issue Type: Improvement > Components: process >Affects Versions: 3.1.4, 3.2.2 >Reporter: Marko A. Rodriguez >Assignee: Marko A. Rodriguez > Fix For: 3.3.0 > > > It took me a long time to realize what was wrong with a traversal I wrote > that used {{optional(blah.sideEffect.blah)}}. {{optional()}} maps to > {{ChooseStep}} under the hood and the provide traversal is first tested for a > {{hasNext()}}. If so, the it plays itself out. The problem is that if there > is a side-effect in the traversal child, then it gets executed twice. > {code} > gremlin> g = TinkerGraph.open().traversal() > ==>graphtraversalsource[tinkergraph[vertices:0 edges:0], standard] > gremlin> g.inject(1).optional(addV('twin')) > ==>v[1] > gremlin> g.V().valueMap(true) > ==>[id:0,label:twin] > ==>[id:1,label:twin] > {code} > We should NOT allow {{optional()}} to have {{SideEffectStep}} steps in it so > as not to cause unexpected behavior. {{StandardVerificationStrategy}} can > analyze and throw an exception if necessary. > Also, {{coalesce()}} has a similar problem, though perhaps it can be a useful > 'technique.' > {code} > gremlin> g = TinkerGraph.open().traversal() > ==>graphtraversalsource[tinkergraph[vertices:0 edges:0], standard] > gremlin> g.inject(1).coalesce(addV('twin1').limit(0), addV('twin2')) > ==>v[1] > gremlin> g.V().valueMap(true) > ==>[id:0,label:twin1] > ==>[id:1,label:twin2] > gremlin> > {code} -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (TINKERPOP-1506) Optional/Coalesce should not allow sideEffect traversals.
[ https://issues.apache.org/jira/browse/TINKERPOP-1506?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15631055#comment-15631055 ] ASF GitHub Bot commented on TINKERPOP-1506: --- Github user asfgit closed the pull request at: https://github.com/apache/tinkerpop/pull/471 > Optional/Coalesce should not allow sideEffect traversals. > - > > Key: TINKERPOP-1506 > URL: https://issues.apache.org/jira/browse/TINKERPOP-1506 > Project: TinkerPop > Issue Type: Improvement > Components: process >Affects Versions: 3.1.4, 3.2.2 >Reporter: Marko A. Rodriguez >Assignee: Marko A. Rodriguez > Fix For: 3.3.0 > > > It took me a long time to realize what was wrong with a traversal I wrote > that used {{optional(blah.sideEffect.blah)}}. {{optional()}} maps to > {{ChooseStep}} under the hood and the provide traversal is first tested for a > {{hasNext()}}. If so, the it plays itself out. The problem is that if there > is a side-effect in the traversal child, then it gets executed twice. > {code} > gremlin> g = TinkerGraph.open().traversal() > ==>graphtraversalsource[tinkergraph[vertices:0 edges:0], standard] > gremlin> g.inject(1).optional(addV('twin')) > ==>v[1] > gremlin> g.V().valueMap(true) > ==>[id:0,label:twin] > ==>[id:1,label:twin] > {code} > We should NOT allow {{optional()}} to have {{SideEffectStep}} steps in it so > as not to cause unexpected behavior. {{StandardVerificationStrategy}} can > analyze and throw an exception if necessary. > Also, {{coalesce()}} has a similar problem, though perhaps it can be a useful > 'technique.' > {code} > gremlin> g = TinkerGraph.open().traversal() > ==>graphtraversalsource[tinkergraph[vertices:0 edges:0], standard] > gremlin> g.inject(1).coalesce(addV('twin1').limit(0), addV('twin2')) > ==>v[1] > gremlin> g.V().valueMap(true) > ==>[id:0,label:twin1] > ==>[id:1,label:twin2] > gremlin> > {code} -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (TINKERPOP-1506) Optional/Coalesce should not allow sideEffect traversals.
[ https://issues.apache.org/jira/browse/TINKERPOP-1506?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15631021#comment-15631021 ] ASF GitHub Bot commented on TINKERPOP-1506: --- Github user twilmes commented on the issue: https://github.com/apache/tinkerpop/pull/471 Nice find. Looks good. VOTE: +1 > Optional/Coalesce should not allow sideEffect traversals. > - > > Key: TINKERPOP-1506 > URL: https://issues.apache.org/jira/browse/TINKERPOP-1506 > Project: TinkerPop > Issue Type: Improvement > Components: process >Affects Versions: 3.1.4, 3.2.2 >Reporter: Marko A. Rodriguez > > It took me a long time to realize what was wrong with a traversal I wrote > that used {{optional(blah.sideEffect.blah)}}. {{optional()}} maps to > {{ChooseStep}} under the hood and the provide traversal is first tested for a > {{hasNext()}}. If so, the it plays itself out. The problem is that if there > is a side-effect in the traversal child, then it gets executed twice. > {code} > gremlin> g = TinkerGraph.open().traversal() > ==>graphtraversalsource[tinkergraph[vertices:0 edges:0], standard] > gremlin> g.inject(1).optional(addV('twin')) > ==>v[1] > gremlin> g.V().valueMap(true) > ==>[id:0,label:twin] > ==>[id:1,label:twin] > {code} > We should NOT allow {{optional()}} to have {{SideEffectStep}} steps in it so > as not to cause unexpected behavior. {{StandardVerificationStrategy}} can > analyze and throw an exception if necessary. > Also, {{coalesce()}} has a similar problem, though perhaps it can be a useful > 'technique.' > {code} > gremlin> g = TinkerGraph.open().traversal() > ==>graphtraversalsource[tinkergraph[vertices:0 edges:0], standard] > gremlin> g.inject(1).coalesce(addV('twin1').limit(0), addV('twin2')) > ==>v[1] > gremlin> g.V().valueMap(true) > ==>[id:0,label:twin1] > ==>[id:1,label:twin2] > gremlin> > {code} -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (TINKERPOP-1506) Optional/Coalesce should not allow sideEffect traversals.
[ https://issues.apache.org/jira/browse/TINKERPOP-1506?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15623856#comment-15623856 ] ASF GitHub Bot commented on TINKERPOP-1506: --- Github user dkuppitz commented on the issue: https://github.com/apache/tinkerpop/pull/471 I've done some manual tests. VOTE: +1 > Optional/Coalesce should not allow sideEffect traversals. > - > > Key: TINKERPOP-1506 > URL: https://issues.apache.org/jira/browse/TINKERPOP-1506 > Project: TinkerPop > Issue Type: Improvement > Components: process >Affects Versions: 3.1.4, 3.2.2 >Reporter: Marko A. Rodriguez > > It took me a long time to realize what was wrong with a traversal I wrote > that used {{optional(blah.sideEffect.blah)}}. {{optional()}} maps to > {{ChooseStep}} under the hood and the provide traversal is first tested for a > {{hasNext()}}. If so, the it plays itself out. The problem is that if there > is a side-effect in the traversal child, then it gets executed twice. > {code} > gremlin> g = TinkerGraph.open().traversal() > ==>graphtraversalsource[tinkergraph[vertices:0 edges:0], standard] > gremlin> g.inject(1).optional(addV('twin')) > ==>v[1] > gremlin> g.V().valueMap(true) > ==>[id:0,label:twin] > ==>[id:1,label:twin] > {code} > We should NOT allow {{optional()}} to have {{SideEffectStep}} steps in it so > as not to cause unexpected behavior. {{StandardVerificationStrategy}} can > analyze and throw an exception if necessary. > Also, {{coalesce()}} has a similar problem, though perhaps it can be a useful > 'technique.' > {code} > gremlin> g = TinkerGraph.open().traversal() > ==>graphtraversalsource[tinkergraph[vertices:0 edges:0], standard] > gremlin> g.inject(1).coalesce(addV('twin1').limit(0), addV('twin2')) > ==>v[1] > gremlin> g.V().valueMap(true) > ==>[id:0,label:twin1] > ==>[id:1,label:twin2] > gremlin> > {code} -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (TINKERPOP-1506) Optional/Coalesce should not allow sideEffect traversals.
[ https://issues.apache.org/jira/browse/TINKERPOP-1506?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15622906#comment-15622906 ] ASF GitHub Bot commented on TINKERPOP-1506: --- GitHub user okram opened a pull request: https://github.com/apache/tinkerpop/pull/471 TINKERPOP-1506: Optional/Coalesce should not allow sideEffect traversals. https://issues.apache.org/jira/browse/TINKERPOP-1506 `g.V(1).optional(addV())` created 2 vertices, not one. This is because `optional()` was implemented with the branch-step `ChooseStep`. This has been fixed with a custom `OptionalStep`. Test cases added to verify correct behavior. Added to `master/` as this is a breaking change. Though, the previous behavior can easily be deemed a bug. VOTE +1. You can merge this pull request into a Git repository by running: $ git pull https://github.com/apache/tinkerpop TINKERPOP-1506 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tinkerpop/pull/471.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #471 commit 40971c95903077d38cb754843eb914b5548f5c1f Author: Marko A. Rodriguez Date: 2016-10-31T18:03:33Z added OptionalStep to be used instead of optional() being backed by ChooseStep. This fixes a side-effect issue. > Optional/Coalesce should not allow sideEffect traversals. > - > > Key: TINKERPOP-1506 > URL: https://issues.apache.org/jira/browse/TINKERPOP-1506 > Project: TinkerPop > Issue Type: Improvement > Components: process >Affects Versions: 3.1.4, 3.2.2 >Reporter: Marko A. Rodriguez > > It took me a long time to realize what was wrong with a traversal I wrote > that used {{optional(blah.sideEffect.blah)}}. {{optional()}} maps to > {{ChooseStep}} under the hood and the provide traversal is first tested for a > {{hasNext()}}. If so, the it plays itself out. The problem is that if there > is a side-effect in the traversal child, then it gets executed twice. > {code} > gremlin> g = TinkerGraph.open().traversal() > ==>graphtraversalsource[tinkergraph[vertices:0 edges:0], standard] > gremlin> g.inject(1).optional(addV('twin')) > ==>v[1] > gremlin> g.V().valueMap(true) > ==>[id:0,label:twin] > ==>[id:1,label:twin] > {code} > We should NOT allow {{optional()}} to have {{SideEffectStep}} steps in it so > as not to cause unexpected behavior. {{StandardVerificationStrategy}} can > analyze and throw an exception if necessary. > Also, {{coalesce()}} has a similar problem, though perhaps it can be a useful > 'technique.' > {code} > gremlin> g = TinkerGraph.open().traversal() > ==>graphtraversalsource[tinkergraph[vertices:0 edges:0], standard] > gremlin> g.inject(1).coalesce(addV('twin1').limit(0), addV('twin2')) > ==>v[1] > gremlin> g.V().valueMap(true) > ==>[id:0,label:twin1] > ==>[id:1,label:twin2] > gremlin> > {code} -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (TINKERPOP-1506) Optional/Coalesce should not allow sideEffect traversals.
[ https://issues.apache.org/jira/browse/TINKERPOP-1506?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15597587#comment-15597587 ] Daniel Kuppitz commented on TINKERPOP-1506: --- Btw., I don't think we should disallow nested side-effect steps, instead we should make it right. The double execution of those nested traversals is a well hidden performance killer - neither {{explain()}} nor {{profile()}} can reveal that there's something that is executed twice. > Optional/Coalesce should not allow sideEffect traversals. > - > > Key: TINKERPOP-1506 > URL: https://issues.apache.org/jira/browse/TINKERPOP-1506 > Project: TinkerPop > Issue Type: Improvement > Components: process >Affects Versions: 3.1.4, 3.2.2 >Reporter: Marko A. Rodriguez > > It took me a long time to realize what was wrong with a traversal I wrote > that used {{optional(blah.sideEffect.blah)}}. {{optional()}} maps to > {{ChooseStep}} under the hood and the provide traversal is first tested for a > {{hasNext()}}. If so, the it plays itself out. The problem is that if there > is a side-effect in the traversal child, then it gets executed twice. > {code} > gremlin> g = TinkerGraph.open().traversal() > ==>graphtraversalsource[tinkergraph[vertices:0 edges:0], standard] > gremlin> g.inject(1).optional(addV('twin')) > ==>v[1] > gremlin> g.V().valueMap(true) > ==>[id:0,label:twin] > ==>[id:1,label:twin] > {code} > We should NOT allow {{optional()}} to have {{SideEffectStep}} steps in it so > as not to cause unexpected behavior. {{StandardVerificationStrategy}} can > analyze and throw an exception if necessary. > Also, {{coalesce()}} has a similar problem, though perhaps it can be a useful > 'technique.' > {code} > gremlin> g = TinkerGraph.open().traversal() > ==>graphtraversalsource[tinkergraph[vertices:0 edges:0], standard] > gremlin> g.inject(1).coalesce(addV('twin1').limit(0), addV('twin2')) > ==>v[1] > gremlin> g.V().valueMap(true) > ==>[id:0,label:twin1] > ==>[id:1,label:twin2] > gremlin> > {code} -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (TINKERPOP-1506) Optional/Coalesce should not allow sideEffect traversals.
[ https://issues.apache.org/jira/browse/TINKERPOP-1506?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15597581#comment-15597581 ] Daniel Kuppitz commented on TINKERPOP-1506: --- These comments still hold true. {{optional}} and {{coalesce}} are not interchangeable. {noformat} gremlin> g.V().coalesce(out("knows"), identity()).coalesce(out("created"), identity()).path() ==>[v[1],v[2],v[2]] ==>[v[1],v[4],v[5]] ==>[v[1],v[4],v[3]] ==>[v[2],v[2],v[2]] ==>[v[3],v[3],v[3]] ==>[v[4],v[4],v[5]] ==>[v[4],v[4],v[3]] ==>[v[5],v[5],v[5]] ==>[v[6],v[6],v[3]] gremlin> g.V().optional(out("knows")).optional(out("created")).path() ==>[v[1],v[2]] ==>[v[1],v[4],v[5]] ==>[v[1],v[4],v[3]] ==>[v[2]] ==>[v[3]] ==>[v[4],v[5]] ==>[v[4],v[3]] ==>[v[5]] ==>[v[6],v[3]] {noformat} > Optional/Coalesce should not allow sideEffect traversals. > - > > Key: TINKERPOP-1506 > URL: https://issues.apache.org/jira/browse/TINKERPOP-1506 > Project: TinkerPop > Issue Type: Improvement > Components: process >Affects Versions: 3.1.4, 3.2.2 >Reporter: Marko A. Rodriguez > > It took me a long time to realize what was wrong with a traversal I wrote > that used {{optional(blah.sideEffect.blah)}}. {{optional()}} maps to > {{ChooseStep}} under the hood and the provide traversal is first tested for a > {{hasNext()}}. If so, the it plays itself out. The problem is that if there > is a side-effect in the traversal child, then it gets executed twice. > {code} > gremlin> g = TinkerGraph.open().traversal() > ==>graphtraversalsource[tinkergraph[vertices:0 edges:0], standard] > gremlin> g.inject(1).optional(addV('twin')) > ==>v[1] > gremlin> g.V().valueMap(true) > ==>[id:0,label:twin] > ==>[id:1,label:twin] > {code} > We should NOT allow {{optional()}} to have {{SideEffectStep}} steps in it so > as not to cause unexpected behavior. {{StandardVerificationStrategy}} can > analyze and throw an exception if necessary. > Also, {{coalesce()}} has a similar problem, though perhaps it can be a useful > 'technique.' > {code} > gremlin> g = TinkerGraph.open().traversal() > ==>graphtraversalsource[tinkergraph[vertices:0 edges:0], standard] > gremlin> g.inject(1).coalesce(addV('twin1').limit(0), addV('twin2')) > ==>v[1] > gremlin> g.V().valueMap(true) > ==>[id:0,label:twin1] > ==>[id:1,label:twin2] > gremlin> > {code} -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (TINKERPOP-1506) Optional/Coalesce should not allow sideEffect traversals.
[ https://issues.apache.org/jira/browse/TINKERPOP-1506?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15597251#comment-15597251 ] pieter martin commented on TINKERPOP-1506: -- `coalesce` was [discussed|https://issues.apache.org/jira/browse/TINKERPOP-968] way back when `optional` was first discussed. [~dkuppitz] comment seems to show that `coalesce` was not what we wanted. [~okram] comment indicated "Ah smart. The reason choose works and coalesce doesn't is because one uses globalTraversals and the other uses localTraversals" Do there comments still hold? > Optional/Coalesce should not allow sideEffect traversals. > - > > Key: TINKERPOP-1506 > URL: https://issues.apache.org/jira/browse/TINKERPOP-1506 > Project: TinkerPop > Issue Type: Improvement > Components: process >Affects Versions: 3.1.4, 3.2.2 >Reporter: Marko A. Rodriguez > > It took me a long time to realize what was wrong with a traversal I wrote > that used {{optional(blah.sideEffect.blah)}}. {{optional()}} maps to > {{ChooseStep}} under the hood and the provide traversal is first tested for a > {{hasNext()}}. If so, the it plays itself out. The problem is that if there > is a side-effect in the traversal child, then it gets executed twice. > {code} > gremlin> g = TinkerGraph.open().traversal() > ==>graphtraversalsource[tinkergraph[vertices:0 edges:0], standard] > gremlin> g.inject(1).optional(addV('twin')) > ==>v[1] > gremlin> g.V().valueMap(true) > ==>[id:0,label:twin] > ==>[id:1,label:twin] > {code} > We should NOT allow {{optional()}} to have {{SideEffectStep}} steps in it so > as not to cause unexpected behavior. {{StandardVerificationStrategy}} can > analyze and throw an exception if necessary. > Also, {{coalesce()}} has a similar problem, though perhaps it can be a useful > 'technique.' > {code} > gremlin> g = TinkerGraph.open().traversal() > ==>graphtraversalsource[tinkergraph[vertices:0 edges:0], standard] > gremlin> g.inject(1).coalesce(addV('twin1').limit(0), addV('twin2')) > ==>v[1] > gremlin> g.V().valueMap(true) > ==>[id:0,label:twin1] > ==>[id:1,label:twin2] > gremlin> > {code} -- This message was sent by Atlassian JIRA (v6.3.4#6332)
Re: [jira] [Commented] (TINKERPOP-1506) Optional/Coalesce should not allow sideEffect traversals.
Hi, `coalesce` was discussed way back when `optional` was first discussed. @Daniel's comment seems to show that `coalesce` was not what we wanted. @Marko's comment indicated "Ah smart. The reason choose works and coalesce doesn't is because one uses globalTraversals and the other uses localTraversals" Do there comments still hold? Thanks Pieter On 21/10/2016 23:42, Marko A. Rodriguez (JIRA) wrote: > [ > https://issues.apache.org/jira/browse/TINKERPOP-1506?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15596463#comment-15596463 > ] > > Marko A. Rodriguez commented on TINKERPOP-1506: > --- > > Huh. I just reailzed we can implement {{optional()}} using {{ColesceStep}} > and we don't have this problem. > > {code} > gremlin> g.inject(1).coalesce(addV('twin'),identity()) > ==>v[0] > gremlin> g.V() > ==>v[0] > {code} > > Thus, {{optional(x)}} -> {{coalesce(x,identity())}}. Easy fix. Any objections > to this direction? > > >> Optional/Coalesce should not allow sideEffect traversals. >> - >> >> Key: TINKERPOP-1506 >> URL: https://issues.apache.org/jira/browse/TINKERPOP-1506 >> Project: TinkerPop >> Issue Type: Improvement >> Components: process >>Affects Versions: 3.1.4, 3.2.2 >>Reporter: Marko A. Rodriguez >> >> It took me a long time to realize what was wrong with a traversal I wrote >> that used {{optional(blah.sideEffect.blah)}}. {{optional()}} maps to >> {{ChooseStep}} under the hood and the provide traversal is first tested for >> a {{hasNext()}}. If so, the it plays itself out. The problem is that if >> there is a side-effect in the traversal child, then it gets executed twice. >> {code} >> gremlin> g = TinkerGraph.open().traversal() >> ==>graphtraversalsource[tinkergraph[vertices:0 edges:0], standard] >> gremlin> g.inject(1).optional(addV('twin')) >> ==>v[1] >> gremlin> g.V().valueMap(true) >> ==>[id:0,label:twin] >> ==>[id:1,label:twin] >> {code} >> We should NOT allow {{optional()}} to have {{SideEffectStep}} steps in it so >> as not to cause unexpected behavior. {{StandardVerificationStrategy}} can >> analyze and throw an exception if necessary. >> Also, {{coalesce()}} has a similar problem, though perhaps it can be a >> useful 'technique.' >> {code} >> gremlin> g = TinkerGraph.open().traversal() >> ==>graphtraversalsource[tinkergraph[vertices:0 edges:0], standard] >> gremlin> g.inject(1).coalesce(addV('twin1').limit(0), addV('twin2')) >> ==>v[1] >> gremlin> g.V().valueMap(true) >> ==>[id:0,label:twin1] >> ==>[id:1,label:twin2] >> gremlin> >> {code} > > > -- > This message was sent by Atlassian JIRA > (v6.3.4#6332)
[jira] [Commented] (TINKERPOP-1506) Optional/Coalesce should not allow sideEffect traversals.
[ https://issues.apache.org/jira/browse/TINKERPOP-1506?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15596463#comment-15596463 ] Marko A. Rodriguez commented on TINKERPOP-1506: --- Huh. I just reailzed we can implement {{optional()}} using {{ColesceStep}} and we don't have this problem. {code} gremlin> g.inject(1).coalesce(addV('twin'),identity()) ==>v[0] gremlin> g.V() ==>v[0] {code} Thus, {{optional(x)}} -> {{coalesce(x,identity())}}. Easy fix. Any objections to this direction? > Optional/Coalesce should not allow sideEffect traversals. > - > > Key: TINKERPOP-1506 > URL: https://issues.apache.org/jira/browse/TINKERPOP-1506 > Project: TinkerPop > Issue Type: Improvement > Components: process >Affects Versions: 3.1.4, 3.2.2 >Reporter: Marko A. Rodriguez > > It took me a long time to realize what was wrong with a traversal I wrote > that used {{optional(blah.sideEffect.blah)}}. {{optional()}} maps to > {{ChooseStep}} under the hood and the provide traversal is first tested for a > {{hasNext()}}. If so, the it plays itself out. The problem is that if there > is a side-effect in the traversal child, then it gets executed twice. > {code} > gremlin> g = TinkerGraph.open().traversal() > ==>graphtraversalsource[tinkergraph[vertices:0 edges:0], standard] > gremlin> g.inject(1).optional(addV('twin')) > ==>v[1] > gremlin> g.V().valueMap(true) > ==>[id:0,label:twin] > ==>[id:1,label:twin] > {code} > We should NOT allow {{optional()}} to have {{SideEffectStep}} steps in it so > as not to cause unexpected behavior. {{StandardVerificationStrategy}} can > analyze and throw an exception if necessary. > Also, {{coalesce()}} has a similar problem, though perhaps it can be a useful > 'technique.' > {code} > gremlin> g = TinkerGraph.open().traversal() > ==>graphtraversalsource[tinkergraph[vertices:0 edges:0], standard] > gremlin> g.inject(1).coalesce(addV('twin1').limit(0), addV('twin2')) > ==>v[1] > gremlin> g.V().valueMap(true) > ==>[id:0,label:twin1] > ==>[id:1,label:twin2] > gremlin> > {code} -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (TINKERPOP-1506) Optional/Coalesce should not allow sideEffect traversals.
[ https://issues.apache.org/jira/browse/TINKERPOP-1506?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15595686#comment-15595686 ] Marko A. Rodriguez commented on TINKERPOP-1506: --- So, we could make it so that {{next()}} returns the result from {{hasNext()}}. However, the way {{BranchStep}} is implemented (for which {{ChooseStep}} is a subclass (for which {{optional()}} is implemented with)), the selection traversal is not the same as the branch traversal. Where the former is a "local child" and the latter, a "global child." Thus, {{optional()}} uses {{clone()}} to get a selection and branch traversal. We could always write an {{OptionalStep}} which would do as you recommend and in fact, this might be the best way forward. > Optional/Coalesce should not allow sideEffect traversals. > - > > Key: TINKERPOP-1506 > URL: https://issues.apache.org/jira/browse/TINKERPOP-1506 > Project: TinkerPop > Issue Type: Improvement > Components: process >Affects Versions: 3.1.4, 3.2.2 >Reporter: Marko A. Rodriguez > > It took me a long time to realize what was wrong with a traversal I wrote > that used {{optional(blah.sideEffect.blah)}}. {{optional()}} maps to > {{ChooseStep}} under the hood and the provide traversal is first tested for a > {{hasNext()}}. If so, the it plays itself out. The problem is that if there > is a side-effect in the traversal child, then it gets executed twice. > {code} > gremlin> g = TinkerGraph.open().traversal() > ==>graphtraversalsource[tinkergraph[vertices:0 edges:0], standard] > gremlin> g.inject(1).optional(addV('twin')) > ==>v[1] > gremlin> g.V().valueMap(true) > ==>[id:0,label:twin] > ==>[id:1,label:twin] > {code} > We should NOT allow {{optional()}} to have {{SideEffectStep}} steps in it so > as not to cause unexpected behavior. {{StandardVerificationStrategy}} can > analyze and throw an exception if necessary. > Also, {{coalesce()}} has a similar problem, though perhaps it can be a useful > 'technique.' > {code} > gremlin> g = TinkerGraph.open().traversal() > ==>graphtraversalsource[tinkergraph[vertices:0 edges:0], standard] > gremlin> g.inject(1).coalesce(addV('twin1').limit(0), addV('twin2')) > ==>v[1] > gremlin> g.V().valueMap(true) > ==>[id:0,label:twin1] > ==>[id:1,label:twin2] > gremlin> > {code} -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (TINKERPOP-1506) Optional/Coalesce should not allow sideEffect traversals.
[ https://issues.apache.org/jira/browse/TINKERPOP-1506?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15573538#comment-15573538 ] Robert Dale commented on TINKERPOP-1506: I don't use optional() but I wonder if that's more of a bug. Why execute twice? Shouldn't next() just return the result of hasNext()? It almost sounds like this bug https://github.com/apache/tinkerpop/pull/346 Also, I disagree on coalesce(). It should continue to work as-is. It's very useful for upsert (update or insert) where you definitely want the second traversal to create an element. I'm open to a better way. But in general, just because you can shoot yourself in the foot, it doesn't make the tool defective. Maybe it would be better to put a NOTE: in the ref guides about doing such things to make to explicitly clear. > Optional/Coalesce should not allow sideEffect traversals. > - > > Key: TINKERPOP-1506 > URL: https://issues.apache.org/jira/browse/TINKERPOP-1506 > Project: TinkerPop > Issue Type: Improvement > Components: process >Affects Versions: 3.1.4, 3.2.2 >Reporter: Marko A. Rodriguez > > It took me a long time to realize what was wrong with a traversal I wrote > that used {{optional(blah.sideEffect.blah)}}. {{optional()}} maps to > {{ChooseStep}} under the hood and the provide traversal is first tested for a > {{hasNext()}}. If so, the it plays itself out. The problem is that if there > is a side-effect in the traversal child, then it gets executed twice. > {code} > gremlin> g = TinkerGraph.open().traversal() > ==>graphtraversalsource[tinkergraph[vertices:0 edges:0], standard] > gremlin> g.inject(1).optional(addV('twin')) > ==>v[1] > gremlin> g.V().valueMap(true) > ==>[id:0,label:twin] > ==>[id:1,label:twin] > {code} > We should NOT allow {{optional()}} to have {{SideEffectStep}} steps in it so > as not to cause unexpected behavior. {{StandardVerificationStrategy}} can > analyze and throw an exception if necessary. > Also, {{coalesce()}} has a similar problem, though perhaps it can be a useful > 'technique.' > {code} > gremlin> g = TinkerGraph.open().traversal() > ==>graphtraversalsource[tinkergraph[vertices:0 edges:0], standard] > gremlin> g.inject(1).coalesce(addV('twin1').limit(0), addV('twin2')) > ==>v[1] > gremlin> g.V().valueMap(true) > ==>[id:0,label:twin1] > ==>[id:1,label:twin2] > gremlin> > {code} -- This message was sent by Atlassian JIRA (v6.3.4#6332)