Re: Restrict copying of invalidated replication slots

Hi. Some review comments for patch v1-0001.

======
1. DOCS?

Shouldn't the documentation [1] for pg_copy_logical_replication_slot()
and pg_copy_physical_replication_slot() be updated to mention this?

======
src/backend/replication/slotfuncs.c

2.
+ /* We should not copy invalidated replication slots */
+ if (src_isinvalidated)
+ ereport(ERROR,
+ (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("cannot copy an invalidated replication slot")));
+

2a.
The "we should not" sounds more like a recommendation than an error.
Comment can just say the same as the as errmsg.

~

2b.
ereport does not need all these parentheses

~

2c.
I felt the errmsg should include the name of the slot.

~~~

2d.
AFAICT this code will emit the same error regardless of
logical/physical slot so maybe you need to modify following to cater
for both kinds of replication_slot:
- the commit message
- docs
- test code

======
src/test/recovery/t/035_standby_logical_decoding.pl

3.
+# Attempting to copy an invalidated slot
+($result, $stdout, $stderr) = $node_standby->psql(

/Attempting/Attempt/

======
[1] https://www.postgresql.org/docs/current/functions-admin.html

Kind Regards,
Peter Smith.
Fujitsu Australia