Re: Aggregate FILTER option is broken in v10

On 2017-10-16 11:12:09 -0400, Tom Lane wrote:
> I wrote:
> > I think possibly the best answer is to revert 8ed3f11bb.  We could
> > think about some compromise solution like merging the projections
> > only for aggregates without FILTER.  But that would require two
> > code paths through the relevant functions in nodeAgg.c, which would
> > be a substantial maintenance burden; and the extra branches required
> > means that this would be a net negative for performance in the
> > simplest case with only one aggregate.
> 
> Hmm ... on closer inspection, the only performance-critical place
> where this matters is advance_aggregates, and that already has a check
> for whether the particular aggregate has a filter.  So we could do
> something like
> 
>         /* Skip anything FILTERed out */
>         if (filter)
>         {
>             // existing code to eval/check filter expr
> +
> +           /* Now it's safe to evaluate this agg's arguments */
> +           slot = ExecProject(pertrans->argproj);
>         }
> +       else
> +           slot = aggstate->evalslot;
> 
> which seems like a pretty minimal extra cost for the normal case
> with no filter.

Thanks, that looks like a reasonable fix.

Greetings,

Andres Freund