Re: Auto-explain patch - Mailing list pgsql-hackers

From ITAGAKI Takahiro
Subject Re: Auto-explain patch
Date
Msg-id 20080901182220.4A20.52131E4D@oss.ntt.co.jp
Whole thread Raw
In response to Re: Auto-explain patch  (Dean Rasheed <dean_rasheed@hotmail.com>)
List pgsql-hackers
Dean Rasheed <dean_rasheed@hotmail.com> wrote:

> > An arguable part is initializing instruments in ExecutorRun_hook.
> > The initialization should be done in ExecutorStart normally, but
> > it is too late in the hook. Is it safe? or are there any better idea?
> 
> How about adding a new hook to control instrumentation of queries in
> ExecutorStart? Something like:
> 
> typedef bool (*ExecutorDoInstrument_hook_type) (QueryDesc *queryDesc, int eflags);
> extern PGDLLIMPORT ExecutorDoInstrument_hook_type ExecutorDoInstrument_hook;

I think it is not good to have any single-purpose hooks -- a new global
variable "bool force_instrument" would be enough for the purpose ;-)

I'd like to suggest on-demand allocation of instruments instead.
PlanState->instrument is not only a runtime statstics collector, but also
represents whether instrumentation is enabled or not. However, we also
have the same information in EState->es_insrument. If we use it instread
of NULL check, we could initialize instrument fields in executor.

[src/backend/executor/execProcnode.c]
ExecProcNode(PlanState *node)
{   ...   if (node->state->es_instrument)   {       if (node->instrument == NULL)           node->instrument =
InstrAlloc(1,long_lived_memory_context);       InstrStartNode(node->instrument);   }
 

Regards,
---
ITAGAKI Takahiro
NTT Open Source Software Center




pgsql-hackers by date:

Previous
From: "Pavel Stehule"
Date:
Subject: Re: Is this really really as designed or defined in some standard
Next
From: Heikki Linnakangas
Date:
Subject: Re: Attaching error cursor position to invalid constant values