[ https://issues.apache.org/jira/browse/TINKERPOP-3147?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17946194#comment-17946194 ]
ASF GitHub Bot commented on TINKERPOP-3147: ------------------------------------------- andreachild commented on code in PR #3096: URL: https://github.com/apache/tinkerpop/pull/3096#discussion_r2052794815 ########## gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/AggregateGlobalStep.java: ########## @@ -69,6 +69,10 @@ public String toString() { @Override public void modulateBy(final Traversal.Admin<?, ?> aggregateTraversal) { + if (this.aggregateTraversal != null) + { Review Comment: Nit: following existing syntax conventions this curly brace should be on the same line as the if statement > Prevent aggregate step from having multiple by modulators > --------------------------------------------------------- > > Key: TINKERPOP-3147 > URL: https://issues.apache.org/jira/browse/TINKERPOP-3147 > Project: TinkerPop > Issue Type: Improvement > Components: process > Affects Versions: 3.7.3 > Reporter: Andrea C > Priority: Minor > > Aggregate step will ignore previous by modulators if multiple are specified, > which can be misleading to the user. Instead, similar to > https://issues.apache.org/jira/browse/TINKERPOP-3121 multiple by modulators > should be prevented with aggregate. > {code:java} > gremlin> g.V().aggregate('x').by('name').cap('x') > ==>[marko,vadas,lop,josh,ripple,peter] > gremlin> g.V().aggregate('x').by('name').by('age').cap('x') > ==>[29,27,32,35] {code} > > Note that `store` step is an alias to `aggregate` and should also be > prevented from having multiple by modulators. -- This message was sent by Atlassian Jira (v8.20.10#820010)