Re: [PATCHES] Auto-explain patch
> On Mon, 2008-01-28 at 09:21 +, Dean Rasheed wrote:
>> This is the patch allows logging of the explain plan for each query run, as
>> described here:
>>
>> http://archives.postgresql.org/pgsql-performance/2008-01/msg00245.php
>>
>> I hope this is useful.
>
> This looks very good, though I don't think its finished yet.
>
Thanks for your comments. They all sound like fair points, but I have
a few questions - I am still pretty new to postgresql and this is the
first time I have looked at the source code, so I may not have fully
understood everything you said.
> We definitely don't want "--- query plan ---" to be
> logged each time we execute each SQL statement. I think we should put
> the EXPLAIN output as the main message, not as additional detail.
>
OK, that's easy to change. I based the message/detail formatting on
the way debug_print_plan produces its output.
> I also think we should only log the EXPLAIN if we have logged the SQL
> statement. It's not much use on its own anyway. This then allows this
> feature to work neatly with log_statement and
> log_min_duration_statement.
>
Wouldn't that mean that only superusers could use it?
Even without logging of the SQL statement, I have found it quite
useful in practice for debugging SQL executed from stored procedures,
where the logged CONTEXT is good enough to identify the SQL being
exectued. Also, from an interactive session logging of the original
SQL doesn't matter so much.
> We need this to work effectively when using v3 protocol prepared
> queries. Since we only plan a query once, printing an EXPLAIN for all
> executions is probably too much information for tuning smaller queries.
>
> So I suggest we call this "log_explain" with settings {off | plan |
> execute}, default off. For simple non-prepared queries plan and execute
> work identically: If log_min_duration_statement> -1 or we are logging
> the statement with log_statement then we will instrument each statement
> to allow us to produce EXPLAIN ANALYZE output if the statement is
> logged. If it is set to "plan" then it will print out the EXPLAIN only
> when the query is planned (on first bind, when replanned etc), so it is
> an EXPLAIN not an EXPLAIN ANALYZE. In this mode it will *not* instrument
> each execution, nor will it print the plan in those cases.
>
I originally anticipated 2 use-cases:
1). A normal user from an interactive session, debugging individual
SQL statements and SQL embedded in stored procudures/triggers.
2). An administrator checking database access (eg. from a web app),
looking for inefficient queries.
I guess that what I have so far is more suited to (1), which is mostly
what I have been using it for (debug output not going to the log file).
I originally thought of doing this as an extra option to EXPLAIN, such
as EXAPLAIN [ANALYSE] ALL, but I thought that it would be easier to
use (and implement) as a settable parameter, mirroring Oracle's
AUTOTRACE.
For (2) I agree that it really needs a way to control the verbosity of
the output, as you suggested. And I guess that this ought to go to
level LOG rather than DEBUG1, to be consistent with the other logging
parameters.
This suggests having 2 separate parameters, one for debugging and one
for logging.
> If that last paragraph sounds too much, perhaps we should just go for on
> | off for the next version of the patch.
>
When you describe logging plans for logged statements only, are you
thinking of just explaining the top-level statement, or would this
include nested statements as well? Perhaps the latter is a 4th,
extra-verbose option "log_explain=all", which would produce EXPLAIN
ANALYSE output for each logged statement, and recursively for each
statement resulting from that top-level statement.
Dean
_
The next generation of Windows Live is here
http://www.windowslive.co.uk/get-live
--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Auto-explain patch
On Sat, 2008-03-29 at 08:47 +, Dean Rasheed wrote: > > I also think we should only log the EXPLAIN if we have logged the SQL > > statement. It's not much use on its own anyway. This then allows this > > feature to work neatly with log_statement and > > log_min_duration_statement. > > > > Wouldn't that mean that only superusers could use it? That's what has currently been agreed for the other logging options. > I originally anticipated 2 use-cases: > > 1). A normal user from an interactive session, debugging individual > SQL statements and SQL embedded in stored procudures/triggers. > > 2). An administrator checking database access (eg. from a web app), > looking for inefficient queries. > > I guess that what I have so far is more suited to (1), which is mostly > what I have been using it for (debug output not going to the log file). > I originally thought of doing this as an extra option to EXPLAIN, such > as EXAPLAIN [ANALYSE] ALL, but I thought that it would be easier to > use (and implement) as a settable parameter, mirroring Oracle's > AUTOTRACE. > > For (2) I agree that it really needs a way to control the verbosity of > the output, as you suggested. And I guess that this ought to go to > level LOG rather than DEBUG1, to be consistent with the other logging > parameters. > > This suggests having 2 separate parameters, one for debugging and one > for logging. That would make sense. The Oracle facility is actually a sql*plus option, so perhaps a psql facility for that would be appropriate for (1), but I think (2) is the more important usage and it would be better to concentrate on that first and then come back for (1) next/later. > > If that last paragraph sounds too much, perhaps we should just go for on > > | off for the next version of the patch. > When you describe logging plans for logged statements only, are you > thinking of just explaining the top-level statement, or would this > include nested statements as well? Perhaps the latter is a 4th, > extra-verbose option "log_explain=all", which would produce EXPLAIN > ANALYSE output for each logged statement, and recursively for each > statement resulting from that top-level statement. Yeh, can't think of any other way. Bottom line is probably that the patch did what it did OK, its just that we probably don't want exactly what it did. It would be very good if you could write up the whole feature description again and re-post to hackers, so we can get wider agreement before working on the patch. Believe me, I understand exactly how you feel when I suggest that. -- Simon Riggs 2ndQuadrant http://www.2ndQuadrant.com PostgreSQL UK 2008 Conference: http://www.postgresql.org.uk -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] create language ... if not exists
Andreas 'ads' Scherbaum wrote: The attached patch for HEAD extends the CREATE LANGUAGE statement by an IF NOT EXISTS option which in effect changes the raised error into a notice. Before i continue working on this patch i would like to know if this extension has a chance to go into PG and what other changes i should apply (beside the missing documentation). The way we've solved this problem for other CREATE commands is to add "OR REPLACE" option, instead of "IF NOT EXISTS". We should do the same here. Regarding the patch itself: You define rule "opt_if_not_exists", but never use it. And you add a new rule for "CREATE LANGUAGE ... HANDLER ...", but forgot "IF_P NOT EXISTS" from the end of that. Looks like you couldn't decide which approach to take, and ended up doing a little bit of both ;-). -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [HACKERS] [PATCHES] Avahi support for Postgresql
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On Sat, Feb 23, 2008 at 01:13:38PM +0100, Mathias Hasselmann wrote: [...] > Avahi/Bonjour/DNS-SD support[1] is very important, for integrating > Postgresql with modern desktop environments like OSX, GNOME, KDE: It's > very convenient to choose active DBMS servers in your local network from > a list, instead of memorizing "cryptic" connection parameters. [...] > People not wanting DNS-SD support for their server can easily control > that feature via the "--with-avahi" configure scripts. Sorry for a dumb question, but I couldn't figure that out from your references [1]..[4]: does that mean that the PostgreSQL server would "advertise itself" on the local net? Or what is the purpose of liking-in libavahi into the postmaster? Surely one wouldn't want this in a data center? Is there a possiblity to disable that at run time? Thanks for any insights - -- tomás -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.1 (GNU/Linux) iD8DBQFH7jUxBcgs9XrR2kYRAsfOAJ0Ulfydo3G1kzQo3HTOdjtswA1A2gCfYYyL VHprJ63unId85/Iht4ha2RE= =get0 -END PGP SIGNATURE- -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] create language ... if not exists
Heikki Linnakangas wrote: Andreas 'ads' Scherbaum wrote: The attached patch for HEAD extends the CREATE LANGUAGE statement by an IF NOT EXISTS option which in effect changes the raised error into a notice. Before i continue working on this patch i would like to know if this extension has a chance to go into PG and what other changes i should apply (beside the missing documentation). The way we've solved this problem for other CREATE commands is to add "OR REPLACE" option, instead of "IF NOT EXISTS". We should do the same here. My recollection is that we only do that where we need to for reasons of dependency. Not sure that applies here. cheers andrew -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] create language ... if not exists
Andrew Dunstan <[EMAIL PROTECTED]> writes: > Heikki Linnakangas wrote: >> The way we've solved this problem for other CREATE commands is to add >> "OR REPLACE" option, instead of "IF NOT EXISTS". We should do the same >> here. > My recollection is that we only do that where we need to for reasons of > dependency. Not sure that applies here. I was about to make the same complaint as Heikki. We currently have two different ways of dealing with this type of scenario: DROP IF EXISTS (for most object types) CREATE OR REPLACE (for functions, rules, views) The OP wants to introduce yet a third variant, implemented for only one kind of object. That's not a feature, it's a wart. Clearly DROP IF EXISTS isn't helpful for the proposed use-case (since you'd lose any pre-existing functions in the language) but I don't see why CREATE OR REPLACE wouldn't serve. regards, tom lane -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] create language ... if not exists
Heikki Linnakangas <[EMAIL PROTECTED]> writes: > The way we've solved this problem for other CREATE commands is to add > "OR REPLACE" option, instead of "IF NOT EXISTS". We should do the same here. If we're willing to consider a solution that is specific to CREATE LANGUAGE (as opposed to implementing IF NOT EXISTS across-the-board, which might happen someday) what I'd suggest is just incorporating the behavior directly into the abbreviated (no parameters) form of CREATE LANGUAGE. If the language already exists and has the same properties specified in pg_pltemplate, don't raise an error. Give a notice maybe. One thing that's not too clear is whether that should happen before or after the privilege check: if a user who doesn't have the rights to create a language issues a CREATE, and the language already exists, should he get a "no privilege" error or an "it already exists" notice? regards, tom lane -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] create language ... if not exists
Tom Lane wrote: Heikki Linnakangas <[EMAIL PROTECTED]> writes: The way we've solved this problem for other CREATE commands is to add "OR REPLACE" option, instead of "IF NOT EXISTS". We should do the same here. If we're willing to consider a solution that is specific to CREATE LANGUAGE (as opposed to implementing IF NOT EXISTS across-the-board, which might happen someday) what I'd suggest is just incorporating the behavior directly into the abbreviated (no parameters) form of CREATE LANGUAGE. If the language already exists and has the same properties specified in pg_pltemplate, don't raise an error. Give a notice maybe. Why not implement "OR REPLACE" like for other things? Still seems the most consistent behavior to me. You might want to get the error if the language already exists, which your proposal wouldn't allow. And it wouldn't help with languages without a pg_pltemplate entry. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Better psql tab-completion support for schemas and tables
Greg Sabino Mullane <[EMAIL PROTECTED]> writes: > Full support for all schema and table name combinations when > getting a list of attributes. All of the following will now work: > select * from information_schema.columns where > select * from foo where > select * from "user" where > select * from "foo" where > select * from "Uppercase".lower where > select * from "gtsm.com"."foo.Bar" where > select * from "GTSM.com".foo where Applied with minor revisions. I noticed while testing this that although you fixed it for the case of the user having unnecessarily quoted the preceding name, for example given a table foo, update "foo" set it's still not bright about letting you complete such an entry in the first place -- try update "f That doesn't invalidate this patch, but there's still more work to do on the completion queries. regards, tom lane -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] create language ... if not exists
Heikki Linnakangas wrote: > Tom Lane wrote: > > Heikki Linnakangas <[EMAIL PROTECTED]> writes: > >> The way we've solved this problem for other CREATE commands is to > >> add "OR REPLACE" option, instead of "IF NOT EXISTS". We should do > >> the same here. > > > > If we're willing to consider a solution that is specific to CREATE > > LANGUAGE (as opposed to implementing IF NOT EXISTS across-the-board, > > which might happen someday) what I'd suggest is just incorporating > > the behavior directly into the abbreviated (no parameters) form of > > CREATE LANGUAGE. If the language already exists and has the same > > properties specified in pg_pltemplate, don't raise an error. Give > > a notice maybe. > > Why not implement "OR REPLACE" like for other things? Still seems the > most consistent behavior to me. > > You might want to get the error if the language already exists, which > your proposal wouldn't allow. And it wouldn't help with languages > without a pg_pltemplate entry. Even though I was the guy originally suggesting that Andreas put forward a patch for IF NOT EXISTS, now that it's being mention I agree with Heikki - it's more consistent. And I see the primary use as being in installation scripts for software that requires pl/pgsql (or any other PL), for which the exact syntax really doesn't matter - it's better to be consistent. If we're implementing IF NOT EXISTS across the board, let's do that for languages at the same time as for others. //Magnus -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] create language ... if not exists
Magnus Hagander <[EMAIL PROTECTED]> writes: > If we're implementing IF NOT EXISTS across the board, let's do that for > languages at the same time as for others. Yeah, if we were going to do it at all it should be handled across-the-board, the way DROP IF EXISTS was. However, I seem to recall that in the discussions leading up to implementing DROP IF EXISTS, we considered and specifically rejected CREATE IF NOT EXISTS. Don't have time right now to troll the archives for the reasoning. regards, tom lane -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] create language ... if not exists
Tom Lane wrote: > Magnus Hagander <[EMAIL PROTECTED]> writes: > > If we're implementing IF NOT EXISTS across the board, let's do that > > for languages at the same time as for others. > > Yeah, if we were going to do it at all it should be handled > across-the-board, the way DROP IF EXISTS was. However, I seem to > recall that in the discussions leading up to implementing DROP IF > EXISTS, we considered and specifically rejected CREATE IF NOT > EXISTS. Don't have time right now to troll the archives for the > reasoning. Right. Which is one of the reasons why I'm suggesting we stick with the CREATE OR REPLACE for now. //Magnus -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] create language ... if not exists
I wrote: > ... However, I seem to recall > that in the discussions leading up to implementing DROP IF EXISTS, > we considered and specifically rejected CREATE IF NOT EXISTS. Don't > have time right now to troll the archives for the reasoning. [ back from dinner party... ] Here's the thread I was remembering: http://archives.postgresql.org/pgsql-hackers/2005-10/msg00632.php The key argument seems to be that it's quite unclear what the state following CREATE IF NOT EXISTS (CINE) should be, if the object does exist but not with the same properties specified in the CINE command. CREATE OR REPLACE resolves that by making it clear that it's gonna be what the command says. Perhaps there is a use-case for the alternate behavior where the pre-existing object doesn't get modified, but I'm not too sure what it would be. regards, tom lane -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Integer datetime by default
On Tue, 2008-03-25 at 12:54 -0700, Neil Conway wrote: > Barring any objections, I'll apply this to HEAD tomorrow. Applied to HEAD. -Neil -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
