[PATCH v35 10/21] Rename confusing function names

Kyotaro Horiguchi <horikyota.ntt@gmail.com>

From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
To:
Date: 2024-10-25T04:02:22Z
Lists: pgsql-hackers
This is the final step in a series of three commits to modify
pendingDeletes.

The pendingDeletes system now handles commit-time processing
only. Previously, its structures and functions managed both commit and
abort cases, so removing abort-time processing alone could cause
confusion. Therefore, update the function names to clarify their
commit-time purpose and remove unnecessary safeguards.
---
 src/backend/access/transam/twophase.c |  2 +-
 src/backend/access/transam/xact.c     |  2 +-
 src/backend/catalog/storage.c         | 44 +++++++++------------------
 src/backend/commands/tablecmds.c      |  2 +-
 src/include/catalog/storage.h         |  4 +--
 5 files changed, 19 insertions(+), 35 deletions(-)

diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
index d3dac67d8cd..ef94aea4ee9 100644
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -1113,7 +1113,7 @@ StartPrepare(GlobalTransaction gxact)
 	hdr.prepared_at = gxact->prepared_at;
 	hdr.owner = gxact->owner;
 	hdr.nsubxacts = xactGetCommittedChildren(&children);
-	hdr.ncommitrels = smgrGetPendingDeletes(true, &commitrels);
+	hdr.ncommitrels = smgrGetCommitPendingDeletes(&commitrels);
 	hdr.ncommitstats =
 		pgstat_get_transactional_drops(true, &commitstats);
 	hdr.nabortstats =
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 1a691c7a30e..33e99bd31ae 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -1340,7 +1340,7 @@ RecordTransactionCommit(void)
 		LogLogicalInvalidations();
 
 	/* Get data needed for commit record */
-	nrels = smgrGetPendingDeletes(true, &rels);
+	nrels = smgrGetCommitPendingDeletes(&rels);
 	nchildren = xactGetCommittedChildren(&children);
 	ndroppedstats = pgstat_get_transactional_drops(true, &droppedstats);
 	if (XLogStandbyInfoActive())
diff --git a/src/backend/catalog/storage.c b/src/backend/catalog/storage.c
index db9cdad25f8..043afa47ced 100644
--- a/src/backend/catalog/storage.c
+++ b/src/backend/catalog/storage.c
@@ -252,45 +252,31 @@ RelationDropStorage(Relation rel)
 }
 
 /*
- * RelationPreserveStorage
+ * RelationPreserveStorageOnCommit
  *		Mark a relation as not to be deleted after all.
  *
- * We need this function because relation mapping changes are committed
- * separately from commit of the whole transaction, so it's still possible
- * for the transaction to abort after the mapping update is done.
- * When a new physical relation is installed in the map, it would be
- * scheduled for delete-on-abort, so we'd delete it, and be in trouble.
- * The relation mapper fixes this by telling us to not delete such relations
- * after all as part of its commit.
+ * This function cancels registered delete-on-commit actions for storage files,
+ * allowing reuse of an existing index build during ALTER TABLE.
  *
- * We also use this to reuse an old build of an index during ALTER TABLE, this
- * time removing the delete-at-commit entry.
+ * Historical note:
+ *   The only caller in abort paths was write_relmapper_file(), intended to 
+ *   preserve committed storage files for mapped relations if outer 
+ *   transactions aborted. However, this case has not occurred in over a decade 
+ *   and is unlikely to be needed in the future. Maintaining the ability for 
+ *   subtransaction-committed storage files to persist after a top-level 
+ *   transaction aborts has added unnecessary complexity and inefficiency to 
+ *   the UNDO log system. Therefore, this feature has been removed, and the 
+ *   function has been renamed.
  *
  * No-op if the relation is not among those scheduled for deletion.
  */
 void
