On Thu, Oct 4, 2018 at 12:33 AM Andres Freund <and...@anarazel.de> wrote: > > Hi, > > There's a few cases where by the time ExecutorFinish() is called, > ExecShutdownNode() has not yet been called. As, among other reasons, > ExecShutdownNode() also collects instrumentation, I think that's > problematic. > > In ExplainOnePlan() we call > > /* run cleanup too */ > ExecutorFinish(queryDesc); > > and then print the majority of the explain data: > > /* Create textual dump of plan tree */ > ExplainPrintPlan(es, queryDesc); > > and only then shut down the entire query: > > ExecutorEnd(queryDesc); > > which seems to mean that if a node hasn't yet been shut down for some > reason, we'll not have information that's normally collected in > ExecShutdownNode(). >
Ideally, ExecShutdownNode would have been called by the end of ExecutorRun phase. Can you tell which particular case you are bothered about? > ISTM, we should have a new EState member that runs ExecShutdownNode() if > in standard_ExecutorEnd() if not already done. > Even if it wasn't called before due to any reason, I think the relevant work should be done via standard_ExecutorEnd->ExecEndPlan->ExecEndNode. For example, for gather node, it will call ExecEndGather to perform the necessary work. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
