diff --git a/src/backend/executor/execTuples.c b/src/backend/executor/execTuples.c
index 8c550f160f62173912a3f0755485bd03b81fb300..56f17abeb02d1a0a5acf5c50d709befb646e3eee 100644
--- a/src/backend/executor/execTuples.c
+++ b/src/backend/executor/execTuples.c
@@ -19,7 +19,7 @@
*
* At ExecutorStart()
* ----------------
-
+ *
* - ExecInitSeqScan() calls ExecInitScanTupleSlot() to construct a
* TupleTableSlots for the tuples returned by the access method, and
* ExecInitResultTypeTL() to define the node's return
@@ -272,7 +272,6 @@ tts_virtual_copy_heap_tuple(TupleTableSlot *slot)
return heap_form_tuple(slot->tts_tupleDescriptor,
slot->tts_values,
slot->tts_isnull);
-
}
static MinimalTuple
@@ -334,6 +333,8 @@ tts_heap_getsysattr(TupleTableSlot *slot, int attnum, bool *isnull)
{
HeapTupleTableSlot *hslot = (HeapTupleTableSlot *) slot;
+ Assert(!TTS_EMPTY(slot));
+
return heap_getsysattr(hslot->tuple, attnum,
slot->tts_tupleDescriptor, isnull);
}
@@ -346,14 +347,19 @@ tts_heap_materialize(TupleTableSlot *slot)
Assert(!TTS_EMPTY(slot));
- /* This slot has it's tuple already materialized. Nothing to do. */
+ /* If slot has its tuple already materialized, nothing to do. */
if (TTS_SHOULDFREE(slot))
return;
- slot->tts_flags |= TTS_FLAG_SHOULDFREE;
-
oldContext = MemoryContextSwitchTo(slot->tts_mcxt);
+ /*
+ * Have to deform from scratch, otherwise tts_values[] entries could point
+ * into the non-materialized tuple (which might be gone when accessed).
+ */
+ slot->tts_nvalid = 0;
+ hslot->off = 0;
+
if (!hslot->tuple)
hslot->tuple = heap_form_tuple(slot->tts_tupleDescriptor,
slot->tts_values,
@@ -368,12 +374,7 @@ tts_heap_materialize(TupleTableSlot *slot)
hslot->tuple = heap_copytuple(hslot->tuple);
}
- /*
- * Have to deform from scratch, otherwise tts_values[] entries could point
- * into the non-materialized tuple (which might be gone when accessed).
- */
- slot->tts_nvalid = 0;
- hslot->off = 0;
+ slot->tts_flags |= TTS_FLAG_SHOULDFREE;
MemoryContextSwitchTo(oldContext);
}
@@ -436,7 +437,7 @@ tts_heap_store_tuple(TupleTableSlot *slot, HeapTuple tuple, bool shouldFree)
slot->tts_nvalid = 0;
hslot->tuple = tuple;
hslot->off = 0;
- slot->tts_flags &= ~TTS_FLAG_EMPTY;
+ slot->tts_flags &= ~(TTS_FLAG_EMPTY | TTS_FLAG_SHOULDFREE);
slot->tts_tid = tuple->t_self;
if (shouldFree)
@@ -509,13 +510,19 @@ tts_minimal_materialize(TupleTableSlot *slot)
Assert(!TTS_EMPTY(slot));
- /* This slot has it's tuple already materialized. Nothing to do. */
+ /* If slot has its tuple already materialized, nothing to do. */
if (TTS_SHOULDFREE(slot))
return;
- slot->tts_flags |= TTS_FLAG_SHOULDFREE;
oldContext = MemoryContextSwitchTo(slot->tts_mcxt);
+ /*
+ * Have to deform from scratch, otherwise tts_values[] entries could point
+ * into the non-materialized tuple (which might be gone when accessed).
+ */
+ slot->tts_nvalid = 0;
+ mslot->off = 0;
+
if (!mslot->mintuple)
{
mslot->mintuple = heap_form_minimal_tuple(slot->tts_tupleDescriptor,
@@ -532,19 +539,14 @@ tts_minimal_materialize(TupleTableSlot *slot)
mslot->mintuple = heap_copy_minimal_tuple(mslot->mintuple);
}
+ slot->tts_flags |= TTS_FLAG_SHOULDFREE;
+
Assert(mslot->tuple == &mslot->minhdr);
mslot->minhdr.t_len = mslot->mintuple->t_len + MINIMAL_TUPLE_OFFSET;
mslot->minhdr.t_data = (HeapTupleHeader) ((char *) mslot->mintuple - MINIMAL_TUPLE_OFFSET);
MemoryContextSwitchTo(oldContext);
-
- /*
- * Have to deform from scratch, otherwise tts_values[] entries could point
- * into the non-materialized tuple (which might be gone when accessed).
- */
- slot->tts_nvalid = 0;
- mslot->off = 0;
}
static void
@@ -615,8 +617,6 @@ tts_minimal_store_tuple(TupleTableSlot *slot, MinimalTuple mtup, bool shouldFree
if (shouldFree)
slot->tts_flags |= TTS_FLAG_SHOULDFREE;
- else
- Assert(!TTS_SHOULDFREE(slot));
}
@@ -651,8 +651,6 @@ tts_buffer_heap_clear(TupleTableSlot *slot)
heap_freetuple(bslot->base.tuple);
slot->tts_flags &= ~TTS_FLAG_SHOULDFREE;
-
- Assert(!BufferIsValid(bslot->buffer));
}
if (BufferIsValid(bslot->buffer))
@@ -681,6 +679,8 @@ tts_buffer_heap_getsysattr(TupleTableSlot *slot, int attnum, bool *isnull)
{
BufferHeapTupleTableSlot *bslot = (BufferHeapTupleTableSlot *) slot;
+ Assert(!TTS_EMPTY(slot));
+
return heap_getsysattr(bslot->base.tuple, attnum,
slot->tts_tupleDescriptor, isnull);
}
@@ -693,14 +693,19 @@ tts_buffer_heap_materialize(TupleTableSlot *slot)
Assert(!TTS_EMPTY(slot));
- /* If already materialized nothing to do. */
+ /* If slot has its tuple already materialized, nothing to do. */
if (TTS_SHOULDFREE(slot))
return;
- slot->tts_flags |= TTS_FLAG_SHOULDFREE;
-
oldContext = MemoryContextSwitchTo(slot->tts_mcxt);
+ /*
+ * Have to deform from scratch, otherwise tts_values[] entries could point
+ * into the non-materialized tuple (which might be gone when accessed).
+ */
+ bslot->base.off = 0;
+ slot->tts_nvalid = 0;
+
if (!bslot->base.tuple)
{
/*
@@ -713,7 +718,6 @@ tts_buffer_heap_materialize(TupleTableSlot *slot)
bslot->base.tuple = heap_form_tuple(slot->tts_tupleDescriptor,
slot->tts_values,
slot->tts_isnull);
-
}
else
{
@@ -723,19 +727,21 @@ tts_buffer_heap_materialize(TupleTableSlot *slot)
* A heap tuple stored in a BufferHeapTupleTableSlot should have a
* buffer associated with it, unless it's materialized or virtual.
*/
- Assert(BufferIsValid(bslot->buffer));
if (likely(BufferIsValid(bslot->buffer)))
ReleaseBuffer(bslot->buffer);
bslot->buffer = InvalidBuffer;
}
- MemoryContextSwitchTo(oldContext);
/*
- * Have to deform from scratch, otherwise tts_values[] entries could point
- * into the non-materialized tuple (which might be gone when accessed).
+ * We don't set TTS_FLAG_SHOULDFREE until after releasing the buffer, if
+ * any. This avoids having a transient state that would fall foul of our
+ * assertions that a slot with TTS_FLAG_SHOULDFREE doesn't own a buffer.
+ * In the unlikely event that ReleaseBuffer() above errors out, we'd
+ * effectively leak the copied tuple, but that seems fairly harmless.
*/
- bslot->base.off = 0;
- slot->tts_nvalid = 0;
+ slot->tts_flags |= TTS_FLAG_SHOULDFREE;
+
+ MemoryContextSwitchTo(oldContext);
}
static void
@@ -756,10 +762,10 @@ tts_buffer_heap_copyslot(TupleTableSlot *dstslot, TupleTableSlot *srcslot)
MemoryContext oldContext;
ExecClearTuple(dstslot);
- dstslot->tts_flags |= TTS_FLAG_SHOULDFREE;
dstslot->tts_flags &= ~TTS_FLAG_EMPTY;
oldContext = MemoryContextSwitchTo(dstslot->tts_mcxt);
bdstslot->base.tuple = ExecCopySlotHeapTuple(srcslot);
+ dstslot->tts_flags |= TTS_FLAG_SHOULDFREE;
MemoryContextSwitchTo(oldContext);
}
else
@@ -1444,10 +1450,10 @@ ExecForceStoreHeapTuple(HeapTuple tuple,
BufferHeapTupleTableSlot *bslot = (BufferHeapTupleTableSlot *) slot;
ExecClearTuple(slot);
- slot->tts_flags |= TTS_FLAG_SHOULDFREE;
slot->tts_flags &= ~TTS_FLAG_EMPTY;
oldContext = MemoryContextSwitchTo(slot->tts_mcxt);
bslot->base.tuple = heap_copytuple(tuple);
+ slot->tts_flags |= TTS_FLAG_SHOULDFREE;
MemoryContextSwitchTo(oldContext);
if (shouldFree)
@@ -1856,7 +1862,6 @@ slot_getmissingattrs(TupleTableSlot *slot, int startAttNum, int lastAttNum)
slot->tts_values[missattnum] = attrmiss[missattnum].am_value;
slot->tts_isnull[missattnum] = !attrmiss[missattnum].am_present;
}
-
}
}
diff --git a/src/include/executor/tuptable.h b/src/include/executor/tuptable.h
index 885b481d9a476d2b604875d47c9d6dcf61cc8167..b7f977233be9c19b3f5d6aae4922908abe339bda 100644
--- a/src/include/executor/tuptable.h
+++ b/src/include/executor/tuptable.h
@@ -261,9 +261,8 @@ typedef struct BufferHeapTupleTableSlot
/*
* If 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_FLAG_SHOULDFREE should not be set be
- * false in such a case, since presumably tts_tuple is pointing at the
- * buffer page.)
+ * reference to that buffer. (TTS_FLAG_SHOULDFREE should not be set in
+ * such a case, since presumably tts_tuple is pointing into the buffer.)
*/
Buffer buffer; /* tuple's buffer, or InvalidBuffer */
} BufferHeapTupleTableSlot;