Re: [PoC] pg_upgrade: allow to upgrade publisher node

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