Thanks to both for the reviews.

On Tue, Jul 28, 2026 at 11:18 PM Jim Jones <[email protected]> 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 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);

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

Attachment: v2-0001-Fix-escapes-for-psql-describe-schema-patterns.patch
Description: Binary data

Reply via email to