From 0518eceec3a1cc2b71da04e839f05f555fdd8567 Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Mon, 22 Jul 2013 13:26:33 -0400 Subject: [PATCH] Adjust HeapTupleSatisfies* routines to take a HeapTuple. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, these functions took a HeapTupleHeader, but upcoming patches for logical replication will introduce new a new snapshot type under which the tuple's TID will be used to lookup (CMIN, CMAX) for visibility determination purposes. This makes that information available. Code churn is minimal since HeapTupleSatisfiesVisibility took the HeapTuple anyway, and deferenced it before calling the satisfies function. Independently of logical replication, this allows t_tableOid and t_self to be cross-checked via assertions in tqual.c. This seems like a useful way to make sure that all callers are setting these values properly, which has been previously put forward as desirable. Andres Freund, reviewed by Álvaro Herrera --- contrib/pgrowlocks/pgrowlocks.c | 2 +- src/backend/access/heap/heapam.c | 13 +++--- src/backend/access/heap/pruneheap.c | 17 +++++++- src/backend/catalog/index.c | 2 +- src/backend/commands/analyze.c | 3 +- src/backend/commands/cluster.c | 2 +- src/backend/commands/vacuumlazy.c | 13 +++--- src/backend/executor/nodeBitmapHeapscan.c | 1 + src/backend/storage/lmgr/predicate.c | 2 +- src/backend/utils/time/tqual.c | 50 +++++++++++++++++++---- src/include/utils/snapshot.h | 4 +- src/include/utils/tqual.h | 20 ++++----- 12 files changed, 91 insertions(+), 38 deletions(-) diff --git a/contrib/pgrowlocks/pgrowlocks.c b/contrib/pgrowlocks/pgrowlocks.c index 075d78131a..8d8e78ea4a 100644 --- a/contrib/pgrowlocks/pgrowlocks.c +++ b/contrib/pgrowlocks/pgrowlocks.c @@ -131,7 +131,7 @@ pgrowlocks(PG_FUNCTION_ARGS) /* must hold a buffer lock to call HeapTupleSatisfiesUpdate */ LockBuffer(scan->rs_cbuf, BUFFER_LOCK_SHARE); - htsu = HeapTupleSatisfiesUpdate(tuple->t_data, + htsu = HeapTupleSatisfiesUpdate(tuple, GetCurrentCommandId(false), scan->rs_cbuf); xmax = HeapTupleHeaderGetRawXmax(tuple->t_data); diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index 5bcbc92bfa..b1a5d9f65a 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -387,6 +387,7 @@ heapgetpage(HeapScanDesc scan, BlockNumber page) HeapTupleData loctup; bool valid; + loctup.t_tableOid = RelationGetRelid(scan->rs_rd); loctup.t_data = (HeapTupleHeader) PageGetItem((Page) dp, lpp); loctup.t_len = ItemIdGetLength(lpp); ItemPointerSet(&(loctup.t_self), page, lineoff); @@ -1715,7 +1716,7 @@ heap_hot_search_buffer(ItemPointer tid, Relation relation, Buffer buffer, heapTuple->t_data = (HeapTupleHeader) PageGetItem(dp, lp); heapTuple->t_len = ItemIdGetLength(lp); - heapTuple->t_tableOid = relation->rd_id; + heapTuple->t_tableOid = RelationGetRelid(relation); heapTuple->t_self = *tid; /* @@ -1763,7 +1764,7 @@ heap_hot_search_buffer(ItemPointer tid, Relation relation, Buffer buffer, * transactions. */ if (all_dead && *all_dead && - !HeapTupleIsSurelyDead(heapTuple->t_data, RecentGlobalXmin)) + !HeapTupleIsSurelyDead(heapTuple, RecentGlobalXmin)) *all_dead = false; /* @@ -1893,6 +1894,7 @@ heap_get_latest_tid(Relation relation, tp.t_self = ctid; tp.t_data = (HeapTupleHeader) PageGetItem(page, lp); tp.t_len = ItemIdGetLength(lp); + tp.t_tableOid = RelationGetRelid(relation); /* * After following a t_ctid link, we might arrive at an unrelated @@ -2591,12 +2593,13 @@ heap_delete(Relation relation, ItemPointer tid, lp = PageGetItemId(page, ItemPointerGetOffsetNumber(tid)); Assert(ItemIdIsNormal(lp)); + tp.t_tableOid = RelationGetRelid(relation); tp.t_data = (HeapTupleHeader) PageGetItem(page, lp); tp.t_len = ItemIdGetLength(lp); tp.t_self = *tid; l1: - result = HeapTupleSatisfiesUpdate(tp.t_data, cid, buffer); + result = HeapTupleSatisfiesUpdate(&tp, cid, buffer); if (result == HeapTupleInvisible) { @@ -3070,7 +3073,7 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup, l2: checked_lockers = false; locker_remains = false; - result = HeapTupleSatisfiesUpdate(oldtup.t_data, cid, buffer); + result = HeapTupleSatisfiesUpdate(&oldtup, cid, buffer); /* see below about the "no wait" case */ Assert(result != HeapTupleBeingUpdated || wait); @@ -3941,7 +3944,7 @@ heap_lock_tuple(Relation relation, HeapTuple tuple, tuple->t_tableOid = RelationGetRelid(relation); l3: - result = HeapTupleSatisfiesUpdate(tuple->t_data, cid, *buffer); + result = HeapTupleSatisfiesUpdate(tuple, cid, *buffer); if (result == HeapTupleInvisible) { diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c index c6e3154293..3ec10a07c0 100644 --- a/src/backend/access/heap/pruneheap.c +++ b/src/backend/access/heap/pruneheap.c @@ -339,6 +339,9 @@ heap_prune_chain(Relation relation, Buffer buffer, OffsetNumber rootoffnum, OffsetNumber chainitems[MaxHeapTuplesPerPage]; int nchain = 0, i; + HeapTupleData tup; + + tup.t_tableOid = RelationGetRelid(relation); rootlp = PageGetItemId(dp, rootoffnum); @@ -348,6 +351,12 @@ heap_prune_chain(Relation relation, Buffer buffer, OffsetNumber rootoffnum, if (ItemIdIsNormal(rootlp)) { htup = (HeapTupleHeader) PageGetItem(dp, rootlp); + + tup.t_data = htup; + tup.t_len = ItemIdGetLength(rootlp); + tup.t_tableOid = RelationGetRelid(relation); + ItemPointerSet(&(tup.t_self), BufferGetBlockNumber(buffer), rootoffnum); + if (HeapTupleHeaderIsHeapOnly(htup)) { /* @@ -368,7 +377,7 @@ heap_prune_chain(Relation relation, Buffer buffer, OffsetNumber rootoffnum, * either here or while following a chain below. Whichever path * gets there first will mark the tuple unused. */ - if (HeapTupleSatisfiesVacuum(htup, OldestXmin, buffer) + if (HeapTupleSatisfiesVacuum(&tup, OldestXmin, buffer) == HEAPTUPLE_DEAD && !HeapTupleHeaderIsHotUpdated(htup)) { heap_prune_record_unused(prstate, rootoffnum); @@ -431,6 +440,10 @@ heap_prune_chain(Relation relation, Buffer buffer, OffsetNumber rootoffnum, Assert(ItemIdIsNormal(lp)); htup = (HeapTupleHeader) PageGetItem(dp, lp); + tup.t_data = htup; + tup.t_len = ItemIdGetLength(lp); + ItemPointerSet(&(tup.t_self), BufferGetBlockNumber(buffer), offnum); + /* * Check the tuple XMIN against prior XMAX, if any */ @@ -448,7 +461,7 @@ heap_prune_chain(Relation relation, Buffer buffer, OffsetNumber rootoffnum, */ tupdead = recent_dead = false; - switch (HeapTupleSatisfiesVacuum(htup, OldestXmin, buffer)) + switch (HeapTupleSatisfiesVacuum(&tup, OldestXmin, buffer)) { case HEAPTUPLE_DEAD: tupdead = true; diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index 8525cb9ec8..7e4d7c0578 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -2251,7 +2251,7 @@ IndexBuildHeapScan(Relation heapRelation, */ LockBuffer(scan->rs_cbuf, BUFFER_LOCK_SHARE); - switch (HeapTupleSatisfiesVacuum(heapTuple->t_data, OldestXmin, + switch (HeapTupleSatisfiesVacuum(heapTuple, OldestXmin, scan->rs_cbuf)) { case HEAPTUPLE_DEAD: diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c index d6d20fde9a..9845b0b50a 100644 --- a/src/backend/commands/analyze.c +++ b/src/backend/commands/analyze.c @@ -1138,10 +1138,11 @@ acquire_sample_rows(Relation onerel, int elevel, ItemPointerSet(&targtuple.t_self, targblock, targoffset); + targtuple.t_tableOid = RelationGetRelid(onerel); targtuple.t_data = (HeapTupleHeader) PageGetItem(targpage, itemid); targtuple.t_len = ItemIdGetLength(itemid); - switch (HeapTupleSatisfiesVacuum(targtuple.t_data, + switch (HeapTupleSatisfiesVacuum(&targtuple, OldestXmin, targbuffer)) { diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c index c20a6fb073..4519c00e22 100644 --- a/src/backend/commands/cluster.c +++ b/src/backend/commands/cluster.c @@ -979,7 +979,7 @@ copy_heap_data(Oid OIDNewHeap, Oid OIDOldHeap, Oid OIDOldIndex, LockBuffer(buf, BUFFER_LOCK_SHARE); - switch (HeapTupleSatisfiesVacuum(tuple->t_data, OldestXmin, buf)) + switch (HeapTupleSatisfiesVacuum(tuple, OldestXmin, buf)) { case HEAPTUPLE_DEAD: /* Definitely dead */ diff --git a/src/backend/commands/vacuumlazy.c b/src/backend/commands/vacuumlazy.c index 078b822059..c34aa53728 100644 --- a/src/backend/commands/vacuumlazy.c +++ b/src/backend/commands/vacuumlazy.c @@ -151,7 +151,7 @@ static void lazy_record_dead_tuple(LVRelStats *vacrelstats, ItemPointer itemptr); static bool lazy_tid_reaped(ItemPointer itemptr, void *state); static int vac_cmp_itemptr(const void *left, const void *right); -static bool heap_page_is_all_visible(Buffer buf, +static bool heap_page_is_all_visible(Relation rel, Buffer buf, TransactionId *visibility_cutoff_xid); @@ -756,10 +756,11 @@ lazy_scan_heap(Relation onerel, LVRelStats *vacrelstats, tuple.t_data = (HeapTupleHeader) PageGetItem(page, itemid); tuple.t_len = ItemIdGetLength(itemid); + tuple.t_tableOid = RelationGetRelid(onerel); tupgone = false; - switch (HeapTupleSatisfiesVacuum(tuple.t_data, OldestXmin, buf)) + switch (HeapTupleSatisfiesVacuum(&tuple, OldestXmin, buf)) { case HEAPTUPLE_DEAD: @@ -1168,7 +1169,7 @@ lazy_vacuum_page(Relation onerel, BlockNumber blkno, Buffer buffer, * check if the page has become all-visible. */ if (!visibilitymap_test(onerel, blkno, vmbuffer) && - heap_page_is_all_visible(buffer, &visibility_cutoff_xid)) + heap_page_is_all_visible(onerel, buffer, &visibility_cutoff_xid)) { Assert(BufferIsValid(*vmbuffer)); PageSetAllVisible(page); @@ -1676,7 +1677,7 @@ vac_cmp_itemptr(const void *left, const void *right) * xmin amongst the visible tuples. */ static bool -heap_page_is_all_visible(Buffer buf, TransactionId *visibility_cutoff_xid) +heap_page_is_all_visible(Relation rel, Buffer buf, TransactionId *visibility_cutoff_xid) { Page page = BufferGetPage(buf); OffsetNumber offnum, @@ -1718,8 +1719,10 @@ heap_page_is_all_visible(Buffer buf, TransactionId *visibility_cutoff_xid) Assert(ItemIdIsNormal(itemid)); tuple.t_data = (HeapTupleHeader) PageGetItem(page, itemid); + tuple.t_len = ItemIdGetLength(itemid); + tuple.t_tableOid = RelationGetRelid(rel); - switch (HeapTupleSatisfiesVacuum(tuple.t_data, OldestXmin, buf)) + switch (HeapTupleSatisfiesVacuum(&tuple, OldestXmin, buf)) { case HEAPTUPLE_LIVE: { diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c index bbb89e6996..45292b2633 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -258,6 +258,7 @@ BitmapHeapNext(BitmapHeapScanState *node) scan->rs_ctup.t_data = (HeapTupleHeader) PageGetItem((Page) dp, lp); scan->rs_ctup.t_len = ItemIdGetLength(lp); + scan->rs_ctup.t_tableOid = scan->rs_rd->rd_id; ItemPointerSet(&scan->rs_ctup.t_self, tbmres->blockno, targoffset); pgstat_count_heap_fetch(scan->rs_rd); diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c index b012df1c5d..d656d62398 100644 --- a/src/backend/storage/lmgr/predicate.c +++ b/src/backend/storage/lmgr/predicate.c @@ -3895,7 +3895,7 @@ CheckForSerializableConflictOut(bool visible, Relation relation, * tuple is visible to us, while HeapTupleSatisfiesVacuum checks what else * is going on with it. */ - htsvResult = HeapTupleSatisfiesVacuum(tuple->t_data, TransactionXmin, buffer); + htsvResult = HeapTupleSatisfiesVacuum(tuple, TransactionXmin, buffer); switch (htsvResult) { case HEAPTUPLE_LIVE: diff --git a/src/backend/utils/time/tqual.c b/src/backend/utils/time/tqual.c index c69ffd306e..da2ce1825e 100644 --- a/src/backend/utils/time/tqual.c +++ b/src/backend/utils/time/tqual.c @@ -163,8 +163,12 @@ HeapTupleSetHintBits(HeapTupleHeader tuple, Buffer buffer, * Xmax is not committed))) that has not been committed */ bool -HeapTupleSatisfiesSelf(HeapTupleHeader tuple, Snapshot snapshot, Buffer buffer) +HeapTupleSatisfiesSelf(HeapTuple htup, Snapshot snapshot, Buffer buffer) { + HeapTupleHeader tuple = htup->t_data; + Assert(ItemPointerIsValid(&htup->t_self)); + Assert(htup->t_tableOid != InvalidOid); + if (!(tuple->t_infomask & HEAP_XMIN_COMMITTED)) { if (tuple->t_infomask & HEAP_XMIN_INVALID) @@ -351,8 +355,12 @@ HeapTupleSatisfiesSelf(HeapTupleHeader tuple, Snapshot snapshot, Buffer buffer) * */ bool -HeapTupleSatisfiesNow(HeapTupleHeader tuple, Snapshot snapshot, Buffer buffer) +HeapTupleSatisfiesNow(HeapTuple htup, Snapshot snapshot, Buffer buffer) { + HeapTupleHeader tuple = htup->t_data; + Assert(ItemPointerIsValid(&htup->t_self)); + Assert(htup->t_tableOid != InvalidOid); + if (!(tuple->t_infomask & HEAP_XMIN_COMMITTED)) { if (tuple->t_infomask & HEAP_XMIN_INVALID) @@ -526,7 +534,7 @@ HeapTupleSatisfiesNow(HeapTupleHeader tuple, Snapshot snapshot, Buffer buffer) * Dummy "satisfies" routine: any tuple satisfies SnapshotAny. */ bool -HeapTupleSatisfiesAny(HeapTupleHeader tuple, Snapshot snapshot, Buffer buffer) +HeapTupleSatisfiesAny(HeapTuple htup, Snapshot snapshot, Buffer buffer) { return true; } @@ -546,9 +554,13 @@ HeapTupleSatisfiesAny(HeapTupleHeader tuple, Snapshot snapshot, Buffer buffer) * table. */ bool -HeapTupleSatisfiesToast(HeapTupleHeader tuple, Snapshot snapshot, +HeapTupleSatisfiesToast(HeapTuple htup, Snapshot snapshot, Buffer buffer) { + HeapTupleHeader tuple = htup->t_data; + Assert(ItemPointerIsValid(&htup->t_self)); + Assert(htup->t_tableOid != InvalidOid); + if (!(tuple->t_infomask & HEAP_XMIN_COMMITTED)) { if (tuple->t_infomask & HEAP_XMIN_INVALID) @@ -627,9 +639,13 @@ HeapTupleSatisfiesToast(HeapTupleHeader tuple, Snapshot snapshot, * distinguish that case must test for it themselves.) */ HTSU_Result -HeapTupleSatisfiesUpdate(HeapTupleHeader tuple, CommandId curcid, +HeapTupleSatisfiesUpdate(HeapTuple htup, CommandId curcid, Buffer buffer) { + HeapTupleHeader tuple = htup->t_data; + Assert(ItemPointerIsValid(&htup->t_self)); + Assert(htup->t_tableOid != InvalidOid); + if (!(tuple->t_infomask & HEAP_XMIN_COMMITTED)) { if (tuple->t_infomask & HEAP_XMIN_INVALID) @@ -849,9 +865,13 @@ HeapTupleSatisfiesUpdate(HeapTupleHeader tuple, CommandId curcid, * for snapshot->xmax and the tuple's xmax. */ bool -HeapTupleSatisfiesDirty(HeapTupleHeader tuple, Snapshot snapshot, +HeapTupleSatisfiesDirty(HeapTuple htup, Snapshot snapshot, Buffer buffer) { + HeapTupleHeader tuple = htup->t_data; + Assert(ItemPointerIsValid(&htup->t_self)); + Assert(htup->t_tableOid != InvalidOid); + snapshot->xmin = snapshot->xmax = InvalidTransactionId; if (!(tuple->t_infomask & HEAP_XMIN_COMMITTED)) @@ -1040,9 +1060,13 @@ HeapTupleSatisfiesDirty(HeapTupleHeader tuple, Snapshot snapshot, * can't see it.) */ bool -HeapTupleSatisfiesMVCC(HeapTupleHeader tuple, Snapshot snapshot, +HeapTupleSatisfiesMVCC(HeapTuple htup, Snapshot snapshot, Buffer buffer) { + HeapTupleHeader tuple = htup->t_data; + Assert(ItemPointerIsValid(&htup->t_self)); + Assert(htup->t_tableOid != InvalidOid); + if (!(tuple->t_infomask & HEAP_XMIN_COMMITTED)) { if (tuple->t_infomask & HEAP_XMIN_INVALID) @@ -1233,9 +1257,13 @@ HeapTupleSatisfiesMVCC(HeapTupleHeader tuple, Snapshot snapshot, * even if we see that the deleting transaction has committed. */ HTSV_Result -HeapTupleSatisfiesVacuum(HeapTupleHeader tuple, TransactionId OldestXmin, +HeapTupleSatisfiesVacuum(HeapTuple htup, TransactionId OldestXmin, Buffer buffer) { + HeapTupleHeader tuple = htup->t_data; + Assert(ItemPointerIsValid(&htup->t_self)); + Assert(htup->t_tableOid != InvalidOid); + /* * Has inserting transaction committed? * @@ -1466,8 +1494,12 @@ HeapTupleSatisfiesVacuum(HeapTupleHeader tuple, TransactionId OldestXmin, * just whether or not the tuple is surely dead). */ bool -HeapTupleIsSurelyDead(HeapTupleHeader tuple, TransactionId OldestXmin) +HeapTupleIsSurelyDead(HeapTuple htup, TransactionId OldestXmin) { + HeapTupleHeader tuple = htup->t_data; + Assert(ItemPointerIsValid(&htup->t_self)); + Assert(htup->t_tableOid != InvalidOid); + /* * If the inserting transaction is marked invalid, then it aborted, and * the tuple is definitely dead. If it's marked neither committed nor diff --git a/src/include/utils/snapshot.h b/src/include/utils/snapshot.h index e747191e79..ed3f58692e 100644 --- a/src/include/utils/snapshot.h +++ b/src/include/utils/snapshot.h @@ -27,8 +27,8 @@ typedef struct SnapshotData *Snapshot; * The specific semantics of a snapshot are encoded by the "satisfies" * function. */ -typedef bool (*SnapshotSatisfiesFunc) (HeapTupleHeader tuple, - Snapshot snapshot, Buffer buffer); +typedef bool (*SnapshotSatisfiesFunc) (HeapTuple htup, + Snapshot snapshot, Buffer buffer); typedef struct SnapshotData { diff --git a/src/include/utils/tqual.h b/src/include/utils/tqual.h index 465231c758..800e366f30 100644 --- a/src/include/utils/tqual.h +++ b/src/include/utils/tqual.h @@ -52,7 +52,7 @@ extern PGDLLIMPORT SnapshotData SnapshotToastData; * if so, the indicated buffer is marked dirty. */ #define HeapTupleSatisfiesVisibility(tuple, snapshot, buffer) \ - ((*(snapshot)->satisfies) ((tuple)->t_data, snapshot, buffer)) + ((*(snapshot)->satisfies) (tuple, snapshot, buffer)) /* Result codes for HeapTupleSatisfiesVacuum */ typedef enum @@ -65,25 +65,25 @@ typedef enum } HTSV_Result; /* These are the "satisfies" test routines for the various snapshot types */ -extern bool HeapTupleSatisfiesMVCC(HeapTupleHeader tuple, +extern bool HeapTupleSatisfiesMVCC(HeapTuple htup, Snapshot snapshot, Buffer buffer); -extern bool HeapTupleSatisfiesNow(HeapTupleHeader tuple, +extern bool HeapTupleSatisfiesNow(HeapTuple htup, Snapshot snapshot, Buffer buffer); -extern bool HeapTupleSatisfiesSelf(HeapTupleHeader tuple, +extern bool HeapTupleSatisfiesSelf(HeapTuple htup, Snapshot snapshot, Buffer buffer); -extern bool HeapTupleSatisfiesAny(HeapTupleHeader tuple, +extern bool HeapTupleSatisfiesAny(HeapTuple htup, Snapshot snapshot, Buffer buffer); -extern bool HeapTupleSatisfiesToast(HeapTupleHeader tuple, +extern bool HeapTupleSatisfiesToast(HeapTuple htup, Snapshot snapshot, Buffer buffer); -extern bool HeapTupleSatisfiesDirty(HeapTupleHeader tuple, +extern bool HeapTupleSatisfiesDirty(HeapTuple htup, Snapshot snapshot, Buffer buffer); /* Special "satisfies" routines with different APIs */ -extern HTSU_Result HeapTupleSatisfiesUpdate(HeapTupleHeader tuple, +extern HTSU_Result HeapTupleSatisfiesUpdate(HeapTuple htup, CommandId curcid, Buffer buffer); -extern HTSV_Result HeapTupleSatisfiesVacuum(HeapTupleHeader tuple, +extern HTSV_Result HeapTupleSatisfiesVacuum(HeapTuple htup, TransactionId OldestXmin, Buffer buffer); -extern bool HeapTupleIsSurelyDead(HeapTupleHeader tuple, +extern bool HeapTupleIsSurelyDead(HeapTuple htup, TransactionId OldestXmin); extern void HeapTupleSetHintBits(HeapTupleHeader tuple, Buffer buffer,