Re: [Doc] pg_restore documentation didn't explain how to use connection string

On Fri, May 17, 2019 at 9:16 AM Lætitia Avrot <laetitia.avrot@gmail.com>
wrote:

Given that, I think it would be a good thing to generalize the -d switch
(and maybe the --maintenance-db switch too).

Just a couple of quick comments:

Some of those tools user --dbname as a long option.
Most of those tools also use the connection environment variables used
by libpq: PGDATABASE
Pgbench is documented [1]https://www.postgresql.org/docs/current/pgbench.html: pgbench [option...] [dbname]

Regards,

Juan José Santamaría Flecha

[1]: https://www.postgresql.org/docs/current/pgbench.html

Hi Juan,

Le ven. 17 mai 2019 à 11:26, Juan José Santamaría Flecha <
juanjo.santamaria@gmail.com> a écrit :

On Fri, May 17, 2019 at 9:16 AM Lætitia Avrot <laetitia.avrot@gmail.com>
wrote:

Given that, I think it would be a good thing to generalize the -d switch
(and maybe the --maintenance-db switch too).

Just a couple of quick comments:

Some of those tools user --dbname as a long option.

You're right. I checked and each and every tool that allow the -d switch
allows the --dbname. So, of course, if -d is implemented for all Postgres
client, --dbname should be allowed too.

Most of those tools also use the connection environment variables used
by libpq: PGDATABASE
Pgbench is documented [1]: pgbench [option...] [dbname]

Maybe I wasn't clear enough. My point was using a connection string is not
documented. Here is PgBench documentation about dbname:

where *dbname* is the name of the already-created database to test in.

(You may also need -h, -p, and/or -U options to specify how to connect to
the database server.)

Cheers,

Lætitia

On Fri, May 17, 2019 at 11:38 AM Lætitia Avrot <laetitia.avrot@gmail.com>
wrote:

Maybe I wasn't clear enough. My point was using a connection string is not
documented. Here is PgBench documentation about dbname:

where *dbname* is the name of the already-created database to test in.

(You may also need -h, -p, and/or -U options to specify how to connect to
the database server.)

In the "Common Options" section of PgBench you can find the connect options.

I really just wanted to make a couple of comments, I have not intention on
reviewing your proposal. So as a final note, dbname defaults to the
username if no other information is found.

Regards,

Juan José Santamaría Flecha

Maybe I wasn't clear enough. My point was using a connection string is
not documented. Here is PgBench documentation about dbname:

where *dbname* is the name of the already-created database to test in.

(You may also need -h, -p, and/or -U options to specify how to connect
to the database server.)

In the "Common Options" section of PgBench you can find the connect
options.

Still nothing about how to use a connection string:

pgbench accepts the following command-line common arguments:

-h hostname
--host=hostname
The database server's host name

-p port
--port=port
The database server's port number

-U login
--username=login
The user name to connect as

I really just wanted to make a couple of comments, I have not intention on

reviewing your proposal. So as a final note, dbname defaults to the
username if no other information is found.

I do really appreciate that you took the time and your point of view is
valuable to me.

Regards,

Lætitia

On Fri, May 17, 2019 at 12:28 PM Lætitia Avrot <laetitia.avrot@gmail.com>
wrote:

I do really appreciate that you took the time and your point of view is
valuable to me.

I did not see your original mail from the 30th, we were talking about
apples and oranges. Sorry for the noise.

I have gone though that original mail and the undocumented behaviour you
are seeing is from libpq itself, maybe not intentional at tool level.

So, if you want to resize your proposal to a more manageable scope breaking
it down at tool level might take you further, there you want to make sure
the behaviour is actually supported.

Regards,

Juan José Santamaría Flecha

Hi all,

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

You'll find it enclosed.

I just think it's too bad you can't use the same syntax with every Postgres
client using connection string. If somebody else feel the same way about
it, please jump into this thread so we can think together how to achieve
this.

Have a nice day,

Lætitia

Le ven. 17 mai 2019 à 09:16, Lætitia Avrot <laetitia.avrot@gmail.com> a
écrit :

Hi all,

It seems my approach was quite candid because, of all postgres client
applications, some document usage of connection string whereas other don't.
Then, several ways of using connection strings are involved.

Here is a little digest:

