MIME-Version: 1.0
References: <D908370F-2695-4231-851D-17179A6A6F2A@gmail.com>
 <CAFPTHDZk-eY1K6OGWhLSWJ0C7qncVQvaxUmBL2r69_1CiznfTA@mail.gmail.com>
 <960DD2C4-A22C-463A-90ED-86E0FABD8D20@gmail.com>
In-Reply-To: <960DD2C4-A22C-463A-90ED-86E0FABD8D20@gmail.com>
From: Ajin Cherian <itsajin@gmail.com>
Date: Fri, 1 May 2026 13:58:51 +1000
Message-ID: 
 <CAFPTHDamW43eQTpZGfUhZAhYxCwYSmCs2ww3sxjBoHDvv7Oawg@mail.gmail.com>
Subject: Re: Bug in ALTER SUBSCRIPTION ... SERVER / ... CONNECTION with broken
 old server
To: Chao Li <li.evan.chao@gmail.com>
Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CAFPTHDamW43eQTpZGfUhZAhYxCwYSmCs2ww3sxjBoHDvv7Oawg%40mail.gmail.com>
Precedence: bulk

On Thu, Apr 30, 2026 at 2:12=E2=80=AFPM Chao Li <li.evan.chao@gmail.com> wr=
ote:
>
>
>
> >
> > 2. I think you should add a comment in the function header above
> > GetSubscription() stating that if aclcheck is false, then the conninfo
> > will be set to null and users need to call GetSubscriptionConnInfo to
> > get the conninfo.
>
> Updated the header comment.

 /*
  * Fetch the subscription from the syscache.
+ *
+ * If missing_ok is false, throw an error if the subscription is not found=
.
+ * If true, return NULL in that case.
+ *
+ * If aclcheck is true, check whether the subscription owner has permissio=
n on
+ * the subscription's foreign server, and load the connection string from =
the
+ * foreign server. Later, GetSubscriptionConnInfo() should be called to ge=
t
+ * the connection string.
  */
 Subscription *
 GetSubscription(Oid subid, bool missing_ok, bool aclcheck)

I don't think this comment is right. If aclcheck is true, users need
not call GetSubscriptionConnInfo() to get the connection string, as it
is populated in this function itself. It is only if aclecheck is
false, do callers need to do that.
Isn't that the case? There are multiple places in the apply worker
where GetSubscription() is called with aclcheck is true and
GetSubscriptionConnInfo() is not called there.

regards,
Ajin Cherian
Fujitsu Australia