Hi Kuroda-san.
I do not have any more review comments for the v5 patch, but here are
a few remaining nitpick items.
======
General
1.
There were a couple of comments that I thought would appear less
squished (aka more readable) if there was a blank line preceding the
XXX.
1a. This one is in getLogicalReplicationSlots
+ /*
+ * Get replication slots.
+ *
+ * XXX: Which information must be extracted from old node? Currently three
+ * attributes are extracted because they are used by
+ * pg_create_logical_replication_slot().
+ * XXX: Do we have to support physical slots?
+ */
~
1b. This one is for the LogicalReplicationSlotInfo typedef
+/*
+ * The LogicalReplicationSlotInfo struct is used to represent replication
+ * slots.
+ * XXX: add more attrbutes if needed
+ */
BTW -- I just noticed there is a typo in that comment. /attrbutes/attributes/
======
src/bin/pg_dump/pg_dump_sort.c
2. describeDumpableObject
+ case DO_LOGICAL_REPLICATION_SLOT:
+ snprintf(buf, bufsize,
+ "REPLICATION SLOT (ID %d NAME %s)",
+ obj->dumpId, obj->name);
+ return;
Since everything else was changed to say logical replication slot,
should this string be changed to "LOGICAL REPLICATION SLOT (ID %d NAME
%s)"?
======
.../pg_upgrade/t/003_logical_replication.pl
3.
Should the name of this TAP test file really be 03_logical_replication_slots.pl?
------
Kind Regards,
Peter Smith.
Fujitsu Australia