From: | Peter Smith <smithpb2250(at)gmail(dot)com> |
---|---|
To: | Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> |
Cc: | Michael Paquier <michael(at)paquier(dot)xyz>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Refactor to use common function 'get_publications_str'. |
Date: | 2024-10-23 22:26:19 |
Message-ID: | CAHut+PshwcOdq+MbgEM2ardEvCVXHxL-6SDzZo4aA=v+qgr7iA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Thu, Oct 24, 2024 at 8:26 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)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
Attachment | Content-Type | Size |
---|---|---|
v3-0001-Refactor-to-use-common-function-GetPublicationsSt.patch | application/octet-stream | 10.2 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | David G. Johnston | 2024-10-23 22:40:17 | Re: Refine comments on usage WL_POSTMASTER_DEATH vs WL_EXIT_ON_PM_DEATH |
Previous Message | Melanie Plageman | 2024-10-23 22:15:37 | Can rs_cindex be < 0 for bitmap heap scans? |