Re: pg_upgrade and logical replication

Here are some review comments for patch set v19*

//////

v19-0001.

No comments

///////

v19-0002.

(I saw that both changes below seemed cut/paste from similar
functions, but I will ask the questions anyway).

======
src/backend/commands/subscriptioncmds.c

1.
+/* Potentially set by pg_upgrade_support functions */
+Oid binary_upgrade_next_pg_subscription_oid = InvalidOid;
+

The comment "by pg_upgrade_support functions" seemed a bit vague. IMO
you might as well tell the name of the function that sets this.

SUGGESTION
Potentially set by the pg_upgrade_support function --
binary_upgrade_set_next_pg_subscription_oid().

~~~

2. CreateSubscription

+ if (!OidIsValid(binary_upgrade_next_pg_subscription_oid))
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("pg_subscription OID value not set when in binary upgrade mode")));

Doesn't this condition mean some kind of impossible internal error
occurred -- i.e. should this be elog instead of ereport?

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