RE: Non-superuser subscription owners

On Friday, March 31, 2023 12:05 AM Robert Haas <robertmhaas(at)gmail(dot)com> wrote:

Hi,

>
> On Tue, Mar 28, 2023 at 1:52 PM Jeff Davis <pgsql(at)j-davis(dot)com> wrote:
> > On Fri, 2023-03-24 at 00:17 -0700, Jeff Davis wrote:
> > > The other patch you posted seems like it makes a lot of progress in
> > > that direction, and I think that should go in first. That was one of
> > > the items I suggested previously[2], so thank you for working on
> > > that.
> >
> > The above is not a hard objection.
>
> The other patch is starting to go in a direction that is going to have some
> conflicts with this one, so I went ahead and committed this one to avoid
> rebasing pain.

I noticed the BF[1] report a core dump after this commit.

#0  0xfd581864 in _lwp_kill () from /usr/lib/libc.so.12
#0  0xfd581864 in _lwp_kill () from /usr/lib/libc.so.12
#1  0xfd5817dc in raise () from /usr/lib/libc.so.12
#2  0xfd581c88 in abort () from /usr/lib/libc.so.12
#3  0x01e6c8d4 in ExceptionalCondition (conditionName=conditionName(at)entry=0x2007758 "IsTransactionState()", fileName=fileName(at)entry=0x20565c4 "catcache.c", lineNumber=lineNumber(at)entry=1208) at assert.c:66
#4  0x01e4e404 in SearchCatCacheInternal (cache=0xfd21e500, nkeys=nkeys(at)entry=1, v1=v1(at)entry=28985, v2=v2(at)entry=0, v3=v3(at)entry=0, v4=v4(at)entry=0) at catcache.c:1208
#5  0x01e4eea0 in SearchCatCache1 (cache=<optimized out>, v1=v1(at)entry=28985) at catcache.c:1162
#6  0x01e66e34 in SearchSysCache1 (cacheId=cacheId(at)entry=11, key1=key1(at)entry=28985) at syscache.c:825
#7  0x01e98c40 in superuser_arg (roleid=28985) at superuser.c:70
#8  0x01c657bc in ApplyWorkerMain (main_arg=<optimized out>) at worker.c:4552
#9  0x01c1ceac in StartBackgroundWorker () at bgworker.c:861
#10 0x01c23be0 in do_start_bgworker (rw=<optimized out>) at postmaster.c:5762
#11 maybe_start_bgworkers () at postmaster.c:5986
#12 0x01c2459c in process_pm_pmsignal () at postmaster.c:5149
#13 ServerLoop () at postmaster.c:1770
#14 0x01c26cdc in PostmasterMain (argc=argc(at)entry=4, argv=argv(at)entry=0xffffe0e4) at postmaster.c:1463
#15 0x01ee2c8c in main (argc=4, argv=0xffffe0e4) at main.c:200

It looks like the super user check is out of a transaction, I haven't checked why
it only failed on one BF animal, but it seems we can put the check into the
transaction like the following:

diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index 6fd674b5d6..08f10fc331 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -4545,12 +4545,13 @@ ApplyWorkerMain(Datum main_arg)
replorigin_session_setup(originid, 0);
replorigin_session_origin = originid;
origin_startpos = replorigin_session_get_progress(false);
- CommitTransactionCommand();

/* Is the use of a password mandatory? */
must_use_password = MySubscription->passwordrequired &&
!superuser_arg(MySubscription->owner);

+ CommitTransactionCommand();
+

[1] https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=mamba&dt=2023-03-30%2019%3A41%3A08

Best Regards,
Hou Zhijie