While profiling some queries and looking at executor overhead, I
realized that we're not making much use of TupleTableSlot's ability to
hold a buffer pin. In a SeqScan, the buffer is held pinned by the
underlying heap-scan anyway. Same with an IndexScan, and the SampleScan.
The only thing that relies on that feature is TidScan, but we could
easily teach TidScan to hold the buffer pin directly.
So, how about we remove the ability of a TupleTableSlot to hold a buffer
pin, per the attached patch? It shaves a few cycles from a
ExecStoreTuple() and ExecClearTuple(), which get called a lot. I
couldn't measure any actual difference from that, though, but it seems
like a good idea from a readability point of view, anyway.
How much do we need to worry about breaking extensions? I.e. to what
extent do we consider the TupleTableSlot API to be stable, for
extensions to use? The FDW API uses TupleTableSlots - this patch had to
fix the ExecStoreTuple calls in postgres_fdw.
We could refrain from changing the signature of ExecStoreTuple(), and
throw an error if you try to pass a valid buffer to it. But I also have
some bigger changes to TupleTableSlot in mind. I think we could gain
some speed by making slots "read-only". For example, it would be nice if
a SeqScan could just replace the tts_tuple pointer in the slot, and not
have to worry that the upper nodes might have materialized the slot, and
freeing the copied tuple. Because that happens very rarely in practice.
It would be better if those few places where we currently materialize an
existing slot, we would create a copy of the slot, and leave the
original slot unmodified. I'll start a different thread on that after
some more experimentation, but if we e.g. get rid of
ExecMaterializeSlot() in its current form, would that be OK?
- Heikki
commit 44ba53760e83921817b4878e48a2e4ad2fd67493
Author: Heikki Linnakangas <heikki.linnakangas@iki.fi>
Date: Tue Aug 30 13:06:59 2016 +0300
Remove support for holding a buffer pin in a TupleTableSlot.
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index daf0438..299fa62 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -1387,7 +1387,6 @@ postgresIterateForeignScan(ForeignScanState *node)
*/
ExecStoreTuple(fsstate->tuples[fsstate->next_tuple++],
slot,
- InvalidBuffer,
false);
return slot;
@@ -3152,7 +3151,7 @@ store_returning_result(PgFdwModifyState *fmstate,
NULL,
fmstate->temp_cxt);
/* tuple will be deleted when it is cleared from the slot */
- ExecStoreTuple(newtup, slot, InvalidBuffer, true);
+ ExecStoreTuple(newtup, slot, true);
}
PG_CATCH();
{
@@ -3258,7 +3257,7 @@ get_returning_data(ForeignScanState *node)
dmstate->retrieved_attrs,
NULL,
dmstate->temp_cxt);
- ExecStoreTuple(newtup, slot, InvalidBuffer, false);
+ ExecStoreTuple(newtup, slot, false);
}
PG_CATCH();
{
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index b0b43cf..91895fd 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -2531,7 +2531,7 @@ IndexBuildHeapRangeScan(Relation heapRelation,
MemoryContextReset(econtext->ecxt_per_tuple_memory);
/* Set up for predicate or expression evaluation */
- ExecStoreTuple(heapTuple, slot, InvalidBuffer, false);
+ ExecStoreTuple(heapTuple, slot, false);
/*
* In a partial index, discard tuples that don't satisfy the
@@ -2679,7 +2679,7 @@ IndexCheckExclusion(Relation heapRelation,
MemoryContextReset(econtext->ecxt_per_tuple_memory);
/* Set up for predicate or expression evaluation */
- ExecStoreTuple(heapTuple, slot, InvalidBuffer, false);
+ ExecStoreTuple(heapTuple, slot, false);
/*
* In a partial index, ignore tuples that don't satisfy the predicate.
@@ -3100,7 +3100,7 @@ validate_index_heapscan(Relation heapRelation,
MemoryContextReset(econtext->ecxt_per_tuple_memory);
/* Set up for predicate or expression evaluation */
- ExecStoreTuple(heapTuple, slot, InvalidBuffer, false);
+ ExecStoreTuple(heapTuple, slot, false);
/*
* In a partial index, discard tuples that don't satisfy the
diff --git a/src/backend/catalog/indexing.c b/src/backend/catalog/indexing.c
index b9fe102..faad4b9 100644
--- a/src/backend/catalog/indexing.c
+++ b/src/backend/catalog/indexing.c
@@ -96,7 +96,7 @@ CatalogIndexInsert(CatalogIndexState indstate, HeapTuple heapTuple)
/* Need a slot to hold the tuple being examined */
slot = MakeSingleTupleTableSlot(RelationGetDescr(heapRelation));
- ExecStoreTuple(heapTuple, slot, InvalidBuffer, false);
+ ExecStoreTuple(heapTuple, slot, false);
/*
* for each index, form and insert the index tuple
diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
index c617abb..6137dda 100644
--- a/src/backend/commands/analyze.c
+++ b/src/backend/commands/analyze.c
@@ -743,7 +743,7 @@ compute_index_stats(Relation onerel, double totalrows,
ResetExprContext(econtext);
/* Set up for predicate or expression evaluation */
- ExecStoreTuple(heapTuple, slot, InvalidBuffer, false);
+ ExecStoreTuple(heapTuple, slot, false);
/* If index is partial, check predicate */
if (predicate != NIL)
diff --git a/src/backend/commands/constraint.c b/src/backend/commands/constraint.c
index 26f9114..baf204c 100644
--- a/src/backend/commands/constraint.c
+++ b/src/backend/commands/constraint.c
@@ -124,7 +124,7 @@ unique_key_recheck(PG_FUNCTION_ARGS)
*/
slot = MakeSingleTupleTableSlot(RelationGetDescr(trigdata->tg_relation));
- ExecStoreTuple(new_row, slot, InvalidBuffer, false);
+ ExecStoreTuple(new_row, slot, false);
/*
* Typically the index won't have expressions, but if it does we need an
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index 5947e72..a2ab4b2 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -2435,7 +2435,7 @@ CopyFrom(CopyState cstate)
/* Place tuple in tuple slot --- but slot shouldn't free it */
slot = myslot;
- ExecStoreTuple(tuple, slot, InvalidBuffer, false);
+ ExecStoreTuple(tuple, slot, false);
skip_tuple = false;
@@ -2603,7 +2603,7 @@ CopyFromInsertBatch(CopyState cstate, EState *estate, CommandId mycid,
List *recheckIndexes;
cstate->cur_lineno = firstBufferedLineNo + i;
- ExecStoreTuple(bufferedTuples[i], myslot, InvalidBuffer, false);
+ ExecStoreTuple(bufferedTuples[i], myslot, false);
recheckIndexes =
ExecInsertIndexTuples(myslot, &(bufferedTuples[i]->t_self),
estate, false, NULL, NIL);
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 86e9814..fdbd86e 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -4142,7 +4142,7 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap, LOCKMODE lockmode)
* Process supplied expressions to replace selected columns.
* Expression inputs come from the old tuple.
*/
- ExecStoreTuple(tuple, oldslot, InvalidBuffer, false);
+ ExecStoreTuple(tuple, oldslot, false);
econtext->ecxt_scantuple = oldslot;
foreach(l, tab->newvals)
@@ -4173,7 +4173,7 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap, LOCKMODE lockmode)
}
/* Now check any constraints on the possibly-changed tuple */
- ExecStoreTuple(tuple, newslot, InvalidBuffer, false);
+ ExecStoreTuple(tuple, newslot, false);
econtext->ecxt_scantuple = newslot;
foreach(l, notnull_attrs)
@@ -7358,7 +7358,7 @@ validateCheckConstraint(Relation rel, HeapTuple constrtup)
while ((tuple = heap_getnext(scan, ForwardScanDirection)) != NULL)
{
- ExecStoreTuple(tuple, slot, InvalidBuffer, false);
+ ExecStoreTuple(tuple, slot, false);
if (!ExecQual(exprstate, econtext, true))
ereport(ERROR,
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index 9de22a1..4ac7b8f 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -2058,7 +2058,7 @@ ExecBRInsertTriggers(EState *estate, ResultRelInfo *relinfo,
if (newslot->tts_tupleDescriptor != tupdesc)
ExecSetSlotDescriptor(newslot, tupdesc);
- ExecStoreTuple(newtuple, newslot, InvalidBuffer, false);
+ ExecStoreTuple(newtuple, newslot, false);
slot = newslot;
}
return slot;
@@ -2133,7 +2133,7 @@ ExecIRInsertTriggers(EState *estate, ResultRelInfo *relinfo,
if (newslot->tts_tupleDescriptor != tupdesc)
ExecSetSlotDescriptor(newslot, tupdesc);
- ExecStoreTuple(newtuple, newslot, InvalidBuffer, false);
+ ExecStoreTuple(newtuple, newslot, false);
slot = newslot;
}
return slot;
@@ -2513,7 +2513,7 @@ ExecBRUpdateTriggers(EState *estate, EPQState *epqstate,
if (newslot->tts_tupleDescriptor != tupdesc)
ExecSetSlotDescriptor(newslot, tupdesc);
- ExecStoreTuple(newtuple, newslot, InvalidBuffer, false);
+ ExecStoreTuple(newtuple, newslot, false);
slot = newslot;
}
return slot;
@@ -2609,7 +2609,7 @@ ExecIRUpdateTriggers(EState *estate, ResultRelInfo *relinfo,
if (newslot->tts_tupleDescriptor != tupdesc)
ExecSetSlotDescriptor(newslot, tupdesc);
- ExecStoreTuple(newtuple, newslot, InvalidBuffer, false);
+ ExecStoreTuple(newtuple, newslot, false);
slot = newslot;
}
return slot;
@@ -2925,7 +2925,7 @@ TriggerEnabled(EState *estate, ResultRelInfo *relinfo,
oldslot = estate->es_trig_oldtup_slot;
if (oldslot->tts_tupleDescriptor != tupdesc)
ExecSetSlotDescriptor(oldslot, tupdesc);
- ExecStoreTuple(oldtup, oldslot, InvalidBuffer, false);
+ ExecStoreTuple(oldtup, oldslot, false);
}
if (HeapTupleIsValid(newtup))
{
@@ -2938,7 +2938,7 @@ TriggerEnabled(EState *estate, ResultRelInfo *relinfo,
newslot = estate->es_trig_newtup_slot;
if (newslot->tts_tupleDescriptor != tupdesc)
ExecSetSlotDescriptor(newslot, tupdesc);
- ExecStoreTuple(newtup, newslot, InvalidBuffer, false);
+ ExecStoreTuple(newtup, newslot, false);
}
/*
diff --git a/src/backend/executor/execIndexing.c b/src/backend/executor/execIndexing.c
index 0e2d834..bcd9894 100644
--- a/src/backend/executor/execIndexing.c
+++ b/src/backend/executor/execIndexing.c
@@ -753,7 +753,7 @@ retry:
* Extract the index column values and isnull flags from the existing
* tuple.
*/
- ExecStoreTuple(tup, existing_slot, InvalidBuffer, false);
+ ExecStoreTuple(tup, existing_slot, false);
FormIndexDatum(indexInfo, existing_slot, estate,
existing_values, existing_isnull);
diff --git a/src/backend/executor/execScan.c b/src/backend/executor/execScan.c
index fb0013d..36fb73b 100644
--- a/src/backend/executor/execScan.c
+++ b/src/backend/executor/execScan.c
@@ -79,7 +79,7 @@ ExecScanFetch(ScanState *node,
/* Store test tuple in the plan node's scan slot */
ExecStoreTuple(estate->es_epqTuple[scanrelid - 1],
- slot, InvalidBuffer, false);
+ slot, false);
/* Check if it meets the access-method conditions */
if (!(*recheckMtd) (node, slot))
diff --git a/src/backend/executor/execTuples.c b/src/backend/executor/execTuples.c
index 533050d..e298d40 100644
--- a/src/backend/executor/execTuples.c
+++ b/src/backend/executor/execTuples.c
@@ -118,7 +118,6 @@ MakeTupleTableSlot(void)
slot->tts_tuple = NULL;
slot->tts_tupleDescriptor = NULL;
slot->tts_mcxt = CurrentMemoryContext;
- slot->tts_buffer = InvalidBuffer;
slot->tts_nvalid = 0;
slot->tts_values = NULL;
slot->tts_isnull = NULL;
@@ -289,14 +288,9 @@ ExecSetSlotDescriptor(TupleTableSlot *slot, /* slot to change */
*
* tuple: tuple to store
* slot: slot to store it in
- * buffer: disk buffer if tuple is in a disk page, else InvalidBuffer
* shouldFree: true if ExecClearTuple should pfree() the tuple
* when done with it
*
- * If 'buffer' is not InvalidBuffer, the tuple table code acquires a pin
- * on the buffer which is held until the slot is cleared, so that the tuple
- * won't go away on us.
- *
* shouldFree is normally set 'true' for tuples constructed on-the-fly.
* It must always be 'false' for tuples that are stored in disk pages,
* since we don't want to try to pfree those.
@@ -322,7 +316,6 @@ ExecSetSlotDescriptor(TupleTableSlot *slot, /* slot to change */
TupleTableSlot *
ExecStoreTuple(HeapTuple tuple,
TupleTableSlot *slot,
- Buffer buffer,
bool shouldFree)
{
/*
@@ -354,24 +347,6 @@ ExecStoreTuple(HeapTuple tuple,
/* Mark extracted state invalid */
slot->tts_nvalid = 0;
- /*
- * If tuple is on a disk page, keep the page pinned as long as we hold a
- * pointer into it. We assume the caller already has such a pin.
- *
- * This is coded to optimize the case where the slot previously held a
- * tuple on the same disk page: in that case releasing and re-acquiring
- * the pin is a waste of cycles. This is a common situation during
- * seqscans, so it's worth troubling over.
- */
- if (slot->tts_buffer != buffer)
- {
- if (BufferIsValid(slot->tts_buffer))
- ReleaseBuffer(slot->tts_buffer);
- slot->tts_buffer = buffer;
- if (BufferIsValid(buffer))
- IncrBufferRefCount(buffer);
- }
-
return slot;
}
@@ -404,14 +379,6 @@ ExecStoreMinimalTuple(MinimalTuple mtup,
heap_free_minimal_tuple(slot->tts_mintuple);
/*
- * Drop the pin on the referenced buffer, if there is one.
- */
- if (BufferIsValid(slot->tts_buffer))
- ReleaseBuffer(slot->tts_buffer);
-
- slot->tts_buffer = InvalidBuffer;
-
- /*
* Store the new tuple into the specified slot.
*/
slot->tts_isempty = false;
@@ -460,14 +427,6 @@ ExecClearTuple(TupleTableSlot *slot) /* slot in which to store tuple */
slot->tts_shouldFreeMin = false;
/*
- * Drop the pin on the referenced buffer, if there is one.
- */
- if (BufferIsValid(slot->tts_buffer))
- ReleaseBuffer(slot->tts_buffer);
-
- slot->tts_buffer = InvalidBuffer;
-
- /*
* Mark it empty.
*/
slot->tts_isempty = true;
@@ -755,14 +714,6 @@ ExecMaterializeSlot(TupleTableSlot *slot)
MemoryContextSwitchTo(oldContext);
/*
- * Drop the pin on the referenced buffer, if there is one.
- */
- if (BufferIsValid(slot->tts_buffer))
- ReleaseBuffer(slot->tts_buffer);
-
- slot->tts_buffer = InvalidBuffer;
-
- /*
* Mark extracted state invalid. This is important because the slot is
* not supposed to depend any more on the previous external data; we
* mustn't leave any dangling pass-by-reference datums in tts_values.
@@ -809,7 +760,7 @@ ExecCopySlot(TupleTableSlot *dstslot, TupleTableSlot *srcslot)
newTuple = ExecCopySlotTuple(srcslot);
MemoryContextSwitchTo(oldContext);
- return ExecStoreTuple(newTuple, dstslot, InvalidBuffer, true);
+ return ExecStoreTuple(newTuple, dstslot, true);
}
diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c
index ce2fc28..8f9b4a0 100644
--- a/src/backend/executor/nodeAgg.c
+++ b/src/backend/executor/nodeAgg.c
@@ -2077,7 +2077,6 @@ agg_retrieve_direct(AggState *aggstate)
*/
ExecStoreTuple(aggstate->grp_firstTuple,
firstSlot,
- InvalidBuffer,
true);
aggstate->grp_firstTuple = NULL; /* don't keep two
* pointers */
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 449aacb..4c489e3 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -274,7 +274,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
*/
ExecStoreTuple(&scan->rs_ctup,
slot,
- scan->rs_cbuf,
false);
/*
diff --git a/src/backend/executor/nodeGather.c b/src/backend/executor/nodeGather.c
index 438d1b2..832409c 100644
--- a/src/backend/executor/nodeGather.c
+++ b/src/backend/executor/nodeGather.c
@@ -295,8 +295,6 @@ gather_getnext(GatherState *gatherstate)
{
ExecStoreTuple(tup, /* tuple to store */
fslot, /* slot in which to store the tuple */
- InvalidBuffer, /* buffer associated with this
- * tuple */
false); /* slot should not pfree tuple */
return fslot;
}
diff --git a/src/backend/executor/nodeIndexscan.c b/src/backend/executor/nodeIndexscan.c
index 3143bd9..187be4a 100644
--- a/src/backend/executor/nodeIndexscan.c
+++ b/src/backend/executor/nodeIndexscan.c
@@ -111,7 +111,6 @@ IndexNext(IndexScanState *node)
*/
ExecStoreTuple(tuple, /* tuple to store */
slot, /* slot to store in */
- scandesc->xs_cbuf, /* buffer containing tuple */
false); /* don't pfree */
/*
@@ -198,7 +197,7 @@ IndexNextWithReorder(IndexScanState *node)
tuple = reorderqueue_pop(node);
/* Pass 'true', as the tuple in the queue is a palloc'd copy */
- ExecStoreTuple(tuple, slot, InvalidBuffer, true);
+ ExecStoreTuple(tuple, slot, true);
return slot;
}
}
@@ -230,7 +229,6 @@ next_indextuple:
*/
ExecStoreTuple(tuple, /* tuple to store */
slot, /* slot to store in */
- scandesc->xs_cbuf, /* buffer containing tuple */
false); /* don't pfree */
/*
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index af7b26c..c70288f 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -751,7 +751,7 @@ ldelete:;
if (slot->tts_tupleDescriptor != RelationGetDescr(resultRelationDesc))
ExecSetSlotDescriptor(slot, RelationGetDescr(resultRelationDesc));
- ExecStoreTuple(&deltuple, slot, InvalidBuffer, false);
+ ExecStoreTuple(&deltuple, slot, false);
}
rslot = ExecProcessReturning(resultRelInfo, slot, planSlot);
@@ -1173,7 +1173,7 @@ ExecOnConflictUpdate(ModifyTableState *mtstate,
ExecCheckHeapTupleVisible(estate, &tuple, buffer);
/* Store target's existing tuple in the state's dedicated slot */
- ExecStoreTuple(&tuple, mtstate->mt_existing, buffer, false);
+ ExecStoreTuple(&tuple, mtstate->mt_existing, false);
/*
* Make tuple and any needed join variables available to ExecQual and
diff --git a/src/backend/executor/nodeSamplescan.c b/src/backend/executor/nodeSamplescan.c
index 9ce7c02..9a9c9d2 100644
--- a/src/backend/executor/nodeSamplescan.c
+++ b/src/backend/executor/nodeSamplescan.c
@@ -65,7 +65,6 @@ SampleNext(SampleScanState *node)
if (tuple)
ExecStoreTuple(tuple, /* tuple to store */
slot, /* slot to store in */
- node->ss.ss_currentScanDesc->rs_cbuf, /* tuple's buffer */
false); /* don't pfree this pointer */
else
ExecClearTuple(slot);
diff --git a/src/backend/executor/nodeSeqscan.c b/src/backend/executor/nodeSeqscan.c
index 00bf3a5..5e26594 100644
--- a/src/backend/executor/nodeSeqscan.c
+++ b/src/backend/executor/nodeSeqscan.c
@@ -83,15 +83,11 @@ SeqNext(SeqScanState *node)
* save the tuple and the buffer returned to us by the access methods in
* our scan tuple slot and return the slot. Note: we pass 'false' because
* tuples returned by heap_getnext() are pointers onto disk pages and were
- * not created with palloc() and so should not be pfree()'d. Note also
- * that ExecStoreTuple will increment the refcount of the buffer; the
- * refcount will not be dropped until the tuple table slot is cleared.
+ * not created with palloc() and so should not be pfree()'d.
*/
if (tuple)
ExecStoreTuple(tuple, /* tuple to store */
slot, /* slot to store in */
- scandesc->rs_cbuf, /* buffer associated with this
- * tuple */
false); /* don't pfree this pointer */
else
ExecClearTuple(slot);
diff --git a/src/backend/executor/nodeSetOp.c b/src/backend/executor/nodeSetOp.c
index 633580b..052ceb9 100644
--- a/src/backend/executor/nodeSetOp.c
+++ b/src/backend/executor/nodeSetOp.c
@@ -273,7 +273,6 @@ setop_retrieve_direct(SetOpState *setopstate)
*/
ExecStoreTuple(setopstate->grp_firstTuple,
resultTupleSlot,
- InvalidBuffer,
true);
setopstate->grp_firstTuple = NULL; /* don't keep two pointers */
diff --git a/src/backend/executor/nodeTidscan.c b/src/backend/executor/nodeTidscan.c
index 2604103..6194453 100644
--- a/src/backend/executor/nodeTidscan.c
+++ b/src/backend/executor/nodeTidscan.c
@@ -256,7 +256,6 @@ TidNext(TidScanState *node)
Relation heapRelation;
HeapTuple tuple;
TupleTableSlot *slot;
- Buffer buffer = InvalidBuffer;
ItemPointerData *tidList;
int numTids;
bool bBackward;
@@ -318,7 +317,14 @@ TidNext(TidScanState *node)
if (node->tss_isCurrentOf)
heap_get_latest_tid(heapRelation, snapshot, &tuple->t_self);
- if (heap_fetch(heapRelation, snapshot, tuple, &buffer, false, NULL))
+ /* Release old buffer, if any */
+ if (BufferIsValid(node->tss_buffer))
+ {
+ ReleaseBuffer(node->tss_buffer);
+ node->tss_buffer = InvalidBuffer;
+ }
+
+ if (heap_fetch(heapRelation, snapshot, tuple, &node->tss_buffer, false, NULL))
{
/*
* store the scanned tuple in the scan tuple slot of the scan
@@ -329,15 +335,8 @@ TidNext(TidScanState *node)
*/
ExecStoreTuple(tuple, /* tuple to store */
slot, /* slot to store in */
- buffer, /* buffer associated with tuple */
false); /* don't pfree */
- /*
- * At this point we have an extra pin on the buffer, because
- * ExecStoreTuple incremented the pin count. Drop our local pin.
- */
- ReleaseBuffer(buffer);
-
return slot;
}
/* Bad TID or failed snapshot qual; try next */
@@ -351,6 +350,12 @@ TidNext(TidScanState *node)
* if we get here it means the tid scan failed so we are at the end of the
* scan..
*/
+ if (BufferIsValid(node->tss_buffer))
+ {
+ ReleaseBuffer(node->tss_buffer);
+ node->tss_buffer = InvalidBuffer;
+ }
+
return ExecClearTuple(slot);
}
@@ -422,6 +427,15 @@ void
ExecEndTidScan(TidScanState *node)
{
/*
+ * Release the buffer
+ */
+ if (BufferIsValid(node->tss_buffer))
+ {
+ ReleaseBuffer(node->tss_buffer);
+ node->tss_buffer = InvalidBuffer;
+ }
+
+ /*
* Free the exprcontext
*/
ExecFreeExprContext(&node->ss.ps);
@@ -505,6 +519,7 @@ ExecInitTidScan(TidScan *node, EState *estate, int eflags)
tidstate->ss.ss_currentRelation = currentRelation;
tidstate->ss.ss_currentScanDesc = NULL; /* no heap scan here */
+ tidstate->tss_buffer = InvalidBuffer;
/*
* get the scan type from the relation descriptor.
diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c
index 56943f2..dad862b 100644
--- a/src/backend/utils/adt/selfuncs.c
+++ b/src/backend/utils/adt/selfuncs.c
@@ -5138,7 +5138,7 @@ get_actual_variable_range(PlannerInfo *root, VariableStatData *vardata,
indexscandir)) != NULL)
{
/* Extract the index column values from the heap tuple */
- ExecStoreTuple(tup, slot, InvalidBuffer, false);
+ ExecStoreTuple(tup, slot, false);
FormIndexDatum(indexInfo, slot, estate,
values, isnull);
@@ -5170,7 +5170,7 @@ get_actual_variable_range(PlannerInfo *root, VariableStatData *vardata,
-indexscandir)) != NULL)
{
/* Extract the index column values from the heap tuple */
- ExecStoreTuple(tup, slot, InvalidBuffer, false);
+ ExecStoreTuple(tup, slot, false);
FormIndexDatum(indexInfo, slot, estate,
values, isnull);
diff --git a/src/backend/utils/sort/tuplesort.c b/src/backend/utils/sort/tuplesort.c
index c8fbcf8..3902068 100644
--- a/src/backend/utils/sort/tuplesort.c
+++ b/src/backend/utils/sort/tuplesort.c
@@ -4176,11 +4176,11 @@ comparetup_cluster(const SortTuple *a, const SortTuple *b,
ecxt_scantuple = GetPerTupleExprContext(state->estate)->ecxt_scantuple;
- ExecStoreTuple(ltup, ecxt_scantuple, InvalidBuffer, false);
+ ExecStoreTuple(ltup, ecxt_scantuple, false);
FormIndexDatum(state->indexInfo, ecxt_scantuple, state->estate,
l_index_values, l_index_isnull);
- ExecStoreTuple(rtup, ecxt_scantuple, InvalidBuffer, false);
+ ExecStoreTuple(rtup, ecxt_scantuple, false);
FormIndexDatum(state->indexInfo, ecxt_scantuple, state->estate,
r_index_values, r_index_isnull);
diff --git a/src/include/executor/tuptable.h b/src/include/executor/tuptable.h
index 5ac0b6a..7e9ad7b 100644
--- a/src/include/executor/tuptable.h
+++ b/src/include/executor/tuptable.h
@@ -82,12 +82,6 @@
* When tts_shouldFree is true, the physical tuple is "owned" by the slot
* and should be freed when the slot's reference to the tuple is dropped.
*
- * If tts_buffer is not InvalidBuffer, then the slot is holding a pin
- * on the indicated buffer page; drop the pin when we release the
- * slot's reference to that buffer. (tts_shouldFree should always be
- * false in such a case, since presumably tts_tuple is pointing at the
- * buffer page.)
- *
* tts_nvalid indicates the number of valid columns in the tts_values/isnull
* arrays. When the slot is holding a "virtual" tuple this must be equal
* to the descriptor's natts. When the slot is holding a physical tuple
@@ -120,7 +114,6 @@ typedef struct TupleTableSlot
HeapTuple tts_tuple; /* physical tuple, or NULL if virtual */
TupleDesc tts_tupleDescriptor; /* slot's tuple descriptor */
MemoryContext tts_mcxt; /* slot itself is in this context */
- Buffer tts_buffer; /* tuple's buffer, or InvalidBuffer */
int tts_nvalid; /* # of valid values in tts_values */
Datum *tts_values; /* current per-attribute values */
bool *tts_isnull; /* current per-attribute isnull flags */
@@ -147,7 +140,6 @@ extern void ExecDropSingleTupleTableSlot(TupleTableSlot *slot);
extern void ExecSetSlotDescriptor(TupleTableSlot *slot, TupleDesc tupdesc);
extern TupleTableSlot *ExecStoreTuple(HeapTuple tuple,
TupleTableSlot *slot,
- Buffer buffer,
bool shouldFree);
extern TupleTableSlot *ExecStoreMinimalTuple(MinimalTuple mtup,
TupleTableSlot *slot,
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index e7fd7bd..cb9146e 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -1457,6 +1457,8 @@ typedef struct BitmapHeapScanState
* NumTids number of tids in this scan
* TidPtr index of currently fetched tid
* TidList evaluated item pointers (array of size NumTids)
+ * htup currently fetched tuple
+ * buffer tuple's buffer, or InvalidBuffer
* ----------------
*/
typedef struct TidScanState
@@ -1468,6 +1470,7 @@ typedef struct TidScanState
int tss_TidPtr;
ItemPointerData *tss_TidList;
HeapTupleData tss_htup;
+ Buffer tss_buffer;
} TidScanState;
/* ----------------
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
