From: Robert Haas on
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

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