database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-06-11 20:28:21 +03:00
Code Activity

Improve the error message given for modifying a window with frame clause.

Browse Source 
For rather inscrutable reasons, SQL:2008 disallows copying-and-modifying a
window definition that has any explicit framing clause.  The error message
we gave for this only made sense if the referencing window definition
itself contains an explicit framing clause, which it might well not.
Moreover, in the context of an OVER clause it's not exactly obvious that
"OVER (windowname)" implies copy-and-modify while "OVER windowname" does
not.  This has led to multiple complaints, eg bug #5199 from Iliya
Krapchatov.  Change to a hopefully more intelligible error message, and
in the case where we have just "OVER (windowname)", add a HINT suggesting
that omitting the parentheses will fix it.  Also improve the related
documentation.  Back-patch to all supported branches.
This commit is contained in:
Tom Lane
2013-11-05 21:58:19 -05:00
parent 4c412ca2e5
commit db157fb141
2 changed files with 36 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files

21
doc/src/sgml/syntax.sgml
View File

@ -1709,10 +1709,10 @@ SELECT string_agg(a ORDER BY a, ',') FROM table; -- incorrect
The syntax of a window function call is one of the following:
<synopsis>
<replaceable>function_name</replaceable> (<optional><replaceable>expression</replaceable> <optional>, <replaceable>expression</replaceable> ... </optional></optional>) OVER ( <replaceable class="parameter">window_definition</replaceable> )
<replaceable>function_name</replaceable> (<optional><replaceable>expression</replaceable> <optional>, <replaceable>expression</replaceable> ... </optional></optional>) OVER <replaceable>window_name</replaceable>
<replaceable>function_name</replaceable> ( * ) OVER ( <replaceable class="parameter">window_definition</replaceable> )
<replaceable>function_name</replaceable> (<optional><replaceable>expression</replaceable> <optional>, <replaceable>expression</replaceable> ... </optional></optional>) OVER ( <replaceable class="parameter">window_definition</replaceable> )
<replaceable>function_name</replaceable> ( * ) OVER <replaceable>window_name</replaceable>
<replaceable>function_name</replaceable> ( * ) OVER ( <replaceable class="parameter">window_definition</replaceable> )
</synopsis>
where <replaceable class="parameter">window_definition</replaceable>
has the syntax
@ -1749,15 +1749,14 @@ UNBOUNDED FOLLOWING
names or numbers.
<replaceable>window_name</replaceable> is a reference to a named window
specification defined in the query's <literal>WINDOW</literal> clause.
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 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"> reference
page for details.
Alternatively, a full <replaceable>window_definition</replaceable> can
be given within parentheses, using the same syntax as for defining a
named window in the <literal>WINDOW</literal> clause; see the
<xref linkend="sql-select"> reference page for details. It's worth
pointing out that <literal>OVER wname</> is not exactly equivalent to
<literal>OVER (wname)</>; the latter implies copying and modifying the
window definition, and will be rejected if the referenced window
specification includes a frame clause.
</para>
<para>

31
src/backend/parser/parse_clause.c
View File

@ -1665,11 +1665,16 @@ 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
* specify its own ordering clause, but only if the previous one 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
* 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.)
* <window clause> syntax rule 10 and general rule 1. The frame
* clause rule is especially bizarre because it makes "OVER foo"
* different from "OVER (foo)", and requires the latter to throw an
* error if foo has a nondefault frame clause. Well, ours not to
* reason why, but we do go out of our way to throw a useful error
* message for such cases.
*/
if (refwc)
{
@ -1708,11 +1713,27 @@ transformWindowDefinitions(ParseState *pstate,
wc->copiedOrder = false;
}
if (refwc && refwc->frameOptions != FRAMEOPTION_DEFAULTS)
{
/*
* Use this message if this is a WINDOW clause, or if it's an OVER
* clause that includes ORDER BY or framing clauses. (We already
* rejected PARTITION BY above, so no need to check that.)
*/
if (windef->name ||
orderClause || windef->frameOptions != FRAMEOPTION_DEFAULTS)
ereport(ERROR,
(errcode(ERRCODE_WINDOWING_ERROR),
errmsg("cannot copy window \"%s\" because it has a frame clause",
windef->refname),
parser_errposition(pstate, windef->location)));
/* Else this clause is just OVER (foo), so say this: */
ereport(ERROR,
(errcode(ERRCODE_WINDOWING_ERROR),
errmsg("cannot override frame clause of window \"%s\"",
windef->refname),
errmsg("cannot copy window \"%s\" because it has a frame clause",
windef->refname),
errhint("Omit the parentheses in this OVER clause."),
parser_errposition(pstate, windef->location)));
}
wc->frameOptions = windef->frameOptions;
/* Process frame offset expressions */
wc->startOffset = transformFrameOffset(pstate, wc->frameOptions,