On Fri, Oct 21, 2016 at 4:45 PM, Peter Geoghegan <p...@heroku.com> wrote:
> More importantly, there are no remaining cases where
> tuplesort_gettuple_common() sets "*should_free = true", because there
> is no remaining need for caller to *ever* pfree() tuple. Moreover, I
> don't anticipate any future need for this -- the scheme recently
> established around per-tape "slab slots" makes it seem unlikely to me
> that the need to "*should_free = true" will ever arise again.

Attached patch 0001-* removes all should_free arguments. To reiterate,
this is purely a refactoring patch.

> Now, it is still true that at least some callers to
> tuplesort_gettupleslot() (but not any other "get tuple" interface
> routine) are going to *require* ownership of memory for returned
> tuples, which means a call to heap_copy_minimal_tuple() remains
> necessary there (see recent commit
> d8589946ddd5c43e1ebd01c5e92d0e177cbfc198 for details). But, that's
> beside the point. In the master branch only, the
> tuplesort_gettupleslot() test "if (!should_free)" seems tautological,
> just as similar tests are for every other tuplesort_gettuple_common()
> caller. So, tuplesort_gettupleslot() can safely assume it should
> *always* heap_copy_minimal_tuple() in the master branch. (BTW, I'm
> going to teach tuplesort_gettuple_common() to avoid this
> heap_copy_minimal_tuple() call for callers that don't actually need
> it, but that's a discussion for another thread).

I decided that it made sense to address this at the same time after
all. So, the second patch attached here, 0002-*, goes on to do so.
This is a performance optimization that we already agreed was a good
idea for Postgres 10 when d8589946ddd5c43e1ebd01c5e92d0e177cbfc198
went in.

Note that the call sites that now opt out of receiving their own
personal copy are essentially returning to their behavior for the
entire beta phase of PostgreSQL 9.6. The problem with that accidental
state of affairs was that it wasn't always safe. But, it was still
generally safe for the majority of callers, I believe, not least
because it took months to hear any complaints at all. That majority of
callers now opt out.

The main question for reviewers of 0002-*, then, is whether or not I
have correctly determined which particular callers can safely opt out.

Finally, 0003-* is a Valgrind suppression borrowed from my parallel
CREATE INDEX patch. It's self-explanatory.

-- 
Peter Geoghegan
From 440f9724362c376807c36d814046015285e5c237 Mon Sep 17 00:00:00 2001
From: Peter Geoghegan <p...@bowt.ie>
Date: Fri, 9 Dec 2016 13:49:18 -0800
Subject: [PATCH 3/3] Add valgrind suppression for logtape_write

This avoids having Valgrind report that logtape.c writes out
uninitialized memory.
---
 src/tools/valgrind.supp | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/src/tools/valgrind.supp b/src/tools/valgrind.supp
index af03051..775db0d 100644
--- a/src/tools/valgrind.supp
+++ b/src/tools/valgrind.supp
@@ -120,6 +120,15 @@
 	fun:RelationMapFinishBootstrap
 }
 
+{
+	logtape_write
+	Memcheck:Param
+	write(buf)
+
+	...
+	fun:LogicalTape*
+}
+
 
 # gcc on ppc64 can generate a four-byte read to fetch the final "char" fields
 # of a FormData_pg_cast.  This is valid compiler behavior, because a proper
-- 
2.7.4

From 3e3ef067713527e210ce76511955a5220fbbf15d Mon Sep 17 00:00:00 2001
From: Peter Geoghegan <p...@bowt.ie>
Date: Thu, 13 Oct 2016 10:54:31 -0700
Subject: [PATCH 2/3] Avoid copying within tuplesort_gettupleslot()

Add a "copy" argument to make it optional to receive a copy of caller
tuple that is safe to use across tuplesort_gettupleslot() calls, as a
performance optimization.  Existing tuplesort_gettupleslot() callers are
made to opt out of copying where that has been determined to be safe.
This brings tuplesort_gettupleslot() in line with
tuplestore_gettupleslot().

In the future, a "copy" tuplesort_getdatum() argument may be added, that
similarly allows callers to opt out of receiving a new copy of tuple
under their direct ownership.
---
 src/backend/executor/nodeAgg.c         |  9 ++++++---
 src/backend/executor/nodeSort.c        |  5 +++--
 src/backend/utils/adt/orderedsetaggs.c |  5 +++--
 src/backend/utils/sort/tuplesort.c     | 17 +++++++++++------
 src/include/utils/tuplesort.h          |  2 +-
 5 files changed, 24 insertions(+), 14 deletions(-)

diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c
index eefb3d6..16a1263 100644
--- a/src/backend/executor/nodeAgg.c
+++ b/src/backend/executor/nodeAgg.c
@@ -570,6 +570,9 @@ initialize_phase(AggState *aggstate, int newphase)
  * Fetch a tuple from either the outer plan (for phase 0) or from the sorter
  * populated by the previous phase.  Copy it to the sorter for the next phase
  * if any.
+ *
+ * Callers cannot rely on memory for tuple in returned slot remaining allocated
+ * past any subsequent call here.  (The sorter may recycle the memory.)
  */
 static TupleTableSlot *
 fetch_input_tuple(AggState *aggstate)
@@ -578,8 +581,8 @@ fetch_input_tuple(AggState *aggstate)
 
 	if (aggstate->sort_in)
 	{
-		if (!tuplesort_gettupleslot(aggstate->sort_in, true, aggstate->sort_slot,
-									NULL))
+		if (!tuplesort_gettupleslot(aggstate->sort_in, true, false,
+									aggstate->sort_slot, NULL))
 			return NULL;
 		slot = aggstate->sort_slot;
 	}
@@ -1273,7 +1276,7 @@ process_ordered_aggregate_multi(AggState *aggstate,
 		ExecClearTuple(slot2);
 
 	while (tuplesort_gettupleslot(pertrans->sortstates[aggstate->current_set],
-								  true, slot1, &newAbbrevVal))
+								  true, true, slot1, &newAbbrevVal))
 	{
 		/*
 		 * Extract the first numTransInputs columns as datums to pass to the
diff --git a/src/backend/executor/nodeSort.c b/src/backend/executor/nodeSort.c
index a34dcc5..8fc7106 100644
--- a/src/backend/executor/nodeSort.c
+++ b/src/backend/executor/nodeSort.c
@@ -132,12 +132,13 @@ ExecSort(SortState *node)
 
 	/*
 	 * Get the first or next tuple from tuplesort. Returns NULL if no more
-	 * tuples.
+	 * tuples. Note that we rely on memory for tuple in slot remaining
+	 * allocated only until subsequent call here.
 	 */
 	slot = node->ss.ps.ps_ResultTupleSlot;
 	(void) tuplesort_gettupleslot(tuplesortstate,
 								  ScanDirectionIsForward(dir),
-								  slot, NULL);
+								  false, slot, NULL);
 	return slot;
 }
 
