Re: [HACKERS] Auto-explain patch

2008-09-27 Thread Alex Hunsaker
On Wed, Aug 27, 2008 at 8:54 PM, ITAGAKI Takahiro [EMAIL PROTECTED] wrote: Here is a contrib version of auto-explain. I'd like to add it the next commit-fest in September. Here is my review: *custom_guc_flags-0828.patch It seems to be windows newline damaged? lots of ^M at the end of the

Re: [HACKERS] Auto-explain patch

2008-09-02 Thread Marko Kreen
On 8/28/08, ITAGAKI Takahiro [EMAIL PROTECTED] wrote: Here is a contrib version of auto-explain. You can use shared_preload_libraries or local_preload_libraries to load the module automatically. If you do so, you also need to add explain in custom_variable_classes and define explain.*

Re: [HACKERS] Auto-explain patch

2008-09-02 Thread ITAGAKI Takahiro
Marko Kreen [EMAIL PROTECTED] wrote: On 8/28/08, ITAGAKI Takahiro [EMAIL PROTECTED] wrote: You can use shared_preload_libraries or local_preload_libraries to load the module automatically. If you do so, you also need to add explain in custom_variable_classes and define explain.*

Re: [HACKERS] Auto-explain patch

2008-09-02 Thread Marko Kreen
On 9/2/08, ITAGAKI Takahiro [EMAIL PROTECTED] wrote: Marko Kreen [EMAIL PROTECTED] wrote: On 8/28/08, ITAGAKI Takahiro [EMAIL PROTECTED] wrote: You can use shared_preload_libraries or local_preload_libraries to load the module automatically. If you do so, you also need to add

Re: [HACKERS] Auto-explain patch

2008-09-01 Thread ITAGAKI Takahiro
Dean Rasheed [EMAIL PROTECTED] 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

Re: [HACKERS] Auto-explain patch

2008-08-31 Thread Dean Rasheed
* auto_explain.tgz A contrib module version of auto-explain. 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? README is a plain-text for

Re: [HACKERS] Auto-explain patch

2008-08-28 Thread Dean Rasheed
Here is a contrib version of auto-explain. OK, I hadn't considered doing it as a module before. Is it only available to superusers? Do we have a general policy on this? Most logging options are superuser-only, but the recent changes to LOG debug_print_* output have left them PGC_USERSET.

Re: [HACKERS] Auto-explain patch

2008-08-28 Thread ITAGAKI Takahiro
Dean Rasheed [EMAIL PROTECTED] wrote: Is it only available to superusers? Presently, yes. Do we have a general policy on this? Most logging options are superuser-only, but the recent changes to LOG debug_print_* output have left them PGC_USERSET. I set it PGC_SUSET because other log_*

Re: [HACKERS] Auto-explain patch

2008-08-27 Thread ITAGAKI Takahiro
Here is a contrib version of auto-explain. I'd like to add it the next commit-fest in September. I set a high value on logging, not on interactive responce because I think it's enough if we use EXPLAIN ANALYZE directly in psql or set min_client_messages to LOG. The module consists of one contrib

Re: [HACKERS] Auto-explain patch

2008-08-26 Thread ITAGAKI Takahiro
Hi, I'm very interested in the auto-explain feature. Are there any plans to re-add it in the next commit-fest? Dean Rasheed [EMAIL PROTECTED] wrote: Please do not export ExplainState --- that's an internal matter for explain.c. Export some wrapper function with a cleaner API than

Re: [HACKERS] Auto-explain patch

2008-08-26 Thread Simon Riggs
On Tue, 2008-08-26 at 19:24 +0900, ITAGAKI Takahiro wrote: I'm very interested in the auto-explain feature. Me too, though must apologise I've had no further time to review this. -- Simon Riggs www.2ndQuadrant.com PostgreSQL Training, Services and Support -- Sent via

Re: [HACKERS] Auto-explain patch

2008-08-03 Thread Dean Rasheed
Thanks for the feedback, and sorry for my delay in replying (I was on holiday). Tom Lane wrote: Comments: I do not think that you should invent a new elog level for this, and especially not one that is designed to send unexpected messages to the client. Having to kluge tab completion like

Re: [HACKERS] Auto-explain patch

2008-07-26 Thread Tom Lane
Dean Rasheed [EMAIL PROTECTED] writes: This new version fixes that and also includes a little patch to psql so that it ignores any backend notices during tab-completion, which otherwise just get in the way. Trace during tab-completion still goes to the server log, if enabled, since this

