Re: Windowing Function Patch Review -> Standard Conformance

Поиск
Список
Период
Сортировка
От Tom Lane
Тема Re: Windowing Function Patch Review -> Standard Conformance
Дата
Msg-id 6363.1229890896@sss.pgh.pa.us
обсуждение исходный текст
Ответ на Re: Windowing Function Patch Review -> Standard Conformance  (Tom Lane <tgl@sss.pgh.pa.us>)
Список pgsql-hackers
I wrote:
> I've been hacking on this and I have a grammar that pretty much works,
> but there's some bizarreness around UNBOUNDED.  I'll post it later.

Here is a proof-of-concept grammar patch that allows frame_bound to use
a_expr instead of a hacked-up constant production (which, as I
complained before, didn't allow all the cases demanded by the spec).
The key changes involved are:

* BETWEEN has to become a fully reserved word instead of type_func_name.

* PARTITION RANGE ROWS FOLLOWING PRECEDING have to be assigned the same
explicit precedence as IDENT.  (This doesn't have any bad consequences
AFAIK, except that you can't use a postfix operator in frame_bound exprs
unless you parenthesize it.)

* UNBOUNDED has to be assigned a precedence slightly lower than
PRECEDING and FOLLOWING, else it's ambiguous whether "UNBOUNDED
PRECEDING" ought to be parsed as "a_expr PRECEDING".  I'm a bit nervous
about this solution; we might someday need to make some of these
keywords at least partly reserved instead.  But right now it doesn't
seem to have any negative consequences.

Making BETWEEN fully reserved is a bit annoying, but it would only
break apps using BETWEEN as a type or function name.  The former doesn't
seem like a problem, the latter might be.

Comments?

            regards, tom lane

PS: I've removed some uninteresting hunks to shorten the diff, so you might
get some chatter from patch if you try to apply the diff.

*** src/backend/parser/gram.y    Sat Dec 20 11:02:55 2008
--- ./gram.y    Sun Dec 21 14:56:17 2008
***************
*** 158,163 ****
--- 158,164 ----
      DefElem                *defelt;
      OptionDefElem        *optdef;
      SortBy                *sortby;
+     WindowDef            *windef;
      JoinExpr            *jexpr;
      IndexElem            *ielem;
      Alias                *alias;
***************
*** 402,407 ****
--- 403,414 ----
  %type <with>     with_clause
  %type <list>    cte_list

+ %type <list>    window_clause window_definition_list opt_partition_clause
+ %type <windef>    window_definition over_clause window_specification
+ %type <str>        opt_existing_window_name
+ %type <node>    opt_frame_clause frame_clause frame_extent
+ %type <node>    frame_bound opt_frame_exclusion
+

  /*
   * If you make any token changes, update the keyword table in
***************
*** 431,440 ****
      DEFERRABLE DEFERRED DEFINER DELETE_P DELIMITER DELIMITERS DESC
      DICTIONARY DISABLE_P DISCARD DISTINCT DO DOCUMENT_P DOMAIN_P DOUBLE_P DROP

!     EACH ELSE ENABLE_P ENCODING ENCRYPTED END_P ENUM_P ESCAPE EXCEPT EXCLUDING
!     EXCLUSIVE EXECUTE EXISTS EXPLAIN EXTERNAL EXTRACT

!     FALSE_P FAMILY FETCH FIRST_P FLOAT_P FOR FORCE FOREIGN FORWARD
      FREEZE FROM FULL FUNCTION

      GLOBAL GRANT GRANTED GREATEST GROUP_P
--- 438,447 ----
      DEFERRABLE DEFERRED DEFINER DELETE_P DELIMITER DELIMITERS DESC
      DICTIONARY DISABLE_P DISCARD DISTINCT DO DOCUMENT_P DOMAIN_P DOUBLE_P DROP

!     EACH ELSE ENABLE_P ENCODING ENCRYPTED END_P ENUM_P ESCAPE EXCEPT EXCLUDE
!     EXCLUDING EXCLUSIVE EXECUTE EXISTS EXPLAIN EXTERNAL EXTRACT

!     FALSE_P FAMILY FETCH FIRST_P FLOAT_P FOLLOWING FOR FORCE FOREIGN FORWARD
      FREEZE FROM FULL FUNCTION

      GLOBAL GRANT GRANTED GREATEST GROUP_P
***************
*** 461,475 ****
      NOT NOTHING NOTIFY NOTNULL NOWAIT NULL_P NULLIF NULLS_P NUMERIC

      OBJECT_P OF OFF OFFSET OIDS OLD ON ONLY OPERATOR OPTION OPTIONS OR
!     ORDER OUT_P OUTER_P OVERLAPS OVERLAY OWNED OWNER

!     PARSER PARTIAL PASSWORD PLACING PLANS POSITION
!     PRECISION PRESERVE PREPARE PREPARED PRIMARY
      PRIOR PRIVILEGES PROCEDURAL PROCEDURE

      QUOTE

!     READ REAL REASSIGN RECHECK RECURSIVE REFERENCES REINDEX RELATIVE_P RELEASE
      RENAME REPEATABLE REPLACE REPLICA RESET RESTART RESTRICT RETURNING RETURNS
      REVOKE RIGHT ROLE ROLLBACK ROW ROWS RULE

--- 468,482 ----
      NOT NOTHING NOTIFY NOTNULL NOWAIT NULL_P NULLIF NULLS_P NUMERIC

      OBJECT_P OF OFF OFFSET OIDS OLD ON ONLY OPERATOR OPTION OPTIONS OR
!     ORDER OTHERS OUT_P OUTER_P OVER OVERLAPS OVERLAY OWNED OWNER

!     PARSER PARTIAL PARTITION PASSWORD PLACING PLANS POSITION
!     PRECEDING PRECISION PRESERVE PREPARE PREPARED PRIMARY
      PRIOR PRIVILEGES PROCEDURAL PROCEDURE

      QUOTE

!     RANGE READ REAL REASSIGN RECHECK RECURSIVE REFERENCES REINDEX RELATIVE_P RELEASE
      RENAME REPEATABLE REPLACE REPLICA RESET RESTART RESTRICT RETURNING RETURNS
      REVOKE RIGHT ROLE ROLLBACK ROW ROWS RULE

***************
*** 479,495 ****
      STATISTICS STDIN STDOUT STORAGE STRICT_P STRIP_P SUBSTRING SUPERUSER_P
      SYMMETRIC SYSID SYSTEM_P

!     TABLE TABLESPACE TEMP TEMPLATE TEMPORARY TEXT_P THEN TIME TIMESTAMP
      TO TRAILING TRANSACTION TREAT TRIGGER TRIM TRUE_P
      TRUNCATE TRUSTED TYPE_P

!     UNCOMMITTED UNENCRYPTED UNION UNIQUE UNKNOWN UNLISTEN UNTIL
      UPDATE USER USING

      VACUUM VALID VALIDATOR VALUE_P VALUES VARCHAR VARIADIC VARYING
      VERBOSE VERSION_P VIEW VOLATILE

!     WHEN WHERE WHITESPACE_P WITH WITHOUT WORK WRAPPER WRITE

      XML_P XMLATTRIBUTES XMLCONCAT XMLELEMENT XMLFOREST XMLPARSE
      XMLPI XMLROOT XMLSERIALIZE
--- 486,502 ----
      STATISTICS STDIN STDOUT STORAGE STRICT_P STRIP_P SUBSTRING SUPERUSER_P
      SYMMETRIC SYSID SYSTEM_P

!     TABLE TABLESPACE TEMP TEMPLATE TEMPORARY TEXT_P THEN TIES TIME TIMESTAMP
      TO TRAILING TRANSACTION TREAT TRIGGER TRIM TRUE_P
      TRUNCATE TRUSTED TYPE_P

!     UNBOUNDED UNCOMMITTED UNENCRYPTED UNION UNIQUE UNKNOWN UNLISTEN UNTIL
      UPDATE USER USING

      VACUUM VALID VALIDATOR VALUE_P VALUES VARCHAR VARIADIC VARYING
      VERBOSE VERSION_P VIEW VOLATILE

!     WHEN WHERE WHITESPACE_P WINDOW WITH WITHOUT WORK WRAPPER WRITE

      XML_P XMLATTRIBUTES XMLCONCAT XMLELEMENT XMLFOREST XMLPARSE
      XMLPI XMLROOT XMLSERIALIZE
***************
*** 523,529 ****
  %nonassoc    BETWEEN
  %nonassoc    IN_P
  %left        POSTFIXOP        /* dummy for postfix Op rules */
! %nonassoc    IDENT            /* to support target_el without AS */
  %left        Op OPERATOR        /* multi-character ops and user-defined operators */
  %nonassoc    NOTNULL
  %nonassoc    ISNULL
--- 530,547 ----
  %nonassoc    BETWEEN
  %nonassoc    IN_P
  %left        POSTFIXOP        /* dummy for postfix Op rules */
! %nonassoc    UNBOUNDED        /* needed for frame_extent, frame_bound */
! /*
!  * To support target_el without AS, we must give IDENT an explicit priority
!  * between POSTFIXOP and Op.  We can safely assign the same priority to
!  * various unreserved keywords as needed to resolve ambiguities (this can't
!  * have any bad effects since obviously the keywords will still behave the
!  * same as if they weren't keywords).  We need to do this for PARTITION,
!  * RANGE, ROWS to support opt_existing_window_name; and for RANGE, ROWS,
!  * FOLLOWING, PRECEDING so that they can follow a_expr without creating
!  * postfix-operator problems.
!  */
! %nonassoc    IDENT PARTITION RANGE ROWS FOLLOWING PRECEDING
  %left        Op OPERATOR        /* multi-character ops and user-defined operators */
  %nonassoc    NOTNULL
  %nonassoc    ISNULL
***************
*** 6825,6831 ****
  simple_select:
              SELECT opt_distinct target_list
              into_clause from_clause where_clause
!             group_clause having_clause
                  {
                      SelectStmt *n = makeNode(SelectStmt);
                      n->distinctClause = $2;
--- 6848,6854 ----
  simple_select:
              SELECT opt_distinct target_list
              into_clause from_clause where_clause
!             group_clause having_clause window_clause
                  {
                      SelectStmt *n = makeNode(SelectStmt);
                      n->distinctClause = $2;
***************
*** 6835,6840 ****
--- 6858,6864 ----
                      n->whereClause = $6;
                      n->groupClause = $7;
                      n->havingClause = $8;
+                     n->windowClause = $9;
                      $$ = (Node *)n;
                  }
              | values_clause                            { $$ = $1; }
***************
*** 8622,8628 ****
   * (Note that many of the special SQL functions wouldn't actually make any
   * sense as functional index entries, but we ignore that consideration here.)
   */
! func_expr:    func_name '(' ')'
                  {
                      FuncCall *n = makeNode(FuncCall);
                      n->funcname = $1;
--- 8655,8661 ----
   * (Note that many of the special SQL functions wouldn't actually make any
   * sense as functional index entries, but we ignore that consideration here.)
   */
! func_expr:    func_name '(' ')' over_clause
                  {
                      FuncCall *n = makeNode(FuncCall);
                      n->funcname = $1;
***************
*** 8630,8639 ****
                      n->agg_star = FALSE;
                      n->agg_distinct = FALSE;
                      n->func_variadic = FALSE;
                      n->location = @1;
                      $$ = (Node *)n;
                  }
!             | func_name '(' expr_list ')'
                  {
                      FuncCall *n = makeNode(FuncCall);
                      n->funcname = $1;
--- 8663,8673 ----
                      n->agg_star = FALSE;
                      n->agg_distinct = FALSE;
                      n->func_variadic = FALSE;
+                     n->over = $4;
                      n->location = @1;
                      $$ = (Node *)n;
                  }
!             | func_name '(' expr_list ')' over_clause
                  {
                      FuncCall *n = makeNode(FuncCall);
                      n->funcname = $1;
***************
*** 8641,8650 ****
                      n->agg_star = FALSE;
                      n->agg_distinct = FALSE;
                      n->func_variadic = FALSE;
                      n->location = @1;
                      $$ = (Node *)n;
                  }
!             | func_name '(' VARIADIC a_expr ')'
                  {
                      FuncCall *n = makeNode(FuncCall);
                      n->funcname = $1;
--- 8675,8685 ----
                      n->agg_star = FALSE;
                      n->agg_distinct = FALSE;
                      n->func_variadic = FALSE;
+                     n->over = $5;
                      n->location = @1;
                      $$ = (Node *)n;
                  }
!             | func_name '(' VARIADIC a_expr ')' /* intentionally not accept over_clause */
                  {
                      FuncCall *n = makeNode(FuncCall);
                      n->funcname = $1;
***************
*** 8652,8661 ****
                      n->agg_star = FALSE;
                      n->agg_distinct = FALSE;
                      n->func_variadic = TRUE;
                      n->location = @1;
                      $$ = (Node *)n;
                  }
!             | func_name '(' expr_list ',' VARIADIC a_expr ')'
                  {
                      FuncCall *n = makeNode(FuncCall);
                      n->funcname = $1;
--- 8687,8697 ----
                      n->agg_star = FALSE;
                      n->agg_distinct = FALSE;
                      n->func_variadic = TRUE;
+                     n->over = NULL;
                      n->location = @1;
                      $$ = (Node *)n;
                  }
!             | func_name '(' expr_list ',' VARIADIC a_expr ')' /* intentionally not accept over_clause */
                  {
                      FuncCall *n = makeNode(FuncCall);
                      n->funcname = $1;
***************
*** 8663,8672 ****
                      n->agg_star = FALSE;
                      n->agg_distinct = FALSE;
                      n->func_variadic = TRUE;
                      n->location = @1;
                      $$ = (Node *)n;
                  }
!             | func_name '(' ALL expr_list ')'
                  {
                      FuncCall *n = makeNode(FuncCall);
                      n->funcname = $1;
--- 8699,8709 ----
                      n->agg_star = FALSE;
                      n->agg_distinct = FALSE;
                      n->func_variadic = TRUE;
+                     n->over = NULL;
                      n->location = @1;
                      $$ = (Node *)n;
                  }
!             | func_name '(' ALL expr_list ')' /* intentionally not accept over_clause */
                  {
                      FuncCall *n = makeNode(FuncCall);
                      n->funcname = $1;
***************
*** 8678,8687 ****
                       * for that in FuncCall at the moment.
                       */
                      n->func_variadic = FALSE;
                      n->location = @1;
                      $$ = (Node *)n;
                  }
!             | func_name '(' DISTINCT expr_list ')'
                  {
                      FuncCall *n = makeNode(FuncCall);
                      n->funcname = $1;
--- 8715,8725 ----
                       * for that in FuncCall at the moment.
                       */
                      n->func_variadic = FALSE;
+                     n->over = NULL;
                      n->location = @1;
                      $$ = (Node *)n;
                  }
!             | func_name '(' DISTINCT expr_list ')' /* intentionally not accept over_clause */
                  {
                      FuncCall *n = makeNode(FuncCall);
                      n->funcname = $1;
***************
*** 8689,8698 ****
                      n->agg_star = FALSE;
                      n->agg_distinct = TRUE;
                      n->func_variadic = FALSE;
                      n->location = @1;
                      $$ = (Node *)n;
                  }
!             | func_name '(' '*' ')'
                  {
                      /*
                       * We consider AGGREGATE(*) to invoke a parameterless
--- 8727,8737 ----
                      n->agg_star = FALSE;
                      n->agg_distinct = TRUE;
                      n->func_variadic = FALSE;
+                     n->over = NULL;
                      n->location = @1;
                      $$ = (Node *)n;
                  }
!             | func_name '(' '*' ')' over_clause
                  {
                      /*
                       * We consider AGGREGATE(*) to invoke a parameterless
***************
*** 8710,8715 ****
--- 8749,8755 ----
                      n->agg_star = TRUE;
                      n->agg_distinct = FALSE;
                      n->func_variadic = FALSE;
+                     n->over = $5;
                      n->location = @1;
                      $$ = (Node *)n;
                  }
***************
*** 9157,9162 ****
--- 9213,9345 ----
          ;

  /*
+  * Window Definitions
+  */
+ window_clause:
+             WINDOW window_definition_list            { $$ = $2; }
+             | /*EMPTY*/                                { $$ = NIL; }
+         ;
+
+ window_definition_list:
+             window_definition                        { $$ = list_make1($1); }
+             | window_definition_list ',' window_definition
+                                                     { $$ = lappend($1, $3); }
+         ;
+
+ window_definition:
+             ColId AS window_specification
+                 {
+                     WindowDef *n = $3;
+                     n->name = $1;
+                     $$ = n;
+                 }
+         ;
+
+ over_clause: OVER window_specification
+                 { $$ = $2; }
+             | OVER ColId
+                 {
+                     WindowDef *n = makeNode(WindowDef);
+                     n->name = NULL;
+                     n->refname = $2;
+                     n->partitionClause = NIL;
+                     n->orderClause = NIL;
+                     n->referenced = false;
+                     n->location = @2;
+                     $$ = n;
+                 }
+             | /*EMPTY*/
+                 { $$ = NULL; }
+         ;
+
+ window_specification: '(' opt_existing_window_name opt_partition_clause
+                         opt_sort_clause opt_frame_clause ')'
+                 {
+                     WindowDef *n = makeNode(WindowDef);
+                     n->name = NULL;
+                     n->refname = $2;
+                     n->partitionClause = $3;
+                     n->orderClause = $4;
+                     n->referenced = false;
+                     n->location = @1;
+                     $$ = n;
+                 }
+         ;
+
+ /*
+  * If we see PARTITION, RANGE, or ROWS as the first token after the '('
+  * of a window_specification, we want the assumption to be that there is
+  * no existing_window_name; but those keywords are unreserved and so could
+  * be ColIds.  We fix this by making them have the same precedence as IDENT
+  * and giving the empty production here a slightly higher precedence, so
+  * that the shift/reduce conflict is resolved in favor of reducing the rule.
+  * These keywords are thus precluded from being an existing_window_name but
+  * are not reserved for any other purpose.
+  */
+ opt_existing_window_name: ColId                        { $$ = $1; }
+             | /*EMPTY*/                %prec Op        { $$ = NULL; }
+         ;
+
+ opt_partition_clause: PARTITION BY expr_list        { $$ = $3; }
+             | /*EMPTY*/                                { $$ = NIL; }
+         ;
+
+ /*
+  * Frame clause is not implemented yet except for the grammar
+  */
+ opt_frame_clause:
+             frame_clause
+             {
+                 ereport(ERROR,
+                         (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                          errmsg("ROWS/RANGE clause of window functions not yet implemented")));
+             }
+             | /*EMPTY*/                                { $$ = NULL; }
+         ;
+
+ frame_clause:
+             ROWS frame_extent opt_frame_exclusion
+             {
+                 $$ = NULL;
+             }
+             | RANGE frame_extent opt_frame_exclusion
+             {
+                 $$ = NULL;
+             }
+         ;
+
+ /*
+  * Since UNBOUNDED is an unreserved keyword, it could possibly be an a_expr.
+  * We fix that by making UNBOUNDED have slightly lower precedence than the
+  * lookahead tokens PRECEDING and FOLLOWING.  This solution might cause
+  * strange behavior if UNBOUNDED is ever used anyplace else in the grammar,
+  * however.
+  */
+ frame_extent:
+             UNBOUNDED PRECEDING                        { $$ = NULL; }
+             | CURRENT_P ROW                            { $$ = NULL; }
+             | a_expr PRECEDING                        { $$ = NULL; }
+             | BETWEEN frame_bound AND frame_bound    { $$ = NULL; }
+         ;
+
+ frame_bound:
+             UNBOUNDED PRECEDING                        { $$ = NULL; }
+             | CURRENT_P ROW                            { $$ = NULL; }
+             | a_expr PRECEDING                        { $$ = NULL; }
+             | UNBOUNDED FOLLOWING                    { $$ = NULL; }
+             | a_expr FOLLOWING                        { $$ = NULL; }
+         ;
+
+ opt_frame_exclusion:
+             EXCLUDE CURRENT_P ROW                    { $$ = NULL; }
+             | EXCLUDE GROUP_P                        { $$ = NULL; }
+             | EXCLUDE TIES                            { $$ = NULL; }
+             | EXCLUDE NO OTHERS                        { $$ = NULL; }
+             | /*EMPTY*/                                { $$ = NULL; }
+         ;
+
+
+ /*
   * Supporting nonterminals for expressions.
   */

***************
*** 9883,9888 ****
--- 10066,10072 ----
              | ENCRYPTED
              | ENUM_P
              | ESCAPE
+             | EXCLUDE
              | EXCLUDING
              | EXCLUSIVE
              | EXECUTE
***************
*** 9890,9895 ****
--- 10074,10080 ----
              | EXTERNAL
              | FAMILY
              | FIRST_P
+             | FOLLOWING
              | FORCE
              | FORWARD
              | FUNCTION
***************
*** 9957,9968 ****
--- 10142,10156 ----
              | OPERATOR
              | OPTION
              | OPTIONS
+             | OTHERS
              | OWNED
              | OWNER
              | PARSER
              | PARTIAL
+             | PARTITION
              | PASSWORD
              | PLANS
+             | PRECEDING
              | PREPARE
              | PREPARED
              | PRESERVE
***************
*** 9971,9976 ****
--- 10159,10165 ----
              | PROCEDURAL
              | PROCEDURE
              | QUOTE
+             | RANGE
              | READ
              | REASSIGN
              | RECHECK
***************
*** 10023,10033 ****
--- 10212,10224 ----
              | TEMPLATE
              | TEMPORARY
              | TEXT_P
+             | TIES
              | TRANSACTION
              | TRIGGER
              | TRUNCATE
              | TRUSTED
              | TYPE_P
+             | UNBOUNDED
              | UNCOMMITTED
              | UNENCRYPTED
              | UNKNOWN
***************
*** 10123,10129 ****
   */
  type_func_name_keyword:
                AUTHORIZATION
-             | BETWEEN
              | BINARY
              | CROSS
              | CURRENT_SCHEMA
--- 10314,10319 ----
***************
*** 10139,10144 ****
--- 10329,10335 ----
              | NATURAL
              | NOTNULL
              | OUTER_P
+             | OVER
              | OVERLAPS
              | RIGHT
              | SIMILAR
***************
*** 10161,10166 ****
--- 10352,10358 ----
              | AS
              | ASC
              | ASYMMETRIC
+             | BETWEEN
              | BOTH
              | CASE
              | CAST
***************
*** 10229,10234 ****
--- 10421,10427 ----
              | VARIADIC
              | WHEN
              | WHERE
+             | WINDOW
              | WITH
          ;


В списке pgsql-hackers по дате отправления:

Предыдущее
От: Tom Lane
Дата:
Сообщение: Re: Windowing Function Patch Review -> Standard Conformance
Следующее
От: Markus Wanner
Дата:
Сообщение: Re: Sync Rep: First Thoughts on Code