Throw error if a <window definition> 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 @@
 <!--
-$PostgreSQL: pgsql/doc/src/sgml/ref/select.sgml,v 1.115 2008/12/31 00:08:35 tgl Exp $
+$PostgreSQL: pgsql/doc/src/sgml/ref/select.sgml,v 1.116 2008/12/31 23:42:56 tgl Exp $
 PostgreSQL documentation
 -->
 
@@ -583,7 +583,7 @@ WINDOW <replaceable class="parameter">window_name</replaceable> AS ( <replaceabl
 [ <replaceable class="parameter">existing_window_name</replaceable> ]
 [ PARTITION BY <replaceable class="parameter">expression</replaceable> [, ...] ]
 [ ORDER BY <replaceable class="parameter">expression</replaceable> [ ASC | DESC | USING <replaceable class="parameter">operator</replaceable> ] [ NULLS { FIRST | LAST } ] [, ...] ]
-[ <replaceable class="parameter">framing_clause</replaceable> ]
+[ <replaceable class="parameter">frame_clause</replaceable> ]
 </synopsis>
    </para>
 
@@ -594,7 +594,8 @@ WINDOW <replaceable class="parameter">window_name</replaceable> AS ( <replaceabl
     as well as its ordering clause if any.  In this case the new window cannot
     specify its own <literal>PARTITION BY</> clause, and it can specify
     <literal>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.
    </para>
 
    <para>
@@ -611,7 +612,7 @@ WINDOW <replaceable class="parameter">window_name</replaceable> AS ( <replaceabl
    </para>
 
    <para>
-    The optional <replaceable class="parameter">framing_clause</> defines
+    The optional <replaceable class="parameter">frame_clause</> defines
     the <firstterm>window frame</> for window functions that depend on the
     frame (not all do).  It can be one of
 <synopsis>
@@ -1486,7 +1487,7 @@ SELECT distributors.* WHERE distributors.name = 'Westward';
 
    <para>
     The SQL standard provides additional options for the window
-    <replaceable class="parameter">framing_clause</>.
+    <replaceable class="parameter">frame_clause</>.
     <productname>PostgreSQL</productname> currently supports only the
     options listed above.
    </para>

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 @@
-<!-- $PostgreSQL: pgsql/doc/src/sgml/syntax.sgml,v 1.128 2008/12/31 00:08:35 tgl Exp $ -->
+<!-- $PostgreSQL: pgsql/doc/src/sgml/syntax.sgml,v 1.129 2008/12/31 23:42:56 tgl Exp $ -->
 
 <chapter id="sql-syntax">
  <title>SQL Syntax</title>
@@ -1588,12 +1588,12 @@ sqrt(2)
     where <replaceable class="parameter">window_definition</replaceable>
     has the syntax
 <synopsis>
-[ <replaceable class="parameter">window_name</replaceable> ]
+[ <replaceable class="parameter">existing_window_name</replaceable> ]
 [ PARTITION BY <replaceable class="parameter">expression</replaceable> [, ...] ]
 [ ORDER BY <replaceable class="parameter">expression</replaceable> [ ASC | DESC | USING <replaceable class="parameter">operator</replaceable> ] [ NULLS { FIRST | LAST } ] [, ...] ]
-[ <replaceable class="parameter">framing_clause</replaceable> ]
+[ <replaceable class="parameter">frame_clause</replaceable> ]
 </synopsis>
-    and the optional <replaceable class="parameter">framing_clause</replaceable>
+    and the optional <replaceable class="parameter">frame_clause</replaceable>
     can be one of
 <synopsis>
 RANGE UNBOUNDED PRECEDING
@@ -1614,7 +1614,8 @@ ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING
     Named window specifications are usually referenced with just
     <literal>OVER</> <replaceable>window_name</replaceable>, 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 <literal>WINDOW</literal> clause; see the
     <xref linkend="sql-select" endterm="sql-select-title"> reference
@@ -1622,6 +1623,9 @@ ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING
    </para>
 
    <para>
+    The <replaceable class="parameter">frame_clause</replaceable> specifies
+    the set of rows constituting the <firstterm>window frame</>, for those
+    window functions that act on the frame instead of the whole partition.
     The default framing option is <literal>RANGE UNBOUNDED PRECEDING</>,
     which is the same as <literal>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
 
    <para>
     The built-in window functions are described in <xref
-    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.
    </para>
 
    <para>

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 <window clause> 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;