diff options
| -rw-r--r-- | src/backend/executor/execTuples.c | 81 | ||||
| -rw-r--r-- | src/include/executor/tuptable.h | 5 |
2 files changed, 45 insertions, 41 deletions
diff --git a/src/backend/executor/execTuples.c b/src/backend/executor/execTuples.c index 87bc510b31b..46d55d944af 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 @@ -273,7 +273,6 @@ tts_virtual_copy_heap_tuple(TupleTableSlot *slot) return heap_form_tuple(slot->tts_tupleDescriptor, slot->tts_values, slot->tts_isnull); - } static MinimalTuple @@ -335,6 +334,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); } @@ -347,14 +348,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, @@ -369,12 +375,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); } @@ -437,7 +438,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) @@ -510,13 +511,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, @@ -533,19 +540,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 @@ -616,8 +618,6 @@ tts_minimal_store_tuple(TupleTableSlot *slot, MinimalTuple mtup, bool shouldFree if (shouldFree) slot->tts_flags |= TTS_FLAG_SHOULDFREE; - else - Assert(!TTS_SHOULDFREE(slot)); } @@ -652,8 +652,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)) @@ -682,6 +680,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); } @@ -694,14 +694,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) { /* @@ -714,7 +719,6 @@ tts_buffer_heap_materialize(TupleTableSlot *slot) bslot->base.tuple = heap_form_tuple(slot->tts_tupleDescriptor, slot->tts_values, slot->tts_isnull); - } else { @@ -724,19 +728,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 @@ -757,10 +763,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 @@ -1445,10 +1451,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) @@ -1857,7 +1863,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 885b481d9a4..b7f977233be 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; |