Dear Peter, Thank you for checking. Then we can wait comments from others. PSA modified version.
> 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? > + */ Added. > 1b. This one is for the LogicalReplicationSlotInfo typedef > > +/* > + * The LogicalReplicationSlotInfo struct is used to represent replication > + * slots. > + * XXX: add more attrbutes if needed > + */ Added. > BTW -- I just noticed there is a typo in that comment. /attrbutes/attributes/ Good finding, replaced. > 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)"? I missed to replace, changed. > .../pg_upgrade/t/003_logical_replication.pl > > 3. > Should the name of this TAP test file really be > 03_logical_replication_slots.pl? > Hmm, not sure. Currently I renamed once according to your advice, but personally another feature which allows to upgrade subscriber[1] should be tested in the same perl file. That's why I named as "003_logical_replication.pl" [1]: https://www.postgresql.org/message-id/20230217075433.u5mjly4d5cr4hcfe%40jrouhaud Best Regards, Hayato Kuroda FUJITSU LIMITED
v6-0001-pg_upgrade-Add-include-logical-replication-slots-.patch
Description: v6-0001-pg_upgrade-Add-include-logical-replication-slots-.patch
