On Fri, Jan 20, 2023 at 12:32:58PM +0900, Michael Paquier wrote: > FWIW, no objections from here. This maps with EXPLAIN where the query > ID is only printed under VERBOSE.
While looking at this change, I have been wondering about something.. Isn't the knowledge of the query ID something that should be pushed within ExplainPrintPlan() so as we don't duplicate in two places the checks that show it? In short, the patch ignores the case where compute_query_id = regress in auto_explain. 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(). 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. -- Michael
signature.asc
Description: PGP signature