Обсуждение: [bug fix] psql's \conninfo reports incorrect destination on Windows

Hello,

I've found a bug that psql's \conninfo displays incorrect information on
Windows.  Please find attached the patch and commit this.

[Problem]
When I run "psql postgres" on Windows and execute \conninfo, it outputs the
text below.  It reports that psql connected to the server via UNIX domain
socket, but the actual connection is of course via TCP/IP socket to
localhost.

You are connected to database "postgres" as user "Administrator" via socket
in "/tmp" at port "5432".

It should output:

You are connected to database "postgres" as user "Administrator" on host
"localhost" at port "5432".


[Cause]
\conninfo calls PQhost(conn) to get the destination info.  PQhost() in this
case returns NULL because conn->pghost and conn->pghostaddr are NULL.  When
\conninfo receives NULL from PQhost(), it assumes /tmp.


[Fix]
PQhost() should return the actual destination.


Regards
MauMau

On Wed, Dec 4, 2013 at 9:21 PM, MauMau <maumau307@gmail.com> wrote:
> Hello,
>
> I've found a bug that psql's \conninfo displays incorrect information on
> Windows.  Please find attached the patch and commit this.
>
> [Problem]
> When I run "psql postgres" on Windows and execute \conninfo, it outputs the
> text below.  It reports that psql connected to the server via UNIX domain
> socket, but the actual connection is of course via TCP/IP socket to
> localhost.
>
> You are connected to database "postgres" as user "Administrator" via socket
> in "/tmp" at port "5432".
>
> It should output:
>
> You are connected to database "postgres" as user "Administrator" on host
> "localhost" at port "5432".

I think that 77035fa8a92d8c39f4c689e54f46813f203f09a8 fixed this problem.
Please check that.

Regards,

-- 
Fujii Masao

From: "Fujii Masao" <masao.fujii@gmail.com>
> I think that 77035fa8a92d8c39f4c689e54f46813f203f09a8 fixed this problem.
> Please check that.

I looked through your patch.  I'm fine with the PQhostaddr().  I didn't 
notice the problem when hostaddr was passed to psql on Windows.

OTOH, regarding PQhost(), I think we had better take my patch. 
connectOptions2() computes and set derived values to PGconn, so that 
PGconn's members have values which are actually used for connection.  To 
follow that rule, PGconn->pghost may as well have "localhost" on machines 
without UNIX domain sockets.  Plus, if we want to refer to the actual 
connection target for libpq implementation in other places than PQhost(), we 
can just use PGconn's members.  If we do so, we don't have to change 
PQhost().

Could you replace the patch?

Regards
MauMau

On Fri, Jan 24, 2014 at 9:00 PM, MauMau <maumau307@gmail.com> wrote:
> From: "Fujii Masao" <masao.fujii@gmail.com>
>
>> I think that 77035fa8a92d8c39f4c689e54f46813f203f09a8 fixed this problem.
>> Please check that.
>
>
> I looked through your patch.  I'm fine with the PQhostaddr().  I didn't
> notice the problem when hostaddr was passed to psql on Windows.
>
> OTOH, regarding PQhost(), I think we had better take my patch.
> connectOptions2() computes and set derived values to PGconn, so that
> PGconn's members have values which are actually used for connection.  To
> follow that rule, PGconn->pghost may as well have "localhost" on machines
> without UNIX domain sockets.

I'm not sure if we should follow that rule. As far as I read the libpq code,
at least connectFailureMessage() and connectDBStart() do the same things
as PQhost() does now.

Regards,

-- 
Fujii Masao

From: "Fujii Masao" <masao.fujii@gmail.com>
> On Fri, Jan 24, 2014 at 9:00 PM, MauMau <maumau307@gmail.com> wrote:
>> OTOH, regarding PQhost(), I think we had better take my patch.
>> connectOptions2() computes and set derived values to PGconn, so that
>> PGconn's members have values which are actually used for connection.  To
>> follow that rule, PGconn->pghost may as well have "localhost" on machines
>> without UNIX domain sockets.
>
> I'm not sure if we should follow that rule. As far as I read the libpq 
> code,
> at least connectFailureMessage() and connectDBStart() do the same things
> as PQhost() does now.

Yes, I found them ugly, too.  Maybe we can refactor them as a separate 
patch.

Regards
MauMau