On 13/07/2026 15:35, torikoshia wrote:
> On 2026-07-13 08:28, Andrei Lepikhov wrote:
>> So, it potentially makes sense to add a statistics field to the activity
>> stat to
>> let users know whether the logging (and how many signals, if replace flag
>> with a
>> counter) is awaiting execution.
>
> That might be useful for some users, but personally I feel that adding
> a column to pg_stat_activity solely for pg_log_query_plan() might be
> excessive.
That part is just an idea. I think it might help if the activity statistics had
a variable-length JSON field where the backend could write some rarely used
flags.
>
>> Maybe it doesn't need to clean up the 'pending' flag at all if no query has
>> been
>> executed yet - the query might be under planning, and the EXPLAIN will be
>> logged
>> right at the beginning of execution.
>
> Without this cleanup, as the comment says, the plan could instead be
> logged when a different query is subsequently executed in the same
> session. I imagine users don't expect this behavior.
Hmmm, asynchronous feature that explains an arbitrary part of the query - for
example, some query inside an evaluated plpgsql function ... I think it is
expected by design.
Also, this feature prints the query, so there is no room for a mismatch. I think
it should print queryId and let people identify specific EXPLAIN more easily -
imagine GB-scale log with multiple explains. The default 'verbose' mode could
also be helpful.
> + ereport(LOG,
> + errmsg("query plan logging was requested but
> there was no opportunity to do it"));
> + LogQueryPlanPending = false;
> + }
> }
>
> What do you think about this approach?
It reduces uncertainty but I think we can do more.
>> In addition, I wonder why this code doesn't use standard planstate walker. It
>> seems to me that something like the following should work:
>
> As discussed in [2], the set of nodes handled here slightly differs
> from that handled by planstate_tree_walker_impl(), so I'd like
> to confirm whether it is appropriate to use the standard walker here.
As correctly mentioned in [2], we don't need any special processing for CTEScan.
So, I think the planstate walker is a correct tool to pass the tree.
I think this tool should be as predictable as possible. People will often use it
when something has already gone wrong. Don't add more stress and uncertainty if
it's doable.
--
regards, Andrei Lepikhov,
pgEdge
