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
>
> 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,
becauseno additional validation is needed:
>
> processSQLNamePattern(pset.db, &buf, pattern, false, false,
> NULL, "n.nspname", NULL,
> NULL, NULL, NULL);
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;");
Other than that the patch LGTM.
Thanks!
Best, Jim
