walterddr commented on code in PR #11937:
URL: https://github.com/apache/pinot/pull/11937#discussion_r1385969803
##########
pinot-query-planner/src/main/java/org/apache/calcite/rel/rules/PinotRuleUtils.java:
##########
@@ -68,19 +69,28 @@ public static boolean isAggregate(RelNode rel) {
}
// TODO: optimize this part out as it is not efficient to scan the entire
subtree for exchanges.
- public static boolean noExchangeInSubtree(RelNode relNode) {
- if (relNode instanceof HepRelVertex) {
- relNode = ((HepRelVertex) relNode).getCurrentRel();
- }
- if (relNode instanceof Exchange) {
+ public static boolean canPushDynamicBroadcastToLeaf(RelNode relNode) {
+ relNode = PinotRuleUtils.unboxRel(relNode);
+ if (relNode instanceof TableScan) {
+ // reaching table means it is plannable.
+ return true;
+ } else if (relNode instanceof Exchange) {
+ // we do not allow any exchanges in between join and table scan.
return false;
- }
- for (RelNode child : relNode.getInputs()) {
- if (!noExchangeInSubtree(child)) {
+ } else if (relNode instanceof Join) {
+ // always check only the left child for dynamic broadcast
+ return canPushDynamicBroadcastToLeaf(((Join) relNode).getLeft());
+ } else if (relNode instanceof Aggregate) {
+ // if there's aggregation before join, join cannot be planned as dynamic
broadcast.
+ return false;
Review Comment:
there's no need to differentiate. from a top-down walk. the moment you reach
agg, you are guarantee that you have join on top of an aggregate (b/c the
starting point of this call is a join
--
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