-RelationPreserveStorage(RelFileLocator rlocator, bool atCommit)
+RelationPreserveStorageOnCommit(RelFileLocator rlocator)
 {
 	PendingRelDelete *pending;
 	PendingRelDelete *prev;
 	PendingRelDelete *next;
 
-	/*
-	 * There is no caller that passes false for atCommit.
-	 *
-	 * The only caller that used to pass false for atCommit was
-	 * write_relmapper_file(), which intended to preserve committed storage
-	 * files for mapped relations if outer transactions aborted. However, this
-	 * has not occurred for more than ten years, and it is unlikely to be
-	 * needed in the future. The code to let storage files committed in
-	 * subtransactions survive after the top transaction aborts makes the UNDO
-	 * log system overly complex and inefficient. Therefore, this feature has
-	 * been removed.  The function signature is left unchanged to make this
-	 * change less invasive and to prevent the function from being mistakenly
-	 * called during transaction aborts.
-	 */
-	Assert (atCommit);
-
 	prev = NULL;
 	for (pending = pendingDeletes; pending != NULL; pending = next)
 	{
@@ -883,15 +869,13 @@ smgrDoPendingSyncs(bool isCommit, bool isParallelWorker)
  * by upper-level transactions.
  */
 int
-smgrGetPendingDeletes(bool forCommit, RelFileLocator **ptr)
+smgrGetCommitPendingDeletes(RelFileLocator **ptr)
 {
 	int			nestLevel = GetCurrentTransactionNestLevel();
 	int			nrels;
 	RelFileLocator *rptr;
 	PendingRelDelete *pending;
 
-	Assert(forCommit);
-
 	nrels = 0;
 	for (pending = pendingDeletes; pending != NULL; pending = pending->next)
 	{
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 56c9d61aa21..251aea55d24 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -9197,7 +9197,7 @@ ATExecAddIndex(AlteredTableInfo *tab, Relation rel,
 
 		irel->rd_createSubid = stmt->oldCreateSubid;
 		irel->rd_firstRelfilelocatorSubid = stmt->oldFirstRelfilelocatorSubid;
-		RelationPreserveStorage(irel->rd_locator, true);
+		RelationPreserveStorageOnCommit(irel->rd_locator);
 		index_close(irel, NoLock);
 	}
 
diff --git a/src/include/catalog/storage.h b/src/include/catalog/storage.h
index 3451d6ac80c..19b02d84a5f 100644
--- a/src/include/catalog/storage.h
+++ b/src/include/catalog/storage.h
@@ -28,7 +28,7 @@ extern SMgrRelation RelationCreateStorage(RelFileLocator rlocator,
 extern void RelationCreateFork(SMgrRelation srel, ForkNumber forkNum,
 							   bool wal_log, bool undo_log);
 extern void RelationDropStorage(Relation rel);
-extern void RelationPreserveStorage(RelFileLocator rlocator, bool atCommit);
+extern void RelationPreserveStorageOnCommit(RelFileLocator rlocator);
 extern void RelationPreTruncate(Relation rel);
 extern void RelationTruncate(Relation rel, BlockNumber nblocks);
 extern void RelationCopyStorage(SMgrRelation src, SMgrRelation dst,
@@ -44,7 +44,7 @@ extern void RestorePendingSyncs(char *startAddress);
  */
 extern void smgrDoPendingDeletes(bool isCommit);
 extern void smgrDoPendingSyncs(bool isCommit, bool isParallelWorker);
-extern int	smgrGetPendingDeletes(bool forCommit, RelFileLocator **ptr);
+extern int	smgrGetCommitPendingDeletes(RelFileLocator **ptr);
 extern void AtSubCommit_smgr(void);
 extern void AtSubAbort_smgr(void);
 extern void PostPrepare_smgr(void);
-- 
2.43.5


----Next_Part(Thu_Oct_31_17_01_30_2024_620)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename="v35-0011-new-indexam-bit-for-unlogged-storage-compatibili.patch"