On Thu, Oct 24, 2024 at 3:17 PM Michael Paquier <mich...@paquier.xyz> wrote: > > On Wed, Oct 23, 2024 at 03:40:19PM -0700, Masahiko Sawada wrote: > > > Now, I've changed the function comment to: > > > /* > > > * Add a comma-separated list of publication names to the 'dest' string. > > > */ > > > > Thank you for updating the patch. The patch looks good to me. > > + /* Build the pub_names comma-separated string. */ > + GetPublicationsStr(MySubscription->publications, pub_names, true); > > In fetch_remote_table_info(), it is true that we may finish by > building the same list two times, but your patch also changes the > logic so as the string is built for nothing when dealing with a server > version of 14 or older. That's a waste in these cases. > --
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); 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. ====== Kind RegArds, Peter Smith. Fujitsu Australia