Re: [WIP] pg_ping utility

Hi Phil,

I am currently looking at your patch.
A lot of people already had a look at at, but I hope I will be helpful in
finalizing it and hand it over to a committer.

Strangely I got the following error when using git apply:
$ git apply ~/download/pg_ping_v3.patch
error: src/bin/scripts/.gitignore: already exists in working directory
error: src/bin/scripts/Makefile: already exists in working directory
I don't really get from where this error comes from, but using patch -p1
made the trick.

So, regarding the review of the patch (v3):
1) pg_ping.c uses 4 spaces instead of 4-space tabs, which is a PostgreSQL
convention. You need to normalize your code respecting that.Take care of
not changing the help output which needs 4 spaces at some places though.
2) As mentionned by Christopher and Peter, pg_ping should perhaps not be
seen as a simple wrapper calling only once PQPing, but as something that
really enhance the libpq calls by incorporating options that could wrap it
more efficiently and give the users a tool to customize the ping to a
server easily. Hence, the following options make sense:
- c for the number of ping packets
- i for the interval between pings
- W for the time to wait for a response
- D output a timestamp at the beginning of ping line
- q quiet the output
Please also not that at the current state, we could do the same thing with
a simple "psql -c 'SELECT 1' postgres", so those additional things look
really necessary.
3) Having an output close to what ping actually does would also be nice,
the current output like Accepting/Rejecting Connections are not that
4) I have also some security concerns about pg_ping. I noticed that even a
user who is rejected by pg_hba.conf can actually ping the server using
pg_ping or PQPing. Is this a wanted behavior? Some input here?
5) You should not use comments like that:
/* Return UNKNOWN*/
Please add a space at the end of comment for clarity like this:
/* Return UNKNOWN */
6) Please use exit(1) instead of exit(3) like the other script utilities.

Thanks,
--
Michael Paquier
http://michael.otacoo.com