I wrote:
> Anyway, I'll go take a look at exactly what would be involved in the
> first choice.
Actually, it seems this way results in a net *savings* of code, because
we can simply remove the code that was responsible for retail pfree'ing
of the transition values. I suppose that code must have predated the
introduction of MemoryContextReset, or it would have occurred to us to
do it like this to begin with.
I think that WindowAgg does not need any changes because it already does
MemoryContextResetAndDeleteChildren(winstate->wincontext) at partition
boundaries. Hitoshi, do you agree?
regards, tom lane
Index: src/backend/executor/nodeAgg.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/executor/nodeAgg.c,v
retrieving revision 1.167
diff -c -r1.167 nodeAgg.c
*** src/backend/executor/nodeAgg.c 17 Jun 2009 16:05:34 -0000 1.167
--- src/backend/executor/nodeAgg.c 23 Jul 2009 19:19:33 -0000
***************
*** 55,60 ****
--- 55,62 ----
* in either case its value need not be preserved. See int8inc() for an
* example. Notice that advance_transition_function() is coded to
avoid a
* data copy step when the previous transition value pointer is returned.
+ * Also, some transition functions make use of the aggcontext to store
+ * working state.
*
*
* Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
***************
*** 273,290 ****
}
/*
- * If we are reinitializing after a group boundary, we have to
free
- * any prior transValue to avoid memory leakage. We must check
not
- * only the isnull flag but whether the pointer is NULL; since
- * pergroupstate is initialized with palloc0, the initial
condition
- * has isnull = 0 and null pointer.
- */
- if (!peraggstate->transtypeByVal &&
- !pergroupstate->transValueIsNull &&
- DatumGetPointer(pergroupstate->transValue) != NULL)
- pfree(DatumGetPointer(pergroupstate->transValue));
-
- /*
* (Re)set transValue to the initial value.
*
* Note that when the initial value is pass-by-ref, we must
copy it
--- 275,280 ----
***************
*** 911,920 ****
}
/*
! * Clear the per-output-tuple context for each group
*/
ResetExprContext(econtext);
/*
* Initialize working state for a new input tuple group
*/
--- 901,915 ----
}
/*
! * Clear the per-output-tuple context for each group, as well as
! * aggcontext (which contains any pass-by-ref transvalues of the
! * old group). We also clear any child contexts of the
aggcontext;
! * some aggregate functions store working state in such
contexts.
*/
ResetExprContext(econtext);
+ MemoryContextResetAndDeleteChildren(aggstate->aggcontext);
+
/*
* Initialize working state for a new input tuple group
*/
***************
*** 1234,1240 ****
* structures and transition values. NOTE: the details of what is
stored
* in aggcontext and what is stored in the regular per-query memory
* context are driven by a simple decision: we want to reset the
! * aggcontext in ExecReScanAgg to recover no-longer-wanted space.
*/
aggstate->aggcontext =
AllocSetContextCreate(CurrentMemoryContext,
--- 1229,1236 ----
* structures and transition values. NOTE: the details of what is
stored
* in aggcontext and what is stored in the regular per-query memory
* context are driven by a simple decision: we want to reset the
! * aggcontext at group boundaries (if not hashing) and in ExecReScanAgg
! * to recover no-longer-wanted space.
*/
aggstate->aggcontext =
AllocSetContextCreate(CurrentMemoryContext,
Index: src/backend/utils/adt/array_userfuncs.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/adt/array_userfuncs.c,v
retrieving revision 1.31
diff -c -r1.31 array_userfuncs.c
*** src/backend/utils/adt/array_userfuncs.c 20 Jun 2009 18:45:28 -0000
1.31
--- src/backend/utils/adt/array_userfuncs.c 23 Jul 2009 19:19:33 -0000
***************
*** 539,545 ****
/*
* Make the result. We cannot release the ArrayBuildState because
! * sometimes aggregate final functions are re-executed.
*/
result = makeMdArrayResult(state, 1, dims, lbs,
CurrentMemoryContext,
--- 539,547 ----
/*
* Make the result. We cannot release the ArrayBuildState because
! * sometimes aggregate final functions are re-executed. Rather, it
! * is nodeAgg.c's responsibility to reset the aggcontext when it's
! * safe to do so.
*/
result = makeMdArrayResult(state, 1, dims, lbs,
CurrentMemoryContext,
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
