Dear Peter, Thanks for reviewing! PSA new version patchset.
> 1. get_logical_slot_infos_per_db > > I noticed that the way this is coded, 'ntups' and 'num_slots' seems to > have exactly the same meaning. IMO you can simplify this by removing > 'ntups'. > > BEFORE > + int ntups; > + int num_slots = 0; > > SUGGESTION > + int num_slots; > > ~ > > BEFORE > + ntups = PQntuples(res); > + > + if (ntups) > + { > > SUGGESTION > + num_slots = PQntuples(res); > + > + if (num_slots) > + { > > ~ > > BEFORE > + slotinfos = (LogicalSlotInfo *) pg_malloc(sizeof(LogicalSlotInfo) * ntups); > > SUGGESTION > + slotinfos = (LogicalSlotInfo *) pg_malloc(sizeof(LogicalSlotInfo) * > num_slots); > > ~ > > BEFORE > + for (slotnum = 0; slotnum < ntups; slotnum++) > + { > + LogicalSlotInfo *curr = &slotinfos[num_slots++]; > > SUGGESTION > + for (slotnum = 0; slotnum < ntups; slotnum++) > + { > + LogicalSlotInfo *curr = &slotinfos[slotnum]; Right, fixed. > 2. get_logical_slot_infos, print_slot_infos > > > > > > > In another thread [1] I am posting some minor patch changes to the > > > VERBOSE logging (changes to double-quotes and commas etc.). Please > > > keep a watch on that thread because if gets pushed then this one will > > > be impacted. e.g. your logging here ought also to include the same > > > suggested double quotes. > > > > I thought it would be pushed soon, so the suggestion was included. > > OK, but I think you have accidentally missed adding similar new double > quotes to all other VERBOSE logging in your patch. > > e.g. see get_logical_slot_infos: > pg_log(PG_VER BOSE, "Database: %s", pDbInfo->db_name); > Oh, I missed it. Fixed. I grepped patches and could not find other lines which should be double-quoted. In addition, I ran pgindent again for 0001. Best Regards, Hayato Kuroda FUJITSU LIMITED
v14-0001-pg_upgrade-Add-include-logical-replication-slots.patch
Description: v14-0001-pg_upgrade-Add-include-logical-replication-slots.patch
v14-0002-Always-persist-to-disk-logical-slots-during-a-sh.patch
Description: v14-0002-Always-persist-to-disk-logical-slots-during-a-sh.patch
v14-0003-pg_upgrade-Add-check-function-for-include-logica.patch
Description: v14-0003-pg_upgrade-Add-check-function-for-include-logica.patch
v14-0004-Change-the-method-used-to-check-logical-replicat.patch
Description: v14-0004-Change-the-method-used-to-check-logical-replicat.patch