Re: Refactor to use common function 'get_publications_str'.

On Thu, Oct 24, 2024 at 5:41 PM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>
> On Thu, Oct 24, 2024 at 05:27:25PM +1100, Peter Smith wrote:
> > Yes, well spotted -- I was aware of that. Originally I had coded a >=
> > PG15 check for that pub_names assignment. e.g.
> >
> > if (walrcv_server_version(LogRepWorkerWalRcvConn) >= 150000)
> > GetPublicationsStr(MySubscription->publications, pub_names, true);
>
> I was wondering about putting the call of GetPublicationsStr() in the
> first block with the makeStringInfo(), have an Assert checking that
> pub_names->data is never NULL in the second block, and destroy the
> StringInfo only if it has been allocated.
>
> > But, somehow it seemed messy to do that just to cater for something I
> > thought was not particularly likely. OTOH, I am happy to put that
> > check back in if you think it is warranted.
>
> I think I would add it. The publication list is not mandatory, but
> your patch makes it look so.
> --

No problem. I will post an updated patch tomorrow.

======
Kind Regards,
Peter Smith.
Fujitsu Australia