On Thu, 1 Jul 2021 at 11:09, Tom Lane <t...@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
