Message-ID: <53a9cddd6a2b5eca78e65979890fa77d690d1279.camel@j-davis.com>
Subject: Re: CREATE SUBSCRIPTION ... SERVER vs. pg_dump, etc.
From: Jeff Davis <pgsql@j-davis.com>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: Noah Misch <noah@leadboat.com>, pgsql-hackers@postgresql.org, "Hayato
 Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>, Shlok Kyal
 <shlok.kyal.oss@gmail.com>, Fujii Masao <masao.fujii@gmail.com>, yuanchao
 zhang <145zhangyc@gmail.com>
Date: Fri, 31 Jul 2026 09:09:27 -0700
In-Reply-To: 
 <CAA4eK1Kz_JWAEmzTMSmGGPnxM0=EAZG_WcNB5dz2We6ezVW1DQ@mail.gmail.com>
References: <20260710195902.4f.noahmisch@microsoft.com>
	 <35e50b0f80c850e5b9eb1dedcc0844852a2d2de0.camel@j-davis.com>
	 <e96efe16fb47fcca4ae0cd0157fb6bc662470712.camel@j-davis.com>
	 <e103ae8daf74485e0c0ebde297fae735d38f54d1.camel@j-davis.com>
	 <c7fe6424386631771dd419d17d0c3d952c2e7619.camel@j-davis.com>
	 <CAA4eK1Kz_JWAEmzTMSmGGPnxM0=EAZG_WcNB5dz2We6ezVW1DQ@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.52.3-0ubuntu1.1 
MIME-Version: 1.0
Archived-At: 
 <https://www.postgresql.org/message-id/53a9cddd6a2b5eca78e65979890fa77d690d1279.camel%40j-davis.com>
Precedence: bulk

On Fri, 2026-07-31 at 14:12 +0530, Amit Kapila wrote:
> However, I feel it is better to detect the same
> at DDL time whenever possible as well as it gives immediate,
> synchronous feedback for interactive CREATE/ALTER, whereas a
> worker-only failure just lands in the server log and the worker keeps
> restarting. Removing it would also mean enabling retain_dead_tuples
> no
> longer validates the publisher at all in the common interactive case.

I believe the only problem case is ALTER SUBSCRIPTION ... ENABLE,
right?

CREATE doesn't do the check when connect=3Dfalse, so that's the same
behavior.

None of ALTER ... SERVER, ALTER ... CONNECTION, or ALTER ... SET
(retain_dead_tuples) are called by restore because it sets those things
with the CREATE statement.


If you'd still like ALTER SUBSCRIPTION ... ENABLE to do the convenience
check, then I think you could clarify the problem case in the comments:

+  /*
+   * During binary upgrade, we only recreate the catalog state and
must not
+   * connect to the publisher. The publisher's suitability for
+   * retain_dead_tuples is validated authoritatively by the apply
worker
+   * when it connects, so skip the opportunistic DDL-time check here.
+   */
+  if (IsBinaryUpgrade)
+    check_pub_rdt =3D false;

During any restore we must not connect to the publisher. It's only a
problem for binary upgrade because that's what issues the ENABLE.

But the overall logic is more like "restore must not create any
connections, therefore it must not issue any commands that set
check_pub_rdt". We can't detect an ordinary restore (because it's
treated the same as interactive SQL), so we just have to be sure not to
introduce check_pub_rdt cases in the ordinary restore path later.

Also, we need to integrate it with the series I posted because we must
not generate the conninfo if IsBinaryUpgrade. I expect yours will go in
first and I can rebase on that, so you don't need to make a change
here.

Regards,
	Jeff Davis