Re: [Doc] pg_restore documentation didn't explain how to useconnection string - Mailing list pgsql-hackers

On Wed, 2019-11-13 at 16:48 +0100, Lætitia Avrot wrote:
> So after some thoughts I did the minimal patch (for now).
> I corrected documentation for the following tools so that now, using connection string
> for Postgres client applications is documented in Postgres:
> - clusterdb
> - pgbench
> - pg_dump
> - pg_restore
> - reindexdb
> - vacuumdb

I think that this patch is a good idea.
Even if it adds some redundancy, that can hardly be avoided because, as you said,
the options to specify the database name are not the same everywhere.

The patch applies and build fine.

I see some room for improvement:

- I think that "connection string" is better than "conninfo string".
  At least the chapter to which you link is headed "Connection Strings".

  This would also be consistent with the use of that term in the
  "pg_basebackup" , "pg_dumpall" and "pg_receivewal" documentation.

  You seem to have copied that wording from the "pg_isready", "psql",
  "reindexdb" and "vacuumdb" documentation, but I think it would be better
  to reword those too.

- You begin your paragraph with "if this parameter contains ...".

  First, I think "argument" might be more appropriate here, as you
  are talking about
  a) the supplied value and
  b) a command line argument or the argument to an option

  Besides, it might be confusing to refer to "*this* parameter" if the text
  is not immediately after what you are referring to, like for example
  in "pgbench", where it might refer to the -h, -p or -U options.

  I think it would be better and less ambiguous to use
  "If <replaceable class="parameter">dbname</replaceable> contains ..."

  In the cases where there is no ambiguity, it might be better to use
  a wording like in the "pg_recvlogical" documentation.

- There are two places you forgot:

  createdb --maintenance-db=dbname
  dropdb --maintenance-db=dbname

While looking at this patch, I noticed that "createuser" and "dropuser"
explicitly connect to the "postgres" database rather than using
"connectMaintenanceDatabase()" like the other scripts, which would try
the database "postgres" first and fall back to "template1".

This is unrelated to the patch, but low-hanging fruit for unified behavior.