Do not modify aggregation state in finalize

[mkindahl]

The function tsl_finalize_agg_ffunc modified the aggregation state by
setting trans_value to the final result when computing the final
value. Since the state can be re-used several times, there could be
several calls to the finalization function, and the finalization
function would be confused when passed a final value instead of a
aggregation state transition value.

This commit fixes this by not modifying the trans_value when
computing the final value and instead just returns it (or the original
trans_value if there is no finalization function).

Fixes #3248

[akuzm]

Didn't know about this. Is it some optimization like using the same state for avg, count and sum?

[mfundul]

I think the comment should say something in the spirit of "the partial state should not be modified by the finalization function because it can be reused". I thought the same thing about avg and sum but apparently the reproducer only has 1 aggregate function.

[mkindahl]

Didn't know about this. Is it some optimization like using the same state for avg, count and sum?

It seems to be mainly used in hash aggregation, but I see it pop up in group aggregation as well. It is the finalize_aggregates function in nodeAgg.c that is calling this using the hashed aggregates.

[codecov]

Codecov Report

Merging #4219 (3ad59bc) into main (a064fd3) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #4219   +/-   ##
=======================================
  Coverage   90.78%   90.79%           
=======================================
  Files         215      215           
  Lines       39530    39537    +7     
=======================================
+ Hits        35888    35898   +10     
+ Misses       3642     3639    -3     

Impacted Files	Coverage Δ
tsl/src/fdw/data_node_scan_exec.c	88.88% <ø> (-0.40%)	⬇️
src/nodes/hypertable_modify.c	70.10% <100.00%> (+0.51%)	⬆️
src/planner.c	95.19% <100.00%> (+0.01%)	⬆️
tsl/src/fdw/data_node_scan_plan.c	98.23% <100.00%> (+0.01%)	⬆️
tsl/src/partialize_finalize.c	96.24% <100.00%> (+0.03%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ae50a53...3ad59bc. Read the comment docs.

[mkindahl]

Using the example from #3248 this is the result:

mats=# \ir t3248.sql
CREATE TABLE
CREATE INDEX
CREATE INDEX
   create_hypertable    
------------------------
 (265,public,metrics,t)
(1 row)

ALTER TABLE
INSERT 0 75
ALTER TABLE
INSERT 0 105
ALTER TABLE
INSERT 0 105
ANALYZE
 count 
-------
   285
(1 row)

psql:t3248.sql:25: NOTICE:  refreshing continuous aggregate "cagg1"
HINT:  Use WITH NO DATA if you do not want to refresh the continuous aggregate on creation.
CREATE MATERIALIZED VIEW
psql:t3248.sql:27: server closed the connection unexpectedly
        This probably means the server terminated abnormally
        before or while processing the request.
psql:t3248.sql:27: fatal: connection to server was lost