Re: FILTER for aggregates [was Re: Department of Redundancy Department: makeNode(FuncCall) division]

On 21 June 2013 10:02, Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com> wrote:
> On 21 June 2013 06:16, David Fetter <david(at)fetter(dot)org> wrote:
>> On Fri, Jun 21, 2013 at 12:10:25AM -0400, Alvaro Herrera wrote:
>>> David Fetter escribió:
>>> > On Thu, Jun 20, 2013 at 08:59:27PM +0100, Dean Rasheed wrote:
>>>
>>> > > In my testing of sub-queries in the FILTER clause (an extension to the
>>> > > spec), I was able to produce the following error:
>>> >
>>> > Per the spec,
>>> >
>>> > B) A <filter clause> shall not contain a <query expression>, a <window function>, or an outer reference.
>>>
>>> If this is not allowed, I think there should be a clearer error message.
>>> What Dean showed is an internal (not user-facing) error message.
>>
>> Please find attached a patch which allows subqueries in the FILTER
>> clause and adds regression testing for same.
>>
>
> Nice!
>
> I should have pointed out that it was already working for some
> sub-queries, just not all. It's good to see that it was basically just
> a one-line fix, because I think it would have been a shame to not
> support sub-queries.
>
> I hope to take another look at it over the weekend.
>

I'm still not happy that this patch is making FILTER a new reserved
keyword, because I think it is a common enough English word (and an
obscure enough SQL keyword) that people may well have used it for
table names or aliases, and so their code will break.

There is another thread discussing a similar issue with lead/lag ignore nulls:
http://www.postgresql.org/message-id/CAOdE5WQnfR737OkxNXuLWnwpL7=OUssC-63ijP2=2RnRTw8emQ@mail.gmail.com

Playing around with the idea proposed there, it seems that it is
possible to make FILTER (and also OVER) unreserved keywords, by
splitting out the production of aggregate/window functions from normal
functions in gram.y. Aggregate/window functions are not allowed in
index_elem or func_table constructs, but they may appear in c_expr's.
That resolves the shift/reduce conflicts that would otherwise occur
from subsequent alias clauses, allowing FILTER and OVER to be
unreserved.

There is a drawback --- certain error messages become less specific,
for example: "SELECT * FROM rank() OVER(ORDER BY random());" is now a
syntax error, rather than the more useful error saying that window
functions aren't allowed in the FROM clause. That doesn't seem like
such a common case though.

What do you think?

Regards,
Dean