Do not modify aggregation state in finalize

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

tsl/src/partialize_finalize.c

@@ -518,14 +518,21 @@ tsl_finalize_agg_sfunc(PG_FUNCTION_ARGS)
 	PG_RETURN_POINTER(tstate);
 }
 
-/* tsl_finalize_agg_ffunc:
+/*
- * apply the finalize function on the state we have accumulated
+ * Apply the finalize function on the state we have accumulated
+ *
+ * The state passed down to this function can be used by other aggregate
+ * functions so it is important to not change the state when computing the
+ * final result.
  */
 Datum
 tsl_finalize_agg_ffunc(PG_FUNCTION_ARGS)
 {
 	FATransitionState *tstate = PG_ARGISNULL(0) ? NULL : (FATransitionState *) PG_GETARG_POINTER(0);
 	MemoryContext fa_context, old_context;
+	Datum result = tstate->per_group_state->trans_value;
+	bool result_isnull = tstate->per_group_state->trans_value_isnull;
+
 	Assert(tstate != NULL);
 	if (!AggCheckCallContext(fcinfo, &fa_context))
 	{
@@ -533,6 +540,7 @@ tsl_finalize_agg_ffunc(PG_FUNCTION_ARGS)
 		elog(ERROR, "finalize_agg_ffunc called in non-aggregate context");
 	}
 	old_context = MemoryContextSwitchTo(fa_context);
+
 	if (OidIsValid(tstate->per_query_state->final_meta.finalfnoid))
 	{
 		/* don't execute if strict and the trans value is NULL or there are extra args (all extra
@@ -547,15 +555,15 @@ tsl_finalize_agg_ffunc(PG_FUNCTION_ARGS)
 			FC_NULL(finalfn_fcinfo, 0) = tstate->per_group_state->trans_value_isnull;
 			finalfn_fcinfo->isnull = false;
 
-			tstate->per_group_state->trans_value = FunctionCallInvoke(finalfn_fcinfo);
+			result = FunctionCallInvoke(finalfn_fcinfo);
-			tstate->per_group_state->trans_value_isnull = finalfn_fcinfo->isnull;
+			result_isnull = finalfn_fcinfo->isnull;
 		}
 	}
 	MemoryContextSwitchTo(old_context);
-	if (tstate->per_group_state->trans_value_isnull)
+	if (result_isnull)
 		PG_RETURN_NULL();
 	else
-		PG_RETURN_DATUM(tstate->per_group_state->trans_value);
+		PG_RETURN_DATUM(result);
 }
 
 /*

tsl/test/expected/continuous_aggs.out

@@ -1703,3 +1703,38 @@ where view_name = 'test_morecols_cagg';
  test_morecols_cagg | t                 | t
 (1 row)
 
+CREATE TABLE issue3248(filler_1 int, filler_2 int, filler_3 int, time timestamptz NOT NULL, device_id int, v0 int, v1 int, v2 float, v3 float);
+CREATE INDEX ON issue3248(time DESC);
+CREATE INDEX ON issue3248(device_id,time DESC);
+SELECT create_hypertable('issue3248','time',create_default_indexes:=false);
+    create_hypertable    
+-------------------------
+ (46,public,issue3248,t)
+(1 row)
+
+ALTER TABLE issue3248 DROP COLUMN filler_1;
+INSERT INTO issue3248(time,device_id,v0,v1,v2,v3)
+SELECT time, device_id, device_id+1,  device_id + 2, device_id + 0.5, NULL
+FROM generate_series('2000-01-01 0:00:00+0'::timestamptz,'2000-01-05 23:55:00+0','8h') gtime(time),
+     generate_series(1,5,1) gdevice(device_id);
+ALTER TABLE issue3248 DROP COLUMN filler_2;
+INSERT INTO issue3248(time,device_id,v0,v1,v2,v3)
+SELECT time, device_id, device_id-1, device_id + 2, device_id + 0.5, NULL
+FROM generate_series('2000-01-06 0:00:00+0'::timestamptz,'2000-01-12 23:55:00+0','8h') gtime(time),
+     generate_series(1,5,1) gdevice(device_id);
+ALTER TABLE issue3248 DROP COLUMN filler_3;
+INSERT INTO issue3248(time,device_id,v0,v1,v2,v3)
+SELECT time, device_id, device_id, device_id + 2, device_id + 0.5, NULL
+FROM generate_series('2000-01-13 0:00:00+0'::timestamptz,'2000-01-19 23:55:00+0','8h') gtime(time),
+     generate_series(1,5,1) gdevice(device_id);
+ANALYZE issue3248;
+CREATE materialized view issue3248_cagg WITH (timescaledb.continuous)
+AS SELECT time_bucket('1h',time), device_id, min(v0), max(v1), avg(v2)
+FROM issue3248 GROUP BY 1,2;
+NOTICE:  refreshing continuous aggregate "issue3248_cagg"
+SELECT
+  FROM issue3248 AS m,
+       LATERAL(SELECT m FROM issue3248_cagg WHERE avg IS NULL LIMIT 1) AS lat;
+--
+(0 rows)
+

tsl/test/sql/continuous_aggs.sql

@@ -1197,3 +1197,32 @@ SELECT view_name, materialized_only, compression_enabled
 FROM timescaledb_information.continuous_aggregates 
 where view_name = 'test_morecols_cagg';
 
+CREATE TABLE issue3248(filler_1 int, filler_2 int, filler_3 int, time timestamptz NOT NULL, device_id int, v0 int, v1 int, v2 float, v3 float);
+CREATE INDEX ON issue3248(time DESC);
+CREATE INDEX ON issue3248(device_id,time DESC);
+SELECT create_hypertable('issue3248','time',create_default_indexes:=false);
+
+ALTER TABLE issue3248 DROP COLUMN filler_1;
+INSERT INTO issue3248(time,device_id,v0,v1,v2,v3)
+SELECT time, device_id, device_id+1,  device_id + 2, device_id + 0.5, NULL
+FROM generate_series('2000-01-01 0:00:00+0'::timestamptz,'2000-01-05 23:55:00+0','8h') gtime(time),
+     generate_series(1,5,1) gdevice(device_id);
+ALTER TABLE issue3248 DROP COLUMN filler_2;
+INSERT INTO issue3248(time,device_id,v0,v1,v2,v3)
+SELECT time, device_id, device_id-1, device_id + 2, device_id + 0.5, NULL
+FROM generate_series('2000-01-06 0:00:00+0'::timestamptz,'2000-01-12 23:55:00+0','8h') gtime(time),
+     generate_series(1,5,1) gdevice(device_id);
+ALTER TABLE issue3248 DROP COLUMN filler_3;
+INSERT INTO issue3248(time,device_id,v0,v1,v2,v3)
+SELECT time, device_id, device_id, device_id + 2, device_id + 0.5, NULL
+FROM generate_series('2000-01-13 0:00:00+0'::timestamptz,'2000-01-19 23:55:00+0','8h') gtime(time),
+     generate_series(1,5,1) gdevice(device_id);
+ANALYZE issue3248;
+
+CREATE materialized view issue3248_cagg WITH (timescaledb.continuous)
+AS SELECT time_bucket('1h',time), device_id, min(v0), max(v1), avg(v2)
+FROM issue3248 GROUP BY 1,2;
+
+SELECT
+  FROM issue3248 AS m,
+       LATERAL(SELECT m FROM issue3248_cagg WHERE avg IS NULL LIMIT 1) AS lat;