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