On Thu, Oct 24, 2024 at 8:26 AM Masahiko Sawada <sawada.m...@gmail.com> wrote:
Thanks for your feedback!
>
> While the changes look good to me, the comment of GetPublicationsStr()
> seems not match what the function actually does:
>
> +/*
> + * Add publication names from the list to a string.
> + */
> +void
> +GetPublicationsStr(List *publications, StringInfo dest, bool quote_literal)
>
> It's true that the function adds publication names to the given
> StringInfo, but it seems to me that the function expects the string is
> empty. For example, if we call this function twice with the same
> publication list, say 'pub1' and 'pub2', it would return a string
> 'pub1,pub2pub1,pub2'.
No, although this function is not designed to be called twice in a
row, there are code examples where this function is being called
passing (non-empty) SQL cmd string.
e.g.
cmd = makeStringInfo();
appendStringInfoString(cmd, "SELECT t.pubname FROM\n"
" pg_catalog.pg_publication t WHERE\n"
" t.pubname IN (");
GetPublicationsStr(publications, cmd, true);
appendStringInfoChar(cmd, ')');
> I think we can improve the description of this
> function to something like "Build a comma-separated string from the
> given list of publication names.".
>
This is a refactoring patch, so I hadn't intended to touch the
function, but I agree the function comment could be better.
Now, I've changed the function comment to:
/*
* Add a comma-separated list of publication names to the 'dest' string.
*/
~~
Please see the attached patch v3.
======
Kind Regards,
Peter Smith.
Fujitsu Australia
v3-0001-Refactor-to-use-common-function-GetPublicationsSt.patch
Description: Binary data
