RE: [16] ALTER SUBSCRIPTION ... SET (run_as_owner = ...) is a no-op

On Monday, September 11, 2023 7:49 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> 
> On Mon, Sep 11, 2023 at 10:46 AM Zhijie Hou (Fujitsu) <houzj.fnst@fujitsu.com>
> wrote:
> >
> > On Sunday, September 10, 2023 4:43 AM Jeff Davis <pgsql@j-davis.com>
> wrote:
> > >
> > >
> > > Repro:
> > >   ALTER SUBSCRIPTION s1 SET (run_as_owner = true);
> > >   SELECT subrunasowner FROM pg_subscription WHERE subname='s1';
> > >    subrunasowner
> > >   ---------------
> > >    f
> > >   (1 row)
> > >
> >
> > Thanks for reporting. I can also reproduce the issue. I think it's
> > because we didn't reflect the option change on catalog. Here is a small patch
> 0001 to fix it.
> >
> 
> Your fix looks good to me.

Thanks for reviewing.

> 
> > > It also looks like a change to that field may not cause the
> > > subscription worker to restart. It would be good to add a test for that case.
> >
> > Currently, the changes on run_as_owner won't cause the worker to
> > restart because we don't need to rebuild the connection in this case.
> > The option change will be caught by apply worker in next loop and the
> > later changes will be applied using the new option. the 0002 patch
> > adds a test to verfiy it, just to show how it behaves.
> >
> 
> Is there a reason to not include 0002 in the commit?

No, I think it's fine to include 0002.

> 
> Shall we push this before 16 or wait for it? I don't if we have enough time or if it
> is worth pushing at the last minute. I can take care of pushing this tomorrow
> morning if we decide that it is okay to go ahead with this unless Jeff or Robert
> pushes it.

I saw the "Stamp 16.0" has came in, so it seems we can only include this fix in 16.1.

Best Regards,
Hou zj