[GitHub] [pinot] Jackie-Jiang commented on a change in pull request #7777: peel off special case for 1 dimensional groupby

[GitBox <gi...@apache.org>]

Jackie-Jiang commented on a change in pull request #7777:
URL: https://github.com/apache/pinot/pull/7777#discussion_r751725837



##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/groupby/DictionaryBasedGroupKeyGenerator.java
##########
@@ -612,7 +656,8 @@ public void processMultiValue(int numDocs, int[][] outGroupIds) {
     private int getGroupId(long rawKey) {
       int numGroups = _groupIdMap.size();
       if (numGroups < _globalGroupIdUpperBound) {
-        return _groupIdMap.computeIfAbsent(rawKey, k -> numGroups);
+        int id = _groupIdMap.putIfAbsent(rawKey, numGroups);
+        return id == _groupIdMap.defaultReturnValue() ? numGroups : id;

Review comment:
       Might be slightly faster to do
   ```suggestion
           return id == INVALID_ID ? numGroups : id;
   ```

##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/groupby/DictionaryBasedGroupKeyGenerator.java
##########
@@ -587,6 +617,20 @@ public LongMapBasedHolder(Long2IntOpenHashMap groupIdMap) {
 
     @Override
     public void processSingleValue(int numDocs, int[] outGroupIds) {
+      if (_numGroupByExpressions == 1) {

Review comment:
       We should never get into `LongMapBasedHolder` for single group-by expression

##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/groupby/DictionaryBasedGroupKeyGenerator.java
##########
@@ -253,12 +252,29 @@ public int getNumKeys() {
   }
 
   private class ArrayBasedHolder implements RawKeyHolder {
-    // TODO: using bitmap might better

Review comment:
       Should we consider using `BitSet` to replace this boolean array?




-- 
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-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org