Re: [HACKERS] auto_explain contrib moudle

2008-11-18 Thread Tom Lane
ITAGAKI Takahiro <[EMAIL PROTECTED]> writes: > Here is an update version of contrib/auto_explain patch. Applied with some editorialization, mostly around the GUC stuff. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make change

Re: [HACKERS] auto_explain contrib moudle

2008-11-17 Thread ITAGAKI Takahiro
Here is an update version of contrib/auto_explain patch. Now it uses new ExecutorStart_hook and ExecutorEnd_hook. When we execute queries using cursor, FETCHes are accumulated and reported only once on CLOSE. A new argument 'flags' is added in DefineCustomXXXVariable() and custom GUC variables ar

Re: [HACKERS] auto_explain contrib moudle

2008-11-17 Thread Tom Lane
ITAGAKI Takahiro <[EMAIL PROTECTED]> writes: > Tom Lane <[EMAIL PROTECTED]> wrote: >> This would likely require adding a struct Instrumentation * field to >> QueryDesc in which to track the total ExecutorRun timing > I think instr_time is enough here, > but why do you think Instrumentation is need

Re: [HACKERS] auto_explain contrib moudle

2008-11-17 Thread ITAGAKI Takahiro
I wrote: > There might be another approach to have a stopwatch stack in > the contrib module instead of the core. I think it is cleaner > because it works even if multiple modules use the stopwatch > at the same time. Am I missing something? Ooops, it should be: ... because each multiple module

Re: [HACKERS] auto_explain contrib moudle

2008-11-17 Thread ITAGAKI Takahiro
Ok, I'm looking at the direction of ExecutorStart/End hooks... Tom Lane <[EMAIL PROTECTED]> wrote: > This would likely require adding a struct Instrumentation * field to > QueryDesc in which to track the total ExecutorRun timing I think instr_time is enough here, but why do you think Instrumenta

Re: [HACKERS] auto_explain contrib moudle

2008-11-16 Thread Tom Lane
Jeff Davis <[EMAIL PROTECTED]> writes: > On Mon, 2008-11-10 at 18:02 +0900, ITAGAKI Takahiro wrote: >> That's because explain.log_analyze requires executor instruments, >> and it's not free. I think we'd better to have an option to avoid >> the overheads... Oops, I found my bug when force_instrumen

Re: [HACKERS] auto_explain contrib moudle

2008-11-14 Thread ITAGAKI Takahiro
Tom Lane <[EMAIL PROTECTED]> wrote: > Now that I look closer, the "contrib infrastructure" item is just a > combination of the auto_explain and pg_stat_statements items, and I > guess the reason you and Matthew were shown as reviewers was that > you'd each been assigned one of those two items. A

Re: [HACKERS] auto_explain contrib moudle

2008-11-14 Thread Tom Lane
Jeff Davis <[EMAIL PROTECTED]> writes: > On Thu, 2008-11-13 at 14:31 -0500, Tom Lane wrote: >> This patch seems to contain a subset of the "contrib infrastructure" >> patch that's listed separately on the commitfest page. While I have >> no strong objection to what's here, I'm wondering what sort

Re: [HACKERS] auto_explain contrib moudle

2008-11-13 Thread Jeff Davis
On Thu, 2008-11-13 at 14:31 -0500, Tom Lane wrote: > Jeff Davis <[EMAIL PROTECTED]> writes: > > Thanks! This patch is ready to go, as far as I'm concerned. > > This patch seems to contain a subset of the "contrib infrastructure" > patch that's listed separately on the commitfest page. While I hav

Re: [HACKERS] auto_explain contrib moudle

2008-11-13 Thread Tom Lane
Jeff Davis <[EMAIL PROTECTED]> writes: > Thanks! This patch is ready to go, as far as I'm concerned. This patch seems to contain a subset of the "contrib infrastructure" patch that's listed separately on the commitfest page. While I have no strong objection to what's here, I'm wondering what sort

Re: [HACKERS] auto_explain contrib moudle

2008-11-11 Thread Jeff Davis
On Mon, 2008-11-10 at 18:02 +0900, ITAGAKI Takahiro wrote: > That's because explain.log_analyze requires executor instruments, > and it's not free. I think we'd better to have an option to avoid > the overheads... Oops, I found my bug when force_instrument is > turned on. It should be enabled only

Re: [HACKERS] auto_explain contrib moudle

2008-11-10 Thread ITAGAKI Takahiro
Thank you for reviewing. Jeff Davis <[EMAIL PROTECTED]> wrote: > Another question: what is explain.log_analyze supposed to do? Changing > it doesn't seem to have an effect; it always prints out the actual time. That's because explain.log_analyze requires executor instruments, and it's not free. I

Re: [HACKERS] auto_explain contrib moudle

