Re: generic explain options v3 - RR Review

On Sun, Jul 19, 2009 at 9:47 AM, Andres Freund<andres(at)anarazel(dot)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