| Postgres Client Application | Connection string syntax
| Documented ? |

|-----------------------------|------------------------------------------------------------------------------------|--------------|
| clusterdb | clusterdb -d <connection_string> or
clusterdb <connection_string> | No |
| createdb | createdb --maintenance-db
<connection_string> | No |
| createuser | Couldn't find if possible
| No |
| dropdb | dropdb --maintenance-db
<connection_string> | No |
| dropuser | Couldn't find if possible
| No |
| pg_basebackup | pg_basebackup -d <connection_string>
| Yes |
| pgbench | Couldn't find if possible
| No |
| pg_dump | pg_dump -d <connection_string>
| Yes |
| pg_dumpall | pg_dumpall -d <connection_string>
| Yes |
| pg_isready | pg_isready -d <connection_string>
| Yes |
| pg_receivewal | pg_receivewal -d <connection_string>
| Yes |
| pg_recvlogical | pg_recvlogical -d <connection_string>
| Yes |
| pg_restore | pg_restore -d <connection_string>
| No |
| psql | psql <connection_string> or psql -d
<connection_string> | Yes |
| reindexdb | reindexdb -d <connection_string> or
reindexdb --maintenance-db <connection_string> | No |
| vacuumdb | vacuumdb -d <connection_string> or
vacuumdb --maintenance-db <connection_string> | No |

And here are some statistics about connection string usage:

| | Number of tool using that syntax |
|------------------|----------------------------------|
| No switch | 2 |
| -d | 11 |
| --maintenance-db | 4 |

- Both tools that allow connection strings without strings also allow the
-d switch.
- From the 4 tools that use the --maintenance-db switch, only 2 won't
allow the -d switch. Those don't have a -d switch now.

Given that, I think it would be a good thing to generalize the -d switch
(and maybe the --maintenance-db switch too).

What do you think ?

Cheers,

Lætitia

Le mar. 30 avr. 2019 à 19:10, Lætitia Avrot <laetitia.avrot@gmail.com> a
écrit :

Hi all,

I'm a big fan a service file to connect to PostgreSQL client
applications. However I know just a few people use them.

I ran into an issue today: I wanted to user pg_restore with my service
file and couldn't find a way to do so.

Documentation didn't help. It was all about "basic" options like
providing host, port, user and database... Nothing about how to connect
using a connection string.

I tried `pg_restore service=my_db <other options> <dumpfile>`, but it
didn't work. `pg_restore` complaining about too many arguments.

I had to ask people or IRC to find out that the `-d` switch accepted
connection strings.

It's really disturbing because :
- It's undocumented
- It doesn't work the way it works with the other PostgreSQL client
applications (For example, `pg_dump` will accept `pg_dump service=my_db
<other_options>`)

***I write a quick patch to document that feature***, but maybe we could
go further. I suggest :

- Creating a "Connection Options" section before the other options (as
the synopsis is pg_restore [*connection-option*...] [*option*...] [
*filename*])
- Put all connection parameters here (including the -d switch witch is
somehow in the middle of the other options
- Change other PostgreSQL client application documentation accordingly
- As a bonus, I'd like pg_restore to accept connection strings just as
other client accept them (without a switch), but maybe it's too difficult

Could you please tell me what you think about it before I make such a
huge change ?

Cheers,

Lætitia
--
*Paper doesn’t grow on trees. Please print responsibly.*

--
*Paper doesn’t grow on trees. Please print responsibly.*

--
*Paper doesn’t grow on trees. Please print responsibly.*

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.

Yours,
Laurenz Albe

Hi Laurenz,

Thank you for taking the time to review that patch.

Le lun. 25 nov. 2019 à 22:34, Laurenz Albe <laurenz.albe@cybertec.at> a
écrit :

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.

I agree.

- 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.

You're right.

- There are two places you forgot:

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

You're perfectly right!

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.

I agree and while trying to unify everything, you'r better try and make
right for all the tools.

I'm not very satisfied with this patch. I think I want to go further with
unifying connection string usage. I'd like at least each and every client
app to accept the same syntax and argument. Let me think a little further
on it, so I try to come up with a simple and neat solution.

Several ones are possible and I'd like to find them all to be able to pick
the best.

Have a nice day,

Lætitia
--
*Paper doesn’t grow on trees. Please print responsibly.*