| From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
|---|---|
| To: | Andres Freund <andres(at)anarazel(dot)de> |
| Cc: | pgsql-hackers(at)postgresql(dot)org |
| Subject: | Re: Aggregate FILTER option is broken in v10 |
| Date: | 2017-10-16 15:12:09 |
| Message-ID: | 2275.1508166729@sss.pgh.pa.us |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
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.
Let me see what I can make of that ...
regards, tom lane
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Eric Radman | 2017-10-16 15:51:42 | [PATCH] Add recovery_min_apply_delay_reconnect recovery option |
| Previous Message | Tom Lane | 2017-10-16 13:42:34 | Aggregate FILTER option is broken in v10 |