2008-11-09 Thread Jeff Davis
On Sat, 2008-11-08 at 11:32 -0800, Jeff Davis wrote: > One thing I'm unsure of (this question is for ITAGAKI Takahiro): why is > it necessary to define a new function DefineCustomVariable(), when there > are already functions DefineCustomBoolVariable() and > DefineCustomIntVariable()? > Oh, I see

Re: [HACKERS] auto_explain contrib moudle

2008-11-08 Thread Jeff Davis
On Fri, 2008-11-07 at 18:03 +0200, Martin Pihlak wrote: > Another thing is a feature I am interested in -- ability to auto-explain > statements > execututed from within functions. I'm thinking of adding an extra boolean GUC > -- > "explain.log_nested_statements" (defaults to false). Quick test se

Re: [HACKERS] auto_explain contrib moudle

2008-11-08 Thread Jeff Davis
On Sat, 2008-11-08 at 12:18 +0200, Martin Pihlak wrote: > For me the primary use of auto-explain would be interactive troubleshooting. > The troublesome statements usually involve several nested function calls and > are tedious to trace manually. With auto-explain I fire up psql, load the > module,

Re: [HACKERS] auto_explain contrib moudle

2008-11-08 Thread Martin Pihlak
Jeff Davis wrote: > I still don't understand why this psql patch is desirable. Who sets > their client_min_messages to LOG in psql? And if they do, why would they > expect different behavior that they always got from the already-existing > GUC log_min_duration_statement? > I know a few ;) In my en

Re: [HACKERS] auto_explain contrib moudle

2008-11-07 Thread Tom Lane
Martin Pihlak <[EMAIL PROTECTED]> writes: > There was actually a patch to disable the psql notices, but there were > some concerns and I think it was removed: > http://archives.postgresql.org/pgsql-hackers/2008-07/msg01264.php > http://archives.postgresql.org/pgsql-hackers/2008-09/msg01752.php > P

Re: [HACKERS] auto_explain contrib moudle

2008-11-07 Thread Jeff Davis
On Fri, 2008-11-07 at 22:23 +0200, Martin Pihlak wrote: > Patch is at: > http://archives.postgresql.org/pgsql-hackers/2008-08/msg01274.php > and seems to get rid of the annoying messages. If there aren't any > major issues with it, I think it should be re-added. > I still don't understand why thi

Re: [HACKERS] auto_explain contrib moudle

2008-11-07 Thread Martin Pihlak
Jeff Davis wrote: > It's logged at the LOG level, just like log_min_duration_statement, and > seems to have the same behavior. What do you think it should do > differently? > There was actually a patch to disable the psql notices, but there were some concerns and I think it was removed: http://arc

Re: [HACKERS] auto_explain contrib moudle

2008-11-07 Thread Jeff Davis
On Fri, 2008-11-07 at 18:03 +0200, Martin Pihlak wrote: > One thing that I noticed was that tab completion queries are also explained > if "explain.log_min_duration" is set to zero. Actually this also applies to > psql \dX commands. Don't know if this is deliberate or not. Example: It's logged at

Re: [HACKERS] auto_explain contrib moudle

2008-11-07 Thread Martin Pihlak
Jeff Davis wrote: > It still needs to be merged with HEAD. > ExecutorRun function signature has changed to return void. Other than that it seems OK. I'll add a few additional notes: One thing that I noticed was that tab completion queries are also explained if "explain.log_min_duration" is set to

Re: [HACKERS] auto_explain contrib moudle

2008-11-07 Thread Jeff Davis
On Thu, 2008-10-09 at 19:06 +0900, ITAGAKI Takahiro wrote: > Thanks for your reviewing, Alex. > I applied your comments to my patch. Hi, This seems like a very useful feature, and it works nicely. Initial comments: 1. Please sync with HEAD. There was a change made on Oct. 31st that affects this

Re: [HACKERS] auto_explain contrib moudle

2008-11-07 Thread Jeff Davis
On Thu, 2008-10-09 at 19:06 +0900, ITAGAKI Takahiro wrote: > Thanks for your reviewing, Alex. > I applied your comments to my patch. > I made a few changes: 1. fixed some minor issues with auto_explain.sgml so that it would build (and renamed to auto-explain.sgml to match other files) 2. added

Re: [HACKERS] auto_explain contrib moudle

2008-11-03 Thread Alex Hunsaker
On Thu, Oct 9, 2008 at 03:06, ITAGAKI Takahiro <[EMAIL PROTECTED]> wrote: > Thanks for your reviewing, Alex. > I applied your comments to my patch. Sorry for the late reply! Somehow I missed this, saw it on the commit fest wiki :) >> *custom_guc_flags-0828.patch >> My only other concern is the c

[HACKERS] auto_explain contrib moudle

2008-10-09 Thread ITAGAKI Takahiro
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" <[EMAIL PROTECTED]> wrote: > *custom_guc_flags-0828.patch > My only other concern is the chan