From 4e213f29851ceee73f9b20846df84f6f9662714c Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Sat, 29 Feb 2020 19:33:21 -0800
Subject: [PATCH v1 2/2] Improve and extend asserts for a snapshot being set.

---
 src/include/utils/snapmgr.h        |  2 ++
 src/backend/access/heap/heapam.c   |  6 ++++--
 src/backend/access/index/indexam.c |  8 +++++++-
 src/backend/catalog/indexing.c     | 11 +++++++++++
 src/backend/utils/time/snapmgr.c   | 19 +++++++++++++++++++
 contrib/amcheck/verify_nbtree.c    |  4 ++--
 6 files changed, 45 insertions(+), 5 deletions(-)

diff --git a/src/include/utils/snapmgr.h b/src/include/utils/snapmgr.h
index b28d13ce841..7738d6a8e01 100644
--- a/src/include/utils/snapmgr.h
+++ b/src/include/utils/snapmgr.h
@@ -116,6 +116,8 @@ extern void PopActiveSnapshot(void);
 extern Snapshot GetActiveSnapshot(void);
 extern bool ActiveSnapshotSet(void);
 
+extern bool SnapshotSet(void);
+
 extern Snapshot RegisterSnapshot(Snapshot snapshot);
 extern void UnregisterSnapshot(Snapshot snapshot);
 extern Snapshot RegisterSnapshotOnOwner(Snapshot snapshot, ResourceOwner owner);
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 29694b8aa4a..912cadeb03a 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -1137,6 +1137,8 @@ heap_beginscan(Relation relation, Snapshot snapshot,
 {
 	HeapScanDesc scan;
 
+	Assert(SnapshotSet());
+
 	/*
 	 * increment relation ref count while scanning relation
 	 *
@@ -1546,7 +1548,7 @@ heap_hot_search_buffer(ItemPointer tid, Relation relation, Buffer buffer,
 	at_chain_start = first_call;
 	skip = !first_call;
 
-	Assert(TransactionIdIsValid(RecentGlobalXmin));
+	Assert(SnapshotSet());
 	Assert(BufferGetBlockNumber(buffer) == blkno);
 
 	/* Scan through possible multiple members of HOT-chain */
@@ -5629,7 +5631,7 @@ heap_abort_speculative(Relation relation, ItemPointer tid)
 	 * RecentGlobalXmin.  That's not pretty, but it doesn't seem worth
 	 * inventing a nicer API for this.
 	 */
-	Assert(TransactionIdIsValid(RecentGlobalXmin));
+	Assert(SnapshotSet());
 	PageSetPrunable(page, RecentGlobalXmin);
 
 	/* store transaction information of xact deleting the tuple */
diff --git a/src/backend/access/index/indexam.c b/src/backend/access/index/indexam.c
index a5210d0b342..558b490d079 100644
--- a/src/backend/access/index/indexam.c
+++ b/src/backend/access/index/indexam.c
@@ -178,6 +178,8 @@ index_insert(Relation indexRelation,
 	RELATION_CHECKS;
 	CHECK_REL_PROCEDURE(aminsert);
 
+	Assert(SnapshotSet());
+
 	if (!(indexRelation->rd_indam->ampredlocks))
 		CheckForSerializableConflictIn(indexRelation,
 									   (ItemPointer) NULL,
@@ -250,6 +252,8 @@ index_beginscan_internal(Relation indexRelation,
 {
 	IndexScanDesc scan;
 
+	Assert(SnapshotSet());
+
 	RELATION_CHECKS;
 	CHECK_REL_PROCEDURE(ambeginscan);
 
@@ -513,7 +517,7 @@ index_getnext_tid(IndexScanDesc scan, ScanDirection direction)
 	SCAN_CHECKS;
 	CHECK_SCAN_PROCEDURE(amgettuple);
 
-	Assert(TransactionIdIsValid(RecentGlobalXmin));
+	Assert(SnapshotSet());
 
 	/*
 	 * The AM's amgettuple proc finds the next index entry matching the scan
@@ -568,6 +572,8 @@ index_fetch_heap(IndexScanDesc scan, TupleTableSlot *slot)
 	bool		all_dead = false;
 	bool		found;
 
+	Assert(SnapshotSet());
+
 	found = table_index_fetch_tuple(scan->xs_heapfetch, &scan->xs_heaptid,
 									scan->xs_snapshot, slot,
 									&scan->xs_heap_continue, &all_dead);
diff --git a/src/backend/catalog/indexing.c b/src/backend/catalog/indexing.c
index d63fcf58cf1..8ba6b3dfa5e 100644
--- a/src/backend/catalog/indexing.c
+++ b/src/backend/catalog/indexing.c
@@ -22,6 +22,7 @@
 #include "catalog/indexing.h"
 #include "executor/executor.h"
 #include "utils/rel.h"
+#include "utils/snapmgr.h"
 
 
 /*
@@ -184,6 +185,8 @@ CatalogTupleInsert(Relation heapRel, HeapTuple tup)
 {
 	CatalogIndexState indstate;
 
+	Assert(SnapshotSet());
+
 	indstate = CatalogOpenIndexes(heapRel);
 
 	simple_heap_insert(heapRel, tup);
@@ -204,6 +207,8 @@ void
 CatalogTupleInsertWithInfo(Relation heapRel, HeapTuple tup,
 						   CatalogIndexState indstate)
 {
+	Assert(SnapshotSet());
+
 	simple_heap_insert(heapRel, tup);
 
 	CatalogIndexInsert(indstate, tup);
@@ -225,6 +230,8 @@ CatalogTupleUpdate(Relation heapRel, ItemPointer otid, HeapTuple tup)
 {
 	CatalogIndexState indstate;
 
+	Assert(SnapshotSet());
+
 	indstate = CatalogOpenIndexes(heapRel);
 
 	simple_heap_update(heapRel, otid, tup);
@@ -245,6 +252,8 @@ void
 CatalogTupleUpdateWithInfo(Relation heapRel, ItemPointer otid, HeapTuple tup,
 						   CatalogIndexState indstate)
 {
+	Assert(SnapshotSet());
+
 	simple_heap_update(heapRel, otid, tup);
 
 	CatalogIndexInsert(indstate, tup);
@@ -268,5 +277,7 @@ CatalogTupleUpdateWithInfo(Relation heapRel, ItemPointer otid, HeapTuple tup,
 void
 CatalogTupleDelete(Relation heapRel, ItemPointer tid)
 {
+	Assert(SnapshotSet());
+
 	simple_heap_delete(heapRel, tid);
 }
diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c
index b5cff157bf6..f7e1665aae6 100644
--- a/src/backend/utils/time/snapmgr.c
+++ b/src/backend/utils/time/snapmgr.c
@@ -857,6 +857,25 @@ ActiveSnapshotSet(void)
 	return ActiveSnapshot != NULL;
 }
 
+/*
+ * Does this transaction have a snapshot.
+ */
+bool
+SnapshotSet(void)
+{
+	/* can't be safe, because somehow xmin is not set */
+	if (!TransactionIdIsValid(MyPgXact->xmin) && HistoricSnapshot == NULL)
+		return false;
+
+	/*
+	 * Can't be safe because no snapshot being registered means invalidation
+	 * processing can change xmin horizon.
+	 */
+	return ActiveSnapshot != NULL ||
+		!pairingheap_is_empty(&RegisteredSnapshots) ||
+		HistoricSnapshot != NULL;
+}
+
 /*
  * RegisterSnapshot
  *		Register a snapshot as being in use by the current resource owner
diff --git a/contrib/amcheck/verify_nbtree.c b/contrib/amcheck/verify_nbtree.c
index ceaaa271680..50a46dca933 100644
--- a/contrib/amcheck/verify_nbtree.c
+++ b/contrib/amcheck/verify_nbtree.c
@@ -412,10 +412,10 @@ bt_check_every_level(Relation rel, Relation heaprel, bool heapkeyspace,
 	Snapshot	snapshot = SnapshotAny;
 
 	/*
-	 * RecentGlobalXmin assertion matches index_getnext_tid().  See note on
 	 * RecentGlobalXmin/B-Tree page deletion.
+	 * This assertion matches the one in index_getnext_tid().  See note on
 	 */
-	Assert(TransactionIdIsValid(RecentGlobalXmin));
+	Assert(SnapshotSet());
 
 	/*
 	 * Initialize state for entire verification operation
-- 
2.25.0.114.g5b0ca878e0

