Revert: Allow locking updated tuples in tuple_update() and tuple_delete()

This commit reverts 87985cc925 and 818861eb57 per review by Andres Freund.

Discussion: https://postgr.es/m/20240410165236.rwyrny7ihi4ddxw4%40awork3.anarazel.de

3 files changed, 28 insertions, 68 deletions

diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index 3abe226bd5d..735662dc9df 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -322,22 +322,19 @@ extern void heap_multi_insert(Relation relation, struct TupleTableSlot **slots,
 							  int ntuples, CommandId cid, int options,
 							  BulkInsertState bistate);
 extern TM_Result heap_delete(Relation relation, ItemPointer tid,
-							 CommandId cid, Snapshot crosscheck, int options,
-							 struct TM_FailureData *tmfd, bool changingPart,
-							 TupleTableSlot *oldSlot);
+							 CommandId cid, Snapshot crosscheck, bool wait,
+							 struct TM_FailureData *tmfd, bool changingPart);
 extern void heap_finish_speculative(Relation relation, ItemPointer tid);
 extern void heap_abort_speculative(Relation relation, ItemPointer tid);
 extern TM_Result heap_update(Relation relation, ItemPointer otid,
 							 HeapTuple newtup,
-							 CommandId cid, Snapshot crosscheck, int options,
+							 CommandId cid, Snapshot crosscheck, bool wait,
 							 struct TM_FailureData *tmfd, LockTupleMode *lockmode,
-							 TU_UpdateIndexes *update_indexes,
-							 TupleTableSlot *oldSlot);
-extern TM_Result heap_lock_tuple(Relation relation, ItemPointer tid,
-								 TupleTableSlot *slot,
-								 CommandId cid, LockTupleMode mode,
-								 LockWaitPolicy wait_policy, bool follow_updates,
-								 struct TM_FailureData *tmfd);
+							 TU_UpdateIndexes *update_indexes);
+extern TM_Result heap_lock_tuple(Relation relation, HeapTuple tuple,
+								 CommandId cid, LockTupleMode mode, LockWaitPolicy wait_policy,
+								 bool follow_updates,
+								 Buffer *buffer, struct TM_FailureData *tmfd);
 
 extern void heap_inplace_update(Relation relation, HeapTuple tuple);
 extern bool heap_prepare_freeze_tuple(HeapTupleHeader tuple,
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index 00deb36f5a4..6ef7714d2bc 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -267,15 +267,6 @@ typedef struct TM_IndexDeleteOp
 /* Follow update chain and lock latest version of tuple */
 #define TUPLE_LOCK_FLAG_FIND_LAST_VERSION		(1 << 1)
 
-/*
- * "options" flag bits for table_tuple_update and table_tuple_delete,
- * Wait for any conflicting update to commit/abort */
-#define TABLE_MODIFY_WAIT			0x0001
-/* Fetch the existing tuple into a dedicated slot */
-#define TABLE_MODIFY_FETCH_OLD_TUPLE 0x0002
-/* On concurrent update, follow the update chain and lock latest version of tuple */
-#define TABLE_MODIFY_LOCK_UPDATED	0x0004
-
 
 /* Typedef for callback function for table_index_build_scan */
 typedef void (*IndexBuildCallback) (Relation index,
@@ -545,10 +536,9 @@ typedef struct TableAmRoutine
 								 CommandId cid,
 								 Snapshot snapshot,
 								 Snapshot crosscheck,
-								 int options,
+								 bool wait,
 								 TM_FailureData *tmfd,
-								 bool changingPart,
-								 TupleTableSlot *oldSlot);
+								 bool changingPart);
 
 	/* see table_tuple_update() for reference about parameters */
 	TM_Result	(*tuple_update) (Relation rel,
@@ -557,11 +547,10 @@ typedef struct TableAmRoutine
 								 CommandId cid,
 								 Snapshot snapshot,
 								 Snapshot crosscheck,
-								 int options,
+								 bool wait,
 								 TM_FailureData *tmfd,
 								 LockTupleMode *lockmode,
-								 TU_UpdateIndexes *update_indexes,
-								 TupleTableSlot *oldSlot);
+								 TU_UpdateIndexes *update_indexes);
 
 	/* see table_tuple_lock() for reference about parameters */
 	TM_Result	(*tuple_lock) (Relation rel,
@@ -1456,7 +1445,7 @@ table_multi_insert(Relation rel, TupleTableSlot **slots, int nslots,
 }
 
 /*
- * Delete a tuple (and optionally lock the last tuple version).
+ * Delete a tuple.
  *
  * NB: do not call this directly unless prepared to deal with
  * concurrent-update conditions.  Use simple_table_tuple_delete instead.
@@ -1467,21 +1456,11 @@ table_multi_insert(Relation rel, TupleTableSlot **slots, int nslots,
  *	cid - delete command ID (used for visibility test, and stored into
  *		cmax if successful)
  *	crosscheck - if not InvalidSnapshot, also check tuple against this
- *	options:
- *		If TABLE_MODIFY_WAIT, wait for any conflicting update to commit/abort.
- *		If TABLE_MODIFY_FETCH_OLD_TUPLE option is given, the existing tuple is
- *		fetched into oldSlot when the update is successful.
- *		If TABLE_MODIFY_LOCK_UPDATED option is given and the tuple is
- *		concurrently updated, then the last tuple version is locked and fetched
- *		into oldSlot.
- *
+ *	wait - true if should wait for any conflicting update to commit/abort
  * Output parameters:
  *	tmfd - filled in failure cases (see below)
  *	changingPart - true iff the tuple is being moved to another partition
  *		table due to an update of the partition key. Otherwise, false.
- *	oldSlot - slot to save the deleted or locked tuple. Can be NULL if none of
- *		TABLE_MODIFY_FETCH_OLD_TUPLE or TABLE_MODIFY_LOCK_UPDATED options
- *		is specified.
  *
  * Normal, successful return value is TM_Ok, which means we did actually
  * delete it.  Failure return codes are TM_SelfModified, TM_Updated, and
@@ -1493,18 +1472,16 @@ table_multi_insert(Relation rel, TupleTableSlot **slots, int nslots,
  */
 static inline TM_Result
 table_tuple_delete(Relation rel, ItemPointer tid, CommandId cid,
-				   Snapshot snapshot, Snapshot crosscheck, int options,
-				   TM_FailureData *tmfd, bool changingPart,
-				   TupleTableSlot *oldSlot)
+				   Snapshot snapshot, Snapshot crosscheck, bool wait,
+				   TM_FailureData *tmfd, bool changingPart)
 {
 	return rel->rd_tableam->tuple_delete(rel, tid, cid,
 										 snapshot, crosscheck,
-										 options, tmfd, changingPart,
-										 oldSlot);
+										 wait, tmfd, changingPart);
 }
 
 /*
- * Update a tuple (and optionally lock the last tuple version).
+ * Update a tuple.
  *
  * NB: do not call this directly unless you are prepared to deal with
  * concurrent-update conditions.  Use simple_table_tuple_update instead.
@@ -1516,23 +1493,13 @@ table_tuple_delete(Relation rel, ItemPointer tid, CommandId cid,
  *	cid - update command ID (used for visibility test, and stored into
  *		cmax/cmin if successful)
  *	crosscheck - if not InvalidSnapshot, also check old tuple against this
- *	options:
- *		If TABLE_MODIFY_WAIT, wait for any conflicting update to commit/abort.
- *		If TABLE_MODIFY_FETCH_OLD_TUPLE option is given, the existing tuple is
- *		fetched into oldSlot when the update is successful.
- *		If TABLE_MODIFY_LOCK_UPDATED option is given and the tuple is
- *		concurrently updated, then the last tuple version is locked and fetched
- *		into oldSlot.
- *
+ *	wait - true if should wait for any conflicting update to commit/abort
  * Output parameters:
  *	tmfd - filled in failure cases (see below)
  *	lockmode - filled with lock mode acquired on tuple
  *  update_indexes - in success cases this is set to true if new index entries
  *		are required for this tuple
- *	oldSlot - slot to save the deleted or locked tuple. Can be NULL if none of
- *		TABLE_MODIFY_FETCH_OLD_TUPLE or TABLE_MODIFY_LOCK_UPDATED options
- *		is specified.
-
+ *
  * Normal, successful return value is TM_Ok, which means we did actually
  * update it.  Failure return codes are TM_SelfModified, TM_Updated, and
  * TM_BeingModified (the last only possible if wait == false).
@@ -1550,15 +1517,13 @@ table_tuple_delete(Relation rel, ItemPointer tid, CommandId cid,
 static inline TM_Result
 table_tuple_update(Relation rel, ItemPointer otid, TupleTableSlot *slot,
 				   CommandId cid, Snapshot snapshot, Snapshot crosscheck,
-				   int options, TM_FailureData *tmfd, LockTupleMode *lockmode,
-				   TU_UpdateIndexes *update_indexes,
-				   TupleTableSlot *oldSlot)
+				   bool wait, TM_FailureData *tmfd, LockTupleMode *lockmode,
+				   TU_UpdateIndexes *update_indexes)
 {
 	return rel->rd_tableam->tuple_update(rel, otid, slot,
 										 cid, snapshot, crosscheck,
-										 options, tmfd,
-										 lockmode, update_indexes,
-										 oldSlot);
+										 wait, tmfd,
+										 lockmode, update_indexes);
 }
 
 /*
@@ -2079,12 +2044,10 @@ table_scan_sample_next_tuple(TableScanDesc scan,
 
 extern void simple_table_tuple_insert(Relation rel, TupleTableSlot *slot);
 extern void simple_table_tuple_delete(Relation rel, ItemPointer tid,
-									  Snapshot snapshot,
-									  TupleTableSlot *oldSlot);
+									  Snapshot snapshot);
 extern void simple_table_tuple_update(Relation rel, ItemPointer otid,
 									  TupleTableSlot *slot, Snapshot snapshot,
-									  TU_UpdateIndexes *update_indexes,
-									  TupleTableSlot *oldSlot);
+									  TU_UpdateIndexes *update_indexes);
 
 
 /* ----------------------------------------------------------------------------
diff --git a/src/include/commands/trigger.h b/src/include/commands/trigger.h
index cb968d03ecd..8a5a9fe6422 100644
--- a/src/include/commands/trigger.h
+++ b/src/include/commands/trigger.h
@@ -216,8 +216,8 @@ extern bool ExecBRDeleteTriggers(EState *estate,
 								 TM_FailureData *tmfd);
 extern void ExecARDeleteTriggers(EState *estate,
 								 ResultRelInfo *relinfo,
+								 ItemPointer tupleid,
 								 HeapTuple fdw_trigtuple,
-								 TupleTableSlot *slot,
 								 TransitionCaptureState *transition_capture,
 								 bool is_crosspart_update);
 extern bool ExecIRDeleteTriggers(EState *estate,
@@ -240,8 +240,8 @@ extern void ExecARUpdateTriggers(EState *estate,
 								 ResultRelInfo *relinfo,
 								 ResultRelInfo *src_partinfo,
 								 ResultRelInfo *dst_partinfo,
+								 ItemPointer tupleid,
 								 HeapTuple fdw_trigtuple,
-								 TupleTableSlot *oldslot,
 								 TupleTableSlot *newslot,
 								 List *recheckIndexes,
 								 TransitionCaptureState *transition_capture,