Throw error if a references a window that already has a

frame clause, as appears to be required by the fine print in the SQL spec.
Per discussion with Pavel, not doing so risks user confusion.

diff --git a/doc/src/sgml/ref/select.sgml b/doc/src/sgml/ref/select.sgml
index ba8bff0e52384b857fec54e6508973eb58e4e4a3..0a19f894e7024b6cc86a0f13902c9afb4adb819d 100644 (file)
--- a/doc/src/sgml/ref/select.sgml
+++ b/doc/src/sgml/ref/select.sgml
@@ -1,5 +1,5 @@
 
 
@@ -583,7 +583,7 @@ WINDOW window_name AS ( 
 [ existing_window_name ]
 [ PARTITION BY expression [, ...] ]
 [ ORDER BY expression [ ASC | DESC | USING operator ] [ NULLS { FIRST | LAST } ] [, ...] ]
-[ framing_clause ]
+[ frame_clause ]
 
    
 
@@ -594,7 +594,8 @@ WINDOW window_name AS ( 
     as well as its ordering clause if any.  In this case the new window cannot
     specify its own PARTITION BY clause, and it can specify
     ORDER BY only if the copied window does not have one.
-    The framing clause is never copied from the existing window.
+    The new window always uses its own frame clause; the copied window
+    must not specify a frame clause.
    
 
    
@@ -611,7 +612,7 @@ WINDOW window_name AS ( 
    
 
    
-    The optional framing_clause defines
+    The optional frame_clause defines
     the window frame for window functions that depend on the
     frame (not all do).  It can be one of
 
@@ -1486,7 +1487,7 @@ SELECT distributors.* WHERE distributors.name = 'Westward';
 
    
     The SQL standard provides additional options for the window
-    framing_clause.
+    frame_clause.
     PostgreSQL currently supports only the
     options listed above.
    

diff --git a/doc/src/sgml/syntax.sgml b/doc/src/sgml/syntax.sgml
index 327bdd7f0f48b47766d955b7e78461a786b96398..6165259a8383796591b1c40d1a0ef45aadcd8e7d 100644 (file)
--- a/doc/src/sgml/syntax.sgml
+++ b/doc/src/sgml/syntax.sgml
@@ -1,4 +1,4 @@
-
+
 
 
  SQL Syntax
@@ -1588,12 +1588,12 @@ sqrt(2)
     where window_definition
     has the syntax
 
-[ window_name ]
+[ existing_window_name ]
 [ PARTITION BY expression [, ...] ]
 [ ORDER BY expression [ ASC | DESC | USING operator ] [ NULLS { FIRST | LAST } ] [, ...] ]
-[ framing_clause ]
+[ frame_clause ]
 
-    and the optional framing_clause
+    and the optional frame_clause
     can be one of
 
 RANGE UNBOUNDED PRECEDING
@@ -1614,7 +1614,8 @@ ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING
     Named window specifications are usually referenced with just
     OVER window_name, but it is
     also possible to write a window name inside the parentheses and then
-    optionally override its ordering clause and/or framing clause.
+    optionally supply an ordering clause and/or frame clause (the referenced
+    window must lack these clauses, if they are supplied here).
     This latter syntax follows the same rules as modifying an existing
     window name within the WINDOW clause; see the
      reference
@@ -1622,6 +1623,9 @@ ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING
    
 
    
+    The frame_clause specifies
+    the set of rows constituting the window frame, for those
+    window functions that act on the frame instead of the whole partition.
     The default framing option is RANGE UNBOUNDED PRECEDING,
     which is the same as RANGE BETWEEN UNBOUNDED PRECEDING AND
     CURRENT ROW; it selects rows up through the current row's last
@@ -1639,8 +1643,9 @@ ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING
 
    
     The built-in window functions are described in 
-    linkend="functions-window-table">.  Also, any built-in or
-    user-defined aggregate function can be used as a window function.
+    linkend="functions-window-table">.  Other window functions can be added by
+    the user.  Also, any built-in or user-defined aggregate function can be
+    used as a window function.
    
 
    


diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c

index 247d54d8782a1dcc41100a2403d86fef9bafb3e6..3e52ce6c3398792cceed8c3b17c7161a003f9f95 100644 (file)

--- a/src/backend/parser/parse_clause.c
+++ b/src/backend/parser/parse_clause.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/parser/parse_clause.c,v 1.183 2008/12/31 00:08:37 tgl Exp $
+ *   $PostgreSQL: pgsql/src/backend/parser/parse_clause.c,v 1.184 2008/12/31 23:42:56 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1554,7 +1554,10 @@ transformWindowDefinitions(ParseState *pstate,
         * Per spec, a windowdef that references a previous one copies the
         * previous partition clause (and mustn't specify its own).  It can
         * specify its own ordering clause. but only if the previous one
-        * had none.  It always specifies its own framing clause.
+        * had none.  It always specifies its own frame clause, and the
+        * previous one must not have a frame clause.  (Yeah, it's bizarre
+        * that each of these cases works differently, but SQL:2008 says so;
+        * see 7.11  syntax rule 10 and general rule 1.)
         */
        if (refwc)
        {
@@ -1592,6 +1595,12 @@ transformWindowDefinitions(ParseState *pstate,
            wc->orderClause = orderClause;
            wc->copiedOrder = false;
        }
+       if (refwc && refwc->frameOptions != FRAMEOPTION_DEFAULTS)
+           ereport(ERROR,
+                   (errcode(ERRCODE_WINDOWING_ERROR),
+                    errmsg("cannot override frame clause of window \"%s\"",
+                           windef->refname),
+                    parser_errposition(pstate, windef->location)));
        wc->frameOptions = windef->frameOptions;
        wc->winref = winref;
 

diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c
index 247d54d8782a1dcc41100a2403d86fef9bafb3e6..3e52ce6c3398792cceed8c3b17c7161a003f9f95 100644 (file)
--- a/src/backend/parser/parse_clause.c
+++ b/src/backend/parser/parse_clause.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/parser/parse_clause.c,v 1.183 2008/12/31 00:08:37 tgl Exp $
+ *   $PostgreSQL: pgsql/src/backend/parser/parse_clause.c,v 1.184 2008/12/31 23:42:56 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1554,7 +1554,10 @@ transformWindowDefinitions(ParseState *pstate,
         * Per spec, a windowdef that references a previous one copies the
         * previous partition clause (and mustn't specify its own).  It can
         * specify its own ordering clause. but only if the previous one
-        * had none.  It always specifies its own framing clause.
+        * had none.  It always specifies its own frame clause, and the
+        * previous one must not have a frame clause.  (Yeah, it's bizarre
+        * that each of these cases works differently, but SQL:2008 says so;
+        * see 7.11  syntax rule 10 and general rule 1.)
         */
        if (refwc)
        {
@@ -1592,6 +1595,12 @@ transformWindowDefinitions(ParseState *pstate,
            wc->orderClause = orderClause;
            wc->copiedOrder = false;
        }
+       if (refwc && refwc->frameOptions != FRAMEOPTION_DEFAULTS)
+           ereport(ERROR,
+                   (errcode(ERRCODE_WINDOWING_ERROR),
+                    errmsg("cannot override frame clause of window \"%s\"",
+                           windef->refname),
+                    parser_errposition(pstate, windef->location)));
        wc->frameOptions = windef->frameOptions;
        wc->winref = winref;