diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 7e4d7c0578c668f4177008ecbadf1e2bf30597e5..b73ee4f2d19f877b30806ce5a092caa4b8b4ed88 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -1258,10 +1258,8 @@ index_constraint_create(Relation heapRelation,
/*
* If needed, mark the index as primary and/or deferred in pg_index.
*
- * Note: since this is a transactional update, it's unsafe against
- * concurrent SnapshotNow scans of pg_index. When making an existing
- * index into a constraint, caller must have a table lock that prevents
- * concurrent table updates; if it's less than a full exclusive lock,
+ * Note: When making an existing index into a constraint, caller must
+ * have a table lock that prevents concurrent table updates; otherwise,
* there is a risk that concurrent readers of the table will miss seeing
* this index at all.
*/
@@ -2989,17 +2987,18 @@ validate_index_heapscan(Relation heapRelation,
* index_set_state_flags - adjust pg_index state flags
*
* This is used during CREATE/DROP INDEX CONCURRENTLY to adjust the pg_index
- * flags that denote the index's state. We must use an in-place update of
- * the pg_index tuple, because we do not have exclusive lock on the parent
- * table and so other sessions might concurrently be doing SnapshotNow scans
- * of pg_index to identify the table's indexes. A transactional update would
- * risk somebody not seeing the index at all. Because the update is not
+ * flags that denote the index's state. Because the update is not
* transactional and will not roll back on error, this must only be used as
* the last step in a transaction that has not made any transactional catalog
* updates!
*
* Note that heap_inplace_update does send a cache inval message for the
* tuple, so other sessions will hear about the update as soon as we commit.
+ *
+ * NB: In releases prior to PostgreSQL 9.4, the use of a non-transactional
+ * update here would have been unsafe; now that MVCC rules apply even for
+ * system catalog scans, we could potentially use a transactional update here
+ * instead.
*/
void
index_set_state_flags(Oid indexId, IndexStateFlagsAction action)
@@ -3207,14 +3206,6 @@ reindex_index(Oid indexId, bool skip_constraint_checks)
* be conservative.) In this case advancing the usability horizon is
* appropriate.
*
- * Note that if we have to update the tuple, there is a risk of concurrent
- * transactions not seeing it during their SnapshotNow scans of pg_index.
- * While not especially desirable, this is safe because no such
- * transaction could be trying to update the table (since we have
- * ShareLock on it). The worst case is that someone might transiently
- * fail to use the index for a query --- but it was probably unusable
- * before anyway, if we are updating the tuple.
- *
* Another reason for avoiding unnecessary updates here is that while
* reindexing pg_index itself, we must not try to update tuples in it.
* pg_index's indexes should always have these flags in their clean state,
diff --git a/src/backend/catalog/pg_enum.c b/src/backend/catalog/pg_enum.c
index a7ef8cda59656b950934ba4f2dd2f2da99f72def..88711e985e9fc20bd309a260f629a837111c4444 100644
--- a/src/backend/catalog/pg_enum.c
+++ b/src/backend/catalog/pg_enum.c
@@ -462,30 +462,9 @@ restart:
* Renumber existing enum elements to have sort positions 1..n.
*
* We avoid doing this unless absolutely necessary; in most installations
- * it will never happen. The reason is that updating existing pg_enum
- * entries creates hazards for other backends that are concurrently reading
- * pg_enum with SnapshotNow semantics. A concurrent SnapshotNow scan could
- * see both old and new versions of an updated row as valid, or neither of
- * them, if the commit happens between scanning the two versions. It's
- * also quite likely for a concurrent scan to see an inconsistent set of
- * rows (some members updated, some not).
- *
- * We can avoid these risks by reading pg_enum with an MVCC snapshot
- * instead of SnapshotNow, but that forecloses use of the syscaches.
- * We therefore make the following choices:
- *
- * 1. Any code that is interested in the enumsortorder values MUST read
- * pg_enum with an MVCC snapshot, or else acquire lock on the enum type
- * to prevent concurrent execution of AddEnumLabel(). The risk of
- * seeing inconsistent values of enumsortorder is too high otherwise.
- *
- * 2. Code that is not examining enumsortorder can use a syscache
- * (for example, enum_in and enum_out do so). The worst that can happen
- * is a transient failure to find any valid value of the row. This is
- * judged acceptable in view of the infrequency of use of RenumberEnumType.
- *
- * XXX. Now that we have MVCC catalog scans, the above reasoning is no longer
- * correct. Should we revisit any decisions here?
+ * it will never happen. Before we had MVCC catalog scans, this function
+ * posed various concurrency hazards. It no longer does, but it's still
+ * extra work, so we don't do it unless it's needed.
*/
static void
RenumberEnumType(Relation pg_enum, HeapTuple *existing, int nelems)
diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
index 4519c00e22363fa32b6097543c5fb03af7042621..051b806aa72d8111b17e640d16f88f8f96b92583 100644
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -476,16 +476,6 @@ check_index_is_clusterable(Relation OldHeap, Oid indexOid, bool recheck, LOCKMOD
* mark_index_clustered: mark the specified index as the one clustered on
*
* With indexOid == InvalidOid, will mark all indexes of rel not-clustered.
- *
- * Note: we do transactional updates of the pg_index rows, which are unsafe
- * against concurrent SnapshotNow scans of pg_index. Therefore this is unsafe
- * to execute with less than full exclusive lock on the parent table;
- * otherwise concurrent executions of RelationGetIndexList could miss indexes.
- *
- * XXX: Now that we have MVCC catalog access, SnapshotNow scans of pg_index
- * shouldn't be common enough to worry about. The above comment needs
- * to be updated, and it may be possible to simplify the logic here in other
- * ways also.
*/
void
mark_index_clustered(Relation rel, Oid indexOid, bool is_internal)
diff --git a/src/backend/utils/time/tqual.c b/src/backend/utils/time/tqual.c
index da2ce1825e043cc5bd21a48f31bdd2438df659b5..38bb704a4d8ae4a2a7ab21ac5dfbf749de3e3b5e 100644
--- a/src/backend/utils/time/tqual.c
+++ b/src/backend/utils/time/tqual.c
@@ -30,10 +30,8 @@
*
* HeapTupleSatisfiesMVCC()
* visible to supplied snapshot, excludes current command
- * HeapTupleSatisfiesNow()
- * visible to instant snapshot, excludes current command
* HeapTupleSatisfiesUpdate()
- * like HeapTupleSatisfiesNow(), but with user-supplied command
+ * visible to instant snapshot, with user-supplied command
* counter and more complex result
* HeapTupleSatisfiesSelf()
* visible to instant snapshot and current command
@@ -68,7 +66,6 @@
/* Static variables representing various special snapshot semantics */
-SnapshotData SnapshotNowData = {HeapTupleSatisfiesNow};
SnapshotData SnapshotSelfData = {HeapTupleSatisfiesSelf};
SnapshotData SnapshotAnyData = {HeapTupleSatisfiesAny};
SnapshotData SnapshotToastData = {HeapTupleSatisfiesToast};
@@ -323,212 +320,6 @@ HeapTupleSatisfiesSelf(HeapTuple htup, Snapshot snapshot, Buffer buffer)
return false;
}
-/*
- * HeapTupleSatisfiesNow
- * True iff heap tuple is valid "now".
- *
- * Here, we consider the effects of:
- * all committed transactions (as of the current instant)
- * previous commands of this transaction
- *
- * Note we do _not_ include changes made by the current command. This
- * solves the "Halloween problem" wherein an UPDATE might try to re-update
- * its own output tuples, http://en.wikipedia.org/wiki/Halloween_Problem.
- *
- * Note:
- * Assumes heap tuple is valid.
- *
- * The satisfaction of "now" requires the following:
- *
- * ((Xmin == my-transaction && inserted by the current transaction
- * Cmin < my-command && before this command, and
- * (Xmax is null || the row has not been deleted, or
- * (Xmax == my-transaction && it was deleted by the current transaction
- * Cmax >= my-command))) but not before this command,
- * || or
- * (Xmin is committed && the row was inserted by a committed transaction, and
- * (Xmax is null || the row has not been deleted, or
- * (Xmax == my-transaction && the row is being deleted by this transaction
- * Cmax >= my-command) || but it's not deleted "yet", or
- * (Xmax != my-transaction && the row was deleted by another transaction
- * Xmax is not committed)))) that has not been committed
- *
- */
-bool
-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)
- return false;
-
- /* Used by pre-9.0 binary upgrades */
- if (tuple->t_infomask & HEAP_MOVED_OFF)
- {
- TransactionId xvac = HeapTupleHeaderGetXvac(tuple);
-
- if (TransactionIdIsCurrentTransactionId(xvac))
- return false;
- if (!TransactionIdIsInProgress(xvac))
- {
- if (TransactionIdDidCommit(xvac))
- {
- SetHintBits(tuple, buffer, HEAP_XMIN_INVALID,
- InvalidTransactionId);
- return false;
- }
- SetHintBits(tuple, buffer, HEAP_XMIN_COMMITTED,
- InvalidTransactionId);
- }
- }
- /* Used by pre-9.0 binary upgrades */
- else if (tuple->t_infomask & HEAP_MOVED_IN)
- {
- TransactionId xvac = HeapTupleHeaderGetXvac(tuple);
-
- if (!TransactionIdIsCurrentTransactionId(xvac))
- {
- if (TransactionIdIsInProgress(xvac))
- return false;
- if (TransactionIdDidCommit(xvac))
- SetHintBits(tuple, buffer, HEAP_XMIN_COMMITTED,
- InvalidTransactionId);
- else
- {
- SetHintBits(tuple, buffer, HEAP_XMIN_INVALID,
- InvalidTransactionId);
- return false;
- }
- }
- }
- else if (TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetXmin(tuple)))
- {
- if (HeapTupleHeaderGetCmin(tuple) >= GetCurrentCommandId(false))
- return false; /* inserted after scan started */
-
- if (tuple->t_infomask & HEAP_XMAX_INVALID) /* xid invalid */
- return true;
-
- if (HEAP_XMAX_IS_LOCKED_ONLY(tuple->t_infomask)) /* not deleter */
- return true;
-
- if (tuple->t_infomask & HEAP_XMAX_IS_MULTI)
- {
- TransactionId xmax;
-
- xmax = HeapTupleGetUpdateXid(tuple);
- if (!TransactionIdIsValid(xmax))
- return true;
-
- /* updating subtransaction must have aborted */
- if (!TransactionIdIsCurrentTransactionId(xmax))
- return true;
- else
- return false;
- }
-
- if (!TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetRawXmax(tuple)))
- {
- /* deleting subtransaction must have aborted */
- SetHintBits(tuple, buffer, HEAP_XMAX_INVALID,
- InvalidTransactionId);
- return true;
- }
-
- if (HeapTupleHeaderGetCmax(tuple) >= GetCurrentCommandId(false))
- return true; /* deleted after scan started */
- else
- return false; /* deleted before scan started */
- }
- else if (TransactionIdIsInProgress(HeapTupleHeaderGetXmin(tuple)))
- return false;
- else if (TransactionIdDidCommit(HeapTupleHeaderGetXmin(tuple)))
- SetHintBits(tuple, buffer, HEAP_XMIN_COMMITTED,
- HeapTupleHeaderGetXmin(tuple));
- else
- {
- /* it must have aborted or crashed */
- SetHintBits(tuple, buffer, HEAP_XMIN_INVALID,
- InvalidTransactionId);
- return false;
- }
- }
-
- /* by here, the inserting transaction has committed */
-
- if (tuple->t_infomask & HEAP_XMAX_INVALID) /* xid invalid or aborted */
- return true;
-
- if (tuple->t_infomask & HEAP_XMAX_COMMITTED)
- {
- if (HEAP_XMAX_IS_LOCKED_ONLY(tuple->t_infomask))
- return true;
- return false;
- }
-
- if (tuple->t_infomask & HEAP_XMAX_IS_MULTI)
- {
- TransactionId xmax;
-
- if (HEAP_XMAX_IS_LOCKED_ONLY(tuple->t_infomask))
- return true;
-
- xmax = HeapTupleGetUpdateXid(tuple);
- if (!TransactionIdIsValid(xmax))
- return true;
- if (TransactionIdIsCurrentTransactionId(xmax))
- {
- if (HeapTupleHeaderGetCmax(tuple) >= GetCurrentCommandId(false))
- return true; /* deleted after scan started */
- else
- return false; /* deleted before scan started */
- }
- if (TransactionIdIsInProgress(xmax))
- return true;
- if (TransactionIdDidCommit(xmax))
- return false;
- return true;
- }
-
- if (TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetRawXmax(tuple)))
- {
- if (HEAP_XMAX_IS_LOCKED_ONLY(tuple->t_infomask))
- return true;
- if (HeapTupleHeaderGetCmax(tuple) >= GetCurrentCommandId(false))
- return true; /* deleted after scan started */
- else
- return false; /* deleted before scan started */
- }
-
- if (TransactionIdIsInProgress(HeapTupleHeaderGetRawXmax(tuple)))
- return true;
-
- if (!TransactionIdDidCommit(HeapTupleHeaderGetRawXmax(tuple)))
- {
- /* it must have aborted or crashed */
- SetHintBits(tuple, buffer, HEAP_XMAX_INVALID,
- InvalidTransactionId);
- return true;
- }
-
- /* xmax transaction committed */
-
- if (HEAP_XMAX_IS_LOCKED_ONLY(tuple->t_infomask))
- {
- SetHintBits(tuple, buffer, HEAP_XMAX_INVALID,
- InvalidTransactionId);
- return true;
- }
-
- SetHintBits(tuple, buffer, HEAP_XMAX_COMMITTED,
- HeapTupleHeaderGetRawXmax(tuple));
- return false;
-}
-
/*
* HeapTupleSatisfiesAny
* Dummy "satisfies" routine: any tuple satisfies SnapshotAny.
@@ -614,10 +405,10 @@ HeapTupleSatisfiesToast(HeapTuple htup, Snapshot snapshot,
/*
* HeapTupleSatisfiesUpdate
*
- * Same logic as HeapTupleSatisfiesNow, but returns a more detailed result
- * code, since UPDATE needs to know more than "is it visible?". Also,
- * tuples of my own xact are tested against the passed CommandId not
- * CurrentCommandId.
+ * This function returns a more detailed result code than most of the
+ * functions in this file, since UPDATE needs to know more than "is it
+ * visible?". It also allows for user-supplied CommandId rather than
+ * relying on CurrentCommandId.
*
* The possible return codes are:
*
@@ -1051,10 +842,6 @@ HeapTupleSatisfiesDirty(HeapTuple htup, Snapshot snapshot,
* transactions started after the snapshot was taken
* changes made by the current command
*
- * This is the same as HeapTupleSatisfiesNow, except that transactions that
- * were in progress or as yet unstarted when the snapshot was taken will
- * be treated as uncommitted, even if they have committed by now.
- *
* (Notice, however, that the tuple status hint bits will be updated on the
* basis of the true state of the transaction, even if we then pretend we
* can't see it.)
diff --git a/src/include/utils/tqual.h b/src/include/utils/tqual.h
index 800e366f30b51d1fde19d53548fb09c0b0fb7a25..19f56e4b4d34feb189a462e98f99a925fcdaa052 100644
--- a/src/include/utils/tqual.h
+++ b/src/include/utils/tqual.h
@@ -19,12 +19,10 @@
/* Static variables representing various special snapshot semantics */
-extern PGDLLIMPORT SnapshotData SnapshotNowData;
extern PGDLLIMPORT SnapshotData SnapshotSelfData;
extern PGDLLIMPORT SnapshotData SnapshotAnyData;
extern PGDLLIMPORT SnapshotData SnapshotToastData;
-#define SnapshotNow (&SnapshotNowData)
#define SnapshotSelf (&SnapshotSelfData)
#define SnapshotAny (&SnapshotAnyData)
#define SnapshotToast (&SnapshotToastData)
@@ -67,8 +65,6 @@ typedef enum
/* These are the "satisfies" test routines for the various snapshot types */
extern bool HeapTupleSatisfiesMVCC(HeapTuple htup,
Snapshot snapshot, Buffer buffer);
-extern bool HeapTupleSatisfiesNow(HeapTuple htup,
- Snapshot snapshot, Buffer buffer);
extern bool HeapTupleSatisfiesSelf(HeapTuple htup,
Snapshot snapshot, Buffer buffer);
extern bool HeapTupleSatisfiesAny(HeapTuple htup,