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 effect is 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);
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
