Re: RFC: Logging plan of the running query

On Mon, Jun 29, 2026 at 1:23 PM torikoshia <torikoshia(at)oss(dot)nttdata(dot)com>
wrote:
>
> On Wed, Jun 24, 2026 at 11:35 PM Andrei Lepikhov <lepihov(at)gmail(dot)com>
> wrote:
> Thanks for your review!
>
> > 2. GetCurrentQueryDesc, SetCurrentQueryDesc - need 'extern' in the .h
> > file
> > 3. ExplainPrintPlan, ExplainPrintTriggers - have exact duplicates in
> > the explain.h
> > 4. ExplainStringAssemble() has '0' input for a boolean parameter.
> > 5. ExecSetExecProcNodeRecurse: I don't see PlanState::initPlan. Is this
> > intentional?
> > 6. INJECTION_POINT("log-query-interrupt", ...) - it is called inside a
> > signal-safe zone. In tests, the caller might do a lot of things in the
> > attached
> > routine, causing hidden problems later.
>
> I agree that these points should be addressed.
> I'll post an updated patch.

Fixed these points.

> I've also received some off-list feedback, and I'm planning to
> incorporate those comments into the updated patch as well.

I also made the following changes:

- Updated the documentation to mention that pg_log_query_plan() is
asynchronous and that plan logging can be delayed if the target backend
does not reach an executor point where the plan can be inspected.
- Fixed LogQueryPlan() so that its temporary memory context is always
restored and deleted, even when there is no current QueryDesc.
- Added defensive checks for pgstat_get_beentry_by_proc_number()
returning NULL, and verified that the backend status entry still matches
the requested PID.
- Wrapped the main LogQueryPlan() body in PG_TRY/PG_FINALLY so
LogQueryPlanPending and memory context cleanup are handled even if
EXPLAIN generation throws an error.

On Wed, Jul 1, 2026 at 7:42 PM Andrei Lepikhov <lepihov(at)gmail(dot)com>
wrote:
> I took a closer look at the solution

Thanks for the comments!
I will mainly consider the 3rd and 4th points.

Regards,

--
Atsushi Torikoshi
Seconded from NTT DATA CORPORATION to SRA OSS K.K.