On Thu, Apr 3, 2025 at 1:32 AM torikoshia <torikoshia@oss.nttdata.com> wrote:
> I think tracking execution progress involves more challenges to solve
> compared to simply outputting the plan.
> For this reason, I believe an incremental approach -- first completing
> the basic plan output functionality in this thread and then extending it
> to support progress tracking -- would be the good way forward.
I think you might be right, although I am not totally certain yet.
> However, for the next patch, I'm considering introducing a GUC to allow
> prior setup before outputting the plan, in response to the previously
> quoted comment:
>
> One way in which this proposal seems safer than previous proposals is
> that previous proposals have involved session A poking session B and
> trying to get session B to emit an EXPLAIN on the fly with no prior
> setup. That would be very useful, but I think it's more difficult and
> more risky than this proposal, where all the configuration happens in
> the session that is going to emit the EXPLAIN output.
When I wrote this comment, I was unaware of Andres's proposal to use
ProcessInterrupts() to install the ExecProcNode() wrapper. With that
approach, which you have already implemented, I don't see a reason to
require prior configuration.
> With this change, it should be possible to use a file-level variable
> instead.
I think the question of whether ActiveQueryDesc can be file-level is
separate from whether prior configuration is needed. If it is
important to touch this from multiple source files, then it is fine
for it to be global. However, if we have a new source file, say
dynamic_explain.c, then you could have functions
ProcessDynamicExplainInterrupt() and DynamicExplainCleanup() in that
file to set, use, clear ActiveQueryDesc, and the rest of the system
might not need to know about it.
--
Robert Haas
EDB: http://www.enterprisedb.com
