Better error message for window-function spec bizarreness
От | Tom Lane |
---|---|
Тема | Better error message for window-function spec bizarreness |
Дата | |
Msg-id | 22948.1383675190@sss.pgh.pa.us обсуждение исходный текст |
Список | pgsql-hackers |
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.nAJHBped009004@wwwmaster.postgresql.org http://www.postgresql.org/message-id/CADyrUxP-5pNAqxjuFx9ToeTEhsxwo942PS3Bk_=JEKdMVg0W7A@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,
В списке pgsql-hackers по дате отправления:
Следующее
От: Tom LaneДата:
Сообщение: Re: Handle LIMIT/OFFSET before select clause (was: Feature request: optimizer improvement)