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
