Dear Wang, Thank you for reviewing! PSA new version.
> 1. In the function getLogicalReplicationSlots > ``` > + /* > + * Note: Currently we do not have any options to include/exclude > slots > + * in dumping, so all the slots must be selected. > + */ > + slotinfo[i].dobj.dump = DUMP_COMPONENT_ALL; > ``` > I think currently we are only dumping the definition of logical replication > slots. It seems better to set it as DUMP_COMPONENT_DEFINITION here. Right. Actually it was harmless because another flags like DUMP_COMPONENT_DEFINITION are not checked in dumpLogicalReplicationSlot(), but changed. > 2. In the function dumpLogicalReplicationSlot > ``` > + ArchiveEntry(fout, slotinfo->dobj.catId, slotinfo->dobj.dumpId, > + ARCHIVE_OPTS(.tag = slotname, > + > .description = "REPLICATION SLOT", > + .section = > SECTION_POST_DATA, > + > .createStmt = query->data)); > ``` > I think if we do not set the member dropStmt in macro ARCHIVE_OPTS here, when > we > specifying the option "--logical-replication-slots-only" and option > "-c/--clean" > together, the "-c/--clean" will not work. > > I think that we could use the function pg_drop_replication_slot to set this > member. Then, in the main function in the pg_dump.c file, we should add a > check > to prevent specifying option "--logical-replication-slots-only" and > option "--if-exists" together. > Or, we could simply add a check to prevent specifying option > "--logical-replication-slots-only" and option "-c/--clean" together. > What do you think? I chose not to allow to combine with -c. Assuming that this option is used only by the pg_upgrade, it is ensured that new node does not have any logical replication slots. So the remove function is not needed. Best Regards, Hayato Kuroda FUJITSU LIMITED
v12-0001-pg_upgrade-Add-include-logical-replication-slots.patch
Description: v12-0001-pg_upgrade-Add-include-logical-replication-slots.patch
v12-0002-Always-persist-to-disk-logical-slots-during-a-sh.patch
Description: v12-0002-Always-persist-to-disk-logical-slots-during-a-sh.patch
v12-0003-pg_upgrade-Add-check-function-for-include-logica.patch
Description: v12-0003-pg_upgrade-Add-check-function-for-include-logica.patch
v12-0004-Change-the-method-used-to-check-logical-replicat.patch
Description: v12-0004-Change-the-method-used-to-check-logical-replicat.patch
