[ 
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)

Reply via email to