Re: Record queryid when auto_explain.log_verbose is on - Mailing list pgsql-hackers

From Michael Paquier
Subject Re: Record queryid when auto_explain.log_verbose is on
Date
Msg-id Y9DeXI/Ukv2Lebmu@paquier.xyz
Whole thread Raw
In response to Re: Record queryid when auto_explain.log_verbose is on  (torikoshia <torikoshia@oss.nttdata.com>)
Responses Re: Record queryid when auto_explain.log_verbose is on
List pgsql-hackers
On Tue, Jan 24, 2023 at 11:01:46PM +0900, torikoshia wrote:
> On 2023-01-23 09:35, Michael Paquier wrote:
>> ExplainPrintTriggers() is kind of different because there is
>> auto_explain_log_triggers.  Still, we could add a flag in ExplainState
>> deciding if the triggers should be printed, so as it would be possible
>> to move ExplainPrintTriggers() and ExplainPrintJITSummary() within
>> ExplainPrintPlan(), as well?  The same kind of logic could be applied
>> for the planning time and the buffer usage if these are tracked in
>> ExplainState rather than being explicit arguments of ExplainOnePlan().
>> Not to mention that this reduces the differences between
>> ExplainOneUtility() and ExplainOnePlan().
>
> Hmm, this refactoring would worth considering, but should be done in another
> patch?

It could be.  That's fine by me to not do that as a first step as the
query ID computation is done just after ExplainPrintPlan().  An
argument could be made about ExplainPrintPlan(), though
compute_query_id = regress offers an option to control that, as well.

>> Leaving this comment aside, I think that this should have at least one
>> regression test in 001_auto_explain.pl, where query_log() can be
>> called while the verbose GUC of auto_explain is enabled.
>
> Agreed.
> Added a test for queryid logging.

Thanks.  Will check and probably apply on HEAD.
--
Michael

Attachment

pgsql-hackers by date:

Previous
From: Michael Paquier
Date:
Subject: Re: recovery modules
Next
From: Peter Eisentraut
Date:
Subject: Re: drop postmaster symlink