Patch: psql \whoami option

Started by David Christensenabout 16 years ago40 messageshackers
Jump to latest
#1David Christensen
david@endpoint.com

-hackers,

In the spirit of small, but hopefully useful interface improvement
patches, enclosed for your review is a patch for providing psql with a
\whoami command (maybe a better name is \conninfo or similar). Its
purpose is to print information about the current connection, by
default in a human-readable format. There is also an optional format
parameter which currently accepts 'dsn' as an option to output the
current connection information as a DSN.

Example output:

$psql -d postgres -p 8555
psql (8.5devel)
You are now connected to database "postgres".

[Tue Jan 26 17:17:31 CST 2010]
machack:postgres:8555=# \whoami
Connected to database: "postgres", user: "machack", port: "8555"
via local domain socket

[Tue Jan 26 17:17:34 CST 2010]
machack:postgres:8555=# \c - - localhost 8555
psql (8.5devel)
You are now connected to database "postgres" on host "localhost".

[Tue Jan 26 17:17:42 CST 2010]
machack:postgres:8555=# \whoami
Connected to database: "postgres", user: "machack", host:
"localhost", port: "8555"

[Tue Jan 26 17:17:46 CST 2010]
machack:postgres:8555=# \whoami dsn
dbname=postgres;user=machack;host=localhost;port=8555

[Tue Jan 26 17:19:02 CST 2010]
machack:postgres:8555=# \q

Regards,

David
--
David Christensen
End Point Corporation
david@endpoint.com

diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql- 
ref.sgml
index 3ce5996..b58b24d 100644
*** a/doc/src/sgml/ref/psql-ref.sgml
--- b/doc/src/sgml/ref/psql-ref.sgml
*************** lo_import 152801
*** 2149,2154 ****
--- 2149,2167 ----
         <varlistentry>
+         <term><literal>\whoami</literal> [ <replaceable  
class="parameter">default</replaceable> | <replaceable  
class="parameter">dsn</replaceable> ] </term>
+         <listitem>
+         <para>
+         Outputs connection information about the current database
+         connection.  When passed parameter <literal>dsn</literal>,
+         outputs as a DSN.  If parameter is unspecified or
+         unrecognized, outputs in a human-readable format.
+         </para>
+         </listitem>
+       </varlistentry>
+
+
+       <varlistentry>
           <term><literal>\x</literal></term>
           <listitem>
           <para>
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 5188b18..21b2468 100644
*** a/src/bin/psql/command.c
--- b/src/bin/psql/command.c
*************** exec_command(const char *cmd,
*** 1106,1111 ****
--- 1106,1156 ----
   		free(fname);
   	}
+ 	/* \whoami -- display information about the current connection	*/
+ 	else if (strcmp(cmd, "whoami") == 0)
+ 	{
+ 		char	   *format = psql_scan_slash_option(scan_state,
+ 													OT_NORMAL, NULL, true);
+ 		char	   *host = PQhost(pset.db);
+
+ 		if (format && !pg_strcasecmp(format, "dsn")) {
+ 			if (host) {
+ 				printf("dbname=%s;user=%s;host=%s;port=%s\n",
+ 					   PQdb(pset.db),
+ 					   PQuser(pset.db),
+ 					   host,
+ 					   PQport(pset.db)
+ 					);
+ 			}
+ 			else {
+ 				printf("dbname=%s;user=%s;port=%s\n",
+ 					   PQdb(pset.db),
+ 					   PQuser(pset.db),
+ 					   PQport(pset.db)
+ 					);
+ 			}
+ 		}
+ 		else {
+ 			/* default case */
+ 			if (host) {
+ 				printf("Connected to database: \"%s\", user: \"%s\", host: \"%s 
\", port: \"%s\"\n",
+ 					   PQdb(pset.db),
+ 					   PQuser(pset.db),
+ 					   host,
+ 					   PQport(pset.db)
+ 					);
+ 			}
+ 			else {
+ 				printf("Connected to database: \"%s\", user: \"%s\", port: \"%s 
\" via local domain socket\n",
+ 					   PQdb(pset.db),
+ 					   PQuser(pset.db),
+ 					   PQport(pset.db)
+ 					);
+ 			}
+ 		}
+ 		free(format);
+ 	}
+
   	/* \x -- toggle expanded table representation */
   	else if (strcmp(cmd, "x") == 0)
   	{
diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
index 6037351..802b76d 100644
*** a/src/bin/psql/help.c
--- b/src/bin/psql/help.c
*************** slashUsage(unsigned short int pager)
*** 249,254 ****
--- 249,256 ----
   			PQdb(pset.db));
   	fprintf(output, _("  \\encoding [ENCODING]   show or set client  
encoding\n"));
   	fprintf(output, _("  \\password [USERNAME]   securely change the  
password for a user\n"));
+ 	fprintf(output, _("  \\whoami [FORMAT]       display information  
about current connection\n"
+ 	                  "                         (FORMAT := {default| 
dsn})\n"));
   	fprintf(output, "\n");
   	fprintf(output, _("Operating System\n"));
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index cb2ae9a..952d2bc 100644
*** a/src/bin/psql/tab-complete.c
--- b/src/bin/psql/tab-complete.c
*************** psql_completion(char *text, int start, i
*** 635,641 ****
   		"\\lo_import", "\\lo_export", "\\lo_list", "\\lo_unlink",
   		"\\o", "\\p", "\\password", "\\prompt", "\\pset", "\\q", "\ 
\qecho", "\\r",
   		"\\set", "\\t", "\\T",
! 		"\\timing", "\\unset", "\\x", "\\w", "\\z", "\\!", NULL
   	};
   	(void) end;					/* not used */
--- 635,641 ----
   		"\\lo_import", "\\lo_export", "\\lo_list", "\\lo_unlink",
   		"\\o", "\\p", "\\password", "\\prompt", "\\pset", "\\q", "\ 
\qecho", "\\r",
   		"\\set", "\\t", "\\T",
! 		"\\timing", "\\unset", "\\x", "\\w", "\\whoami", "\\z", "\\!", NULL
   	};

(void) end; /* not used */

#2Josh Berkus
josh@agliodbs.com
In reply to: David Christensen (#1)
Re: Patch: psql \whoami option

On 1/26/10 3:24 PM, David Christensen wrote:

-hackers,

In the spirit of small, but hopefully useful interface improvement
patches, enclosed for your review is a patch for providing psql with a
\whoami command (maybe a better name is \conninfo or similar). Its
purpose is to print information about the current connection, by default
in a human-readable format. There is also an optional format parameter
which currently accepts 'dsn' as an option to output the current
connection information as a DSN.

oooh, I could really use this. +1 to put it in 9.1-first CF.

however, \conninfo is probably the better name. And what about a
postgresql function version for non-psql connections?

--Josh Berkus

#3Magnus Hagander
magnus@hagander.net
In reply to: Josh Berkus (#2)
Re: Patch: psql \whoami option

2010/1/27 Josh Berkus <josh@agliodbs.com>:

On 1/26/10 3:24 PM, David Christensen wrote:

-hackers,

In the spirit of small, but hopefully useful interface improvement
patches, enclosed for your review is a patch for providing psql with a
\whoami command (maybe a better name is \conninfo or similar).  Its
purpose is to print information about the current connection, by default
in a human-readable format.  There is also an optional format parameter
which currently accepts 'dsn' as an option to output the current
connection information as a DSN.

On a first note, it seems like the check for the parameter "dsn" isn't
"complete". Without testing it, it looks like it would be possible to
run "\whoami foobar", which should give an error.

oooh, I could really use this.  +1 to put it in 9.1-first CF.

however, \conninfo is probably the better name.  And what about a

+1 on that name.

postgresql function version for non-psql connections?

How could that function possibly know what the connection looks like
from the client side? Think NAT, think proxies, think connection
poolers.

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

#4Martin Atukunda
matlads@gmail.com
In reply to: Magnus Hagander (#3)
Re: Patch: psql \whoami option

How about using the psql prompt to convey this information? IIRC the
psql prompt can be configured to show the hostname, server, port and
other fields. Wouldn't this be enough? or am I missing something?

- Martin -

On 27 Jan 2010, at 13:01, Magnus Hagander wrote:

Show quoted text

2010/1/27 Josh Berkus <josh@agliodbs.com>:

On 1/26/10 3:24 PM, David Christensen wrote:

-hackers,

In the spirit of small, but hopefully useful interface improvement
patches, enclosed for your review is a patch for providing psql
with a
\whoami command (maybe a better name is \conninfo or similar). Its
purpose is to print information about the current connection, by
default
in a human-readable format. There is also an optional format
parameter
which currently accepts 'dsn' as an option to output the current
connection information as a DSN.

On a first note, it seems like the check for the parameter "dsn" isn't
"complete". Without testing it, it looks like it would be possible to
run "\whoami foobar", which should give an error.

oooh, I could really use this. +1 to put it in 9.1-first CF.

however, \conninfo is probably the better name. And what about a

+1 on that name.

postgresql function version for non-psql connections?

How could that function possibly know what the connection looks like
from the client side? Think NAT, think proxies, think connection
poolers.

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#5Magnus Hagander
magnus@hagander.net
In reply to: Martin Atukunda (#4)
Re: Patch: psql \whoami option

I think the idea is that if you do that, it'll be there all the time,
potentially "crowding the space".

//Magnus

2010/1/27 Martin Atukunda <matlads@gmail.com>:

How about using the psql prompt to convey this information? IIRC the psql prompt can be configured to show the hostname, server, port and other fields. Wouldn't this be enough? or am I missing something?

- Martin -

On 27 Jan 2010, at 13:01, Magnus Hagander wrote:

2010/1/27 Josh Berkus <josh@agliodbs.com>:

On 1/26/10 3:24 PM, David Christensen wrote:

-hackers,

In the spirit of small, but hopefully useful interface improvement
patches, enclosed for your review is a patch for providing psql with a
\whoami command (maybe a better name is \conninfo or similar).  Its
purpose is to print information about the current connection, by default
in a human-readable format.  There is also an optional format parameter
which currently accepts 'dsn' as an option to output the current
connection information as a DSN.

On a first note, it seems like the check for the parameter "dsn" isn't
"complete". Without testing it, it looks like it would be possible to
run "\whoami foobar", which should give an error.

oooh, I could really use this.  +1 to put it in 9.1-first CF.

however, \conninfo is probably the better name.  And what about a

+1 on that name.

postgresql function version for non-psql connections?

How could that function possibly know what the connection looks like
from the client side? Think NAT, think proxies, think connection
poolers.

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

#6Thom Brown
thombrown@gmail.com
In reply to: Josh Berkus (#2)
Re: Patch: psql \whoami option

2010/1/27 Josh Berkus <josh@agliodbs.com>

however, \conninfo is probably the better name.

+1

Something along the lines of: "Connected to localhost port 5432 as user
thomb"?

Thom

#7Thom Brown
thombrown@gmail.com
In reply to: Thom Brown (#6)
Re: Patch: psql \whoami option

2010/1/27 Thom Brown <thombrown@gmail.com>

Show quoted text

2010/1/27 Josh Berkus <josh@agliodbs.com>

however, \conninfo is probably the better name.

+1

Something along the lines of: "Connected to localhost port 5432 as user
thomb"?

Thom

Er... ignore that. Just saw the other examples which are better ;)

#8David Christensen
david@endpoint.com
In reply to: Magnus Hagander (#3)
Re: Patch: psql \whoami option

On Jan 27, 2010, at 4:01 AM, Magnus Hagander wrote:

2010/1/27 Josh Berkus <josh@agliodbs.com>:

On 1/26/10 3:24 PM, David Christensen wrote:

-hackers,

In the spirit of small, but hopefully useful interface improvement
patches, enclosed for your review is a patch for providing psql
with a
\whoami command (maybe a better name is \conninfo or similar). Its
purpose is to print information about the current connection, by
default
in a human-readable format. There is also an optional format
parameter
which currently accepts 'dsn' as an option to output the current
connection information as a DSN.

On a first note, it seems like the check for the parameter "dsn" isn't
"complete". Without testing it, it looks like it would be possible to
run "\whoami foobar", which should give an error.

Yeah, I debated that; right now, it just ignores any output it doesn't
know about and spits out the human-readable format.

oooh, I could really use this. +1 to put it in 9.1-first CF.

however, \conninfo is probably the better name. And what about a

+1 on that name.

That makes at least three, including me. :-)

postgresql function version for non-psql connections?

How could that function possibly know what the connection looks like
from the client side? Think NAT, think proxies, think connection
poolers.

Yes, this doesn't seem to be a feasible thing to detect in all (many?)
cases.

Regards,

David
--
David Christensen
End Point Corporation
david@endpoint.com

#9David Christensen
david@endpoint.com
In reply to: Martin Atukunda (#4)
Re: Patch: psql \whoami option

On Jan 27, 2010, at 5:23 AM, Martin Atukunda wrote:

How about using the psql prompt to convey this information? IIRC the
psql prompt can be configured to show the hostname, server, port and
other fields. Wouldn't this be enough? or am I missing something?

Prompt customization is certainly something that could be done (and I
use in my .psqlrc), but consider someone unaware of the psql prompt
customization or people who are not using their own setup/account,
etc. This is a command that could be useful for anyone; as experts,
we tend to miss some of the holes in the current interfaces.

Regards,

David
--
David Christensen
End Point Corporation
david@endpoint.com

#10Magnus Hagander
magnus@hagander.net
In reply to: David Christensen (#8)
Re: Patch: psql \whoami option

2010/1/27 David Christensen <david@endpoint.com>:

On Jan 27, 2010, at 4:01 AM, Magnus Hagander wrote:

2010/1/27 Josh Berkus <josh@agliodbs.com>:

On 1/26/10 3:24 PM, David Christensen wrote:

-hackers,

In the spirit of small, but hopefully useful interface improvement
patches, enclosed for your review is a patch for providing psql with a
\whoami command (maybe a better name is \conninfo or similar).  Its
purpose is to print information about the current connection, by default
in a human-readable format.  There is also an optional format parameter
which currently accepts 'dsn' as an option to output the current
connection information as a DSN.

On a first note, it seems like the check for the parameter "dsn" isn't
"complete". Without testing it, it looks like it would be possible to
run "\whoami foobar", which should give an error.

Yeah, I debated that; right now, it just ignores any output it doesn't know about and spits out the human-readable format.

yeah, that's not very forwards-compatible. Someone uses it in the
wrong way, and suddenly their stuff gets broken if we choose to modify
it in the future. If we say we're only going ot accept two options,
let's enforce that and show an error/help message if the user typos.

Guessing is not what we do :-) We leave that to other databases...

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

#11David Christensen
david@endpoint.com
In reply to: Magnus Hagander (#10)
Re: Patch: psql \whoami option

On Jan 27, 2010, at 8:08 AM, Magnus Hagander wrote:

2010/1/27 David Christensen <david@endpoint.com>:

On Jan 27, 2010, at 4:01 AM, Magnus Hagander wrote:

2010/1/27 Josh Berkus <josh@agliodbs.com>:

On 1/26/10 3:24 PM, David Christensen wrote:

-hackers,

In the spirit of small, but hopefully useful interface improvement
patches, enclosed for your review is a patch for providing psql
with a
\whoami command (maybe a better name is \conninfo or similar).
Its
purpose is to print information about the current connection, by
default
in a human-readable format. There is also an optional format
parameter
which currently accepts 'dsn' as an option to output the current
connection information as a DSN.

On a first note, it seems like the check for the parameter "dsn"
isn't
"complete". Without testing it, it looks like it would be possible
to
run "\whoami foobar", which should give an error.

Yeah, I debated that; right now, it just ignores any output it
doesn't know about and spits out the human-readable format.

yeah, that's not very forwards-compatible. Someone uses it in the
wrong way, and suddenly their stuff gets broken if we choose to modify
it in the future. If we say we're only going ot accept two options,
let's enforce that and show an error/help message if the user typos.

That's a good point about forward-compatibility. In that case, I'm
not sure if "default" is the best name for the human-readable format,
but I didn't like "human-readable" ;-). I assume that should have an
explicit spelling, and not just be the format that we get if we don't
otherwise specify. Ideas, anyone?

Regards,

David
--
David Christensen
End Point Corporation
david@endpoint.com

#12Tom Lane
tgl@sss.pgh.pa.us
In reply to: David Christensen (#11)
Re: Patch: psql \whoami option

David Christensen <david@endpoint.com> writes:

That's a good point about forward-compatibility. In that case, I'm
not sure if "default" is the best name for the human-readable format,
but I didn't like "human-readable" ;-). I assume that should have an
explicit spelling, and not just be the format that we get if we don't
otherwise specify. Ideas, anyone?

I think this patch has near zero usecase already, and more than one
output format is *definitely* a waste of time. Forget the argument and
just print the "human readable" format.

regards, tom lane

#13Tom Lane
tgl@sss.pgh.pa.us
In reply to: Magnus Hagander (#5)
Re: Patch: psql \whoami option

Magnus Hagander <magnus@hagander.net> writes:

2010/1/27 Martin Atukunda <matlads@gmail.com>:

How about using the psql prompt to convey this information?

I think the idea is that if you do that, it'll be there all the time,
potentially "crowding the space".

I had the same reaction as Martin. If you want this info all the time,
setting the prompt is the way to go. If you only want it occasionally,
you're probably more likely to remember other ways of getting the info
first (session_user, pg_stat_activity, etc etc). There may be some
use-case in between where another backslash command would be helpful,
but it seems a tad marginal to me. I don't object as long as it's not
overdesigned, but let's keep the bells and whistles to a minimum.

regards, tom lane

#14Bruce Momjian
bruce@momjian.us
In reply to: David Christensen (#1)
Re: Patch: psql \whoami option

I have added this patch to the next commit-fest. Thanks:

https://commitfest.postgresql.org/action/commitfest_view?id=6

---------------------------------------------------------------------------

David Christensen wrote:

-hackers,

In the spirit of small, but hopefully useful interface improvement
patches, enclosed for your review is a patch for providing psql with a
\whoami command (maybe a better name is \conninfo or similar). Its
purpose is to print information about the current connection, by
default in a human-readable format. There is also an optional format
parameter which currently accepts 'dsn' as an option to output the
current connection information as a DSN.

Example output:

$psql -d postgres -p 8555
psql (8.5devel)
You are now connected to database "postgres".

[Tue Jan 26 17:17:31 CST 2010]
machack:postgres:8555=# \whoami
Connected to database: "postgres", user: "machack", port: "8555"
via local domain socket

[Tue Jan 26 17:17:34 CST 2010]
machack:postgres:8555=# \c - - localhost 8555
psql (8.5devel)
You are now connected to database "postgres" on host "localhost".

[Tue Jan 26 17:17:42 CST 2010]
machack:postgres:8555=# \whoami
Connected to database: "postgres", user: "machack", host:
"localhost", port: "8555"

[Tue Jan 26 17:17:46 CST 2010]
machack:postgres:8555=# \whoami dsn
dbname=postgres;user=machack;host=localhost;port=8555

[Tue Jan 26 17:19:02 CST 2010]
machack:postgres:8555=# \q

Regards,

David
--
David Christensen
End Point Corporation
david@endpoint.com

diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql- 
ref.sgml
index 3ce5996..b58b24d 100644
*** a/doc/src/sgml/ref/psql-ref.sgml
--- b/doc/src/sgml/ref/psql-ref.sgml
*************** lo_import 152801
*** 2149,2154 ****
--- 2149,2167 ----
<varlistentry>
+         <term><literal>\whoami</literal> [ <replaceable  
class="parameter">default</replaceable> | <replaceable  
class="parameter">dsn</replaceable> ] </term>
+         <listitem>
+         <para>
+         Outputs connection information about the current database
+         connection.  When passed parameter <literal>dsn</literal>,
+         outputs as a DSN.  If parameter is unspecified or
+         unrecognized, outputs in a human-readable format.
+         </para>
+         </listitem>
+       </varlistentry>
+
+
+       <varlistentry>
<term><literal>\x</literal></term>
<listitem>
<para>
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 5188b18..21b2468 100644
*** a/src/bin/psql/command.c
--- b/src/bin/psql/command.c
*************** exec_command(const char *cmd,
*** 1106,1111 ****
--- 1106,1156 ----
free(fname);
}
+ 	/* \whoami -- display information about the current connection	*/
+ 	else if (strcmp(cmd, "whoami") == 0)
+ 	{
+ 		char	   *format = psql_scan_slash_option(scan_state,
+ 													OT_NORMAL, NULL, true);
+ 		char	   *host = PQhost(pset.db);
+
+ 		if (format && !pg_strcasecmp(format, "dsn")) {
+ 			if (host) {
+ 				printf("dbname=%s;user=%s;host=%s;port=%s\n",
+ 					   PQdb(pset.db),
+ 					   PQuser(pset.db),
+ 					   host,
+ 					   PQport(pset.db)
+ 					);
+ 			}
+ 			else {
+ 				printf("dbname=%s;user=%s;port=%s\n",
+ 					   PQdb(pset.db),
+ 					   PQuser(pset.db),
+ 					   PQport(pset.db)
+ 					);
+ 			}
+ 		}
+ 		else {
+ 			/* default case */
+ 			if (host) {
+ 				printf("Connected to database: \"%s\", user: \"%s\", host: \"%s 
\", port: \"%s\"\n",
+ 					   PQdb(pset.db),
+ 					   PQuser(pset.db),
+ 					   host,
+ 					   PQport(pset.db)
+ 					);
+ 			}
+ 			else {
+ 				printf("Connected to database: \"%s\", user: \"%s\", port: \"%s 
\" via local domain socket\n",
+ 					   PQdb(pset.db),
+ 					   PQuser(pset.db),
+ 					   PQport(pset.db)
+ 					);
+ 			}
+ 		}
+ 		free(format);
+ 	}
+
/* \x -- toggle expanded table representation */
else if (strcmp(cmd, "x") == 0)
{
diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
index 6037351..802b76d 100644
*** a/src/bin/psql/help.c
--- b/src/bin/psql/help.c
*************** slashUsage(unsigned short int pager)
*** 249,254 ****
--- 249,256 ----
PQdb(pset.db));
fprintf(output, _("  \\encoding [ENCODING]   show or set client  
encoding\n"));
fprintf(output, _("  \\password [USERNAME]   securely change the  
password for a user\n"));
+ 	fprintf(output, _("  \\whoami [FORMAT]       display information  
about current connection\n"
+ 	                  "                         (FORMAT := {default| 
dsn})\n"));
fprintf(output, "\n");
fprintf(output, _("Operating System\n"));
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index cb2ae9a..952d2bc 100644
*** a/src/bin/psql/tab-complete.c
--- b/src/bin/psql/tab-complete.c
*************** psql_completion(char *text, int start, i
*** 635,641 ****
"\\lo_import", "\\lo_export", "\\lo_list", "\\lo_unlink",
"\\o", "\\p", "\\password", "\\prompt", "\\pset", "\\q", "\ 
\qecho", "\\r",
"\\set", "\\t", "\\T",
! 		"\\timing", "\\unset", "\\x", "\\w", "\\z", "\\!", NULL
};
(void) end;					/* not used */
--- 635,641 ----
"\\lo_import", "\\lo_export", "\\lo_list", "\\lo_unlink",
"\\o", "\\p", "\\password", "\\prompt", "\\pset", "\\q", "\ 
\qecho", "\\r",
"\\set", "\\t", "\\T",
! 		"\\timing", "\\unset", "\\x", "\\w", "\\whoami", "\\z", "\\!", NULL
};

(void) end; /* not used */

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

#15Steve Singer
steve@ssinger.info
In reply to: David Christensen (#1)
Re: Patch: psql \whoami option

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

#16Robert Haas
robertmhaas@gmail.com
In reply to: Steve Singer (#15)
Re: Patch: psql \whoami option

On Sun, Jun 20, 2010 at 10:51 PM, Steve Singer <ssinger_pg@sympatico.ca> wrote:

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.

It seems like for user and database it might be sensible to apply
PQescapeIdentifier to the value before printing it. This will
double-quote it and escape any internal double-quotes appropriately.
The port is, I guess, being stored as a string, but doesn't it have to
be an integer? In which case, why quote it at all?

Is there really a point to the non-DSN format or should we just use
the DSN format always?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

#17Tom Lane
tgl@sss.pgh.pa.us
In reply to: Robert Haas (#16)
Re: Patch: psql \whoami option

Robert Haas <robertmhaas@gmail.com> writes:

On Sun, Jun 20, 2010 at 10:51 PM, Steve Singer <ssinger_pg@sympatico.ca> wrote:

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.

This is the same as standard practice in just about every other
message...

It seems like for user and database it might be sensible to apply
PQescapeIdentifier to the value before printing it.

I think this would actually be a remarkably bad idea in this particular
instance, because in the majority of cases psql does not apply
identifier dequoting rules to user and database names. What is printed
should be the same as what you'd need to give to \connect, for example.

The port is, I guess, being stored as a string, but doesn't it have to
be an integer? In which case, why quote it at all?

Agreed, no need for quotes there.

regards, tom lane

#18Tom Lane
tgl@sss.pgh.pa.us
In reply to: Robert Haas (#16)
Re: Patch: psql \whoami option

Robert Haas <robertmhaas@gmail.com> writes:

Is there really a point to the non-DSN format or should we just use
the DSN format always?

BTW, didn't have an opinion on that to start with, but after thinking
about it I'd turn it around. psql doesn't deal in DSN format anywhere
else, so why should it do so here? To make the point more obvious,
what's the justification for printing DSN format and not, say, JDBC URL
format? I'd vote for removing the DSN printout option, not the other
way round. If there was some mechanically readable format to offer to
print, it would be conninfo string format, which you can actually use
with psql if you have a mind to.

regards, tom lane

#19David Christensen
david@endpoint.com
In reply to: Tom Lane (#17)
Re: Patch: psql \whoami option

On Jun 21, 2010, at 9:00 AM, Tom Lane wrote:

Robert Haas <robertmhaas@gmail.com> writes:

On Sun, Jun 20, 2010 at 10:51 PM, Steve Singer <ssinger_pg@sympatico.ca> wrote:

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.

This is the same as standard practice in just about every other
message...

It seems like for user and database it might be sensible to apply
PQescapeIdentifier to the value before printing it.

I think this would actually be a remarkably bad idea in this particular
instance, because in the majority of cases psql does not apply
identifier dequoting rules to user and database names. What is printed
should be the same as what you'd need to give to \connect, for example.

So I'm not quite sure how the above two paragraphs resolve? Should the user/database names be quoted or not? I have a new version of this patch available which has incorporated the feedback to this point?

As an example of the current behavior, consider:

machack:machack:5432=# create database "foo""bar"
machack-# ;
CREATE DATABASE

[Sun Jul 18 12:14:49 CDT 2010]
machack:machack:5432=# \c foo"bar
unterminated quoted string
You are now connected to database "machack".

[Sun Jul 18 12:14:53 CDT 2010]
machack:machack:5432=# \c "foo"bar"
unterminated quoted string
You are now connected to database "machack".

[Sun Jul 18 12:14:59 CDT 2010]
machack:machack:5432=# \c "foo""bar"
You are now connected to database "foo"bar".

As you can see, the value passed to connect differs from the output in the "connected to database" string.

Regards,

David
--
David Christensen
End Point Corporation
david@endpoint.com

#20David Christensen
david@endpoint.com
In reply to: David Christensen (#19)
Re: Patch: psql \whoami option

On Jul 18, 2010, at 12:17 PM, David Christensen wrote:

On Jun 21, 2010, at 9:00 AM, Tom Lane wrote:

Robert Haas <robertmhaas@gmail.com> writes:

On Sun, Jun 20, 2010 at 10:51 PM, Steve Singer <ssinger_pg@sympatico.ca> wrote:

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.

This is the same as standard practice in just about every other
message...

It seems like for user and database it might be sensible to apply
PQescapeIdentifier to the value before printing it.

I think this would actually be a remarkably bad idea in this particular
instance, because in the majority of cases psql does not apply
identifier dequoting rules to user and database names. What is printed
should be the same as what you'd need to give to \connect, for example.

So I'm not quite sure how the above two paragraphs resolve? Should the user/database names be quoted or not? I have a new version of this patch available which has incorporated the feedback to this point?

As an example of the current behavior, consider:

machack:machack:5432=# create database "foo""bar"
machack-# ;
CREATE DATABASE

[Sun Jul 18 12:14:49 CDT 2010]
machack:machack:5432=# \c foo"bar
unterminated quoted string
You are now connected to database "machack".

[Sun Jul 18 12:14:53 CDT 2010]
machack:machack:5432=# \c "foo"bar"
unterminated quoted string
You are now connected to database "machack".

[Sun Jul 18 12:14:59 CDT 2010]
machack:machack:5432=# \c "foo""bar"
You are now connected to database "foo"bar".

As you can see, the value passed to connect differs from the output in the "connected to database" string.

It's helpful when you attach said patch. This has been rebased to current HEAD.

Regards,

David
--
David Christensen
End Point Corporation
david@endpoint.com

Attachments:

psql-conninfo-v2.patchapplication/octet-stream; name=psql-conninfo-v2.patch; x-unix-mode=0644Download+39-4
#21Tom Lane
tgl@sss.pgh.pa.us
In reply to: David Christensen (#19)
#22David Christensen
david@endpoint.com
In reply to: Tom Lane (#21)
#23David Christensen
david@endpoint.com
In reply to: David Christensen (#22)
#24Steve Singer
steve@ssinger.info
In reply to: David Christensen (#20)
#25Robert Haas
robertmhaas@gmail.com
In reply to: David Christensen (#23)
#26David Christensen
david@endpoint.com
In reply to: Robert Haas (#25)
#27Robert Haas
robertmhaas@gmail.com
In reply to: David Christensen (#26)
#28David Christensen
david@endpoint.com
In reply to: Robert Haas (#25)
#29Robert Haas
robertmhaas@gmail.com
In reply to: David Christensen (#28)
#30Robert Haas
robertmhaas@gmail.com
In reply to: Robert Haas (#29)
#31David Christensen
david@endpoint.com
In reply to: Robert Haas (#30)
#32Robert Haas
robertmhaas@gmail.com
In reply to: David Christensen (#31)
#33Fujii Masao
masao.fujii@gmail.com
In reply to: Robert Haas (#32)
#34Robert Haas
robertmhaas@gmail.com
In reply to: Fujii Masao (#33)
#35Fujii Masao
masao.fujii@gmail.com
In reply to: Robert Haas (#34)
#36David Christensen
david@endpoint.com
In reply to: Fujii Masao (#35)
#37Robert Haas
robertmhaas@gmail.com
In reply to: Fujii Masao (#35)
#38Robert Haas
robertmhaas@gmail.com
In reply to: David Christensen (#36)
#39Fujii Masao
masao.fujii@gmail.com
In reply to: Robert Haas (#37)
#40Robert Haas
robertmhaas@gmail.com
In reply to: Fujii Masao (#39)