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

On Thu, Jun 20, 2013 at 08:59:27PM +0100, Dean Rasheed wrote:
> On 17 June 2013 06:36, David Fetter <david(at)fetter(dot)org> wrote:
> >> > > Please find attached two versions of a patch which provides optional
> >> > > FILTER clause for aggregates (T612, "Advanced OLAP operations").
> >> > >
> >> > > The first is intended to be applied on top of the previous patch, the
> >> > > second without it. The first is, I believe, clearer in what it's
> >> > > doing. Rather than simply mechanically visiting every place a
> >> > > function call might be constructed, it visits a central one to change
> >> > > the default, then goes only to the places where it's relevant.
> >> > >
> >> > > The patches are both early WIP as they contain no docs or regression
> >> > > tests yet.
> >> >
> >> > Docs and regression tests added, makeFuncArgs approached dropped for
> >> > now, will re-visit later.
> >>
> >> Regression tests added to reflect bug fixes in COLLATE.
> >
> > Rebased vs. master.
>
> Hi,
>
> I've been looking at this patch, which adds support for the SQL
> standard feature of applying a filter to rows used in an aggregate.
> The syntax matches the spec:
>
> agg_fn ( <args> [ ORDER BY <sort_clause> ] ) [ FILTER ( WHERE <qual> ) ]
>
> and this patch makes FILTER a new reserved keyword, usable as a
> function or type, but not in other contexts, e.g., as a table name or
> alias.
>
> I'm not sure if that might be a problem for some people, but I can't
> see any way round it, since otherwise there would be no way for the
> parser to distinguish a filter clause from an alias expression.
>
> The feature appears to work per-spec, and the code and doc changes
> look reasonable. However, it looks a little light on regression tests,

What tests do you think should be there that aren't?

> and so I think some necessary changes have been overlooked.
>
> 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.

I'd be happy to see about adding one despite this, though. That they
didn't figure out how doesn't mean we shouldn't eventually, ideally in
time for 9.4.

Cheers,
David.
--
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david(dot)fetter(at)gmail(dot)com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate