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 по дате отправления: