siddharthteotia commented on code in PR #8558:
URL: https://github.com/apache/pinot/pull/8558#discussion_r855374289
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/planner/StagePlanner.java:
##########
@@ -67,11 +68,12 @@ public QueryPlan makePlan(RelNode relRoot) {
StageNode globalStageRoot = walkRelPlan(relRoot, getNewStageId());
// global root needs to send results back to the ROOT, a.k.a. the client
response node.
- // the last stage is always a broadcast-gather.
+ // the last stage only has one receiver so doesn't matter what the
exchange type is.
Review Comment:
I agree that the last exchange at the top (between sender(s) in 1st stage
and single receiver in 0th stage) can be anything. May be
`RelDistribution.Type.ANY` is more suitable ?
Slightly related question - I think exchange type information is enough to
decide the kind of sender (like right now it will be broadcast, later it could
be shuffle / hash partition etc).
How do we decide the kind of exact receiver ? Especially for the receiver in
the root stage, we ideally want to know if the receiver is unordered or ordered
for simple v/s sorted merge respectively ?
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/planner/StagePlanner.java:
##########
@@ -95,12 +97,15 @@ private StageNode walkRelPlan(RelNode node, int
currentStageId) {
if (isExchangeNode(node)) {
// 1. exchangeNode always have only one input, get its input converted
as a new stage root.
StageNode nextStageRoot = walkRelPlan(node.getInput(0), getNewStageId());
- RelDistribution.Type exchangeType = ((LogicalExchange)
node).distribution.getType();
+ RelDistribution distribution = ((LogicalExchange)
node).getDistribution();
+ RelDistribution.Type exchangeType = distribution.getType();
// 2. make an exchange sender and receiver node pair
- StageNode mailboxReceiver = new MailboxReceiveNode(currentStageId,
nextStageRoot.getStageId(), exchangeType);
- StageNode mailboxSender = new
MailboxSendNode(nextStageRoot.getStageId(), mailboxReceiver.getStageId(),
+ StageNode mailboxReceiver = new MailboxReceiveNode(currentStageId,
node.getRowType(), nextStageRoot.getStageId(),
exchangeType);
+ StageNode mailboxSender = new
MailboxSendNode(nextStageRoot.getStageId(), node.getRowType(),
+ mailboxReceiver.getStageId(), exchangeType, exchangeType ==
RelDistribution.Type.HASH_DISTRIBUTED
+ ? new FieldSelectionKeySelector(distribution.getKeys().get(0)) :
null);
Review Comment:
I thought initially we are going to support only broadcast based
distribution / exchange ?
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/planner/RelToStageConverter.java:
##########
@@ -24,16 +24,18 @@
import java.util.stream.Collectors;
import org.apache.calcite.rel.RelNode;
import org.apache.calcite.rel.core.JoinRelType;
-import org.apache.calcite.rel.logical.LogicalCalc;
Review Comment:
Is LogicalCalc even useful ? It is essentially same functionality provided
together by LogicalProject and LogicalFilter
--
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]
