On 23 May 2017 at 07:26, Euler Taveira <eu...@timbira.com.br> wrote: > > ReplicationSlotValidateName() should be called in parse_subscription_options() to avoid a pilot error. > IMHO we should prevent a future error (use of invalid slot name).
Yes, I see now. I assume this little patch should be enough for that.
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c index 86eb31d..625b5e9 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" @@ -138,6 +139,9 @@ parse_subscription_options(List *options, bool *connect, bool *enabled_given, *slot_name_given = true; *slot_name = defGetString(defel); + /* Validate slot_name even if create_slot = false */ + ReplicationSlotValidateName(*slot_name, ERROR); + /* Setting slot_name = NONE is treated as no slot name. */ if (strcmp(*slot_name, "none") == 0) *slot_name = NULL; diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out index 91ba8ab..6334a18 100644 --- a/src/test/regress/expected/subscription.out +++ b/src/test/regress/expected/subscription.out @@ -62,6 +62,9 @@ CREATE SUBSCRIPTION testsub2 CONNECTION 'dbname=doesnotexist' PUBLICATION testpu ERROR: subscription with slot_name = NONE must also set create_slot = false CREATE SUBSCRIPTION testsub2 CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (slot_name = NONE, create_slot = false); ERROR: subscription with slot_name = NONE must also set enabled = false +CREATE SUBSCRIPTION testsub2 CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (slot_name = "invalid name", create_slot = false); +ERROR: replication slot name "invalid name" contains invalid character +HINT: Replication slot names may only contain lower case letters, numbers, and the underscore character. -- ok - with slot_name = NONE CREATE SUBSCRIPTION testsub3 CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (slot_name = NONE, connect = false); WARNING: tables were not subscribed, you will have to run ALTER SUBSCRIPTION ... REFRESH PUBLICATION to subscribe the tables diff --git a/src/test/regress/sql/subscription.sql b/src/test/regress/sql/subscription.sql index 4b694a3..5b635bc 100644 --- a/src/test/regress/sql/subscription.sql +++ b/src/test/regress/sql/subscription.sql @@ -47,6 +47,7 @@ CREATE SUBSCRIPTION testsub2 CONNECTION 'dbname=doesnotexist' PUBLICATION testpu CREATE SUBSCRIPTION testsub2 CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (slot_name = NONE); CREATE SUBSCRIPTION testsub2 CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (slot_name = NONE, enabled = false); CREATE SUBSCRIPTION testsub2 CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (slot_name = NONE, create_slot = false); +CREATE SUBSCRIPTION testsub2 CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (slot_name = "invalid name", create_slot = false); -- ok - with slot_name = NONE CREATE SUBSCRIPTION testsub3 CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (slot_name = NONE, connect = false);
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
