walterddr commented on code in PR #9630:
URL: https://github.com/apache/pinot/pull/9630#discussion_r1005180715
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/planner/ExplainPlanStageVisitor.java:
##########
@@ -44,7 +44,7 @@
*/
public class ExplainPlanStageVisitor implements
StageNodeVisitor<StringBuilder, ExplainPlanStageVisitor.Context> {
- private final QueryPlan _queryPlan;
Review Comment:
i see your point. in this case i would suggest
- keep this line change (e.g. should keep stage `MetadadataMap` instead of
the `QueryPlan` as static context )
- make another entry point `public static String explain(DistributeStagePlan
distributedStagePlan)` and make sure it doesn't step beyond mailbox boundaries
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/plan/DistributedStagePlan.java:
##########
@@ -73,4 +74,9 @@ public void setServerInstance(ServerInstance serverInstance) {
public void setStageRoot(StageNode stageRoot) {
_stageRoot = stageRoot;
}
+
+ public String explain() {
+ return ExplainPlanStageVisitor.explainFrom(_metadataMap, _stageRoot,
_serverInstance);
Review Comment:
see my other comment. we should have an entrypoint for
`DistributedStagePlan`
and make `explainFrom` function private
without the context of whether it is explaining a stage plan or a global
plan, the explain result can be wrong.
--
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]
