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

On Tue, Sep 12, 2023 at 7:10 AM Zhijie Hou (Fujitsu)
<houzj(dot)fnst(at)fujitsu(dot)com> wrote:
>
> On Monday, September 11, 2023 7:49 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> >
> > On Mon, Sep 11, 2023 at 10:46 AM Zhijie Hou (Fujitsu) <houzj(dot)fnst(at)fujitsu(dot)com>
> > wrote:
> > >
> > > On Sunday, September 10, 2023 4:43 AM Jeff Davis <pgsql(at)j-davis(dot)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.
>

Pushed by slightly adjusting the comments in the test case.

--
With Regards,
Amit Kapila.