Re: Bug in ALTER SUBSCRIPTION ... SERVER / ... CONNECTION with broken old server
Jeff Davis <pgsql@j-davis.com>
From: Jeff Davis <pgsql@j-davis.com>
To: Chao Li <li.evan.chao@gmail.com>
Cc: Zsolt Parragi <zsolt.parragi@percona.com>, "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>, Ajin Cherian <itsajin@gmail.com>,
PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2026-05-15T23:18:43Z
Lists: pgsql-hackers
Attachments
- v6-0001-Avoid-errors-during-ALTER-SUBSCRIPTION.patch (text/x-patch)
- v6-0002-Avoid-errors-during-DROP-SUBSCRIPTION-when-slot_n.patch (text/x-patch)
- v6-0003-DROP-SUBSCRIPTION-improve-error-message.patch (text/x-patch)
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