This is a review for the \whoami patch (changed to \conninfo).
This review was done on the Feb 2 2010 version of the patch (rebased to
head) that reflects some of the feedback from -hackers on the initial
submission. The commitfest entry should be updated to reflect the most
recent version of this patch that David emailed to me.
Content & Purpose
========================
The patch adds a \conninfo command to psql to print connection information
for the current connection. The patch includes documentation updates but no
regression test changes. I don't see regression tests for other psql '\'
commands so I don't think they are required in this case either.
Usability Review
==========================
The initial discussion on -hackers recommened renaming the command to
\conninfo which was done.
One comment I have on the output format is that values (ie the database
name) are enclosed in double quotes but the values being quoted can contain
double quotes that are not being escaped. For example
Connected to database: "testing"er", user: "ssinger", port: "5432" via local
domain socket
(where my database name is testing"er ). Programs will have a hard time
parsing this. I'm not sure if this is a valid concern but I'm mentioning
it.
Initial Run
==============
Connecting both through tcp/ip and unix domain sockets produces valid
\conninfo output. The regression tests pass when the patch is applied.
Performance
=============
I see no performance implications of this patch.
Code & Nitpicking
================
in command.c you have the opening brace on the same line as the if. See
"if (host) {"
and the associated "else {"
The block " else if (strcmp(cmd, "conninfo") == 0)" is in between the
commands "\c" and "\cd" it looks like the commands are ordered
alphabetically. Wouldn't conninfo fit in after "\cd" but before "\copy"
In help.c you don't update the row count at the top of slashUsage() per the
comment you should increment it.
Other than those issues the patch looks fine.
Steve