Skip site navigation (1) Skip section navigation (2)

auto_explain contrib moudle

From: ITAGAKI Takahiro <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp>
To: "Alex Hunsaker" <badalex(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: auto_explain contrib moudle
Date: 2008-10-09 10:06:18
Message-ID: 20081009165157.9BE4.52131E4D@oss.ntt.co.jp (view raw or flat)
Thread:
Lists: pgsql-hackers
Thanks for your reviewing, Alex.
I applied your comments to my patch.

- auto_explain.patch : patch against HEAD
- auto_explain.tgz   : contrib module
- autoexplain.sgml   : documentation

"Alex Hunsaker" <badalex(at)gmail(dot)com> wrote:

> *custom_guc_flags-0828.patch
> My only other concern is the changes to DefineCustom*() to tag the new
> flags param.  Now I think anyone who uses Custom gucs will want/should
> be able to set that. I did not see any people in contrib using it but
> did not look on PGfoundry.  Do we need to document the change
> somewhere for people who might be using it???

Now it is done with DefineCustomVariable(type, variable) and keep
existing functions as-is for backward compatibility.

Some people will be happy if the functions are documented,
but we need to define 'stable-internal-functions' between
SPI (stable expoted functions) and unstable internal functions.


> *psql_ignore_notices-0828.patch:
> I think this should get dropped.

Hmm, I agree that hiding all messages is not good. I removed it.
If some people need it, we can reconsider it in a separated discussion.


> *auto_explalin.c:
> init_instrument()
> The only "cleaner" way I can
> see is to add a hook for CreateQueryDesc so we can overload
> doInstrument and ExecInitNode will InstrAlloc them all for us.

I wanted to avoid modifying core codes as far as possible,
but I see it was ugly. Now I added 'force_instrument' global
variable as a hook for CreateQueryDesc.


> the only other comment I have is suset_assign() do we really need to
> be a superuser if its all going to LOG ? There was some concern about
> explaining security definer functions right? but surely a regular
> explain on those shows the same thing as this explain?  Or what am I
> missing?

Almost logging options in postgres are only for superusers. So I think
auto_explain options should not be modified by non-superusers, too.

If you want to permit usage for users, you can create a security definer
wrapper function to allow it, no?

  CREATE FUNCTION set_explain_log_min_duration(text) RETURNS text AS
    $$ SELECT set_config('explain.log_min_duration', $1, false); $$
  LANGUAGE sql SECURITY DEFINER;

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



Attachment: autoexplain.sgml
Description: application/octet-stream (4.1 KB)
Attachment: auto_explain.tgz
Description: application/octet-stream (1.8 KB)
Attachment: auto_explain.patch
Description: application/octet-stream (13.9 KB)

In response to

Responses

pgsql-hackers by date

Next:From: Matthew WakelingDate: 2008-10-09 10:10:08
Subject: Re: CREATE DATABASE vs delayed table unlink
Previous:From: Zdenek KotalaDate: 2008-10-09 07:38:22
Subject: Re: [WIP] Reduce alignment requirements on 64-bit systems.

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group