On Tue, Oct 6, 2026 at 5:18 AM surya poondla <[email protected]> wrote: > > Hi Peter, > > Thanks for v7. It applies cleanly on master, builds without warnings, and > make check passes. > The quoting fix itself looks right: both queries now take the actual schema > name via appendStringLiteralConn(). I tried names with embedded ', ", \ > and ., plus mixed case. They all display correctly with the right > publications footer, and \dn "e_sch1" now shows the footer that master > silently left out. > > I have a couple of comments: > 1. Backpatching. Nishant showed the bug goes back to PG15, but v7 also > changes what \dn prints whenever a pattern is given: > - each match is now its own "Schema "x"" table instead of one "List of > schemas" > - the "(N rows)" footer is gone > - in non-quiet mode, a pattern with no match is now an error > > I don't think changes to user-visible output can go into 15–18. Could this be > split into two patches? 0001 would be a minimal escaping fix that can be > backpatched, e.g. keying the footer off PQgetvalue(res, 0, 0) when exactly > one row comes back. 0002 would be the per-schema display, master only, and > would probably need its own discussion. psql-ref.sgml also still says > matching schemas "are listed", so 0002 would need a doc update. > > 2. The no-match behaviour doesn't actually match \dt. The comment says "to be > same as \dt", but listTables() prints the error and then returns true, while > listSchemas() jumps to error_return and returns false. So under > ON_ERROR_STOP, \dn nosuch now aborts a script where it used to print an empty > table: > > $ psql -v ON_ERROR_STOP=1 -c '\dn e_nosuch' -c 'select 1'; echo "exit=$?" > Did not find any schemas named "e_nosuch". > exit=1 > $ psql -v ON_ERROR_STOP=1 -c '\dt e_nosuch' -c 'select 1'; echo "exit=$?" > Did not find any tables named "e_nosuch". > ?column? > ---------- > 1 > (1 row) > > exit=0 > > Also: > - the no-pattern message "Did not find any schemas" has no trailing > period, unlike the other messages in describe.c > - in quiet mode a no-match still prints the old empty "List of schemas" > table, so there are two output styles depending on -q > - pg_regress runs psql with -q, so none of the new error paths are > exercised by the regression tests > > 3. \dn * now prints a separate table for every schema in the database, > pg_catalog, pg_toast and information_schema included. With CSV output, a > multi-match pattern now produces several header rows in a row, which would > break anything that parses the output: > \pset format csv > \dn e_sch* > Name,Owner > e_sch1,surya > Name,Owner > e_sch2,surya > Name,Owner > e_sch3,surya > This is one more reason to keep the redesign out of the bug fix, or at least > call it out explicitly. > > 4. Minor issues in describeOneSchemaDetails(): > - The footer query has no separator after the literal. With ECHO_HIDDEN > it shows up as WHERE n.nspname = 'e_sch1'ORDER BY 1;. It works, but a "\n" > before ORDER BY would fix it. > - The footer query still uses appendPQExpBuffer() with no format > arguments. Jim pointed this out for v1, but v2 only changed the ORDER BY > line. appendPQExpBufferStr() would do. > - myopt.topt.default_footer = false is only set inside the sversion >= > 150000 branch, so against an older server each schema table still ends with > "(1 row)". > > 5. Tests: > - CREATE PUBLICATION pub_sch_1 FOR TABLES IN SCHEMA SCH_1 is unquoted, > so it publishes sch_1, not "SCH_1". The expected output is correct, but a > reader will probably assume the opposite. Could you add a comment, or use > distinct names? > - There's no coverage for \dn+ with a pattern or for names containing a > backslash. > > Regards, > Surya Poondla
Hi Surya, Thanks for your review comments! I am not is there are any the backpatching requirements for this since its a rare (nobody raised it before me) psql display-only bug. OTOH, I recognise your point that I need to separate the minimal bug-fix, from where I went beyond that and tampered with existing \dn behaviour. I am currently splitting the patches and plan to post something new next week... ====== Kind Regards, Peter Smith. Fujitsu Australia
