[
https://issues.apache.org/jira/browse/TINKERPOP-3110?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17879216#comment-17879216
]
ASF GitHub Bot commented on TINKERPOP-3110:
-------------------------------------------
saikiranboga commented on code in PR #2757:
URL: https://github.com/apache/tinkerpop/pull/2757#discussion_r1743706799
##########
CHANGELOG.asciidoc:
##########
@@ -38,6 +38,7 @@ This release also includes changes from <<release-3-6-8,
3.6.8>>.
* Added getter method to `ConcatStep`, `ConjoinStep`, `SplitGlobalStep` and
`SplitLocalStep` for their private fields.
* Gremlin Server docker containers shutdown gracefully when receiving a
SIGTERM.
* TINKERPOP-3080 Support to specify Operator as a reducer in withSideEffect
when parsing with the grammar
+* TINKERPOP-3110 Fixed bug in Bytecode build logic for strategies
Review Comment:
Removed JIRA ref. and expanded on the bug.
> Incorrect Bytecode when multiple options are used in traversal
> --------------------------------------------------------------
>
> Key: TINKERPOP-3110
> URL: https://issues.apache.org/jira/browse/TINKERPOP-3110
> Project: TinkerPop
> Issue Type: Bug
> Components: server
> Reporter: Saikiran Boga
> Priority: Major
>
> When multiple options like with("x",1).with("y",1) are used to build a
> traversal, the Bytecode building logic when constructing the traversal on the
> server keeps appending the Bytecode incrementally instead of replacing the
> previous Bytecode instruction.
> For a query like below:
> {code:java}
> g
> .with('evaluationTimeout',300000L)
> .with('a', 1)
> .with('b', 2)
> .with('c', 3)
> .V(){code}
> The Bytecode attached to traversal looks as:
> {code:java}
> g
> .withStrategies(new
> org.apache.tinkerpop.gremlin.process.traversal.strategy.decoration.OptionsStrategy(evaluationTimeout:
> 300000L))
> .withStrategies(new
> org.apache.tinkerpop.gremlin.process.traversal.strategy.decoration.OptionsStrategy(evaluationTimeout:
> 300000L, a: (int) 1))
> .withStrategies(new
> org.apache.tinkerpop.gremlin.process.traversal.strategy.decoration.OptionsStrategy(evaluationTimeout:
> 300000L, a: (int) 1, b: (int) 2))
> .withStrategies(new
> org.apache.tinkerpop.gremlin.process.traversal.strategy.decoration.OptionsStrategy(evaluationTimeout:
> 300000L, a: (int) 1, b: (int) 2, c: (int) 3))
> .V() {code}
> Instead it should be just single instruction for OptionsStrategy as:
>
> {code:java}
> g
> .withStrategies(new
> org.apache.tinkerpop.gremlin.process.traversal.strategy.decoration.OptionsStrategy(evaluationTimeout:
> 300000L, a: (int) 1, b: (int) 2, c: (int) 3))
> .V() {code}
>
> We always build a new OptionsStrategy here and carry over the previous
> options
> [https://github.com/apache/tinkerpop/blob/master/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/TraversalSource.java#L116-L122]
> as OptionsStrategy itself is immutable and we cannot add new options after
> creating it.
>
> We build the traversal strategies set and bytecode here:
> [https://github.com/apache/tinkerpop/blob/master/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/TraversalSource.java#L132-L133]
>
> The traversal strategies set itself is built correctly and we remove
> duplicates
> [https://github.com/apache/tinkerpop/blob/master/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/DefaultTraversalStrategies.java#L42-L50]
>
> But Bytecode just appends the new strategy as instruction
> [https://github.com/apache/tinkerpop/blob/master/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Bytecode.java#L82-L83]
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
