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

Поиск
Список
Период
Сортировка
От Chao Li
Тема Re: Bug in ALTER SUBSCRIPTION ... SERVER / ... CONNECTION with broken old server
Дата
Msg-id 013CFA76-B42A-4351-8698-0B293BABDAE3@gmail.com
обсуждение
Ответ на Re: Bug in ALTER SUBSCRIPTION ... SERVER / ... CONNECTION with broken old server  (Ajin Cherian <itsajin@gmail.com>)
Список pgsql-hackers

> On May 1, 2026, at 11:58, Ajin Cherian <itsajin@gmail.com> wrote:
>
> On Thu, Apr 30, 2026 at 2:12 PM Chao Li <li.evan.chao@gmail.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
returnsit; 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()
whereappropriate. Sorry about that. 

I will address both issues in v3.

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







В списке pgsql-hackers по дате отправления: