We've had a couple of complaints about the error message that's thrown
for the case where you try to copy-and-modify a window definition that
includes a frame clause:
http://www.postgresql.org/message-id/200911191711.najhbped009...@wwwmaster.postgresql.org
http://www.postgresql.org/message-id/CADyrUxP-5pNAqxjuFx9ToeTEhsxwo942PS3Bk_=jekdmvg0...@mail.gmail.com
I propose the attached patch, which changes the text of the message to
"cannot copy window \"%s\" because it has a frame clause", and then adds
a HINT "Omit the parentheses in this OVER clause." if the clause is just
OVER (windowname) and doesn't include any attempt to modify the window's
properties.
I think we should back-patch this into all versions that have window
functions (ie, all supported branches).
Any objections or better ideas?
regards, tom lane
diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c
index ea90e58..29183c1 100644
*** a/src/backend/parser/parse_clause.c
--- b/src/backend/parser/parse_clause.c
*************** transformWindowDefinitions(ParseState *p
*** 1735,1745 ****
/*
* 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 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)
{
--- 1735,1750 ----
/*
* 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 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. The frame
! * clause rule is especially bizarre because it makes "OVER foo"
! * different from "OVER (foo)", solely in that the latter is required
! * 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)
{
*************** transformWindowDefinitions(ParseState *p
*** 1778,1788 ****
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;
/* Process frame offset expressions */
wc->startOffset = transformFrameOffset(pstate, wc->frameOptions,
--- 1783,1809 ----
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 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,
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
