Remove uses of MemoryContextContains in nodeAgg.c and nodeWindowAgg.c.
MemoryContextContains is no longer reliable in the wake of c6e0fe1f2,
so we need to get rid of these uses.
It appears that there's no really good reason to force the result of
an aggregate's finalfn or serialfn to be allocated in the per-tuple
context. The only other plausible case is that the result points to
or into the aggregate's transition value, and that's fine because it
will last as long as we need it to. (This conclusion depends on the
assumption that finalfns are not allowed to scribble on the transition
value, but we've long required that.) So we can just drop the
MemoryContextContains plus datumCopy business, although we do need
to take care to not return a read-write pointer when the transition
value is an expanded datum.
Likewise, we don't really need to force the result of a window
function to be in the output context. In this case, the plausible
alternative is that it's pointing into the temporary tuple slot used
by WinGetFuncArgInPartition or WinGetFuncArgInFrame (since those
functions could return such a pointer, which might become the window
function's result). That will hold still for long enough, unless
there is another window function using the same WindowObject.
I'm content to always perform a datumCopy when there's more than one
such function.
On net, these changes should provide small speed improvements as well
as removing problematic code.
Tom Lane and David Rowley
Discussion: https://postgr.es/m/1913788.1664898906@sss.pgh.pa.us
Branch
------
master
Details
-------
https://git.postgresql.org/pg/commitdiff/42b746d4c982257bf3f924176632b04dc288174b
Modified Files
--------------
src/backend/executor/nodeAgg.c | 32 ++++++++++----------------------
src/backend/executor/nodeWindowAgg.c | 30 ++++++++++++++----------------
src/test/regress/expected/window.out | 17 +++++++++++++++++
src/test/regress/sql/window.sql | 4 ++++
4 files changed, 45 insertions(+), 38 deletions(-)
