gortiz commented on code in PR #17323:
URL: https://github.com/apache/pinot/pull/17323#discussion_r2591648557
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/planner/physical/MailboxAssignmentVisitor.java:
##########
@@ -187,6 +188,12 @@ private void connectWorkers(int stageId, Map<Integer,
QueryServerInstance> serve
List<Integer> workerIds = entry.getValue();
mailboxInfoList.add(new MailboxInfo(server.getHostname(),
server.getQueryMailboxPort(), workerIds));
}
+ // Sort by first workerId to ensure deterministic ordering.
+ // This is critical for hash-distributed exchanges where (hash %
numMailboxes) is used as an index to choose the
+ // receiving worker.
+ // Without this sorting, different stages could route the same hash value
to different workers, resulting in
+ // incorrect join/union/intersect results for pre-partitioned sends (where
a full partition shuffle is skipped).
+ mailboxInfoList.sort(Comparator.comparingInt(info ->
info.getWorkerIds().get(0)));
Review Comment:
Are we sure the worker ids is going to have exactly one instance? Shouldn't
we sort by all elements in case the first element is equal?
--
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]
