From: Amit Kapila <akapila@postgresql.org>
Date: Mon, 19 Jul 2021 05:53:35 +0000 (+0530)
Subject: Don't allow to set replication slot_name as ''.
X-Git-Tag: REL_11_13~35
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=eb158e74afafd052339729efd179dbf74ecd9683;p=thirdparty%2Fpostgresql.git

Don't allow to set replication slot_name as ''.

We don't allow to create replication slot_name as an empty string ('') via
SQL API pg_create_logical_replication_slot() but it is allowed to be set
via Alter Subscription command. This will lead to apply worker repeatedly
keep trying to stream data via slot_name '' and the user is not allowed to
create the slot with that name.

Author: Japin Li
Reviewed-By: Ranier Vilela, Amit Kapila
Backpatch-through: 10, where it was introduced
Discussion: https://postgr.es/m/MEYP282MB1669CBD98E721C77CA696499B61A9@MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM
---

diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index 7a56fa44953..7c981212e75 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -39,6 +39,7 @@
 
 #include "replication/logicallauncher.h"
 #include "replication/origin.h"
+#include "replication/slot.h"
 #include "replication/walreceiver.h"
 #include "replication/walsender.h"
 #include "replication/worker_internal.h"
@@ -145,6 +146,8 @@ parse_subscription_options(List *options, bool *connect, bool *enabled_given,
 			/* Setting slot_name = NONE is treated as no slot name. */
 			if (strcmp(*slot_name, "none") == 0)
 				*slot_name = NULL;
+			else
+				ReplicationSlotValidateName(*slot_name, ERROR);
 		}
 		else if (strcmp(defel->defname, "copy_data") == 0 && copy_data)
 		{
diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out
index 9aa0b702d79..4b635e4039e 100644
--- a/src/test/regress/expected/subscription.out
+++ b/src/test/regress/expected/subscription.out
@@ -86,6 +86,9 @@ ALTER SUBSCRIPTION testsub SET PUBLICATION testpub2, testpub3 WITH (refresh = fa
 ALTER SUBSCRIPTION testsub CONNECTION 'dbname=doesnotexist2';
 ALTER SUBSCRIPTION testsub SET (slot_name = 'newname');
 -- fail
+ALTER SUBSCRIPTION testsub SET (slot_name = '');
+ERROR:  replication slot name "" is too short
+-- fail
 ALTER SUBSCRIPTION doesnotexist CONNECTION 'dbname=doesnotexist2';
 ERROR:  subscription "doesnotexist" does not exist
 ALTER SUBSCRIPTION testsub SET (create_slot = false);
diff --git a/src/test/regress/sql/subscription.sql b/src/test/regress/sql/subscription.sql
index 5efff08c6d0..d0dfaf5ba3e 100644
--- a/src/test/regress/sql/subscription.sql
+++ b/src/test/regress/sql/subscription.sql
@@ -65,6 +65,9 @@ ALTER SUBSCRIPTION testsub SET PUBLICATION testpub2, testpub3 WITH (refresh = fa
 ALTER SUBSCRIPTION testsub CONNECTION 'dbname=doesnotexist2';
 ALTER SUBSCRIPTION testsub SET (slot_name = 'newname');
 
+-- fail
+ALTER SUBSCRIPTION testsub SET (slot_name = '');
+
 -- fail
 ALTER SUBSCRIPTION doesnotexist CONNECTION 'dbname=doesnotexist2';
 ALTER SUBSCRIPTION testsub SET (create_slot = false);