database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-08-19 23:22:23 +03:00
Code Activity

Rename ExecAggTransReparent, and improve its documentation.

Browse Source 
The name of this function suggests that it ought to reparent R/W
expanded objects to be children of the persistent aggcontext, instead
of copying them.  In fact it does no such thing, and if you try to
make it do so you will see multiple regression failures.  Rename it
to the less-misleading ExecAggCopyTransValue, and add commentary
about why that attractive-sounding optimization won't work.  Also
adjust comments at call sites, some of which were describing logic
that has since been moved into ExecAggCopyTransValue.

Discussion: https://postgr.es/m/3004282.1681930251@sss.pgh.pa.us
This commit is contained in:
Tom Lane
2023-04-24 13:01:33 -04:00
parent 5a8366ad75
commit fce3b26e97
6 changed files with 51 additions and 28 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
src/backend/executor/nodeWindowAgg.c
View File

@@ -368,7 +368,8 @@ advance_windowaggregate(WindowAggState *winstate,
* free the prior transValue. But if transfn returned a pointer to its
* first input, we don't need to do anything. Also, if transfn returned a
* pointer to a R/W expanded object that is already a child of the
* aggcontext, assume we can adopt that value without copying it.
* aggcontext, assume we can adopt that value without copying it. (See
* comments for ExecAggCopyTransValue, which this code duplicates.)
*/
if (!peraggstate->transtypeByVal &&
DatumGetPointer(newVal) != DatumGetPointer(peraggstate->transValue))
@@ -533,7 +534,8 @@ advance_windowaggregate_base(WindowAggState *winstate,
* free the prior transValue. But if invtransfn returned a pointer to its
* first input, we don't need to do anything. Also, if invtransfn
* returned a pointer to a R/W expanded object that is already a child of
* the aggcontext, assume we can adopt that value without copying it.
* the aggcontext, assume we can adopt that value without copying it. (See
* comments for ExecAggCopyTransValue, which this code duplicates.)
*
* Note: the checks for null values here will never fire, but it seems
* best to have this stanza look just like advance_windowaggregate.