RE: [PoC] pg_upgrade: allow to upgrade publisher node

Dear Wang,

Thank you for reviewing! PSA new version.

> For patches 0001
> 
> 1. The latest patch set fails to apply because the new commit (0245f8d) in HEAD.

I didn't notice that. Thanks, fixed.

> 2. In file pg_dump.h.
> ```
> +/*
> + * The LogicalReplicationSlotInfo struct is used to represent replication
> + * slots.
> + *
> + * XXX: add more attributes if needed
> + */
> +typedef struct _LogicalReplicationSlotInfo
> +{
> +	DumpableObject dobj;
> +	char	   *plugin;
> +	char	   *slottype;
> +	bool		twophase;
> +} LogicalReplicationSlotInfo;
> ```
> 
> Do we need the structure member "slottype"? It seems we do not use "slottype"
> because we only dump logical replication slot.

As you said, this attribute is not needed. This is a garbage of previous efforts.
Removed.

> For patch 0002
> 
> 3. In the function SaveSlotToPath
> ```
> -	/* and don't do anything if there's nothing to write */
> -	if (!was_dirty)
> +	/*
> +	 * and don't do anything if there's nothing to write, unless it's this is
> +	 * called for a logical slot during a shutdown checkpoint, as we want to
> +	 * persist the confirmed_flush_lsn in that case, even if that's the only
> +	 * modification.
> +	 */
> +	if (!was_dirty && !is_shutdown && !SlotIsLogical(slot))
> ```
> It seems that the code isn't consistent with our expectation.
> If this is called for a physical slot during a shutdown checkpoint and there's
> nothing to write, I think it will also persist physical slots to disk.

You meant to say that we should not change handlings for physical case, right?

> For patch 0003
> 
> 4. In the function check_for_parameter_settings
> ```
> +	/* --include-logical-replication-slots can be used since PG	16. */
> +	if (GET_MAJOR_VERSION(new_cluster->major_version < 1600))
> +		return;
> ```
> It seems that there is a slight mistake (the input of GET_MAJOR_VERSION) in the
> if-condition:
> GET_MAJOR_VERSION(new_cluster->major_version < 1600)
> ->
> GET_MAJOR_VERSION(new_cluster->major_version) <= 1500
> 
> Please also check the similar if-conditions in the below two functions
> check_for_confirmed_flush_lsn (in 0003 patch)
> check_are_logical_slots_active (in 0004 patch)

Done. I grepped with GET_MAJOR_VERSION, and confirmed they were fixed.

Best Regards,
Hayato Kuroda
FUJITSU LIMITED