Re: [HACKERS] generic explain options v3
Robert Haas robertmh...@gmail.com writes: Here's the update. There are a few things that I'm not entirely happy with here, but not quite sure what to do about either. Committed with a few editorializations. - ExplainPrintPlan() is now almost trivial. It seems like there should be some way to get rid of this altogether, but I'm not quite sure how. I thought about ripping pstmt and rtable out of ExplainState and just storying queryDesc there. But that involves changing a lot of code, and while it makes some things simpler, it makes other parts more complex. I'm not sure whether it's a win or not; I'm also not sure how much brainpower it's worth spending on this. I think the problem here is that you chose to treat ExplainState.pstmt as a parameter, when it's better considered as an internal field. I changed it to the latter approach. - It's becoming increasingly evident to me that the explain stuff in prepare.c has no business being there and should be moved to explain.c. I haven't done that here, but it's worth thinking about. I'm unconvinced. The reason that code is that way is that the alternative would require explain.c to know quite a lot about prepared plans, which does not seem like an improvement. - The hack needed in ExplainLogLevel is just that. Yeah, I thought that was okay. We could alternatively refactor the code so that the parameter analysis code is a separate function that utility.c could call, but it's unclear that it's worth the trouble. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] generic explain options v3
On Sun, Jul 26, 2009 at 7:40 PM, Tom Lanet...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: Here's the update. There are a few things that I'm not entirely happy with here, but not quite sure what to do about either. Committed with a few editorializations. Thanks. - ExplainPrintPlan() is now almost trivial. It seems like there should be some way to get rid of this altogether, but I'm not quite sure how. I thought about ripping pstmt and rtable out of ExplainState and just storying queryDesc there. But that involves changing a lot of code, and while it makes some things simpler, it makes other parts more complex. I'm not sure whether it's a win or not; I'm also not sure how much brainpower it's worth spending on this. I think the problem here is that you chose to treat ExplainState.pstmt as a parameter, when it's better considered as an internal field. I changed it to the latter approach. Sounds fine. - It's becoming increasingly evident to me that the explain stuff in prepare.c has no business being there and should be moved to explain.c. I haven't done that here, but it's worth thinking about. I'm unconvinced. The reason that code is that way is that the alternative would require explain.c to know quite a lot about prepared plans, which does not seem like an improvement. I didn't consider that. As it is, prepare.c has to know quite a lot about explaining, so it may be six of one, half a dozen of the other. - The hack needed in ExplainLogLevel is just that. Yeah, I thought that was okay. We could alternatively refactor the code so that the parameter analysis code is a separate function that utility.c could call, but it's unclear that it's worth the trouble. OK. It seems I have quite a bit of work in front of me unbreaking the machine-readable explain patch. I started grinding through it, but it's not pretty. I'll post an updated version when I have it. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] generic explain options v3
Robert Haas robertmh...@gmail.com writes: Ugh. I took a look at this and it turns out that there are some tentacles. It doesn't seem very sane to actually do anything with a list of DefElem nodes, so we really need to parse that list and convert it to a more sensible format right away (this also seems important for proper error checking). Yeah, the standard approach is to convert it into a group of values at the start of execution of the utility command. The obvious solution to that is to create the ExplainState sooner, back up at the ExplainQuery level. If we do that, though, then ExplainState will need to become a public API, because contrib/auto_explain calls ExplainPrintPlan(). Well, if we add any more options to EXPLAIN then auto_explain may well be interested in them, so I'm not sure this is bad. The alternative is to keep adding retail parameters to the public functions. And if we do that, then probably we should declare it in include/nodes/execnodes.h and make it a node type... No, just a struct declared in commands/explain.h. There's no reason for it to be part of the Node system. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] generic explain options v3
On Thu, Jul 23, 2009 at 12:08 PM, Tom Lanet...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: Ugh. I took a look at this and it turns out that there are some tentacles. It doesn't seem very sane to actually do anything with a list of DefElem nodes, so we really need to parse that list and convert it to a more sensible format right away (this also seems important for proper error checking). Yeah, the standard approach is to convert it into a group of values at the start of execution of the utility command. The obvious solution to that is to create the ExplainState sooner, back up at the ExplainQuery level. If we do that, though, then ExplainState will need to become a public API, because contrib/auto_explain calls ExplainPrintPlan(). Well, if we add any more options to EXPLAIN then auto_explain may well be interested in them, so I'm not sure this is bad. The alternative is to keep adding retail parameters to the public functions. And if we do that, then probably we should declare it in include/nodes/execnodes.h and make it a node type... No, just a struct declared in commands/explain.h. There's no reason for it to be part of the Node system. Oh, OK. That will work. Thanks. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] generic explain options v3
On Tue, Jul 21, 2009 at 10:29 PM, Robert Haasrobertmh...@gmail.com wrote: On Tue, Jul 21, 2009 at 10:05 PM, Tom Lanet...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Tue, Jul 21, 2009 at 7:47 PM, Tom Lanet...@sss.pgh.pa.us wrote: Also, I'd suggest changing the ExplainStmt struct to have a list of DefElem options instead of hard-wiring the option set at that level. What is the advantage of that? Fewer places to change when you add a new option; in particular, not copyfuncs or equalfuncs. Also, the way you are doing it is gratuitously unlike every other command that has similar issues to deal with. Everybody else parses their DefElem list at execution time. I think you should have the legacy ANALYZE and VERBOSE syntax elements generate DefElem list members that get examined at execution. Not having to touch copyfuncs or equalfuncs for future options is a definite plus, so I'll rework along these lines. Ugh. I took a look at this and it turns out that there are some tentacles. It doesn't seem very sane to actually do anything with a list of DefElem nodes, so we really need to parse that list and convert it to a more sensible format right away (this also seems important for proper error checking). The natural place to do this would be in ExplainPrintPlan(), which is already copying the relevant fields from the ExplainStmt over into an ExplainState, but that's too far down the call tree, which (for a non-utility statement when ExplainOneQuery_hook is null) looks like this: ExplainQuery - ExplainOneQuery - ExplainOnePlan - ExplainPrintPlan The obvious solution to that is to create the ExplainState sooner, back up at the ExplainQuery level. If we do that, though, then ExplainState will need to become a public API, because contrib/auto_explain calls ExplainPrintPlan(). And if we do that, then probably we should declare it in include/nodes/execnodes.h and make it a node type... and if we do that then we'll be back to a copyfuncs/equalfuncs change every time we add a flag. Now that's not to say there's no advantage in the proposed refactoring - it's still more consistent with the way things are done elsewhere. But since it's going to be a fair amount of work and fail to achieve one of the two goals you set forth for it, I'd like to get confirmation before proceeding if possible, and any suggestions you may have for how to make it as clean as possible. Thanks, ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] generic explain options v3
Robert Haas robertmh...@gmail.com writes: Here is an updated version of my generic options for explain patch. What is the rationale for essentially duplicating defGetBoolean()? Also, I'd suggest changing the ExplainStmt struct to have a list of DefElem options instead of hard-wiring the option set at that level. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] generic explain options v3
On Tue, Jul 21, 2009 at 7:47 PM, Tom Lanet...@sss.pgh.pa.us wrote:
Robert Haas robertmh...@gmail.com writes:
Here is an updated version of my generic options for explain patch.
What is the rationale for essentially duplicating defGetBoolean()?
I just didn't realize we already had something along those lines.
Updated patch attached, to which I've also applied Andres Freund's
parser changes, suggested here:
http://archives.postgresql.org/pgsql-hackers/2009-07/msg01213.php
Also, I'd suggest changing the ExplainStmt struct to have a list of
DefElem options instead of hard-wiring the option set at that level.
What is the advantage of that?
...Robert
*** a/contrib/auto_explain/auto_explain.c
--- b/contrib/auto_explain/auto_explain.c
***
*** 14,19
--- 14,20
#include commands/explain.h
#include executor/instrument.h
+ #include nodes/makefuncs.h
#include utils/guc.h
PG_MODULE_MAGIC;
***
*** 196,207 explain_ExecutorEnd(QueryDesc *queryDesc)
msec = queryDesc-totaltime-total * 1000.0;
if (msec = auto_explain_log_min_duration)
{
StringInfoData buf;
initStringInfo(buf);
! ExplainPrintPlan(buf, queryDesc,
! queryDesc-doInstrument auto_explain_log_analyze,
! auto_explain_log_verbose);
/* Remove last line break */
if (buf.len 0 buf.data[buf.len - 1] == '\n')
--- 197,210
msec = queryDesc-totaltime-total * 1000.0;
if (msec = auto_explain_log_min_duration)
{
+ ExplainStmt *stmt = makeExplain(NIL, NULL);
StringInfoData buf;
initStringInfo(buf);
! stmt-analyze =
! (queryDesc-doInstrument auto_explain_log_analyze);
! stmt-verbose = auto_explain_log_verbose;
! ExplainPrintPlan(buf, queryDesc, stmt);
/* Remove last line break */
if (buf.len 0 buf.data[buf.len - 1] == '\n')
*** a/doc/src/sgml/ref/explain.sgml
--- b/doc/src/sgml/ref/explain.sgml
***
*** 31,36 PostgreSQL documentation
--- 31,37
refsynopsisdiv
synopsis
+ EXPLAIN [ ( [ { ANALYZE | VERBOSE | COSTS } [ replaceable class=parameterboolean_value/replaceable ] ] [, ...] ) ] replaceable class=parameterstatement/replaceable
EXPLAIN [ ANALYZE ] [ VERBOSE ] replaceable class=parameterstatement/replaceable
/synopsis
/refsynopsisdiv
***
*** 70,75 EXPLAIN [ ANALYZE ] [ VERBOSE ] replaceable class=parameterstatement/replac
--- 71,86
are close to reality.
/para
+ para
+Only the literalANALYZE/literal and literalVERBOSE/literal options
+can be specified, and only in the order, without surrounding the option list
+in parentheses. Prior to productnamePostgreSQL/productname 8.5, the
+unparenthesized syntax was the only one supported. It is expected that
+all new options will be supported only when using the parenthesized syntax,
+which also allows a value to be specified for each option
+(e.g. literalTRUE/literal or literalFALSE/literal).
+ /para
+
important
para
Keep in mind that the statement is actually executed when
***
*** 99,105 ROLLBACK;
termliteralANALYZE/literal/term
listitem
para
! Carry out the command and show the actual run times.
/para
/listitem
/varlistentry
--- 110,117
termliteralANALYZE/literal/term
listitem
para
! Carry out the command and show the actual run times. This
! parameter defaults to commandFALSE/command.
/para
/listitem
/varlistentry
***
*** 108,114 ROLLBACK;
termliteralVERBOSE/literal/term
listitem
para
! Include the output column list for each node in the plan tree.
/para
/listitem
/varlistentry
--- 120,151
termliteralVERBOSE/literal/term
listitem
para
! Include the output column list for each node in the plan tree. This
! parameter defaults to commandFALSE/command.
! /para
! /listitem
!/varlistentry
!
!varlistentry
! termliteralCOSTS/literal/term
! listitem
! para
! Include information on the estimated startup and total cost of each
! plan node, as well as the estimated number of rows and the estimated
! width of each row. This parameter defaults to commandTRUE/command.
! /para
! /listitem
!/varlistentry
!
!varlistentry
! termreplaceable class=parameter /boolean_value/replaceable/term
! listitem
! para
! Specifies whether the named parameter should be turned on or off. You
! can use the values literalTRUE/literal or literal1/literal to
! request the stated option, and literalFALSE/literal
! or literal0/literal. If the Boolean value is omitted, it defaults
! to literalTRUE/literal.
/para
/listitem
/varlistentry
***
*** 202,207 EXPLAIN SELECT * FROM foo WHERE i = 4;
--- 239,258
/para
Re: [HACKERS] generic explain options v3
Robert Haas robertmh...@gmail.com writes: On Tue, Jul 21, 2009 at 7:47 PM, Tom Lanet...@sss.pgh.pa.us wrote: Also, I'd suggest changing the ExplainStmt struct to have a list of DefElem options instead of hard-wiring the option set at that level. What is the advantage of that? Fewer places to change when you add a new option; in particular, not copyfuncs or equalfuncs. Also, the way you are doing it is gratuitously unlike every other command that has similar issues to deal with. Everybody else parses their DefElem list at execution time. I think you should have the legacy ANALYZE and VERBOSE syntax elements generate DefElem list members that get examined at execution. BTW, I see that your explain refactoring patch is marked ready for committer, but is it actually sane to apply it before the other two? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] generic explain options v3
On Tue, Jul 21, 2009 at 10:05 PM, Tom Lanet...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Tue, Jul 21, 2009 at 7:47 PM, Tom Lanet...@sss.pgh.pa.us wrote: Also, I'd suggest changing the ExplainStmt struct to have a list of DefElem options instead of hard-wiring the option set at that level. What is the advantage of that? Fewer places to change when you add a new option; in particular, not copyfuncs or equalfuncs. Also, the way you are doing it is gratuitously unlike every other command that has similar issues to deal with. Everybody else parses their DefElem list at execution time. I think you should have the legacy ANALYZE and VERBOSE syntax elements generate DefElem list members that get examined at execution. Not having to touch copyfuncs or equalfuncs for future options is a definite plus, so I'll rework along these lines. BTW, I see that your explain refactoring patch is marked ready for committer, but is it actually sane to apply it before the other two? I think so. It's all code cleanup, with no behavioral changes, and is intended to contain only the stuff that seemed to me as thought it would still be worth doing even if the rest of the patch set were rejected. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] generic explain options v3 - RR Review
On Sun, Jul 19, 2009 at 03:15:38AM +0200, Andres Freund wrote:
Hi Robert, Hi All,
Patch applies with some offset changes, code changes look sensible, I
personally like the new syntax and the features it may allow in future. One,
possibly big, gripe remains though:
The formerly valid statement which cannot be written without the parentheses
and stay semantically equivalent:
EXPLAIN (SELECT 1 ORDER BY 1) UNION ALL (SELECT 2 ORDER BY 1);
is now not valid anymore (The added %prec UMINUS causes the first '(' to be
recognize as start of the option list as intended).
This currently can only be resolved by using an option list like:
EXPLAIN (VERBOSE OFF) ...
Its also currently impossible to use an empty set of parentheses to resolve
this - this could easily be changed though.
I have to admit I don't see a nice solution here except living with the
incompatibility... Perhaps somebody has a better idea?
I think another possibility might be to allow the syntax:
EXPLAIN VERBOSE ANALYSE (options) SELECt ...;
Sure, it's a bit ugly, but in the grammer you could then do:
ExplainStmt: EXPLAIN opt_analyze opt_verbose ExplainableStmt
| EXPLAIN opt_analyze opt_verbose '(' explain_option_list
')' ExplainableStmt
Which means that (I think) bison can use the token *after* the '(' to
disambiguate, and since SELECT is a reserved word I think the problem
may be solved.
(The point being that then Bison can reduce the opt_analyze for both
cases).
Hope this helps,
--
Martijn van Oosterhout klep...@svana.org http://svana.org/kleptog/
Please line up in a tree and maintain the heap invariant while
boarding. Thank you for flying nlogn airlines.
signature.asc
Description: Digital signature
Re: [HACKERS] generic explain options v3 - RR Review
On Sunday 19 July 2009 14:39:33 Martijn van Oosterhout wrote:
On Sun, Jul 19, 2009 at 03:15:38AM +0200, Andres Freund wrote:
Hi Robert, Hi All,
Patch applies with some offset changes, code changes look sensible, I
personally like the new syntax and the features it may allow in future.
One, possibly big, gripe remains though:
The formerly valid statement which cannot be written without the
parentheses and stay semantically equivalent:
EXPLAIN (SELECT 1 ORDER BY 1) UNION ALL (SELECT 2 ORDER BY 1);
is now not valid anymore (The added %prec UMINUS causes the first '(' to
be recognize as start of the option list as intended).
This currently can only be resolved by using an option list like:
EXPLAIN (VERBOSE OFF) ...
Its also currently impossible to use an empty set of parentheses to
resolve this - this could easily be changed though.
I have to admit I don't see a nice solution here except living with the
incompatibility... Perhaps somebody has a better idea?
I think another possibility might be to allow the syntax:
EXPLAIN VERBOSE ANALYSE (options) SELECt ...;
Sure, it's a bit ugly, but in the grammer you could then do:
ExplainStmt: EXPLAIN opt_analyze opt_verbose ExplainableStmt
| EXPLAIN opt_analyze opt_verbose '(' explain_option_list
')'
| ExplainableStmt
Which means that (I think) bison can use the token *after* the '(' to
disambiguate, and since SELECT is a reserved word I think the problem
may be solved.
I think that does not work since explain_option_name has to include keywords
to be able to use ANALYZE and VERBOSE.
Its solvable by not allowing all keywords there but only ANALYZE and VERBOSE.
Involves some duplication though...
Patch attached.
Andres
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 1d51032..175929c 100644
*** a/src/backend/parser/gram.y
--- b/src/backend/parser/gram.y
*** static TypeName *TableFuncTypeName(List
*** 321,327
%type list opt_interval interval_second
%type node overlay_placing substr_from substr_for
! %type boolean opt_instead opt_analyze
%type boolean index_opt_unique opt_verbose opt_full
%type boolean opt_freeze opt_default opt_recheck
%type defelt opt_binary opt_oids copy_delimiter
--- 321,327
%type list opt_interval interval_second
%type node overlay_placing substr_from substr_for
! %type boolean opt_instead
%type boolean index_opt_unique opt_verbose opt_full
%type boolean opt_freeze opt_default opt_recheck
%type defelt opt_binary opt_oids copy_delimiter
*** opt_name_list:
*** 6456,6468
*
*/
! ExplainStmt: EXPLAIN opt_analyze opt_verbose ExplainableStmt
{
ExplainStmt *n = makeExplain(NIL, (Node *) $4);
! n-analyze = $2;
n-verbose = $3;
$$ = (Node *)n;
}
| EXPLAIN '(' explain_option_list ')' ExplainableStmt
{
$$ = (Node *) makeExplain((List *) $3, (Node *) $5);
--- 6456,6480
*
*/
! ExplainStmt:
! EXPLAIN ExplainableStmt
! {
! ExplainStmt *n = makeExplain(NIL, (Node *) $2);
! $$ = (Node *)n;
! }
! | EXPLAIN ANALYZE opt_verbose ExplainableStmt
{
ExplainStmt *n = makeExplain(NIL, (Node *) $4);
! n-analyze = TRUE;
n-verbose = $3;
$$ = (Node *)n;
}
+ | EXPLAIN VERBOSE ExplainableStmt
+ {
+ ExplainStmt *n = makeExplain(NIL, (Node *) $3);
+ n-verbose = TRUE;
+ $$ = (Node *)n;
+ }
| EXPLAIN '(' explain_option_list ')' ExplainableStmt
{
$$ = (Node *) makeExplain((List *) $3, (Node *) $5);
*** ExplainableStmt:
*** 6479,6502
| ExecuteStmt /* by default all are $$=$1 */
;
- /*
- * The precedence declaration for the opt_analyze EMPTY case, below, is
- * necessary to prevent a shift/reduce conflict in the second production for
- * ExplainStmt, above. Otherwise, when the parser encounters EXPLAIN (, it
- * can't tell whether the ( is the beginning of a SelectStmt or the beginning
- * of the options list. The precedence declaration below forces the latter
- * interpretation.
- *
- * It might seem that we could get away with simply changing the definition of
- * ExplainableStmt to use select_without_parens rather than SelectStmt, but
- * that does not work, because select_without_parens produces expressions such
- * as (SELECT NULL) ORDER BY 1 that we interpret as legal queries.
- */
- opt_analyze:
- analyze_keyword { $$ = TRUE; }
- | /* EMPTY */ %prec UMINUS { $$ = FALSE; }
- ;
-
explain_option_list:
explain_option_elem
{
--- 6491,6496
*** explain_option_elem:
*** 6516,6522
;
explain_option_name:
! ColLabel { $$ = $1; }
;
explain_option_arg:
--- 6510,6518
;
Re: [HACKERS] generic explain options v3 - RR Review
On Sun, Jul 19, 2009 at 9:47 AM, Andres Freundand...@anarazel.de wrote:
On Sunday 19 July 2009 14:39:33 Martijn van Oosterhout wrote:
On Sun, Jul 19, 2009 at 03:15:38AM +0200, Andres Freund wrote:
Hi Robert, Hi All,
Patch applies with some offset changes, code changes look sensible, I
personally like the new syntax and the features it may allow in future.
One, possibly big, gripe remains though:
The formerly valid statement which cannot be written without the
parentheses and stay semantically equivalent:
EXPLAIN (SELECT 1 ORDER BY 1) UNION ALL (SELECT 2 ORDER BY 1);
is now not valid anymore (The added %prec UMINUS causes the first '(' to
be recognize as start of the option list as intended).
This currently can only be resolved by using an option list like:
EXPLAIN (VERBOSE OFF) ...
Its also currently impossible to use an empty set of parentheses to
resolve this - this could easily be changed though.
I have to admit I don't see a nice solution here except living with the
incompatibility... Perhaps somebody has a better idea?
I think another possibility might be to allow the syntax:
EXPLAIN VERBOSE ANALYSE (options) SELECt ...;
Sure, it's a bit ugly, but in the grammer you could then do:
ExplainStmt: EXPLAIN opt_analyze opt_verbose ExplainableStmt
| EXPLAIN opt_analyze opt_verbose '('
explain_option_list ')'
| ExplainableStmt
Which means that (I think) bison can use the token *after* the '(' to
disambiguate, and since SELECT is a reserved word I think the problem
may be solved.
I think that does not work since explain_option_name has to include keywords
to be able to use ANALYZE and VERBOSE.
Its solvable by not allowing all keywords there but only ANALYZE and VERBOSE.
Involves some duplication though...
Patch attached.
Hmm, good idea. I will update and resubmit.
...Robert
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] generic explain options v3 - RR Review
Hi Robert, Hi All,
Patch applies with some offset changes, code changes look sensible, I
personally like the new syntax and the features it may allow in future. One,
possibly big, gripe remains though:
The formerly valid statement which cannot be written without the parentheses
and stay semantically equivalent:
EXPLAIN (SELECT 1 ORDER BY 1) UNION ALL (SELECT 2 ORDER BY 1);
is now not valid anymore (The added %prec UMINUS causes the first '(' to be
recognize as start of the option list as intended).
This currently can only be resolved by using an option list like:
EXPLAIN (VERBOSE OFF) ...
Its also currently impossible to use an empty set of parentheses to resolve
this - this could easily be changed though.
I have to admit I don't see a nice solution here except living with the
incompatibility... Perhaps somebody has a better idea?
Andres
PS: The 'offset corrected' version I tested with is attached
diff --git a/contrib/auto_explain/auto_explain.c b/contrib/auto_explain/auto_explain.c
index b7c0ef9..956af94 100644
*** a/contrib/auto_explain/auto_explain.c
--- b/contrib/auto_explain/auto_explain.c
***
*** 14,19
--- 14,20
#include commands/explain.h
#include executor/instrument.h
+ #include nodes/makefuncs.h
#include utils/guc.h
PG_MODULE_MAGIC;
*** explain_ExecutorEnd(QueryDesc *queryDesc
*** 196,207
msec = queryDesc-totaltime-total * 1000.0;
if (msec = auto_explain_log_min_duration)
{
StringInfoData buf;
initStringInfo(buf);
! ExplainPrintPlan(buf, queryDesc,
! queryDesc-doInstrument auto_explain_log_analyze,
! auto_explain_log_verbose);
/* Remove last line break */
if (buf.len 0 buf.data[buf.len - 1] == '\n')
--- 197,210
msec = queryDesc-totaltime-total * 1000.0;
if (msec = auto_explain_log_min_duration)
{
+ ExplainStmt *stmt = makeExplain(NIL, NULL);
StringInfoData buf;
initStringInfo(buf);
! stmt-analyze =
! (queryDesc-doInstrument auto_explain_log_analyze);
! stmt-verbose = auto_explain_log_verbose;
! ExplainPrintPlan(buf, queryDesc, stmt);
/* Remove last line break */
if (buf.len 0 buf.data[buf.len - 1] == '\n')
diff --git a/doc/src/sgml/ref/explain.sgml b/doc/src/sgml/ref/explain.sgml
index ea9b3a6..afcab08 100644
*** a/doc/src/sgml/ref/explain.sgml
--- b/doc/src/sgml/ref/explain.sgml
*** PostgreSQL documentation
*** 31,36
--- 31,37
refsynopsisdiv
synopsis
+ EXPLAIN [ ( [ { ANALYZE | VERBOSE | COSTS } [ replaceable class=parameterboolean_value/replaceable ] ] [, ...] ) ] replaceable class=parameterstatement/replaceable
EXPLAIN [ ANALYZE ] [ VERBOSE ] replaceable class=parameterstatement/replaceable
/synopsis
/refsynopsisdiv
*** EXPLAIN [ ANALYZE ] [ VERBOSE ] replace
*** 70,75
--- 71,86
are close to reality.
/para
+ para
+Only the literalANALYZE/literal and literalVERBOSE/literal options
+can be specified, and only in the order, without surrounding the option list
+in parentheses. Prior to productnamePostgreSQL/productname 8.5, the
+unparenthesized syntax was the only one supported. It is expected that
+all new options will be supported only when using the parenthesized syntax,
+which also allows a value to be specified for each option
+(e.g. literalTRUE/literal or literalFALSE/literal).
+ /para
+
important
para
Keep in mind that the statement is actually executed when
*** ROLLBACK;
*** 99,105
termliteralANALYZE/literal/term
listitem
para
! Carry out the command and show the actual run times.
/para
/listitem
/varlistentry
--- 110,117
termliteralANALYZE/literal/term
listitem
para
! Carry out the command and show the actual run times. This
! parameter defaults to commandOFF/command.
/para
/listitem
/varlistentry
*** ROLLBACK;
*** 108,114
termliteralVERBOSE/literal/term
listitem
para
! Include the output column list for each node in the plan tree.
/para
/listitem
/varlistentry
--- 120,152
termliteralVERBOSE/literal/term
listitem
para
! Include the output column list for each node in the plan tree. This
! parameter defaults to commandOFF/command.
! /para
! /listitem
!/varlistentry
!
!varlistentry
! termliteralCOSTS/literal/term
! listitem
! para
! Include information on the estimated startup and total cost of each
! plan node, as well as the estimated number of rows and the estimated
! width of each row. This parameter defaults to commandON/command.
! /para
! /listitem
!/varlistentry
!
!varlistentry
! termreplaceable class=parameter /boolean_value/replaceable/term
! listitem
! para
! Specifies whether the named
