For inplace update durability, make heap_update() callers wait.

The previous commit fixed some ways of losing an inplace update.  It
remained possible to lose one when a backend working toward a
heap_update() copied a tuple into memory just before inplace update of
that tuple.  In catalogs eligible for inplace update, use LOCKTAG_TUPLE
to govern admission to the steps of copying an old tuple, modifying it,
and issuing heap_update().  This includes MERGE commands.  To avoid
changing most of the pg_class DDL, don't require LOCKTAG_TUPLE when
holding a relation lock sufficient to exclude inplace updaters.
Back-patch to v12 (all supported versions).  In v13 and v12, "UPDATE
pg_class" or "UPDATE pg_database" can still lose an inplace update.  The
v14+ UPDATE fix needs commit 86dc90056dfdbd9d1b891718d2e5614e3e432f35,
and it wasn't worth reimplementing that fix without such infrastructure.

Reviewed by Nitin Motiani and (in earlier versions) Heikki Linnakangas.

Discussion: https://postgr.es/m/20231027214946.79.nmisch@google.com

1 files changed, 26 insertions, 0 deletions

diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index 1242c1da3f5..d25074181f9 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -1282,6 +1282,7 @@ ExecUpdate(ModifyTableState *mtstate,
 	}
 	else
 	{
+		ItemPointerData lockedtid PG_USED_FOR_ASSERTS_ONLY;
 		LockTupleMode lockmode;
 		bool		partition_constraint_failed;
 		bool		update_indexes;
@@ -1473,6 +1474,26 @@ lreplace:
 			ExecConstraints(resultRelInfo, slot, estate);
 
 		/*
+		 * We lack the infrastructure to follow rules in README.tuplock
+		 * section "Locking to write inplace-updated tables".  Specifically,
+		 * we lack infrastructure to lock tupleid before this file's
+		 * ExecProcNode() call fetches the tuple's old columns.  Just take a
+		 * lock that silences check_lock_if_inplace_updateable_rel().  This
+		 * doesn't actually protect inplace updates like those rules intend,
+		 * so we may lose an inplace update that overlaps a superuser running
+		 * "UPDATE pg_class" or "UPDATE pg_database".
+		 */
+#ifdef USE_ASSERT_CHECKING
+		if (IsInplaceUpdateRelation(resultRelationDesc))
+		{
+			lockedtid = *tupleid;
+			LockTuple(resultRelationDesc, &lockedtid, InplaceUpdateTupleLock);
+		}
+		else
+			ItemPointerSetInvalid(&lockedtid);
+#endif
+
+		/*
 		 * replace the heap tuple
 		 *
 		 * Note: if es_crosscheck_snapshot isn't InvalidSnapshot, we check
@@ -1488,6 +1509,11 @@ lreplace:
 									true /* wait for commit */ ,
 									&tmfd, &lockmode, &update_indexes);
 
+#ifdef USE_ASSERT_CHECKING
+		if (ItemPointerIsValid(&lockedtid))
+			UnlockTuple(resultRelationDesc, &lockedtid, InplaceUpdateTupleLock);
+#endif
+
 		switch (result)
 		{
 			case TM_SelfModified: