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
signature.asc
Description: PGP signature