Re: [HACKERS] Auto-explain patch

2008-07-11 Thread Dean Rasheed
Ooops. That last patch had an embarrassing little typo which caused it to not actually record the planner times. This new version fixes that and also includes a little patch to psql so that it ignores any backend notices during tab-completion, which otherwise just get in the way. Trace during

Re: [HACKERS] Auto-explain patch

2008-07-11 Thread Simon Riggs
On Fri, 2008-07-11 at 09:33 +, Dean Rasheed wrote: This new version Thanks, I'll review this next week now. -- Simon Riggs www.2ndQuadrant.com PostgreSQL Training, Services and Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to

Re: [HACKERS] Auto-explain patch

2008-07-10 Thread Simon Riggs
On Wed, 2008-07-09 at 09:11 +, Dean Rasheed wrote: Simon, I like your proposal, and I think I can see how to code it fairly easily. There is one thing that it doesn't allow, however, which the debug_xxx parameters do, and that is for a non-superuser to trace SQL used in functions, from

Re: [HACKERS] Auto-explain patch

2008-07-10 Thread Gregory Stark
Simon Riggs [EMAIL PROTECTED] writes: On Wed, 2008-07-09 at 09:11 +, Dean Rasheed wrote: So I suggest grouping these parameters in their own category (eg. sql_trace) and then having additional parameters to control where the output would go. So the sql_trace parameters would be: *

Re: [HACKERS] Auto-explain patch

2008-07-10 Thread Dean Rasheed
After sleeping on this, I think we should follow your idea. Hmm. I preferred your idea ;-) It reduces the number of new parameters back down to 3, which makes it easier to use: * trace_min_planner_duration - int, PGC_USERSET * trace_min_executor_duration - int, PGC_USERSET *

Re: [HACKERS] Auto-explain patch

2008-07-09 Thread Dean Rasheed
, and if both are off, then the sql_trace parameters would do nothing. Dean Subject: RE: [HACKERS] Auto-explain patch From: [EMAIL PROTECTED] To: [EMAIL PROTECTED] CC: pgsql-hackers@postgresql.org Date: Mon, 7 Jul 2008 18:03:15 +0100 On Sun, 2008-07-06 at 17:58 +, Dean Rasheed wrote: OK

Re: [HACKERS] Auto-explain patch

2008-07-09 Thread ITAGAKI Takahiro
Dean Rasheed [EMAIL PROTECTED] wrote: * client_sql_trace = on | off - settable by a normal user to allow a client session to see the sql_trace output. If this parameter is on, the sql_trace will be logged as NOTICE output. In terms of security, is it ok to show normal users SQLs used in

Re: [HACKERS] Auto-explain patch

2008-07-09 Thread Simon Riggs
On Wed, 2008-07-09 at 09:11 +, Dean Rasheed wrote: Simon, I like your proposal, and I think I can see how to code it fairly easily. There is one thing that it doesn't allow, however, which the debug_xxx parameters do, and that is for a non-superuser to trace SQL used in functions, from

Re: [HACKERS] Auto-explain patch

2008-07-09 Thread Marko Kreen
On 7/9/08, ITAGAKI Takahiro [EMAIL PROTECTED] wrote: Dean Rasheed [EMAIL PROTECTED] wrote: * client_sql_trace = on | off - settable by a normal user to allow a client session to see the sql_trace output. If this parameter is on, the sql_trace will be logged as NOTICE output. In terms

Re: [HACKERS] Auto-explain patch

2008-07-09 Thread Dean Rasheed
Just set client_min_messages = 'LOG'; True, but you would still need to be a superuser to to set the min_durations and explain parameters. The other advantage of client_sql_trace is that you could debug your own functions without filling up the log file. It would work better for multiple users

Re: [HACKERS] Auto-explain patch

2008-07-09 Thread Dean Rasheed
+0300 From: [EMAIL PROTECTED] To: [EMAIL PROTECTED] Subject: Re: [HACKERS] Auto-explain patch CC: [EMAIL PROTECTED]; [EMAIL PROTECTED]; pgsql-hackers@postgresql.org On 7/9/08, ITAGAKI Takahiro wrote: Dean Rasheed wrote: * client_sql_trace = on | off - settable by a normal user to allow

Re: [HACKERS] Auto-explain patch

