Re: Invalidate the subscription worker in cases where a user loses their superuser status
Peter Smith <smithpb2250@gmail.com>
From: Peter Smith <smithpb2250@gmail.com>
To: vignesh C <vignesh21@gmail.com>
Cc: Amit Kapila <amit.kapila16@gmail.com>, Jeff Davis <pgsql@j-davis.com>, Robert Haas <robertmhaas@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2023-10-03T00:39:09Z
Lists: pgsql-hackers
Some review comments for v5. ====== src/backend/catalog/pg_subscription.c 1. GetSubscription - comment + /* Get superuser for subscription owner */ + sub->ownersuperuser = superuser_arg(sub->owner); + The comment doesn't seem very good. SUGGESTION /* Is the subscription owner a superuser? */ ====== 2. General - consistency Below are the code fragments using the new Subscription field. AlterSubscription_refresh: must_use_password = !sub->ownersuperuser && sub->passwordrequired; AlterSubscription: walrcv_check_conninfo(stmt->conninfo, sub->passwordrequired && !sub->ownersuperuser); LogicalRepSyncTableStart: must_use_password = MySubscription->passwordrequired && !MySubscription->ownersuperuser; run_apply_worker: must_use_password = MySubscription->passwordrequired && !MySubscription->ownersuperuser; ~ It is not a difference caused by this patch, but since you are modifying these lines anyway, I felt it would be better if all the expressions were consistent. So, in AlterSubscription_refresh IMO it would be better like: BEFORE must_use_password = !sub->ownersuperuser && sub->passwordrequired; SUGGESTION must_use_password = sub->passwordrequired && !sub->ownersuperuser; ====== Other than those trivial things, v5 LGTM. ====== Kind Regards, Peter Smith. Fujitsu Australia
Commits
-
Restart the apply worker if the privileges have been revoked.
- 79243de13f4d 17.0 landed