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@gmail.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