From 9ad369d158b2bb07dff11c4b035a30aca10a9443 Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Thu, 7 Mar 2019 16:30:12 -0800 Subject: [PATCH v18 04/18] tableam: Add fetch_row_version. Author: Reviewed-By: Discussion: https://postgr.es/m/ Backpatch: --- src/backend/access/heap/heapam_handler.c | 28 ++++++++++ src/backend/commands/trigger.c | 70 ++++-------------------- src/backend/executor/execMain.c | 12 +--- src/backend/executor/nodeLockRows.c | 10 +--- src/backend/executor/nodeModifyTable.c | 23 ++------ src/backend/executor/nodeTidscan.c | 15 +---- src/include/access/tableam.h | 25 +++++++++ 7 files changed, 73 insertions(+), 110 deletions(-) diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c index 3285197c558..318e393dbde 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -148,6 +148,33 @@ heapam_index_fetch_tuple(struct IndexFetchTableData *scan, * ------------------------------------------------------------------------ */ +static bool +heapam_fetch_row_version(Relation relation, + ItemPointer tid, + Snapshot snapshot, + TupleTableSlot *slot, + Relation stats_relation) +{ + BufferHeapTupleTableSlot *bslot = (BufferHeapTupleTableSlot *) slot; + Buffer buffer; + + Assert(TTS_IS_BUFFERTUPLE(slot)); + + if (heap_fetch(relation, tid, snapshot, &bslot->base.tupdata, &buffer, stats_relation)) + { + /* store in slot, transferring existing pin */ + ExecStorePinnedBufferHeapTuple(&bslot->base.tupdata, slot, buffer); + + slot->tts_tableOid = RelationGetRelid(relation); + + return true; + } + + slot->tts_tableOid = RelationGetRelid(relation); + + return false; +} + static bool heapam_tuple_satisfies_snapshot(Relation rel, TupleTableSlot *slot, Snapshot snapshot) @@ -546,6 +573,7 @@ static const TableAmRoutine heapam_methods = { .tuple_update = heapam_heap_update, .tuple_lock = heapam_lock_tuple, + .tuple_fetch_row_version = heapam_fetch_row_version, .tuple_satisfies_snapshot = heapam_tuple_satisfies_snapshot, }; diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index 5cc15bcfef0..6fbf0c2b81e 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -14,10 +14,11 @@ #include "postgres.h" #include "access/genam.h" -#include "access/heapam.h" -#include "access/tableam.h" -#include "access/sysattr.h" #include "access/htup_details.h" +#include "access/relation.h" +#include "access/sysattr.h" +#include "access/table.h" +#include "access/tableam.h" #include "access/xact.h" #include "catalog/catalog.h" #include "catalog/dependency.h" @@ -3382,43 +3383,8 @@ GetTupleForTrigger(EState *estate, } else { - - Page page; - ItemId lp; - Buffer buffer; - BufferHeapTupleTableSlot *boldslot; - HeapTuple tuple; - - Assert(TTS_IS_BUFFERTUPLE(oldslot)); - ExecClearTuple(oldslot); - boldslot = (BufferHeapTupleTableSlot *) oldslot; - tuple = &boldslot->base.tupdata; - - buffer = ReadBuffer(relation, ItemPointerGetBlockNumber(tid)); - - /* - * Although we already know this tuple is valid, we must lock the - * buffer to ensure that no one has a buffer cleanup lock; otherwise - * they might move the tuple while we try to copy it. But we can - * release the lock before actually doing the heap_copytuple call, - * since holding pin is sufficient to prevent anyone from getting a - * cleanup lock they don't already hold. - */ - LockBuffer(buffer, BUFFER_LOCK_SHARE); - - page = BufferGetPage(buffer); - lp = PageGetItemId(page, ItemPointerGetOffsetNumber(tid)); - - Assert(ItemIdIsNormal(lp)); - - tuple->t_data = (HeapTupleHeader) PageGetItem(page, lp); - tuple->t_len = ItemIdGetLength(lp); - tuple->t_self = *tid; - tuple->t_tableOid = RelationGetRelid(relation); - - LockBuffer(buffer, BUFFER_LOCK_UNLOCK); - - ExecStorePinnedBufferHeapTuple(tuple, oldslot, buffer); + if (!table_fetch_row_version(relation, tid, SnapshotAny, oldslot, NULL)) + elog(ERROR, "couldn't fetch tuple"); } return true; @@ -4197,8 +4163,6 @@ AfterTriggerExecute(EState *estate, AfterTriggerShared evtshared = GetTriggerSharedData(event); Oid tgoid = evtshared->ats_tgoid; TriggerData LocTriggerData; - HeapTupleData tuple1; - HeapTupleData tuple2; HeapTuple rettuple; int tgindx; bool should_free_trig = false; @@ -4275,19 +4239,12 @@ AfterTriggerExecute(EState *estate, default: if (ItemPointerIsValid(&(event->ate_ctid1))) { - Buffer buffer; - LocTriggerData.tg_trigslot = ExecGetTriggerOldSlot(estate, relInfo); - ItemPointerCopy(&(event->ate_ctid1), &(tuple1.t_self)); - if (!heap_fetch(rel, &(tuple1.t_self), SnapshotAny, &tuple1, &buffer, NULL)) + if (!table_fetch_row_version(rel, &(event->ate_ctid1), SnapshotAny, LocTriggerData.tg_trigslot, NULL)) elog(ERROR, "failed to fetch tuple1 for AFTER trigger"); - ExecStorePinnedBufferHeapTuple(&tuple1, - LocTriggerData.tg_trigslot, - buffer); LocTriggerData.tg_trigtuple = - ExecFetchSlotHeapTuple(LocTriggerData.tg_trigslot, false, - &should_free_trig); + ExecFetchSlotHeapTuple(LocTriggerData.tg_trigslot, false, &should_free_trig); } else { @@ -4299,19 +4256,12 @@ AfterTriggerExecute(EState *estate, AFTER_TRIGGER_2CTID && ItemPointerIsValid(&(event->ate_ctid2))) { - Buffer buffer; - LocTriggerData.tg_newslot = ExecGetTriggerNewSlot(estate, relInfo); - ItemPointerCopy(&(event->ate_ctid2), &(tuple2.t_self)); - if (!heap_fetch(rel, &(tuple2.t_self), SnapshotAny, &tuple2, &buffer, NULL)) + if (!table_fetch_row_version(rel, &(event->ate_ctid2), SnapshotAny, LocTriggerData.tg_newslot, NULL)) elog(ERROR, "failed to fetch tuple2 for AFTER trigger"); - ExecStorePinnedBufferHeapTuple(&tuple2, - LocTriggerData.tg_newslot, - buffer); LocTriggerData.tg_newtuple = - ExecFetchSlotHeapTuple(LocTriggerData.tg_newslot, false, - &should_free_new); + ExecFetchSlotHeapTuple(LocTriggerData.tg_newslot, false, &should_free_new); } else { diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index 8723e32dd58..e70e9f08e42 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -2638,17 +2638,9 @@ EvalPlanQualFetchRowMarks(EPQState *epqstate) else { /* ordinary table, fetch the tuple */ - HeapTupleData tuple; - Buffer buffer; - - tuple.t_self = *((ItemPointer) DatumGetPointer(datum)); - if (!heap_fetch(erm->relation, &tuple.t_self, SnapshotAny, - &tuple, &buffer, NULL)) + if (!table_fetch_row_version(erm->relation, (ItemPointer) DatumGetPointer(datum), + SnapshotAny, slot, NULL)) elog(ERROR, "failed to fetch tuple for EvalPlanQual recheck"); - - /* successful, store tuple */ - ExecStorePinnedBufferHeapTuple(&tuple, slot, buffer); - ExecMaterializeSlot(slot); } } else diff --git a/src/backend/executor/nodeLockRows.c b/src/backend/executor/nodeLockRows.c index 91f46b88ed8..aedc5297e3b 100644 --- a/src/backend/executor/nodeLockRows.c +++ b/src/backend/executor/nodeLockRows.c @@ -21,7 +21,6 @@ #include "postgres.h" -#include "access/heapam.h" #include "access/htup_details.h" #include "access/tableam.h" #include "access/xact.h" @@ -275,8 +274,6 @@ lnext: ExecAuxRowMark *aerm = (ExecAuxRowMark *) lfirst(lc); ExecRowMark *erm = aerm->rowmark; TupleTableSlot *markSlot; - HeapTupleData tuple; - Buffer buffer; markSlot = EvalPlanQualSlot(&node->lr_epqstate, erm->relation, erm->rti); @@ -297,12 +294,9 @@ lnext: Assert(ItemPointerIsValid(&(erm->curCtid))); /* okay, fetch the tuple */ - tuple.t_self = erm->curCtid; - if (!heap_fetch(erm->relation, &tuple.t_self, SnapshotAny, &tuple, &buffer, - NULL)) + if (!table_fetch_row_version(erm->relation, &erm->curCtid, SnapshotAny, markSlot, + NULL)) elog(ERROR, "failed to fetch tuple for EvalPlanQual recheck"); - ExecStorePinnedBufferHeapTuple(&tuple, markSlot, buffer); - ExecMaterializeSlot(markSlot); /* successful, use tuple in slot */ } diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 5b079d8302a..b4247ec33b4 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -230,18 +230,15 @@ ExecCheckTIDVisible(EState *estate, TupleTableSlot *tempSlot) { Relation rel = relinfo->ri_RelationDesc; - Buffer buffer; - HeapTupleData tuple; /* Redundantly check isolation level */ if (!IsolationUsesXactSnapshot()) return; - tuple.t_self = *tid; - if (!heap_fetch(rel, &tuple.t_self, SnapshotAny, &tuple, &buffer, NULL)) + if (!table_fetch_row_version(rel, tid, SnapshotAny, tempSlot, NULL)) elog(ERROR, "failed to fetch conflicting tuple for ON CONFLICT"); - ExecStorePinnedBufferHeapTuple(&tuple, tempSlot, buffer); ExecCheckHeapTupleVisible(estate, rel, tempSlot); + ExecClearTuple(tempSlot); } /* ---------------------------------------------------------------- @@ -851,21 +848,9 @@ ldelete:; } else { - BufferHeapTupleTableSlot *bslot; - HeapTuple deltuple; - Buffer buffer; - - Assert(TTS_IS_BUFFERTUPLE(slot)); - ExecClearTuple(slot); - bslot = (BufferHeapTupleTableSlot *) slot; - deltuple = &bslot->base.tupdata; - - deltuple->t_self = *tupleid; - if (!heap_fetch(resultRelationDesc, &deltuple->t_self, SnapshotAny, - deltuple, &buffer, NULL)) + if (!table_fetch_row_version(resultRelationDesc, tupleid, SnapshotAny, + slot, NULL)) elog(ERROR, "failed to fetch deleted tuple for DELETE RETURNING"); - - ExecStorePinnedBufferHeapTuple(deltuple, slot, buffer); } } diff --git a/src/backend/executor/nodeTidscan.c b/src/backend/executor/nodeTidscan.c index b819cf2383e..7d496cc4105 100644 --- a/src/backend/executor/nodeTidscan.c +++ b/src/backend/executor/nodeTidscan.c @@ -310,7 +310,6 @@ TidNext(TidScanState *node) Relation heapRelation; HeapTuple tuple; TupleTableSlot *slot; - Buffer buffer = InvalidBuffer; ItemPointerData *tidList; int numTids; bool bBackward; @@ -376,19 +375,9 @@ TidNext(TidScanState *node) if (node->tss_isCurrentOf) heap_get_latest_tid(heapRelation, snapshot, &tuple->t_self); - if (heap_fetch(heapRelation, &tuple->t_self, snapshot, tuple, &buffer, NULL)) - { - /* - * Store the scanned tuple in the scan tuple slot of the scan - * state, transferring the pin to the slot. - */ - ExecStorePinnedBufferHeapTuple(tuple, /* tuple to store */ - slot, /* slot to store in */ - buffer); /* buffer associated with - * tuple */ - + if (table_fetch_row_version(heapRelation, &tuple->t_self, snapshot, slot, NULL)) return slot; - } + /* Bad TID or failed snapshot qual; try next */ if (bBackward) node->tss_TidPtr--; diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h index b34e903d84d..abbd7f63385 100644 --- a/src/include/access/tableam.h +++ b/src/include/access/tableam.h @@ -218,6 +218,13 @@ typedef struct TableAmRoutine * ------------------------------------------------------------------------ */ + + bool (*tuple_fetch_row_version) (Relation rel, + ItemPointer tid, + Snapshot snapshot, + TupleTableSlot *slot, + Relation stats_relation); + /* * Does the tuple in `slot` satisfy `snapshot`? The slot needs to be of * the appropriate type for the AM. @@ -542,6 +549,24 @@ table_index_fetch_tuple(struct IndexFetchTableData *scan, * ------------------------------------------------------------------------ */ + +/* + * table_fetch_row_version - retrieve tuple with given tid + * + * XXX: This shouldn't just take a tid, but tid + additional information + */ +static inline bool +table_fetch_row_version(Relation rel, + ItemPointer tid, + Snapshot snapshot, + TupleTableSlot *slot, + Relation stats_relation) +{ + return rel->rd_tableam->tuple_fetch_row_version(rel, tid, + snapshot, slot, + stats_relation); +} + /* * Return true iff tuple in slot satisfies the snapshot. * -- 2.21.0.dirty