0002-Avoid-copying-within-tuplesort_gettupleslot.patch

From 541a7f0ae6060763cd4448359159c1a2c5980a68 Mon Sep 17 00:00:00 2001
From: Peter Geoghegan <pg@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 71524c2..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, true);
+		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