gortiz commented on code in PR #12783:
URL: https://github.com/apache/pinot/pull/12783#discussion_r1552031853


##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/SortOperator.java:
##########
@@ -129,16 +128,19 @@ private TransferableBlock produceSortedBlock() {
         return TransferableBlockUtils.getEndOfStreamTransferableBlock();
       }
     } else {
-      LinkedList<Object[]> rows = new LinkedList<>();
-      while (_priorityQueue.size() > _offset) {
-        Object[] row = _priorityQueue.poll();
-        rows.addFirst(row);
-      }
-      if (rows.size() == 0) {
+      int resultSize = _priorityQueue.size() - _offset;
+      if (resultSize == 0) {
         return TransferableBlockUtils.getEndOfStreamTransferableBlock();
-      } else {
-        return new TransferableBlock(rows, _dataSchema, DataBlock.Type.ROW);
       }
+      ArrayList<Object[]> rows = new ArrayList<>(resultSize);
+      for (int i = 0; i < resultSize; i++) {

Review Comment:
   It is needed because otherwise `ArrayList.set` will fail. The size used as 
argument in the constructor means the initial size of the array used under the 
hood (aka its capacity), but set requires requires the its first argument to be 
`0 <= arg <= list.size()`. By the time the first element is added, 
`list.size()` is 0, so when we add the latest element, even if the array used 
under the hood has capacity to store it, `ArrayList.set` throws an exception.
   
   Ideally we wouldn't need to do that, but AFAIK there is no way to do that in 
Java without reflection. I don't think the cost is actually problematic. 
Ideally the JIT can get rid of the loop (but I didn't try). In the worse case 
what we are doing is basically to increase the size counter in the array in a 
loop. No resize is going to be executed.



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

Reply via email to