From 1a1d69334b9cbecca5038528bc7a5bddcc5f05ca Mon Sep 17 00:00:00 2001 From: David Rowley Date: Fri, 9 Sep 2022 13:16:46 +1200 Subject: [PATCH v1 3/4] Adjust memory context API with aggregate and window functions This makes it so an aggregate function's final function always returns any byref types in memory allocated in CurrentMemoryContext. The same is also done here for window functions. A window function may now call the PG_WINDOW_RESULTINFO() macro to obtain a WindowResultInfo in order to determine the result type details of the return value. Functions which do not currently return in CurrentMemoryContext must perform a datumCopy(). Extension authors who have created aggregates with a final function or created their own window functions must ensure their C code conforms. --- src/backend/executor/nodeWindowAgg.c | 10 +++++++--- src/backend/utils/adt/numeric.c | 2 +- src/backend/utils/adt/windowfuncs.c | 21 +++++++++++++++++++++ src/include/windowapi.h | 12 ++++++++++++ 4 files changed, 41 insertions(+), 4 deletions(-) diff --git a/src/backend/executor/nodeWindowAgg.c b/src/backend/executor/nodeWindowAgg.c index 8b0858e9f5..ba119046f4 100644 --- a/src/backend/executor/nodeWindowAgg.c +++ b/src/backend/executor/nodeWindowAgg.c @@ -1034,9 +1034,13 @@ eval_windowfunction(WindowAggState *winstate, WindowStatePerFunc perfuncstate, { LOCAL_FCINFO(fcinfo, FUNC_MAX_ARGS); MemoryContext oldContext; + WindowResultInfo resultinfo; oldContext = MemoryContextSwitchTo(winstate->ss.ps.ps_ExprContext->ecxt_per_tuple_memory); + resultinfo.resulttypeLen = perfuncstate->resulttypeLen; + resultinfo.resulttypeByVal = perfuncstate->resulttypeByVal; + /* * We don't pass any normal arguments to a window function, but we do pass * it the number of arguments, in order to permit window function @@ -1046,7 +1050,8 @@ eval_windowfunction(WindowAggState *winstate, WindowStatePerFunc perfuncstate, InitFunctionCallInfoData(*fcinfo, &(perfuncstate->flinfo), perfuncstate->numArguments, perfuncstate->winCollation, - (void *) perfuncstate->winobj, NULL); + (void *) perfuncstate->winobj, + (void *) &resultinfo); /* Just in case, make all the regular argument slots be null */ for (int argno = 0; argno < perfuncstate->numArguments; argno++) fcinfo->args[argno].isnull = true; @@ -1062,8 +1067,7 @@ eval_windowfunction(WindowAggState *winstate, WindowStatePerFunc perfuncstate, * tuple, as is entirely possible.) */ if (!perfuncstate->resulttypeByVal && !fcinfo->isnull && - !MemoryContextContains(CurrentMemoryContext, - DatumGetPointer(*result))) + GetMemoryChunkContext(DatumGetPointer(*result)) != CurrentMemoryContext) *result = datumCopy(*result, perfuncstate->resulttypeByVal, perfuncstate->resulttypeLen); diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c index 920a63b008..2ac7de70c5 100644 --- a/src/backend/utils/adt/numeric.c +++ b/src/backend/utils/adt/numeric.c @@ -6643,7 +6643,7 @@ int2int4_sum(PG_FUNCTION_ARGS) if (transdata->count == 0) PG_RETURN_NULL(); - PG_RETURN_DATUM(Int64GetDatumFast(transdata->sum)); + PG_RETURN_DATUM(Int64GetDatum(transdata->sum)); } diff --git a/src/backend/utils/adt/windowfuncs.c b/src/backend/utils/adt/windowfuncs.c index 596564fa15..40f0a08123 100644 --- a/src/backend/utils/adt/windowfuncs.c +++ b/src/backend/utils/adt/windowfuncs.c @@ -15,6 +15,7 @@ #include "nodes/supportnodes.h" #include "utils/builtins.h" +#include "utils/datum.h" #include "windowapi.h" /* @@ -350,6 +351,7 @@ leadlag_common(FunctionCallInfo fcinfo, bool forward, bool withoffset, bool withdefault) { WindowObject winobj = PG_WINDOW_OBJECT(); + WindowResultInfo *wresinfo = PG_WINDOW_RESULTINFO(); int32 offset; bool const_offset; Datum result; @@ -388,6 +390,10 @@ leadlag_common(FunctionCallInfo fcinfo, if (isnull) PG_RETURN_NULL(); + if (!wresinfo->resulttypeByVal) + result = datumCopy(result, wresinfo->resulttypeByVal, + wresinfo->resulttypeLen); + PG_RETURN_DATUM(result); } @@ -470,6 +476,7 @@ Datum window_first_value(PG_FUNCTION_ARGS) { WindowObject winobj = PG_WINDOW_OBJECT(); + WindowResultInfo *wresinfo = PG_WINDOW_RESULTINFO(); Datum result; bool isnull; @@ -479,6 +486,10 @@ window_first_value(PG_FUNCTION_ARGS) if (isnull) PG_RETURN_NULL(); + if (!wresinfo->resulttypeByVal) + result = datumCopy(result, wresinfo->resulttypeByVal, + wresinfo->resulttypeLen); + PG_RETURN_DATUM(result); } @@ -491,6 +502,7 @@ Datum window_last_value(PG_FUNCTION_ARGS) { WindowObject winobj = PG_WINDOW_OBJECT(); + WindowResultInfo *wresinfo = PG_WINDOW_RESULTINFO(); Datum result; bool isnull; @@ -500,6 +512,10 @@ window_last_value(PG_FUNCTION_ARGS) if (isnull) PG_RETURN_NULL(); + if (!wresinfo->resulttypeByVal) + result = datumCopy(result, wresinfo->resulttypeByVal, + wresinfo->resulttypeLen); + PG_RETURN_DATUM(result); } @@ -512,6 +528,7 @@ Datum window_nth_value(PG_FUNCTION_ARGS) { WindowObject winobj = PG_WINDOW_OBJECT(); + WindowResultInfo *wresinfo = PG_WINDOW_RESULTINFO(); bool const_offset; Datum result; bool isnull; @@ -533,5 +550,9 @@ window_nth_value(PG_FUNCTION_ARGS) if (isnull) PG_RETURN_NULL(); + if (!wresinfo->resulttypeByVal) + result = datumCopy(result, wresinfo->resulttypeByVal, + wresinfo->resulttypeLen); + PG_RETURN_DATUM(result); } diff --git a/src/include/windowapi.h b/src/include/windowapi.h index 5a620a2e42..c3d65b161c 100644 --- a/src/include/windowapi.h +++ b/src/include/windowapi.h @@ -36,7 +36,19 @@ /* this struct is private in nodeWindowAgg.c */ typedef struct WindowObjectData *WindowObject; +/* + * Evaluation of window functions are passed this object and it's available + * via fcinfo->resultinfo and can be accessed in the window function via the + * PG_WINDOW_RESULTINFO macro. + */ +typedef struct WindowResultInfo +{ + int16 resulttypeLen; /* type length of wfunc's result type */ + bool resulttypeByVal; /* true if wfunc's result is byval */ +} WindowResultInfo; + #define PG_WINDOW_OBJECT() ((WindowObject) fcinfo->context) +#define PG_WINDOW_RESULTINFO() ((WindowResultInfo *) fcinfo->resultinfo) #define WindowObjectIsValid(winobj) \ ((winobj) != NULL && IsA(winobj, WindowObjectData)) -- 2.34.1