Re: nodeAgg perf tweak

Neil Conway <neilc@samurai.com> writes:
> - yours would mean that int8inc() and similar aggregates wouldn't ever
> need to do palloc(); mine would require a palloc() every k calls to the
> transition function.

No.  The current code involves two pallocs per cycle, one inside the
aggregate function to construct its result value, and then one in
datumCopy to copy the result into the proper context.  Your patch
reduces that to 1 + 1/k pallocs per cycle, mine to zero.

The fact that it's a central fix for all aggregate functions is
definitely a nice feature of your approach, but I am concerned about the
possible side-effects on user-defined aggregate functions that may not
work as you expect them to.  I think it's safer to keep the aggregate
code behaving as-is and get the performance win in the individual
functions.  There are not that many aggregates that we really care that
much about.
        regards, tom lane