From 6b11c3146d12f8ce3701d648e3dde0a8d94e4fd8 Mon Sep 17 00:00:00 2001
From: Shlok Kyal <shlok.kyal.oss@gmail.com>
Date: Wed, 10 Sep 2025 15:08:23 +0530
Subject: [PATCH v4] 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                | 53 +++----------------
 .../t/040_standby_failover_slots_sync.pl      | 34 ++++++++++++
 2 files changed, 41 insertions(+), 46 deletions(-)

diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index fd0fdb96d42..f3f2fc8b8fc 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -2745,53 +2745,20 @@ GetSlotInvalidationCauseName(ReplicationSlotInvalidationCause cause)
 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)
-	{
-		/*
-		 * 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);
-
-		foreach_ptr(char, name, *elemlist)
-		{
-			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;
-			}
-		}
-
-		LWLockRelease(ReplicationSlotControlLock);
+	foreach_ptr(char, name, *elemlist)
+	{
+		if (!ReplicationSlotValidateName(name, false, WARNING))
+			return false;
 	}
 
-	return ok;
+	return true;
 }
 
 /*
@@ -2949,12 +2916,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/recovery/t/040_standby_failover_slots_sync.pl b/src/test/recovery/t/040_standby_failover_slots_sync.pl
index 2c61c51e914..ae2119a186d 100644
--- a/src/test/recovery/t/040_standby_failover_slots_sync.pl
+++ b/src/test/recovery/t/040_standby_failover_slots_sync.pl
@@ -966,4 +966,38 @@ $result = $standby1->safe_psql('postgres',
 
 is($result, '1', "data can be consumed using snap_test_slot");
 
+##################################################
+# Test for GUC synchronized standby slots
+##################################################
+
+# Cannot be set synchronized_standby_slots to a reserved slot name
+($result, $stdout, $stderr) = $primary->psql('postgres',
+	"ALTER SYSTEM SET synchronized_standby_slots='pg_conflict_detection'");
+ok( $stderr =~
+	  m/WARNING:  replication slot name "pg_conflict_detection" is reserved/,
+	"Cannot use a reserverd slot name");
+
+# Cannot be set synchronized_standby_slots to slot name with invalid characters
+($result, $stdout, $stderr) = $primary->psql('postgres',
+	"ALTER SYSTEM SET synchronized_standby_slots='invalid*'");
+ok( $stderr =~
+	  m/WARNING:  replication slot name "invalid\*" contains invalid character/,
+	"Cannot use a invalid slot name");
+
+# Can set synchronized_standby_slots to a non-existent slot name
+$primary->safe_psql(
+	'postgres', qq[
+	ALTER SYSTEM SET synchronized_standby_slots='missing';
+	SELECT pg_reload_conf();
+]);
+
+# Parallel worker does not throw error during startup.
+$primary->safe_psql(
+	'postgres', qq[
+	SET min_parallel_table_scan_size TO 0;
+	SET parallel_setup_cost TO 0;
+	SET parallel_tuple_cost TO 0;
+	SELECT * from pg_namespace;
+]);
+
 done_testing();
-- 
2.34.1