Thread

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

Jeff Davis <pgsql@j-davis.com> — 2026-05-15T23:18:43Z

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.

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?

Regards,
	Jeff Davis