On Thu, 1 Jul 2021 at 11:09, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Just reading it over quickly, I noticed a comment mentioning
> "aggcombinedfn" which I suppose should be "aggcombinefn".
Thanks. I've fixed that locally.
> No particular opinion on whether this is a net reduction
> of logical complexity.
I had another look over it and I think we do need to be more clear
about when we're talking about aggtransfn and aggcombinefn. The
existing code uses variables name aggtransfn when the value stored
could be the value for the aggcombinefn. Additionally, the other
change to remove the special case build_aggregate_combinefn_expr()
function seems good in a sense of reusing more code and reducing the
amount of code in that file.
Unless anyone thinks differently about this, I plan on pushing the
patch in the next day or so.
David