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 ----
;
explain_option_name:
! ColId { $$ = $1; }
! | VERBOSE { $$ = "verbose"; }
! | analyze_keyword { $$ = "analyze"; }
;
explain_option_arg:
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
