v9_REL_17-0001-Remove-the-validation-from-the-GUC-check-h.txt

From 9aca60e26cb22c13d6876d7868ec0e6fd3c12ff6 Mon Sep 17 00:00:00 2001
From: Shlok Kyal <shlok.kyal.oss@gmail.com>
Date: Thu, 23 Oct 2025 11:09:47 +0530
Subject: [PATCH v9_REL_17] Remove the validation from the GUC check hook and
 add parsing check

The validation in check_synchronized_standby_slots cannot be run
in postmaster and hence can result inconsistent values in some
backends that inherit the value from postmaster and those
that are started newly. Also, this validation results in parallel
workers fail to with error. This causes all the commands run by
parallel workers to fail, which seems unnecesary. This validation
already happens in StandbySlotsHaveCaughtup() where this GUC is
used, so it can be removed from the GUC check. Also added a
parsing check for the slot names specified in this GUC.
---
 src/backend/replication/slot.c    | 59 +++++++++----------------------
 src/test/regress/expected/guc.out | 10 ++++++
 src/test/regress/sql/guc.sql      | 10 ++++++
 3 files changed, 36 insertions(+), 43 deletions(-)

diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index 80b8abde3a2..4bc2be33396 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -2468,53 +2468,32 @@ GetSlotInvalidationCause(const char *invalidation_reason)
 static bool
 validate_sync_standby_slots(char *rawname, List **elemlist)
 {
-	bool		ok;
-
 	/* Verify syntax and parse string into a list of identifiers */
-	ok = SplitIdentifierString(rawname, ',', elemlist);
-
-	if (!ok)
+	if (!SplitIdentifierString(rawname, ',', elemlist))
 	{
 		GUC_check_errdetail("List syntax is invalid.");
+		return false;
 	}
-	else if (MyProc)
+
+	/* Iterate the list to validate each slot name */
+	foreach_ptr(char, name, *elemlist)
 	{
-		/*
-		 * Check that each specified slot exist and is physical.
-		 *
-		 * Because we need an LWLock, we cannot do this on processes without a
-		 * PGPROC, so we skip it there; but see comments in
-		 * StandbySlotsHaveCaughtup() as to why that's not a problem.
-		 */
-		LWLockAcquire(ReplicationSlotControlLock, LW_SHARED);
+		int			err_code;
+		char	   *err_msg = NULL;
+		char	   *err_hint = NULL;
 
-		foreach_ptr(char, name, *elemlist)
+		if (!ReplicationSlotValidateNameInternal(name, &err_code, &err_msg,
+												 &err_hint))
 		{
-			ReplicationSlot *slot;
-
-			slot = SearchNamedReplicationSlot(name, false);
-
-			if (!slot)
-			{
-				GUC_check_errdetail("replication slot \"%s\" does not exist",
-									name);
-				ok = false;
-				break;
-			}
-
-			if (!SlotIsPhysical(slot))
-			{
-				GUC_check_errdetail("\"%s\" is not a physical replication slot",
-									name);
-				ok = false;
-				break;
-			}
+			GUC_check_errcode(err_code);
+			GUC_check_errdetail("%s", err_msg);
+			if (err_hint != NULL)
+				GUC_check_errhint("%s", err_hint);
+			return false;
 		}
-
-		LWLockRelease(ReplicationSlotControlLock);
 	}
 
-	return ok;
+	return true;
 }
 
 /*
@@ -2672,12 +2651,6 @@ StandbySlotsHaveCaughtup(XLogRecPtr wait_for_lsn, int elevel)
 		/*
 		 * If a slot name provided in synchronized_standby_slots does not
 		 * exist, report a message and exit the loop.
-		 *
-		 * Though validate_sync_standby_slots (the GUC check_hook) tries to
-		 * avoid this, it can nonetheless happen because the user can specify
-		 * a nonexistent slot name before server startup. That function cannot
-		 * validate such a slot during startup, as ReplicationSlotCtl is not
-		 * initialized by then.  Also, the user might have dropped one slot.
 		 */
 		if (!slot)
 		{
diff --git a/src/test/regress/expected/guc.out b/src/test/regress/expected/guc.out
index 455b6d6c0ce..f03e722a441 100644
--- a/src/test/regress/expected/guc.out
+++ b/src/test/regress/expected/guc.out
@@ -888,3 +888,13 @@ SELECT name FROM tab_settings_flags
 (0 rows)
 
 DROP TABLE tab_settings_flags;
+-- Test for GUC synchronized standby slots.
+-- Cannot set synchronized_standby_slots to an invalid slot name.
+ALTER SYSTEM SET synchronized_standby_slots='invalid*';
+ERROR:  invalid value for parameter "synchronized_standby_slots": "invalid*"
+DETAIL:  replication slot name "invalid*" contains invalid character
+HINT:  Replication slot names may only contain lower case letters, numbers, and the underscore character.
+-- Can set synchronized_standby_slots to a non-existent slot name.
+ALTER SYSTEM SET synchronized_standby_slots='missing';
+-- Reset the GUC.
+ALTER SYSTEM RESET synchronized_standby_slots;
diff --git a/src/test/regress/sql/guc.sql b/src/test/regress/sql/guc.sql
index dc79761955d..17ed240bf55 100644
--- a/src/test/regress/sql/guc.sql
+++ b/src/test/regress/sql/guc.sql
@@ -353,3 +353,13 @@ SELECT name FROM tab_settings_flags
   WHERE no_reset AND NOT no_reset_all
   ORDER BY 1;
 DROP TABLE tab_settings_flags;
+
+-- Test for GUC synchronized standby slots.
+-- Cannot set synchronized_standby_slots to an invalid slot name.
+ALTER SYSTEM SET synchronized_standby_slots='invalid*';
+
+-- Can set synchronized_standby_slots to a non-existent slot name.
+ALTER SYSTEM SET synchronized_standby_slots='missing';
+
+-- Reset the GUC.
+ALTER SYSTEM RESET synchronized_standby_slots;
-- 
2.34.1