Re: PSQL schema "describe" \dn is not escaping quotes - Mailing list pgsql-hackers

From surya poondla
Subject Re: PSQL schema "describe" \dn is not escaping quotes
Date
Msg-id CAOVWO5owc=bd05vqu4-3J5BbKwih8wwB11gFBQ9c5V+qqtOqJA@mail.gmail.com
Whole thread
In response to Re: PSQL schema "describe" \dn is not escaping quotes  (Peter Smith <smithpb2250@gmail.com>)
Responses Re: PSQL schema "describe" \dn is not escaping quotes
List pgsql-hackers
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

pgsql-hackers by date:

Previous
From: Greg Burd
Date:
Subject: Re: Let an ordering index scan hand its ORDER BY value to the target list
Next
From: Manu
Date:
Subject: Re: UNDO with constant time recovery (CTR)