RE: [PoC] pg_upgrade: allow to upgrade publisher node - Mailing list pgsql-hackers

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