Much of nodeAgg.c does not really know the difference between the aggregate's combine function and the aggregate's transition function. This was done on purpose so that we can keep as much code the same as possible between partial aggregate and finalize aggregate.
We can take this a bit further with the attached patch which managed a net reduction of about 3 dozen lines of code. 3 files changed, 118 insertions(+), 155 deletions(-) I also did some renaming to try and make it more clear about when we're talking about aggtransfn and when we're just talking about the transition function that's being used, which in the finalize aggregate case will be the combine function. I proposed this a few years ago in [1], but at the time we went with a more minimal patch to fix the bug being reported there with plans to come back and do a bit more once we branched. I've rebased this and I'd like to propose this small cleanup for pg15. The patch is basically making build_pertrans_for_aggref() oblivious to if it's working with the aggtransfn or the aggcombinefn and all the code that needs to treat them differently is moved up into ExecInitAgg(). This allows us to just completely get rid of build_aggregate_combinefn_expr() and just use build_aggregate_transfn_expr() instead. I feel this is worth doing as nodeAgg.c has grown quite a bit over the years. Shrinking it down a bit and maybe making it a bit more readable seems like a worthy goal. Heikki took a big step forward towards that goal in 0a2bc5d61e. This, arguably, helps a little more. I've included Andres and Horiguchi-san because they were part of the discussion on the original thread. David [1] https://www.postgresql.org/message-id/CAKJS1f-Qu2Q9g6Xfcf5dctg99oDkbV9LyW4Lym9C4L1vEHTN%3Dg%40mail.gmail.com
v2-0001-Cleanup-some-aggregate-code-in-the-executor.patch
Description: Binary data
