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.
