Re: Bug in ALTER SUBSCRIPTION ... SERVER / ... CONNECTION with broken old server

On Sun, May 17, 2026 at 10:29 PM Chao Li <li.evan.chao@gmail.com> wrote:
>
> > On May 16, 2026, at 07:18, Jeff Davis <pgsql@j-davis.com> wrote:
> >
> > On Fri, 2026-05-15 at 15:18 +0800, Chao Li wrote:
> >> 0002 adds this Assert:
> >> ```
> >> if (update_failover || update_two_phase || check_pub_rdt)
> >> {
> >> bool must_use_password;
> >> char    *err;
> >> WalReceiverConn *wrconn;
> >>
> >> Assert(conninfo_needed);
> >> ```
> >>
> >> So, for those two paths, if check_pub_rdt is true, then the Assert
> >> will be fired, is that intentional?
> >
> > I've fixed it to be Assert(new_conninfo || orig_conninfo_needed).
> >
> > Also, the code above was missing the case of SUBOPT_ORIGIN which could
> > set check_pub_rdt. I changed it to be more conservative and set
> > orig_conninfo_needed=false when one of:
> >
> >  ALTER SUBSCRIPTION ... SERVER
> >  ALTER SUBSCRIPTION ... CONNECTION
> >  ALTER SUBSCRIPTION ... SET (slot_name=NONE)
> >
> > and not try to be precise about which other settings might need
> > check_pub_rdt or not.
>
> Yep, this part looks good now.
>
> >
> > What do you think of v6-0003? Is it over-engineered? Should the
> > subtransaction happen at a lower level? Is there an alternative to
> > using a subtransaction?
> >
>
> For the reason you described in the commit message, catching the error and later reporting it through ReportSlotConnectionError(), I don't think this is over-engineered. I also think keeping the subtransaction inside construct_subserver_conninfo() is reasonable, because this error-capturing behavior seems specific to the DROP SUBSCRIPTION path.
>
> As for whether the HINT itself really helps, I would reserve my opinion. As the test output shows:
> ```
> DROP SUBSCRIPTION regress_testsub6;
> ERROR:  could not connect to publisher when attempting to drop replication slot "dummy": user mapping not found for user "regress_subscription_user3", server "test_server"
> HINT:  Use ALTER SUBSCRIPTION ... DISABLE to disable the subscription, and then use ALTER SUBSCRIPTION ... SET (slot_name = NONE) to disassociate it from the slot.
> ```
>
> The error message already says that the problem is “user mapping not found”, so fixing the user mapping could also be a solution. So, the HINT is still useful, but it might not be the most direct fix in some case.
>

Right, the hint doesn't sound to be a right solution for the problem reported.

-- 
With Regards,
Amit Kapila.