v4-0001-Don-t-allow-to-set-replication-slot_name-as.patch

application/octet-stream

Filename: v4-0001-Don-t-allow-to-set-replication-slot_name-as.patch
Type: application/octet-stream
Part: 0
Message: Re: Why ALTER SUBSCRIPTION ... SET (slot_name='none') requires subscription disabled?

Patch

Format: format-patch
Series: patch v4-0001
Subject: Don't allow to set replication slot_name as ''.
File+
src/backend/commands/subscriptioncmds.c 2 0
src/test/regress/expected/subscription.out 3 0
src/test/regress/sql/subscription.sql 3 0
From b139ce3193a0b3600ab9e055485fbf92b4e317c0 Mon Sep 17 00:00:00 2001
From: Amit Kapila <akapila@postgresql.org>
Date: Fri, 16 Jul 2021 10:31:04 +0530
Subject: [PATCH v4 1/2] 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
---
 src/backend/commands/subscriptioncmds.c    | 2 ++
 src/test/regress/expected/subscription.out | 3 +++
 src/test/regress/sql/subscription.sql      | 3 +++
 3 files changed, 8 insertions(+)

diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index 5f834a9c30..fb0b0058f2 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -180,6 +180,8 @@ parse_subscription_options(List *stmt_options, bits32 supported_opts, SubOpts *o
 			/* Setting slot_name = NONE is treated as no slot name. */
 			if (strcmp(opts->slot_name, "none") == 0)
 				opts->slot_name = NULL;
+			else
+				ReplicationSlotValidateName(opts->slot_name, ERROR);
 		}
 		else if (IsSet(supported_opts, SUBOPT_COPY_DATA) &&
 				 strcmp(defel->defname, "copy_data") == 0)
diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out
index ad6b4e4bd3..67f92b3878 100644
--- a/src/test/regress/expected/subscription.out
+++ b/src/test/regress/expected/subscription.out
@@ -86,6 +86,9 @@ ALTER SUBSCRIPTION regress_testsub SET PUBLICATION testpub2, testpub3 WITH (refr
 ALTER SUBSCRIPTION regress_testsub CONNECTION 'dbname=regress_doesnotexist2';
 ALTER SUBSCRIPTION regress_testsub SET (slot_name = 'newname');
 -- fail
+ALTER SUBSCRIPTION regress_testsub SET (slot_name = '');
+ERROR:  replication slot name "" is too short
+-- fail
 ALTER SUBSCRIPTION regress_doesnotexist CONNECTION 'dbname=regress_doesnotexist2';
 ERROR:  subscription "regress_doesnotexist" does not exist
 ALTER SUBSCRIPTION regress_testsub SET (create_slot = false);
diff --git a/src/test/regress/sql/subscription.sql b/src/test/regress/sql/subscription.sql
index b732871407..88743ab33b 100644
--- a/src/test/regress/sql/subscription.sql
+++ b/src/test/regress/sql/subscription.sql
@@ -65,6 +65,9 @@ ALTER SUBSCRIPTION regress_testsub SET PUBLICATION testpub2, testpub3 WITH (refr
 ALTER SUBSCRIPTION regress_testsub CONNECTION 'dbname=regress_doesnotexist2';
 ALTER SUBSCRIPTION regress_testsub SET (slot_name = 'newname');
 
+-- fail
+ALTER SUBSCRIPTION regress_testsub SET (slot_name = '');
+
 -- fail
 ALTER SUBSCRIPTION regress_doesnotexist CONNECTION 'dbname=regress_doesnotexist2';
 ALTER SUBSCRIPTION regress_testsub SET (create_slot = false);
-- 
2.28.0.windows.1