Prev: [COMMITTERS] pgsql: pgindent run for 9.0, second run
Next: psql \conninfo command (was: Patch: psql \whoami option)
From: Robert Haas on 19 Jul 2010 23:34 On Sun, Jul 18, 2010 at 2:00 PM, David Christensen <david(a)endpoint.com> wrote: > Updated the commitfest entry with the patch, updated the title to reflect the actual name of the command, and marked as ready for committer. I took a look at this patch. One problem is that it doesn't handle the case where there is no database connection (for example, shut down the database with pg_ctl, then do select 1, then do \conninfo). I've fixed that in the attached version. However, I'm also wondering about the output format. I feel like it would make sense to use a format similar to what we do for \c, which looks like this: You are now connected to database "%s". It prints out more parameters if they've changed. The longest possible version is: You are now connected to database "%s" on host "%s" at port "%s" as user "%s". My suggestion is that we use the same format, except (1) always include all the components, since that's the point; (2) don't include the word "now"; and (3) if there is no host, then print "via local socket" rather than on host...port.... So where the current patch prints: Connected to database: "rhaas", user: "rhaas", port: 5432 via local domain socket I would propose to print instead: You are connected to database "rhaas" via local socket as user "rhaas". If people strongly prefer the way the patch does it now, I don't think it's horrible... but it seems like it would be nicer to be somewhat consistent with the existing message. Thoughts? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres Company
From: David Christensen on 19 Jul 2010 23:41 On Jul 19, 2010, at 10:34 PM, Robert Haas wrote: > On Sun, Jul 18, 2010 at 2:00 PM, David Christensen <david(a)endpoint.com> wrote: >> Updated the commitfest entry with the patch, updated the title to reflect the actual name of the command, and marked as ready for committer. > > I took a look at this patch. One problem is that it doesn't handle > the case where there is no database connection (for example, shut down > the database with pg_ctl, then do select 1, then do \conninfo). I've > fixed that in the attached version. Thanks, I hadn't considered that case. > However, I'm also wondering about the output format. I feel like it > would make sense to use a format similar to what we do for \c, which > looks like this: > > You are now connected to database "%s". > > It prints out more parameters if they've changed. The longest > possible version is: > > You are now connected to database "%s" on host "%s" at port "%s" as user "%s". > > My suggestion is that we use the same format, except (1) always > include all the components, since that's the point; (2) don't include > the word "now"; and (3) if there is no host, then print "via local > socket" rather than on host...port.... So where the current patch > prints: > > Connected to database: "rhaas", user: "rhaas", port: 5432 via local > domain socket > > I would propose to print instead: > > You are connected to database "rhaas" via local socket as user "rhaas". > > If people strongly prefer the way the patch does it now, I don't think > it's horrible... but it seems like it would be nicer to be somewhat > consistent with the existing message. Thoughts? +1 from me; I don't care what color the bikeshed is, as long as it gets the point across, which this does, and is consistent to boot. Regards, David -- David Christensen End Point Corporation david(a)endpoint.com -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Robert Haas on 19 Jul 2010 23:55 On Mon, Jul 19, 2010 at 11:41 PM, David Christensen <david(a)endpoint.com> wrote: >> I took a look at this patch. �One problem is that it doesn't handle >> the case where there is no database connection (for example, shut down >> the database with pg_ctl, then do select 1, then do \conninfo). �I've >> fixed that in the attached version. > > Thanks, I hadn't considered that case. For some reason, I end up crashing a lot of databases during development (must be my lousy coding). So I've had occasion to run across this, ahem, a few times... > +1 from me; I don't care what color the bikeshed is, as long as it gets the point across, which this does, and is consistent to boot. Great, committed that way. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres Company -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: David Christensen on 20 Jul 2010 00:02 > I would propose to print instead: > > You are connected to database "rhaas" via local socket as user "rhaas". One minor quibble here; you lose the ability to see which pg instance you're running on if there are multiple ones running on different local sockets, so maybe either the port or the socket path should show up here still. Regards, David -- David Christensen End Point Corporation david(a)endpoint.com -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Robert Haas on 20 Jul 2010 00:07
On Tue, Jul 20, 2010 at 12:02 AM, David Christensen <david(a)endpoint.com> wrote: >> I would propose to print instead: >> >> You are connected to database "rhaas" via local socket as user "rhaas". > > > One minor quibble here; you lose the ability to see which pg instance you're running on if there are multiple ones running on different local sockets, so maybe either the port or the socket path should show up here still. Doh. Will fix. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres Company -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers |