From: | ITAGAKI Takahiro <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp> |
---|---|
To: | Dean Rasheed <dean_rasheed(at)hotmail(dot)com> |
Cc: | <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Auto-explain patch |
Date: | 2008-09-01 09:47:10 |
Message-ID: | 20080901182220.4A20.52131E4D@oss.ntt.co.jp |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Dean Rasheed <dean_rasheed(at)hotmail(dot)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
From | Date | Subject | |
---|---|---|---|
Next Message | Heikki Linnakangas | 2008-09-01 10:59:49 | Re: Attaching error cursor position to invalid constant values |
Previous Message | Pavel Stehule | 2008-09-01 09:15:56 | Re: Is this really really as designed or defined in some standard |