walterddr commented on code in PR #11932:
URL: https://github.com/apache/pinot/pull/11932#discussion_r1397700997
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/combine/GroupByCombineOperator.java:
##########
@@ -188,6 +186,14 @@ protected void processSegments() {
mergedKeys++;
}
}
+
+ // Set groups limit reached flag.
+ if (resultsBlock.isNumGroupsLimitReached()) {
Review Comment:
both the numsGroupsLimit config and the trim threshold on the lookupMap of
the index table are describing the maximum number of keys allows in the group
by operator.
i was wondering what's the reason to have the 2 separate in the first place?
logically speaking they seems the same to me, can't we use just 1?
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/combine/GroupByCombineOperator.java:
##########
@@ -188,6 +186,14 @@ protected void processSegments() {
mergedKeys++;
}
}
+
+ // Set groups limit reached flag.
+ if (resultsBlock.isNumGroupsLimitReached()) {
Review Comment:
both the numsGroupsLimit config and the trim threshold on the lookupMap of
the index table are describing the maximum number of keys allows in the group
by operator.
i was wondering what's the reason to have the 2 separate in the first place?
logically speaking they seems the same to me, can't we unify them?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
