Re: pg_upgrade and logical replication

I have only trivial review comments for patch v18-0001

======
src/bin/pg_upgrade/check.c

1. check_new_cluster_subscription_configuration

+ /*
+ * A slot not created yet refers to the 'i' (initialize) state, while
+ * 'r' (ready) state refer to a slot created previously but already
+ * dropped. These states are supported states for upgrade. The other
+ * states listed below are not ok:
+ *
+ * a) SUBREL_STATE_DATASYNC: A relation upgraded while in this state
+ * would retain a replication slot, which could not be dropped by the
+ * sync worker spawned after the upgrade because the subscription ID
+ * tracked by the publisher does not match anymore.
+ *
+ * b) SUBREL_STATE_SYNCDONE: A relation upgraded while in this state
+ * would retain the replication origin in certain cases.
+ *
+ * c) SUBREL_STATE_FINISHEDCOPY: A tablesync worker spawned to work on
+ * a relation upgraded while in this state would expect an origin ID
+ * with the OID of the subscription used before the upgrade, causing
+ * it to fail.
+ *
+ * d) SUBREL_STATE_SYNCWAIT, SUBREL_STATE_CATCHUP and
+ * SUBREL_STATE_UNKNOWN: These states are not stored in the catalog,
+ * so we need not allow these states.
+ */

1a.
/while 'r' (ready) state refer to a slot/while 'r' (ready) state
refers to a slot/

1b.
/These states are supported states for upgrade./These states are
supported for pg_upgrade./

1c
/The other states listed below are not ok./The other states listed
below are not supported./

======
src/bin/pg_upgrade/t/004_subscription.pl

2.
+# ------------------------------------------------------
+# Check that pg_upgrade refuses to run in:
+# a) if there's a subscription with tables in a state different than
+#    'r' (ready) or 'i' (init) state and/or
+# b) if the subscription has no replication origin.
+# ------------------------------------------------------

/if there's a subscription with tables in a state different than 'r'
(ready) or 'i' (init) state and/if there's a subscription with tables
in a state other than 'r' (ready) or 'i' (init) and/

======
Kind Regards,
Peter Smith.
Fujitsu Australia