gortiz commented on code in PR #14507:
URL: https://github.com/apache/pinot/pull/14507#discussion_r1895374402


##########
pinot-query-planner/src/main/java/org/apache/pinot/query/planner/physical/DispatchablePlanVisitor.java:
##########
@@ -37,10 +40,7 @@
 
 
 public class DispatchablePlanVisitor implements PlanNodeVisitor<Void, 
DispatchablePlanContext> {
-  public static final DispatchablePlanVisitor INSTANCE = new 
DispatchablePlanVisitor();
-
-  private DispatchablePlanVisitor() {
-  }
+  private final Set<MailboxSendNode> _visited = Collections.newSetFromMap(new 
IdentityHashMap<>());

Review Comment:
   Given we have spools now, a mailbox send may be visited by this visitor 
twice. This keeps track of the mailbox send nodes that have been visited to do 
not visit them again. That is unnecessary and may be problematic (called code 
may assume each mailbox is visited only once. I don't remember the kind of 
errors this may produce, but probably I wouldn't have detected it in case it 
didn't fail :laughing: 



-- 
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

Reply via email to