walterddr commented on code in PR #10570:
URL: https://github.com/apache/pinot/pull/10570#discussion_r1160771190
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/plan/PhysicalPlanVisitor.java:
##########
@@ -61,12 +63,20 @@ public static OpChain build(StageNode node,
PlanRequestContext context) {
@Override
public MultiStageOperator visitMailboxReceive(MailboxReceiveNode node,
PlanRequestContext context) {
- MailboxReceiveOperator mailboxReceiveOperator =
- new MailboxReceiveOperator(context.getOpChainExecutionContext(),
node.getExchangeType(),
- node.getCollationKeys(), node.getCollationDirections(),
node.isSortOnSender(), node.isSortOnReceiver(),
- node.getDataSchema(), node.getSenderStageId(), node.getStageId());
- context.addReceivingMailboxes(mailboxReceiveOperator.getSendingMailbox());
- return mailboxReceiveOperator;
+ if (!CollectionUtils.isEmpty(node.getCollationKeys()) &&
node.isSortOnReceiver()) {
Review Comment:
> So now with my changes the following will hold:
>
> * `node.isSortOnReceiver() == true`: only set if actual sorting needs to
be performed on the receiver
>
> * Window functions that need ORDER BY
> * ORDER BY queries that have at least one collation key
> * `node.isSortOnReceiver() == false`: set when sorting not required on
receiver side.
> * Can be `false` if collation key is non-empty (we don't use this code
path today) such as when in the future we have partitioned data and decide to
sort on sender side only.
[RR] hmm. could you explain more as: under what circumstances will this be
true? did you mean there's an "ORDER BY " clause in the SQL, or there is
actually a collation key specified on the SortExchange RelNode after Calcite
has finish optimizer phase?
>
> The `SortNode` will still be created where needed, just if it's child
operator is sorted then we set that the input is already sorted, otherwise we
set input is not sorted (just as before). This discussion is more around when
to create the sorted mailbox receive vs. non-sorted one
[RR] the above-described behavior is identical to prior to
https://github.com/apache/pinot/pull/10408 yes? just instead of doing the
sorting in `SortExchange`, previously we will create a SortNode on top of the
Exchange Node?
--
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]
