Re: Gram.y patches for better parenthesis handling.

Applied.   Thanks.


> Okay, here's my attempt at fixing the problems with parentheses in
> subqueries.  It passes the normal 'runcheck' tests, and I've tried
> a few simple things like 
>   select 1 as foo union (((((select 2))))) order by foo;
> 
> There are a few things that it doesn't do that have been talked 
> about here at least a little:
> 
> 1) It doesn't allow things like "IN(((select 1)))" -- the select
> here has to be at the top level.  This is not new.
> 
> 2) It does NOT preserve the odd syntax I found when I started looking
> at this, where a SELECT statement could begin with parentheses.  Thus,
>   (SELECT a from foo) order by a;
> fails.
> 
> I have preserved the ability, used in the regression tests, to
> have a single select statement in what appears to be a RuleActionMulti
> (but wasn't -- the parens were part of select_clause syntax).
> In my version, this is a special form.
> 
> This may cause some discussion: I have differentiated the two kinds
> of RuleActionMulti.  Perhaps nobody knew there were two kinds, because
> I don't think the second form appears in the regression tests. This
> one uses square brackets instead of parentheses, but originally was
> otherwise the same as the one in parentheses.  In this version of
> gram.y, the square bracket form treats SELECT statements the same
> as the other allowed statements.  As discussed before on this list,
> psql cannot make sense out of the results of such a thing, but an
> application might.  And I have designs on just such an application.
> 
> ++ kevin
> 
> 
> 
> -- 
> Kevin O'Gorman  (805) 650-6274  mailto:kogorman@pacbell.net
> Permanent e-mail forwarder:  mailto:Kevin.O'Gorman.64@Alum.Dartmouth.org
> At school: mailto:kogorman@cs.ucsb.edu
> Web: http://www.cs.ucsb.edu/~kogorman/index.html
> Web: http://trixie.kosman.via.ayuda.com/~kevin/index.html
> 
> "There is a freedom lying beyond circumstance,
> derived from the direct intuition that life can
> be grounded upon its absorption in what is
> changeless amid change" 
>    -- Alfred North Whitehead

> --- gram.y.orig    Thu Oct 26 13:13:04 2000
> +++ gram.y    Fri Oct 27 17:37:58 2000
> @@ -124,14 +124,15 @@
>          DropGroupStmt, DropPLangStmt, DropSchemaStmt, DropStmt, DropTrigStmt,
>          DropUserStmt, DropdbStmt, ExplainStmt, ExtendStmt, FetchStmt,
>          GrantStmt, IndexStmt, InsertStmt, ListenStmt, LoadStmt, LockStmt,
> -        NotifyStmt, OptimizableStmt, ProcedureStmt, ReindexStmt,
> +        NotifyStmt, OptimizableStmt, ProcedureStmt
> +        QualifiedSelectStmt, ReindexStmt,
>          RemoveAggrStmt, RemoveFuncStmt, RemoveOperStmt, RemoveStmt,
>          RenameStmt, RevokeStmt, RuleActionStmt, RuleActionStmtOrEmpty,
>          RuleStmt, SelectStmt, SetSessionStmt, TransactionStmt, TruncateStmt,
>          UnlistenStmt, UpdateStmt, VacuumStmt, VariableResetStmt,
>          VariableSetStmt, VariableShowStmt, ViewStmt
>  
> -%type <node>    select_clause, select_subclause
> +%type <node>    subquery, simple_select, select_head, set_select
>  
>  %type <list>    SessionList
>  %type <node>    SessionClause
> @@ -174,19 +175,20 @@
>          result, OptTempTableName, relation_name_list, OptTableElementList,
>          OptUnder, OptInherit, definition, opt_distinct,
>          opt_with, func_args, func_args_list, func_as,
> -        oper_argtypes, RuleActionList, RuleActionMulti,
> +        oper_argtypes, RuleActionList, RuleActionMulti, 
> +        RuleActionOrSelectMulti, RuleActions, RuleActionBracket,
>          opt_column_list, columnList, opt_va_list, va_list,
>          sort_clause, sortby_list, index_params, index_list, name_list,
>          from_clause, from_list, opt_array_bounds,
>          expr_list, attrs, target_list, update_target_list,
>          def_list, opt_indirection, group_clause, TriggerFuncArgs,
> -        opt_select_limit
> +        opt_select_limit, select_limit
>  
>  %type <typnam>    func_arg, func_return, aggr_argtype
>  
>  %type <boolean>    opt_arg, TriggerForOpt, TriggerForType, OptTemp
>  
> -%type <list>    for_update_clause, update_list
> +%type <list>    opt_for_update_clause, for_update_clause, update_list
>  %type <boolean>    opt_all
>  %type <boolean>    opt_table
>  %type <boolean>    opt_chain, opt_trans
> @@ -2689,7 +2691,7 @@
>  RuleStmt:  CREATE RULE name AS
>             { QueryIsRule=TRUE; }
>             ON event TO event_object where_clause
> -           DO opt_instead RuleActionList
> +           DO opt_instead RuleActions
>                  {
>                      RuleStmt *n = makeNode(RuleStmt);
>                      n->rulename = $3;
> @@ -2702,17 +2704,42 @@
>                  }
>          ;
>  
> -RuleActionList:  NOTHING                { $$ = NIL; }
> -        | SelectStmt                    { $$ = makeList1($1); }
> -        | RuleActionStmt                { $$ = makeList1($1); }
> -        | '[' RuleActionMulti ']'        { $$ = $2; }
> -        | '(' RuleActionMulti ')'        { $$ = $2; } 
> +RuleActions:  NOTHING                { $$ = NIL; }
> +        | RuleActionStmt            { $$ = makeList1($1); }
> +        | SelectStmt                { $$ = makeList1($1); }
> +        | RuleActionList
> +        | RuleActionBracket
> +        ;
> +
> +/* LEGACY: Version 7.0 did not like SELECT statements in these lists,
> + * but because of an oddity in the syntax for select_clause, allowed
> + * certain forms like "DO INSTEAD (select 1)", and this is used in
> + * the regression tests.
> + * Here, we're allowing just one SELECT in parentheses, to preserve
> + * any such expectations, and make the regression tests work.
> + *               ++ KO'G
> + */
> +RuleActionList:        '(' RuleActionMulti ')'        { $$ = $2; } 
> +        | '(' SelectStmt ')'    { $$ = makeList1($2); }
> +        ;
> +
> +/* An undocumented feature, bracketed lists are allowed to contain
> + * SELECT statements on the same basis as the others.  Before this,
> + * they were the same as parenthesized lists, and did not allow
> + * SelectStmts.  Anybody know why they were here originally?  Or if
> + * they're in the regression tests at all?
> + *               ++ KO'G
> + */
> +RuleActionBracket:    '[' RuleActionOrSelectMulti ']'        { $$ = $2; } 
>          ;
>  
>  /* the thrashing around here is to discard "empty" statements... */
>  RuleActionMulti:  RuleActionMulti ';' RuleActionStmtOrEmpty
>                  { if ($3 != (Node *) NULL)
> -                    $$ = lappend($1, $3);
> +                    if ($1 != NIL)
> +                        $$ = lappend($1, $3);
> +                    else
> +                        $$ = makeList1($3);
>                    else
>                      $$ = $1;
>                  }
> @@ -2724,6 +2751,31 @@
>                  }
>          ;
>  
> +RuleActionOrSelectMulti: RuleActionOrSelectMulti ';' RuleActionStmtOrEmpty
> +                { if ($3 != (Node *) NULL)
> +                    if ($1 != NIL)
> +                        $$ = lappend($1, $3);
> +                    else
> +                        $$ = makeList1($3);
> +                  else
> +                    $$ = $1;
> +                }
> +        | RuleActionOrSelectMulti ';' SelectStmt
> +                { if ($1 != NIL)
> +                        $$ = lappend($1, $3);
> +                  else
> +                        $$ = makeList1($3);
> +                }
> +        | RuleActionStmtOrEmpty
> +                { if ($1 != (Node *) NULL)
> +                    $$ = makeList1($1);
> +                  else
> +                    $$ = NIL;
> +                }
> +        | SelectStmt        { $$ = makeList1($1); }
> +        ;
> +
> +
>  RuleActionStmt:    InsertStmt
>          | UpdateStmt
>          | DeleteStmt
> @@ -3289,7 +3341,12 @@
>   * However, this is not checked by the grammar; parse analysis must check it.
>   */
>  
> -SelectStmt:      select_clause sort_clause for_update_clause opt_select_limit
> +SelectStmt:    QualifiedSelectStmt
> +        | select_head
> +        ;
> +
> +QualifiedSelectStmt:
> +          select_head sort_clause opt_for_update_clause opt_select_limit
>              {
>                  SelectStmt *n = findLeftmostSelect($1);
>  
> @@ -3299,34 +3356,35 @@
>                  n->limitCount = nth(1, $4);
>                  $$ = $1;
>              }
> -        ;
> -
> -/* This rule parses Select statements that can appear within set operations,
> - * including UNION, INTERSECT and EXCEPT.  '(' and ')' can be used to specify
> - * the ordering of the set operations.  Without '(' and ')' we want the
> - * operations to be ordered per the precedence specs at the head of this file.
> - *
> - * Since parentheses around SELECTs also appear in the expression grammar,
> - * there is a parse ambiguity if parentheses are allowed at the top level of a
> - * select_clause: are the parens part of the expression or part of the select?
> - * We separate select_clause into two levels to resolve this: select_clause
> - * can have top-level parentheses, select_subclause cannot.
> - *
> - * Note that sort clauses cannot be included at this level --- a sort clause
> - * can only appear at the end of the complete Select, and it will be handled
> - * by the topmost SelectStmt rule.  Likewise FOR UPDATE and LIMIT.
> - */
> -select_clause: '(' select_subclause ')'
> +        | select_head for_update_clause opt_select_limit
>              {
> -                $$ = $2; 
> +                SelectStmt *n = findLeftmostSelect($1);
> +
> +                n->sortClause = NULL;
> +                n->forUpdate = $2;
> +                n->limitOffset = nth(0, $3);
> +                n->limitCount = nth(1, $3);
> +                $$ = $1;
>              }
> -        | select_subclause
> +        | select_head select_limit
>              {
> -                $$ = $1; 
> +                SelectStmt *n = findLeftmostSelect($1);
> +
> +                n->sortClause = NULL;
> +                n->forUpdate = NULL;
> +                n->limitOffset = nth(0, $2);
> +                n->limitCount = nth(1, $2);
> +                $$ = $1;
>              }
>          ;
>  
> -select_subclause: SELECT opt_distinct target_list
> +subquery:    '(' subquery ')'            { $$ = $2; }
> +        | '(' QualifiedSelectStmt ')'    { $$ = $2; }
> +        | '(' set_select ')'            { $$ = $2; }
> +        | simple_select                    { $$ = $1; }
> +        ;
> +
> +simple_select: SELECT opt_distinct target_list
>               result from_clause where_clause
>               group_clause having_clause
>                  {
> @@ -3341,7 +3399,13 @@
>                      n->havingClause = $8;
>                      $$ = (Node *)n;
>                  }
> -        | select_clause UNION opt_all select_clause
> +        ;
> +
> +select_head: simple_select            { $$ = $1; }
> +        |    set_select                { $$ = $1; }
> +        ;
> +
> +set_select: select_head UNION opt_all subquery
>              {    
>                  SetOperationStmt *n = makeNode(SetOperationStmt);
>                  n->op = SETOP_UNION;
> @@ -3350,7 +3414,7 @@
>                  n->rarg = $4;
>                  $$ = (Node *) n;
>              }
> -        | select_clause INTERSECT opt_all select_clause
> +        | select_head INTERSECT opt_all subquery
>              {
>                  SetOperationStmt *n = makeNode(SetOperationStmt);
>                  n->op = SETOP_INTERSECT;
> @@ -3359,7 +3423,7 @@
>                  n->rarg = $4;
>                  $$ = (Node *) n;
>              }
> -        | select_clause EXCEPT opt_all select_clause
> +        | select_head EXCEPT opt_all subquery
>              {
>                  SetOperationStmt *n = makeNode(SetOperationStmt);
>                  n->op = SETOP_EXCEPT;
> @@ -3424,7 +3488,6 @@
>          ;
>  
>  sort_clause:  ORDER BY sortby_list                { $$ = $3; }
> -        | /*EMPTY*/                                { $$ = NIL; }
>          ;
>  
>  sortby_list:  sortby                            { $$ = makeList1($1); }
> @@ -3446,7 +3509,7 @@
>          ;
>  
>  
> -opt_select_limit:    LIMIT select_limit_value ',' select_offset_value
> +select_limit:    LIMIT select_limit_value ',' select_offset_value
>              { $$ = makeList2($4, $2); }
>          | LIMIT select_limit_value OFFSET select_offset_value
>              { $$ = makeList2($4, $2); }
> @@ -3456,6 +3519,9 @@
>              { $$ = makeList2($2, $4); }
>          | OFFSET select_offset_value
>              { $$ = makeList2($2, NULL); }
> +        ;
> +
> +opt_select_limit: select_limit    { $$ = $1; }
>          | /* EMPTY */
>              { $$ = makeList2(NULL, NULL); }
>          ;
> @@ -3555,6 +3621,9 @@
>  
>  for_update_clause:  FOR UPDATE update_list        { $$ = $3; }
>          | FOR READ ONLY                            { $$ = NULL; }
> +        ;
> +
> +opt_for_update_clause:    for_update_clause        { $$ = $1; }
>          | /* EMPTY */                            { $$ = NULL; }
>          ;
>  
> @@ -3598,7 +3667,7 @@
>                      $1->name = $2;
>                      $$ = (Node *) $1;
>                  }
> -        | '(' select_subclause ')' alias_clause
> +        | '(' SelectStmt ')' alias_clause
>                  {
>                      RangeSubselect *n = makeNode(RangeSubselect);
>                      n->subquery = $2;
> @@ -4134,7 +4203,7 @@
>   * Define row_descriptor to allow yacc to break the reduce/reduce conflict
>   *  with singleton expressions.
>   */
> -row_expr: '(' row_descriptor ')' IN '(' select_subclause ')'
> +row_expr: '(' row_descriptor ')' IN '(' SelectStmt ')'
>                  {
>                      SubLink *n = makeNode(SubLink);
>                      n->lefthand = $2;
> @@ -4144,7 +4213,7 @@
>                      n->subselect = $6;
>                      $$ = (Node *)n;
>                  }
> -        | '(' row_descriptor ')' NOT IN '(' select_subclause ')'
> +        | '(' row_descriptor ')' NOT IN '(' SelectStmt ')'
>                  {
>                      SubLink *n = makeNode(SubLink);
>                      n->lefthand = $2;
> @@ -4154,7 +4223,7 @@
>                      n->subselect = $7;
>                      $$ = (Node *)n;
>                  }
> -        | '(' row_descriptor ')' all_Op sub_type '(' select_subclause ')'
> +        | '(' row_descriptor ')' all_Op sub_type '(' SelectStmt ')'
>                  {
>                      SubLink *n = makeNode(SubLink);
>                      n->lefthand = $2;
> @@ -4167,7 +4236,7 @@
>                      n->subselect = $7;
>                      $$ = (Node *)n;
>                  }
> -        | '(' row_descriptor ')' all_Op '(' select_subclause ')'
> +        | '(' row_descriptor ')' all_Op '(' SelectStmt ')'
>                  {
>                      SubLink *n = makeNode(SubLink);
>                      n->lefthand = $2;
> @@ -4498,7 +4567,7 @@
>                          $$ = n;
>                      }
>                  }
> -        | a_expr all_Op sub_type '(' select_subclause ')'
> +        | a_expr all_Op sub_type '(' SelectStmt ')'
>                  {
>                      SubLink *n = makeNode(SubLink);
>                      n->lefthand = makeList1($1);
> @@ -4894,7 +4963,7 @@
>                      n->agg_distinct = FALSE;
>                      $$ = (Node *)n;
>                  }
> -        | '(' select_subclause ')'
> +        | '(' SelectStmt ')'
>                  {
>                      SubLink *n = makeNode(SubLink);
>                      n->lefthand = NIL;
> @@ -4904,7 +4973,7 @@
>                      n->subselect = $2;
>                      $$ = (Node *)n;
>                  }
> -        | EXISTS '(' select_subclause ')'
> +        | EXISTS '(' SelectStmt ')'
>                  {
>                      SubLink *n = makeNode(SubLink);
>                      n->lefthand = NIL;
> @@ -5003,7 +5072,7 @@
>                  { $$ = $1; }
>          ;
>  
> -in_expr:  select_subclause
> +in_expr:  SelectStmt
>                  {
>                      SubLink *n = makeNode(SubLink);
>                      n->subselect = $1;


--  Bruce Momjian                        |  http://candle.pha.pa.us pgman@candle.pha.pa.us               |  (610)
853-3000+  If your life is a hard drive,     |  830 Blythe Avenue +  Christ can be your backup.        |  Drexel Hill,
Pennsylvania19026