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
