On Sun, May 17, 2026 at 10:29 PM Chao Li <[email protected]> wrote: > > > On May 16, 2026, at 07:18, Jeff Davis <[email protected]> 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.
