[GitHub] [druid] abhishekagarwal87 commented on a change in pull request #10685: Subtotal queries give incorrect results if query has a limit spec

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

abhishekagarwal87 commented on a change in pull request #10685:
URL: https://github.com/apache/druid/pull/10685#discussion_r544939164



##########
File path: processing/src/main/java/org/apache/druid/query/groupby/epinephelinae/ByteBufferMinMaxOffsetHeap.java
##########
@@ -59,6 +59,32 @@ public ByteBufferMinMaxOffsetHeap(
     this.heapIndexUpdater = heapIndexUpdater;
   }
 
+  public ByteBufferMinMaxOffsetHeap copy()

Review comment:
       Let's also add some unit tests for this method. 

##########
File path: processing/src/main/java/org/apache/druid/query/groupby/epinephelinae/LimitedBufferHashGrouper.java
##########
@@ -318,6 +336,8 @@ public void close()
   private CloseableIterator<Entry<KeyType>> makeHeapIterator()
   {
     final int initialHeapSize = offsetHeap.getHeapSize();
+    ByteBufferMinMaxOffsetHeap newHeap = offsetHeap.copy();

Review comment:
       can you add a comment here on why we are creating another copy? 

##########
File path: processing/src/main/java/org/apache/druid/query/groupby/epinephelinae/LimitedBufferHashGrouper.java
##########
@@ -318,6 +336,8 @@ public void close()
   private CloseableIterator<Entry<KeyType>> makeHeapIterator()

Review comment:
       Please add some unit tests in the `LimitedBufferHashGrouperTest` where an iterator is created multiple times and still returns the same results. 




----------------------------------------------------------------
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.

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



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