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

> On May 1, 2026, at 11:58, Ajin Cherian <itsajin(at)gmail(dot)com> wrote:
>
> On Thu, Apr 30, 2026 at 2:12 PM Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
>>
>>
>>
>>>
>>> 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 permission on
> + * the subscription's foreign server, and load the connection string from the
> + * foreign server. Later, GetSubscriptionConnInfo() should be called to get
> + * 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

Hi Ajin,

Thank you for the comment. After rereading that part, I agree the wording is not clear.

What I meant is that GetSubscriptionConnInfo() is a safe accessor, if sub->conninfo has already been resolved, it just returns it; otherwise, it resolves it on demand. So the intended usage is that callers can consistently use GetSubscriptionConnInfo() when they need the connection string.

I also missed doing a broader search for direct uses of sub->conninfo and replacing them with GetSubscriptionConnInfo() where appropriate. Sorry about that.

I will address both issues in v3.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/