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