Thanks to both for the reviews.
On Tue, Jul 28, 2026 at 11:18 PM Jim Jones <jim.jones@uni-muenster.de> wrote:
>
> Hi Peter & Steven
>
> On 28/07/2026 10:19, Steven Niu wrote:
> > 1. Indentation
> >
> > if (!validateSQLNamePattern(...))
> > goto error_return; // should be indented one more tab
Now I have run pgindent.
> >
> > 2. The new validateSQLNamePattern() call duplicates validation that
> > already happened.
> >
> > The validateSQLNamePattern() has been called before your change. Since only the escaping/query-building side
effectis actually
> > needed here. You can call the lower-level processSQLNamePattern() directly and doesn't check its return
value,because no additional validation is needed:
> >
> > processSQLNamePattern(pset.db, &buf, pattern, false, false,
> > NULL, "n.nspname", NULL,
> > NULL, NULL, NULL);
Done. Thanks for pointing me to this lower function.
>
>
> I'm also wondering if we should use appendPQExpBufferStr instead of
> appendPQExpBuffer here, since there are no format strings -- just to be
> consistent with the other queries in the same file.
>
> appendPQExpBuffer(&buf,
> "SELECT pubname \n"
> "FROM pg_catalog.pg_publication p\n"
> " JOIN pg_catalog.pg_publication_namespace pn ON p.oid = pn.pnpubid\n"
> " JOIN pg_catalog.pg_namespace n ON n.oid = pn.pnnspid \n");
> ...
> appendPQExpBuffer(&buf, "ORDER BY 1;");
Done. Oops.
~~~
PSA patch v2 to address all of the above.
======
Kind Regards,
Peter Smith.
Fujitsu Australia
