npawar commented on code in PR #10172:
URL: https://github.com/apache/pinot/pull/10172#discussion_r1087164008
##########
pinot-core/src/main/java/org/apache/pinot/core/query/executor/ServerQueryExecutorV1Impl.java:
##########
@@ -475,6 +476,10 @@ private static void getSegmentExplainPlanRowData(Operator
node, ExplainPlanRows
}
}
+ if (node instanceof AcquireReleaseColumnsSegmentOperator) {
+ ((AcquireReleaseColumnsSegmentOperator) node).materializeChildOperator();
Review Comment:
Hi @saurabhd336 ,
Jackie and I just discussed this.
Ideally we want the contract to be, that prefetch/acquire/release has
happened before any physical access related to the query. While just accessing
planNodes (which is the case for explain plan, similar to our planning phase)
shouldn't really do physical access, we previously discovered that it does do
physical access (as you pointed out in your example above). So in a sense, the
planning we have is "physical planning", and the explain plan is actually an
"explain physical plan".
In case of the planning part, we overcame this by passing the planNode to
the AcquireReleaseOperator and having it run within the getNextBlock, after
prefetch/acquire has already happened.
It would be good if we can maintain the same contract for the planNode.run
that's happening in explain physical plan i.e. ensure prefetch/acquire is done
and a release is called after.
Could we add a new API to all operators, something like "getExplainPlan" and
move all this logic into the respective operators? That way we'd avoid all of
this if-else happening here (even for EmptyFilterOperator and
MatchAllFilterOperator). In the InstanceResponseOperator's getExplainPlan
method, we'd prefetch and release (similar to getNextBlock), and in
AcquireRelease we'd acquire/release
--
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]
