From 80f9ee78ad72675fb1e2e1d28f1cfe75a1d1e18b Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Date: Tue, 10 Sep 2024 17:51:56 +0000
Subject: [PATCH v2] Disallow altering invalidated replication slots

ALTER_REPLICATION_SLOT for invalid replication slots should not be
allowed because there is no way to get back the invalidated
(logical) slot to working. This commit adds an error.

Author: Bharath Rupireddy
Reviewed-by: Peter Smith, shveta malik
Discussion: https://www.postgresql.org/message-id/CAA4eK1%2Bszcosq0nS109mMSxPWyNT1Q%3DUNYCJgXKYuCceaPS%2BhA%40mail.gmail.com
---
 src/backend/replication/slot.c                      |  7 +++++++
 src/test/recovery/t/035_standby_logical_decoding.pl | 12 +++++++++++-
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index 0a03776156..6828100cf1 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -820,6 +820,13 @@ ReplicationSlotAlter(const char *name, const bool *failover,
 				errmsg("cannot use %s with a physical replication slot",
 					   "ALTER_REPLICATION_SLOT"));
 
+	if (MyReplicationSlot->data.invalidated != RS_INVAL_NONE)
+		ereport(ERROR,
+				errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+				errmsg("cannot alter invalid replication slot \"%s\"", name),
+				errdetail("This replication slot has been invalidated due to \"%s\".",
+						  SlotInvalidationCauses[MyReplicationSlot->data.invalidated]));
+
 	if (RecoveryInProgress())
 	{
 		/*
diff --git a/src/test/recovery/t/035_standby_logical_decoding.pl b/src/test/recovery/t/035_standby_logical_decoding.pl
index 4185b803b2..d658fdf417 100644
--- a/src/test/recovery/t/035_standby_logical_decoding.pl
+++ b/src/test/recovery/t/035_standby_logical_decoding.pl
@@ -495,7 +495,8 @@ $node_subscriber->stop;
 # Scenario 1: hot_standby_feedback off and vacuum FULL
 #
 # In passing, ensure that replication slot stats are not removed when the
-# active slot is invalidated.
+# active slot is invalidated, and check that an error occurs when
+# attempting to alter the invalid slot.
 ##################################################
 
 # One way to produce recovery conflict is to create/drop a relation and
@@ -526,6 +527,15 @@ check_for_invalidation('vacuum_full_', 1, 'with vacuum FULL on pg_class');
 # Verify reason for conflict is 'rows_removed' in pg_replication_slots
 check_slots_conflict_reason('vacuum_full_', 'rows_removed');
 
+# Attempting to alter an invalidated slot should result in an error
+($result, $stdout, $stderr) = $node_standby->psql(
+    'postgres',
+    qq[ALTER_REPLICATION_SLOT vacuum_full_inactiveslot (failover);],
+    replication => 'database');
+ok($stderr =~ /ERROR:  cannot alter invalid replication slot "vacuum_full_inactiveslot"/ &&
+   $stderr =~ /DETAIL:  This replication slot has been invalidated due to "rows_removed"./,
+    "invalidated slot cannot be altered");
+
 # Ensure that replication slot stats are not removed after invalidation.
 is( $node_standby->safe_psql(
 		'testdb',
-- 
2.43.0

