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

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,
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:

CREATE TABLE t1(a1 int);
CREATE TABLE t2(a2 int);
INSERT INTO t1 SELECT * FROM generate_series(1,10);
INSERT INTO t2 SELECT * FROM generate_series(3,6);

SELECT array_agg(a1) FILTER (WHERE a1 IN (SELECT a2 FROM t2)) FROM t1;

ERROR: plan should not reference subplan's variable

which looks to be related to subselect.c's handling of sub-queries in
aggregates.

Regards,
Dean