Hi, hackers
The documentation [1] says:
When dropping a subscription that is associated with a replication slot on the
remote host (the normal state), DROP SUBSCRIPTION will connect to the remote
host and try to drop the replication slot as part of its operation. This is
necessary so that the resources allocated for the subscription on the remote
host are released. If this fails, either because the remote host is not
reachable or because the remote replication slot cannot be dropped or does not
exist or never existed, the DROP SUBSCRIPTION command will fail. To proceed in
this situation, disassociate the subscription from the replication slot by
executing ALTER SUBSCRIPTION ... SET (slot_name = NONE).
However, when I try this, it complains the subscription is enabled, this command
requires the subscription disabled. Why we need this limitation?
In src/backend/commands/subscriptioncmds.c:
if (IsSet(opts.specified_opts, SUBOPT_SLOT_NAME))
{
if (sub->enabled && !opts.slot_name)
ereport(ERROR,
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
errmsg("cannot set %s for enabled subscription",
"slot_name = NONE")));
if (opts.slot_name)
values[Anum_pg_subscription_subslotname - 1] =
DirectFunctionCall1(namein,
CStringGetDatum(opts.slot_name));
else
nulls[Anum_pg_subscription_subslotname - 1] = true;
replaces[Anum_pg_subscription_subslotname - 1] = true;
}
OTOH, when I execute ALTER SUBSCRIPTION ... SET (slot_name=''), it doesn't
complain. However,
SELECT select pg_create_logical_replication_slot('', 'pgoutput') complains slot
name is too
short. Although, the slot will be created at publisher, and validate the slot
name, IMO, we
can also validate the slot_name in parse_subscription_options() to get the
error early.
Attached fixes it. Any thoughts?
[1] https://www.postgresql.org/docs/current/sql-dropsubscription.html
--
Regrads,
Japin Li.
ChengDu WenWu Information Technology Co.,Ltd.
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index eb88d877a5..fa07203b03 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -173,6 +173,8 @@ parse_subscription_options(List *stmt_options, bits32 supported_opts, SubOpts *o
opts->specified_opts |= SUBOPT_SLOT_NAME;
opts->slot_name = defGetString(defel);
+ ReplicationSlotValidateName(opts->slot_name, ERROR);
+
/* Setting slot_name = NONE is treated as no slot name. */
if (strcmp(opts->slot_name, "none") == 0)
opts->slot_name = NULL;
diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out
index 57f7dd9b0a..d1ff3531a4 100644
--- a/src/test/regress/expected/subscription.out
+++ b/src/test/regress/expected/subscription.out
@@ -85,6 +85,8 @@ ERROR: invalid connection string syntax: missing "=" after "foobar" in connecti
ALTER SUBSCRIPTION regress_testsub SET PUBLICATION testpub2, testpub3 WITH (refresh = false);
ALTER SUBSCRIPTION regress_testsub CONNECTION 'dbname=regress_doesnotexist2';
ALTER SUBSCRIPTION regress_testsub SET (slot_name = 'newname');
+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
diff --git a/src/test/regress/sql/subscription.sql b/src/test/regress/sql/subscription.sql
index 308c098c14..1500a96e2b 100644
--- a/src/test/regress/sql/subscription.sql
+++ b/src/test/regress/sql/subscription.sql
@@ -64,6 +64,7 @@ ALTER SUBSCRIPTION regress_testsub CONNECTION 'foobar';
ALTER SUBSCRIPTION regress_testsub SET PUBLICATION testpub2, testpub3 WITH (refresh = false);
ALTER SUBSCRIPTION regress_testsub CONNECTION 'dbname=regress_doesnotexist2';
ALTER SUBSCRIPTION regress_testsub SET (slot_name = 'newname');
+ALTER SUBSCRIPTION regress_testsub SET (slot_name = '');
-- fail
ALTER SUBSCRIPTION regress_doesnotexist CONNECTION 'dbname=regress_doesnotexist2';
