diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c index a4ca363f20d..1e9f4602c69 100644 --- a/src/backend/replication/slot.c +++ b/src/backend/replication/slot.c @@ -2784,53 +2784,32 @@ GetSlotInvalidationCauseName(ReplicationSlotInvalidationCause cause) static bool validate_sync_standby_slots(char *rawname, List **elemlist) { - bool ok; - /* Verify syntax and parse string into a list of identifiers */ - ok = SplitIdentifierString(rawname, ',', elemlist); - - if (!ok) + if (!SplitIdentifierString(rawname, ',', elemlist)) { GUC_check_errdetail("List syntax is invalid."); + return false; } - else if (MyProc) + + /* Iterate the list to validate each slot name */ + foreach_ptr(char, name, *elemlist) { - /* - * Check that each specified slot exist and is physical. - * - * Because we need an LWLock, we cannot do this on processes without a - * PGPROC, so we skip it there; but see comments in - * StandbySlotsHaveCaughtup() as to why that's not a problem. - */ - LWLockAcquire(ReplicationSlotControlLock, LW_SHARED); + int err_code; + char *err_msg = NULL; + char *err_hint = NULL; - foreach_ptr(char, name, *elemlist) + if (!ReplicationSlotValidateNameInternal(name, false, &err_code, + &err_msg, &err_hint)) { - ReplicationSlot *slot; - - slot = SearchNamedReplicationSlot(name, false); - - if (!slot) - { - GUC_check_errdetail("Replication slot \"%s\" does not exist.", - name); - ok = false; - break; - } - - if (!SlotIsPhysical(slot)) - { - GUC_check_errdetail("\"%s\" is not a physical replication slot.", - name); - ok = false; - break; - } + GUC_check_errcode(err_code); + GUC_check_errdetail("%s", err_msg); + if (err_hint != NULL) + GUC_check_errhint("%s", err_hint); + return false; } - - LWLockRelease(ReplicationSlotControlLock); } - return ok; + return true; } /* @@ -2988,12 +2967,6 @@ StandbySlotsHaveCaughtup(XLogRecPtr wait_for_lsn, int elevel) /* * If a slot name provided in synchronized_standby_slots does not * exist, report a message and exit the loop. - * - * Though validate_sync_standby_slots (the GUC check_hook) tries to - * avoid this, it can nonetheless happen because the user can specify - * a nonexistent slot name before server startup. That function cannot - * validate such a slot during startup, as ReplicationSlotCtl is not - * initialized by then. Also, the user might have dropped one slot. */ if (!slot) { diff --git a/src/test/modules/unsafe_tests/expected/guc_privs.out b/src/test/modules/unsafe_tests/expected/guc_privs.out index 6c0ad898341..3cf2f2fdb85 100644 --- a/src/test/modules/unsafe_tests/expected/guc_privs.out +++ b/src/test/modules/unsafe_tests/expected/guc_privs.out @@ -581,6 +581,21 @@ DROP ROLE regress_host_resource_newadmin; -- ok, nothing was transferred -- Use "drop owned by" so we can drop the role DROP OWNED BY regress_host_resource_admin; -- ok DROP ROLE regress_host_resource_admin; -- ok +-- Test for GUC synchronized standby slots +-- Cannot set synchronized_standby_slots to a reserved slot name +ALTER SYSTEM SET synchronized_standby_slots='pg_conflict_detection'; +ERROR: invalid value for parameter "synchronized_standby_slots": "pg_conflict_detection" +DETAIL: replication slot name "pg_conflict_detection" is reserved +HINT: The name "pg_conflict_detection" is reserved for the conflict detection slot. +-- Cannot set synchronized_standby_slots to an invalid slot name +ALTER SYSTEM SET synchronized_standby_slots='invalid*'; +ERROR: invalid value for parameter "synchronized_standby_slots": "invalid*" +DETAIL: replication slot name "invalid*" contains invalid character +HINT: Replication slot names may only contain lower case letters, numbers, and the underscore character. +-- Can set synchronized_standby_slots to a non-existent slot name +ALTER SYSTEM SET synchronized_standby_slots='missing'; +-- Reset the GUC +ALTER SYSTEM RESET synchronized_standby_slots; -- Clean up RESET SESSION AUTHORIZATION; DROP ROLE regress_admin; -- ok diff --git a/src/test/modules/unsafe_tests/sql/guc_privs.sql b/src/test/modules/unsafe_tests/sql/guc_privs.sql index 9bcbbfa9040..d0d16f3c29f 100644 --- a/src/test/modules/unsafe_tests/sql/guc_privs.sql +++ b/src/test/modules/unsafe_tests/sql/guc_privs.sql @@ -262,6 +262,16 @@ DROP ROLE regress_host_resource_newadmin; -- ok, nothing was transferred DROP OWNED BY regress_host_resource_admin; -- ok DROP ROLE regress_host_resource_admin; -- ok +-- Test for GUC synchronized standby slots +-- Cannot set synchronized_standby_slots to a reserved slot name +ALTER SYSTEM SET synchronized_standby_slots='pg_conflict_detection'; +-- Cannot set synchronized_standby_slots to an invalid slot name +ALTER SYSTEM SET synchronized_standby_slots='invalid*'; +-- Can set synchronized_standby_slots to a non-existent slot name +ALTER SYSTEM SET synchronized_standby_slots='missing'; +-- Reset the GUC +ALTER SYSTEM RESET synchronized_standby_slots; + -- Clean up RESET SESSION AUTHORIZATION; DROP ROLE regress_admin; -- ok