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

On Wed, 27 Sept 2023 at 12:28, Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Wed, Sep 27, 2023 at 6:58 AM Peter Smith <smithpb2250@gmail.com> wrote:
> >
> > On Tue, Sep 26, 2023 at 11:57 PM vignesh C <vignesh21@gmail.com> wrote:
> > >
> > > On Tue, 26 Sept 2023 at 13:03, Peter Smith <smithpb2250@gmail.com> wrote:
> > > >
> > > > 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?
> > >
> > > When I was doing this change the same thought had come to my mind too
> > > but later I noticed that in case of owner change there was no separate
> > > log message. Since superuser check is somewhat similar to owner
> > > change, I felt no need to make any change for this.
> > >
> >
> > Yeah, I had seen the same already before my comment. Anyway, YMMV.
> >
>
> But OTOH, the owner of the subscription can be changed by the Alter
> Subscription command whereas superuser status can't be changed. I
> think we should consider changing the message for this case.

Modified

> BTW, do we want to backpatch this patch? I think we should backatch to
> PG16 as it impacts password_required functionality. Before this patch
> even if the subscription owner's superuser status is lost, it won't
> use a password for connection till the server gets restarted or the
> apply worker gets restarted due to some other reason. What do you
> think?

I felt since password_required functionality is there in PG16, we
should fix this in PG16 too. I have checked that password_required
functionality is not there in PG15, so no need to make any change in
PG15

The updated patch has the changes for the same.

Regards,
Vignesh