Hi Peter,
Thanks for the patch. Nice review comments.
I applied v3 on master and did some testing of my own. A few observations.
1) The patch fixes more than the reported case, which I think is worth
calling out in the commit message. Because the old code compared
n.nspname against the raw pattern text, the footer was silently missing for
*any* double-quoted pattern, not just ones containing an embedded single
quote:
-- unpatched
test=# \dn sch_a
List of schemas
Name | Owner
-------+----------
sch_a | postgres
Included in publications:
"pub_a"
test=# \dn "sch_a"
List of schemas
Name | Owner
-------+----------
sch_a | postgres
(1 row) <-- footer silently missing
FWIW I checked describe.c for other places where a user-supplied pattern is
interpolated into query text, and this is the only one
every other use of 'pattern' is either a pg_log_error() message or goes
through validateSQLNamePattern().
2) I'm less sure about switching this query to pattern matching.
processSQLNamePattern() turns it into a regex match, so it can now
match several schemas, but the footer belongs to the whole table rather
than to any one row:
-- v3
test=# \dn sch*
List of schemas
Name | Owner
-------+----------
sch_a | postgres
sch_b | postgres
Included in publications:
"pub_a"
"pub_b"
There's no way to tell which publication goes with which schema, and with
"\dn *" you get every schema publication in the database lumped
into one footer.
Unpatched, "\dn sch*" prints no footer at all (nspname = 'sch*' matches
nothing), so this is new behaviour.
Since the footer only really makes sense for a single schema, how about
keying off the row the main query already returned, rather than
re-interpreting the pattern?
if (pattern && PQntuples(res) == 1 && pset.sversion >= 150000)
{
...
appendPQExpBufferStr(&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"
"WHERE n.nspname = ");
appendStringLiteralConn(&buf, PQgetvalue(res, 0, 0), pset.db);
appendPQExpBufferStr(&buf, "\nORDER BY 1");
That is exactly escaped, can't drift from what's actually displayed, and
skips the second query entirely when it wouldn't be meaningful.
Minor nit:
- One test line has a stray trailing semicolon:
\d "it's my public table";
psql strips it so it's harmless, but the neighbouring lines don't have one.
Regards,
Surya Poondla
