Re: Skipping schema changes in publication

[Amit Kapila]

On Mon, Mar 16, 2026 at 12:06 PM Peter Smith <smithpb2...@gmail.com> wrote:
>
> ======
> src/backend/commands/tablecmds.c
>
> ATExecAttachPartition:
>
> 3.
> + bool first = true;
> + StringInfoData pubnames;
> +
> + initStringInfo(&pubnames);
> +
> + foreach_oid(pubid, exceptpuboids)
> + {
> + char    *pubname = get_publication_name(pubid, false);
> +
> + if (!first)
> + {
> + /*
> + * translator: This is a separator in a list of publication
> + * names.
> + */
> + appendStringInfoString(&pubnames, _(", "));
> + }
> +
> + first = false;
> +
> + appendStringInfo(&pubnames, _("\"%s\""), pubname);
> + }
>
> 3a.
> Why is appendStringInfo(&pubnames, _("\"%s\""), pubname) using the
> "_(" macro?. AFAIK it does not make sense to call gettext() for a
> pubname.
>

Why? I see a similar usage in logicalrep_get_attrs_str().

> ~
>
> 3b.
> Postgres already has a function to make a CSV list of pubnames. Can't
> we build a list of pubnames here, then just call the common
> 'GetPublicationsStr' to get that as a CSV string. PSA a patch
> demonstrating what I mean.
>

Here you are first forming a list then adding commas to it by parsing
it. I think such a duplicate effort is not required.

-- 
With Regards,
Amit Kapila.