On Wed, 29 Jun 2022 at 12:59, David Rowley <[email protected]> wrote:
> I noticed while doing some memory context related work that since we
> now use generation.c memory contexts for tuplesorts (40af10b57) that
> tuplesort_putindextuplevalues() causes memory "leaks" in the
> generation context due to index_form_tuple() being called while we're
> switched into the state->tuplecontext.
I voiced my dislike for the patch I came up with to fix this issue to
Andres. He suggested that I just add a version of index_form_tuple
that can be given a MemoryContext pointer to allocate the returned
tuple into.
I like that idea much better, so I've attached a patch to fix it that way.
If there are no objections, I plan to push this in the next 24 hours.
David
diff --git a/src/backend/access/common/indextuple.c
b/src/backend/access/common/indextuple.c
index 3065730bae..c0bad3cd95 100644
--- a/src/backend/access/common/indextuple.c
+++ b/src/backend/access/common/indextuple.c
@@ -33,20 +33,39 @@
* ----------------------------------------------------------------
*/
+ /* ----------------
+ * index_form_tuple
+ *
+ * As index_form_tuple_context, but allocates the returned tuple
in the
+ * CurrentMemoryContext.
+ * ----------------
+ */
+IndexTuple
+index_form_tuple(TupleDesc tupleDescriptor,
+ Datum *values,
+ bool *isnull)
+{
+ return index_form_tuple_context(tupleDescriptor, values, isnull,
+
CurrentMemoryContext);
+}
+
/* ----------------
- * index_form_tuple
+ * index_form_tuple_context
*
* This shouldn't leak any memory; otherwise, callers such as
* tuplesort_putindextuplevalues() will be very unhappy.
*
* This shouldn't perform external table access provided caller
* does not pass values that are stored EXTERNAL.
+ *
+ * Allocates returned tuple in provided 'context'.
* ----------------
*/
IndexTuple
-index_form_tuple(TupleDesc tupleDescriptor,
- Datum *values,
- bool *isnull)
+index_form_tuple_context(TupleDesc tupleDescriptor,
+ Datum *values,
+ bool *isnull,
+ MemoryContext context)
{
char *tp; /* tuple pointer */
IndexTuple tuple; /* return tuple */
@@ -143,7 +162,7 @@ index_form_tuple(TupleDesc tupleDescriptor,
size = hoff + data_size;
size = MAXALIGN(size); /* be conservative */
- tp = (char *) palloc0(size);
+ tp = (char *) MemoryContextAllocZero(context, size);
tuple = (IndexTuple) tp;
heap_fill_tuple(tupleDescriptor,
diff --git a/src/backend/utils/sort/tuplesort.c
b/src/backend/utils/sort/tuplesort.c
index 31554fd867..421afcf47d 100644
--- a/src/backend/utils/sort/tuplesort.c
+++ b/src/backend/utils/sort/tuplesort.c
@@ -1884,12 +1884,13 @@ tuplesort_putindextuplevalues(Tuplesortstate *state,
Relation rel,
ItemPointer self,
Datum *values,
bool *isnull)
{
- MemoryContext oldcontext = MemoryContextSwitchTo(state->tuplecontext);
+ MemoryContext oldcontext;
SortTuple stup;
Datum original;
IndexTuple tuple;
- stup.tuple = index_form_tuple(RelationGetDescr(rel), values, isnull);
+ stup.tuple = index_form_tuple_context(RelationGetDescr(rel), values,
+
isnull, state->tuplecontext);
tuple = ((IndexTuple) stup.tuple);
tuple->t_tid = *self;
USEMEM(state, GetMemoryChunkSpace(stup.tuple));
@@ -1899,7 +1900,7 @@ tuplesort_putindextuplevalues(Tuplesortstate *state,
Relation rel,
RelationGetDescr(state->indexRel),
&stup.isnull1);
- MemoryContextSwitchTo(state->sortcontext);
+ oldcontext = MemoryContextSwitchTo(state->sortcontext);
if (!state->sortKeys || !state->sortKeys->abbrev_converter ||
stup.isnull1)
{
diff --git a/src/include/access/itup.h b/src/include/access/itup.h
index 2c8877e991..7458bc2fda 100644
--- a/src/include/access/itup.h
+++ b/src/include/access/itup.h
@@ -150,6 +150,9 @@ typedef IndexAttributeBitMapData * IndexAttributeBitMap;
/* routines in indextuple.c */
extern IndexTuple index_form_tuple(TupleDesc tupleDescriptor,
Datum
*values, bool *isnull);
+extern IndexTuple index_form_tuple_context(TupleDesc tupleDescriptor,
+
Datum *values, bool *isnull,
+
MemoryContext context);
extern Datum nocache_index_getattr(IndexTuple tup, int attnum,
TupleDesc
tupleDesc);
extern void index_deform_tuple(IndexTuple tup, TupleDesc tupleDescriptor,
