somandal commented on code in PR #10408:
URL: https://github.com/apache/pinot/pull/10408#discussion_r1136538505
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/MailboxReceiveOperator.java:
##########
@@ -161,7 +194,12 @@ protected TransferableBlock getNextBlock() {
return _upstreamErrorBlock;
}
if (!block.isEndOfStreamBlock()) {
- return block;
+ if (_priorityQueue != null) {
+ List<Object[]> container = block.getContainer();
+ _priorityQueue.addAll(container);
+ } else {
+ return block;
+ }
Review Comment:
So the code here does return a no-op block (after exiting out of the loop,
i.e processing all the mailboxes in the `_sendingMailbox` list). Are you
recommending instead to return a no-op block for each mailbox? (at least it
seems so from the recommended change)
I've created the util as recommended and it explicitly returns a no-op block
after adding all the rows to the `PriorityQueue` if the data needs to be
ordered for that given mailbox.
Any reason why we should not keep processing all mailboxes and instead
return a no-op for each mailbox in the `_sendingMailbox` list as recommended?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
