Re: Invalidate the subscription worker in cases where a user loses their superuser status

Here are some comments for patch v2-0001.

======
src/backend/replication/logical/worker.c

1. maybe_reread_subscription

ereport(LOG,
(errmsg("logical replication worker for subscription \"%s\"
will restart because of a parameter change",
MySubscription->name)));

Is this really a "parameter change" though? It might be a stretch to
call the user role change a subscription parameter change. Perhaps
this case needs its own LOG message?

======
src/include/catalog/pg_subscription.h

2.
char *origin; /* Only publish data originating from the
* specified origin */
+ bool isownersuperuser; /* Is subscription owner superuser? */
} Subscription;

Is a new Subscription field member really necessary? The Subscription
already has 'owner' -- why doesn't function
maybe_reread_subscription() just check:

(!superuser_arg(sub->owner) && superuser_arg(MySubscription->owner))

======
src/test/subscription/t/027_nosuperuser.pl

3.
# The apply worker should get restarted after the superuser prvileges are
# revoked for subscription owner alice.

typo

/prvileges/privileges/

~

4.
+# After the user becomes non-superuser the apply worker should be restarted and
+# it should fail with 'password is required' error as password option is not
+# part of the connection string.

/as password option/because the password option/

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