Re: Refactor to use common function 'get_publications_str'.

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.
--
Michael