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