2008-07-09 Thread Marko Kreen
On 7/9/08, Dean Rasheed [EMAIL PROTECTED] wrote: Of course you can still sort of see the SQL used in functions declared SECURITY DEFINER, using debug_print_parse, but this would be opening up a much more transparent way to do it. Do I need to worry about this? If this allows to see

Re: [HACKERS] Auto-explain patch

2008-07-09 Thread Tom Lane
Marko Kreen [EMAIL PROTECTED] writes: On 7/9/08, Dean Rasheed [EMAIL PROTECTED] wrote: Do I need to worry about this? If this allows to see values, then yes. Otherwise no. It definitely would open a hole that doesn't exist now, which is to see values that are inserted into an EXECUTE'd

Re: [HACKERS] Auto-explain patch

2008-07-09 Thread Simon Riggs
On Wed, 2008-07-09 at 12:01 +, Dean Rasheed wrote: Just set client_min_messages = 'LOG'; True, but you would still need to be a superuser to to set the min_durations and explain parameters. No The other advantage of client_sql_trace is that you could debug your own functions

Re: [HACKERS] Auto-explain patch

2008-07-07 Thread Simon Riggs
On Sun, 2008-07-06 at 17:58 +, Dean Rasheed wrote: OK, this one should work against CVS HEAD. OK, still getting some offsets, but it applies. The patch now outputs things in a useful way to avoid a flood of information, that's good. The code seems fine, but it doesn't link up with the

Re: [HACKERS] Auto-explain patch

2008-07-06 Thread Simon Riggs
On Thu, 2008-07-03 at 16:58 +, Dean Rasheed wrote: Here is an updated version of the patch Dean, I'm getting 4 chunks fail when trying to apply your patch onto CVS HEAD. I'm sure its just a little bitrot. Can you update the patch please? Thanks, -- Simon Riggs

Re: [HACKERS] Auto-explain patch

2008-07-06 Thread Dean Rasheed
OK, this one should work against CVS HEAD. Dean. Subject: Re: [HACKERS] Auto-explain patch From: [EMAIL PROTECTED] To: [EMAIL PROTECTED] CC: pgsql-hackers@postgresql.org Date: Sun, 6 Jul 2008 16:42:55 +0100 On Thu, 2008-07-03 at 16:58 +, Dean Rasheed wrote: Here is an updated

Re: [HACKERS] Auto-explain patch

2008-07-03 Thread Dean Rasheed
? Regards, Dean From: [EMAIL PROTECTED] To: [EMAIL PROTECTED] CC: pgsql-hackers@postgresql.org Subject: RE: [HACKERS] Auto-explain patch Date: Wed, 2 Jul 2008 19:42:06 + Its certainly not useful to *me* in its current form. It would produce way

Re: [HACKERS] Auto-explain patch

2008-07-03 Thread Alex Hunsaker
On Thu, Jul 3, 2008 at 10:58 AM, Dean Rasheed [EMAIL PROTECTED] wrote: Here is an updated version of the patch, with a debug_explain_min_duration parameter to allow explaining of just slow-running queries. I've also incorporated a couple of Simon Riggs' suggestions for formatting the output

Re: [HACKERS] Auto-explain patch

2008-07-02 Thread Dean Rasheed
Its certainly not useful to *me* in its current form. It would produce way to much (usless) output. However if it were tied to log_min_duration_statement so I get auto explains for long running queries... That would be very useful indeed. Even if it has to explain everything just to toss out

Re: [HACKERS] Auto-explain patch

2008-06-30 Thread Dean Rasheed
Hi, This is a small patch I posted a few months back, and then kinda forgot about / got distracted with other things. Is there any interest in this? If so I am willing to put more work into it, if people like it or have suggested improvements. Otherwise I'll let it drop. Here's what is does:

Re: [HACKERS] Auto-explain patch

2008-06-30 Thread Marko Kreen
On 6/30/08, Dean Rasheed [EMAIL PROTECTED] wrote: This is a small patch I posted a few months back, and then kinda forgot about / got distracted with other things. Is there any interest in this? If so I am willing to put more work into it, if people like it or have suggested improvements.

Re: [HACKERS] Auto-explain patch

2008-06-30 Thread Alex Hunsaker
On Mon, Jun 30, 2008 at 6:34 AM, Dean Rasheed [EMAIL PROTECTED] wrote: Hi, This is a small patch I posted a few months back, and then kinda forgot about / got distracted with other things. Is there any interest in this? If so I am willing to put more work into it, if people like it or have