diff --git a/src/backend/utils/adt/orderedsetaggs.c b/src/backend/utils/adt/orderedsetaggs.c
index fe44d56..3c23329 100644
--- a/src/backend/utils/adt/orderedsetaggs.c
+++ b/src/backend/utils/adt/orderedsetaggs.c
@@ -1189,7 +1189,7 @@ hypothetical_rank_common(FunctionCallInfo fcinfo, int flag,
 	tuplesort_performsort(osastate->sortstate);
 
 	/* iterate till we find the hypothetical row */
-	while (tuplesort_gettupleslot(osastate->sortstate, true, slot, NULL))
+	while (tuplesort_gettupleslot(osastate->sortstate, true, true, slot, NULL))
 	{
 		bool		isnull;
 		Datum		d = slot_getattr(slot, nargs + 1, &isnull);
@@ -1352,7 +1352,8 @@ hypothetical_dense_rank_final(PG_FUNCTION_ARGS)
 	slot2 = extraslot;
 
 	/* iterate till we find the hypothetical row */
-	while (tuplesort_gettupleslot(osastate->sortstate, true, slot, &abbrevVal))
+	while (tuplesort_gettupleslot(osastate->sortstate, true, true, slot,
+								  &abbrevVal))
 	{
 		bool		isnull;
 		Datum		d = slot_getattr(slot, nargs + 1, &isnull);
diff --git a/src/backend/utils/sort/tuplesort.c b/src/backend/utils/sort/tuplesort.c
index 0320f0d..d60159a 100644
--- a/src/backend/utils/sort/tuplesort.c
+++ b/src/backend/utils/sort/tuplesort.c
@@ -2083,12 +2083,15 @@ tuplesort_gettuple_common(Tuplesortstate *state, bool forward,
  * NULL value in leading attribute will set abbreviated value to zeroed
  * representation, which caller may rely on in abbreviated inequality check.
  *
- * The slot receives a copied tuple (sometimes allocated in caller memory
- * context) that will stay valid regardless of future manipulations of the
- * tuplesort's state.
+ * If copy is TRUE, the slot receives a copied tuple that will stay valid
+ * regardless of future manipulations of the tuplesort's state.  Memory is
+ * owned by the caller.  If copy is FALSE, the slot may just receive a pointer
+ * to a tuple held within the tuplesort.  The latter is more efficient, but
+ * the slot contents may be corrupted if there is another call here before
+ * previous slot contents are used.
  */
 bool
-tuplesort_gettupleslot(Tuplesortstate *state, bool forward,
+tuplesort_gettupleslot(Tuplesortstate *state, bool forward, bool copy,
 					   TupleTableSlot *slot, Datum *abbrev)
 {
 	MemoryContext oldcontext = MemoryContextSwitchTo(state->sortcontext);
@@ -2105,8 +2108,10 @@ tuplesort_gettupleslot(Tuplesortstate *state, bool forward,
 		if (state->sortKeys->abbrev_converter && abbrev)
 			*abbrev = stup.datum1;
 
-		stup.tuple = heap_copy_minimal_tuple((MinimalTuple) stup.tuple);
-		ExecStoreMinimalTuple((MinimalTuple) stup.tuple, slot, false);
+		if (copy)
+			stup.tuple = heap_copy_minimal_tuple((MinimalTuple) stup.tuple);
+
+		ExecStoreMinimalTuple((MinimalTuple) stup.tuple, slot, copy);
 		return true;
 	}
 	else
diff --git a/src/include/utils/tuplesort.h b/src/include/utils/tuplesort.h
index 38af746..ad7adca 100644
--- a/src/include/utils/tuplesort.h
+++ b/src/include/utils/tuplesort.h
@@ -93,7 +93,7 @@ extern void tuplesort_putdatum(Tuplesortstate *state, Datum val,
 extern void tuplesort_performsort(Tuplesortstate *state);
 
 extern bool tuplesort_gettupleslot(Tuplesortstate *state, bool forward,
-					   TupleTableSlot *slot, Datum *abbrev);
+					   bool copy, TupleTableSlot *slot, Datum *abbrev);
 extern HeapTuple tuplesort_getheaptuple(Tuplesortstate *state, bool forward);
 extern IndexTuple tuplesort_getindextuple(Tuplesortstate *state, bool forward);
 extern bool tuplesort_getdatum(Tuplesortstate *state, bool forward,
-- 
2.7.4

From 6ca426173e0de6499ebb1d136a2442a518a46215 Mon Sep 17 00:00:00 2001
From: Peter Geoghegan <p...@bowt.ie>
Date: Thu, 8 Dec 2016 14:37:54 -0800
Subject: [PATCH 1/3] Remove should_free tuplesort routine arguments

Tuplesort routine should_free status arguments are redundant, as memory
for tuples returned from a sort are always owned by tuplesort in
practice.  This has been the case since commit e94568ecc removed any
instances of caller tuples being returned as anything other than
pointers into tuplesort-owned buffers.  Refactor interface of certain
tuplesort routines to reflect this general state of affairs.

There are two remaining cases where we must always copy caller tuples
because callers are likely to require ownership of memory: when the
caller passes a TupleTableSlot which tuplesort stores a heap tuple in
for caller, and when the caller gets a pass-by-reference Datum tuple
from tuplesort.  In the future, it may be possible for some of the
callers of these two remaining functions to opt out of having tuplesort
produce a new copy for them, as a performance optimization, but that has
nothing to do with tuplesort memory management, and everything to do
with the requirements of those callers.
---
 src/backend/access/hash/hashsort.c  |  6 +---
 src/backend/access/nbtree/nbtsort.c | 24 ++++----------
 src/backend/commands/cluster.c      |  6 +---
 src/backend/utils/sort/tuplesort.c  | 62 +++++++++++++------------------------
 src/include/utils/tuplesort.h       |  6 ++--
 5 files changed, 31 insertions(+), 73 deletions(-)

diff --git a/src/backend/access/hash/hashsort.c b/src/backend/access/hash/hashsort.c
index 8938ab5..12fb78a 100644
--- a/src/backend/access/hash/hashsort.c
+++ b/src/backend/access/hash/hashsort.c
@@ -104,15 +104,13 @@ void
 _h_indexbuild(HSpool *hspool)
 {
 	IndexTuple	itup;
-	bool		should_free;
 #ifdef USE_ASSERT_CHECKING
 	uint32		hashkey = 0;
 #endif
 
 	tuplesort_performsort(hspool->sortstate);
 
-	while ((itup = tuplesort_getindextuple(hspool->sortstate,
-										   true, &should_free)) != NULL)
+	while ((itup = tuplesort_getindextuple(hspool->sortstate, true)) != NULL)
 	{
 		/*
 		 * Technically, it isn't critical that hash keys be found in sorted
@@ -129,7 +127,5 @@ _h_indexbuild(HSpool *hspool)
 #endif
 
 		_hash_doinsert(hspool->index, itup);
-		if (should_free)
-			pfree(itup);
 	}
 }
diff --git a/src/backend/access/nbtree/nbtsort.c b/src/backend/access/nbtree/nbtsort.c
index 99a014e..7f65b5a 100644
--- a/src/backend/access/nbtree/nbtsort.c
+++ b/src/backend/access/nbtree/nbtsort.c
@@ -680,9 +680,7 @@ _bt_load(BTWriteState *wstate, BTSpool *btspool, BTSpool *btspool2)
 	bool		merge = (btspool2 != NULL);
 	IndexTuple	itup,
 				itup2 = NULL;
-	bool		should_free,
-				should_free2,
-				load1;
+	bool		load1;
 	TupleDesc	tupdes = RelationGetDescr(wstate->index);
 	int			i,
 				keysz = RelationGetNumberOfAttributes(wstate->index);
@@ -697,10 +695,8 @@ _bt_load(BTWriteState *wstate, BTSpool *btspool, BTSpool *btspool2)
 		 */
 
 		/* the preparation of merge */
-		itup = tuplesort_getindextuple(btspool->sortstate,
-									   true, &should_free);
-		itup2 = tuplesort_getindextuple(btspool2->sortstate,
-										true, &should_free2);
+		itup = tuplesort_getindextuple(btspool->sortstate, true);
+		itup2 = tuplesort_getindextuple(btspool2->sortstate, true);
 		indexScanKey = _bt_mkscankey_nodata(wstate->index);
 
 		/* Prepare SortSupport data for each column */
@@ -775,18 +771,12 @@ _bt_load(BTWriteState *wstate, BTSpool *btspool, BTSpool *btspool2)
 			if (load1)
 			{
 				_bt_buildadd(wstate, state, itup);
-				if (should_free)
-					pfree(itup);
-				itup = tuplesort_getindextuple(btspool->sortstate,
-											   true, &should_free);
+				itup = tuplesort_getindextuple(btspool->sortstate, true);
 			}
 			else
 			{
 				_bt_buildadd(wstate, state, itup2);
-				if (should_free2)
-					pfree(itup2);
-				itup2 = tuplesort_getindextuple(btspool2->sortstate,
-												true, &should_free2);
+				itup2 = tuplesort_getindextuple(btspool2->sortstate, true);
 			}
 		}
 		pfree(sortKeys);
@@ -795,15 +785,13 @@ _bt_load(BTWriteState *wstate, BTSpool *btspool, BTSpool *btspool2)
 	{
 		/* merge is unnecessary */
 		while ((itup = tuplesort_getindextuple(btspool->sortstate,
-											   true, &should_free)) != NULL)
+											   true)) != NULL)
 		{
 			/* When we see first tuple, create first index page */
 			if (state == NULL)
 				state = _bt_pagestate(wstate, 0);
 
 			_bt_buildadd(wstate, state, itup);
-			if (should_free)
-				pfree(itup);
 		}
 	}
 
diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
index dc1f79f..2131226 100644
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -1057,11 +1057,10 @@ copy_heap_data(Oid OIDNewHeap, Oid OIDOldHeap, Oid OIDOldIndex, bool verbose,
 		for (;;)
 		{
 			HeapTuple	tuple;
-			bool		shouldfree;
 
 			CHECK_FOR_INTERRUPTS();
 
-			tuple = tuplesort_getheaptuple(tuplesort, true, &shouldfree);
+			tuple = tuplesort_getheaptuple(tuplesort, true);
 			if (tuple == NULL)
 				break;
 
@@ -1069,9 +1068,6 @@ copy_heap_data(Oid OIDNewHeap, Oid OIDOldHeap, Oid OIDOldIndex, bool verbose,
 									 oldTupDesc, newTupDesc,
 									 values, isnull,
 									 NewHeap->rd_rel->relhasoids, rwstate);
-
-			if (shouldfree)
-				heap_freetuple(tuple);
 		}
 
 		tuplesort_end(tuplesort);
diff --git a/src/backend/utils/sort/tuplesort.c b/src/backend/utils/sort/tuplesort.c
index baf87b3..0320f0d 100644
--- a/src/backend/utils/sort/tuplesort.c
+++ b/src/backend/utils/sort/tuplesort.c
@@ -1841,12 +1841,12 @@ tuplesort_performsort(Tuplesortstate *state)
 /*
  * Internal routine to fetch the next tuple in either forward or back
  * direction into *stup.  Returns FALSE if no more tuples.
- * If *should_free is set, the caller must pfree stup.tuple when done with it.
- * Otherwise, caller should not use tuple following next call here.
+ * Returned tuple belongs to tuplesort memory context, and must not be freed
+ * by caller.  Caller should not use tuple following next call here.
  */
 static bool
 tuplesort_gettuple_common(Tuplesortstate *state, bool forward,
-						  SortTuple *stup, bool *should_free)
+						  SortTuple *stup)
 {
 	unsigned int tuplen;
 
@@ -1855,7 +1855,6 @@ tuplesort_gettuple_common(Tuplesortstate *state, bool forward,
 		case TSS_SORTEDINMEM:
 			Assert(forward || state->randomAccess);
 			Assert(!state->slabAllocatorUsed);
-			*should_free = false;
 			if (forward)
 			{
 				if (state->current < state->memtupcount)
@@ -1927,7 +1926,6 @@ tuplesort_gettuple_common(Tuplesortstate *state, bool forward,
 					 */
 					state->lastReturnedTuple = stup->tuple;
 
-					*should_free = false;
 					return true;
 				}
 				else
@@ -2008,14 +2006,12 @@ tuplesort_gettuple_common(Tuplesortstate *state, bool forward,
 			 */
 			state->lastReturnedTuple = stup->tuple;
 
-			*should_free = false;
 			return true;
 
 		case TSS_FINALMERGE:
 			Assert(forward);
 			/* We are managing memory ourselves, with the slab allocator. */
 			Assert(state->slabAllocatorUsed);
-			*should_free = false;
 
 			/*
 			 * The slab slot holding the tuple that we returned in previous
@@ -2097,9 +2093,8 @@ tuplesort_gettupleslot(Tuplesortstate *state, bool forward,
 {
 	MemoryContext oldcontext = MemoryContextSwitchTo(state->sortcontext);
 	SortTuple	stup;
-	bool		should_free;
 
-	if (!tuplesort_gettuple_common(state, forward, &stup, &should_free))
+	if (!tuplesort_gettuple_common(state, forward, &stup))
 		stup.tuple = NULL;
 
 	MemoryContextSwitchTo(oldcontext);
@@ -2110,12 +2105,8 @@ tuplesort_gettupleslot(Tuplesortstate *state, bool forward,
 		if (state->sortKeys->abbrev_converter && abbrev)
 			*abbrev = stup.datum1;
 
-		if (!should_free)
-		{
-			stup.tuple = heap_copy_minimal_tuple((MinimalTuple) stup.tuple);
-			should_free = true;
-		}
-		ExecStoreMinimalTuple((MinimalTuple) stup.tuple, slot, should_free);
+		stup.tuple = heap_copy_minimal_tuple((MinimalTuple) stup.tuple);
+		ExecStoreMinimalTuple((MinimalTuple) stup.tuple, slot, false);
 		return true;
 	}
 	else
@@ -2127,18 +2118,17 @@ tuplesort_gettupleslot(Tuplesortstate *state, bool forward,
 
 /*
  * Fetch the next tuple in either forward or back direction.
- * Returns NULL if no more tuples.  If *should_free is set, the
- * caller must pfree the returned tuple when done with it.
- * If it is not set, caller should not use tuple following next
- * call here.
+ * Returns NULL if no more tuples.  Returned tuple belongs to tuplesort memory
+ * context, and must not be freed by caller.  Caller should not use tuple
+ * following next call here.
  */
 HeapTuple
-tuplesort_getheaptuple(Tuplesortstate *state, bool forward, bool *should_free)
+tuplesort_getheaptuple(Tuplesortstate *state, bool forward)
 {
 	MemoryContext oldcontext = MemoryContextSwitchTo(state->sortcontext);
 	SortTuple	stup;
 
-	if (!tuplesort_gettuple_common(state, forward, &stup, should_free))
+	if (!tuplesort_gettuple_common(state, forward, &stup))
 		stup.tuple = NULL;
 
 	MemoryContextSwitchTo(oldcontext);
@@ -2148,19 +2138,17 @@ tuplesort_getheaptuple(Tuplesortstate *state, bool forward, bool *should_free)
 
 /*
  * Fetch the next index tuple in either forward or back direction.
- * Returns NULL if no more tuples.  If *should_free is set, the
- * caller must pfree the returned tuple when done with it.
- * If it is not set, caller should not use tuple following next
- * call here.
+ * Returns NULL if no more tuples.  Returned tuple belongs to tuplesort memory
+ * context, and must not be freed by caller.  Caller should not use tuple
+ * following next call here.
  */
 IndexTuple
-tuplesort_getindextuple(Tuplesortstate *state, bool forward,
-						bool *should_free)
+tuplesort_getindextuple(Tuplesortstate *state, bool forward)
 {
 	MemoryContext oldcontext = MemoryContextSwitchTo(state->sortcontext);
 	SortTuple	stup;
 
-	if (!tuplesort_gettuple_common(state, forward, &stup, should_free))
+	if (!tuplesort_gettuple_common(state, forward, &stup))
 		stup.tuple = NULL;
 
 	MemoryContextSwitchTo(oldcontext);
@@ -2173,7 +2161,8 @@ tuplesort_getindextuple(Tuplesortstate *state, bool forward,
  * Returns FALSE if no more datums.
  *
  * If the Datum is pass-by-ref type, the returned value is freshly palloc'd
- * and is now owned by the caller.
+ * and is now owned by the caller (this differs from similar routines for
+ * other types of tuplesorts).
  *
  * Caller may optionally be passed back abbreviated value (on TRUE return
  * value) when abbreviation was used, which can be used to cheaply avoid
@@ -2188,9 +2177,8 @@ tuplesort_getdatum(Tuplesortstate *state, bool forward,
 {
 	MemoryContext oldcontext = MemoryContextSwitchTo(state->sortcontext);
 	SortTuple	stup;
-	bool		should_free;
 
-	if (!tuplesort_gettuple_common(state, forward, &stup, &should_free))
+	if (!tuplesort_gettuple_common(state, forward, &stup))
 	{
 		MemoryContextSwitchTo(oldcontext);
 		return false;
@@ -2208,11 +2196,7 @@ tuplesort_getdatum(Tuplesortstate *state, bool forward,
 	else
 	{
 		/* use stup.tuple because stup.datum1 may be an abbreviation */
-
-		if (should_free)
-			*val = PointerGetDatum(stup.tuple);
-		else
-			*val = datumCopy(PointerGetDatum(stup.tuple), false, state->datumTypeLen);
+		*val = datumCopy(PointerGetDatum(stup.tuple), false, state->datumTypeLen);
 		*isNull = false;
 	}
 
@@ -2270,16 +2254,12 @@ tuplesort_skiptuples(Tuplesortstate *state, int64 ntuples, bool forward)
 			while (ntuples-- > 0)
 			{
 				SortTuple	stup;
-				bool		should_free;
 
-				if (!tuplesort_gettuple_common(state, forward,
-											   &stup, &should_free))
+				if (!tuplesort_gettuple_common(state, forward, &stup))
 				{
 					MemoryContextSwitchTo(oldcontext);
 					return false;
 				}
-				if (should_free && stup.tuple)
-					pfree(stup.tuple);
 				CHECK_FOR_INTERRUPTS();
 			}
 			MemoryContextSwitchTo(oldcontext);
diff --git a/src/include/utils/tuplesort.h b/src/include/utils/tuplesort.h
index 5cecd6d..38af746 100644
--- a/src/include/utils/tuplesort.h
+++ b/src/include/utils/tuplesort.h
@@ -94,10 +94,8 @@ extern void tuplesort_performsort(Tuplesortstate *state);
 
 extern bool tuplesort_gettupleslot(Tuplesortstate *state, bool forward,
 					   TupleTableSlot *slot, Datum *abbrev);
-extern HeapTuple tuplesort_getheaptuple(Tuplesortstate *state, bool forward,
-					   bool *should_free);
-extern IndexTuple tuplesort_getindextuple(Tuplesortstate *state, bool forward,
-						bool *should_free);
+extern HeapTuple tuplesort_getheaptuple(Tuplesortstate *state, bool forward);
+extern IndexTuple tuplesort_getindextuple(Tuplesortstate *state, bool forward);
 extern bool tuplesort_getdatum(Tuplesortstate *state, bool forward,
 				   Datum *val, bool *isNull, Datum *abbrev);
 
-- 
2.7.4

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to