From: Noah Misch <noah@leadboat.com>
To: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: Fabien COELHO <coelho@cri.ensmp.fr>
Cc: PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>
Cc: Arthur Zakirov <a.zakirov@postgrespro.ru>
Cc: Robert Haas <robertmhaas@gmail.com>
Subject: Re: fix psql \conninfo & \connect when using hostaddr
Date: Mon, 27 May 2019 16:37:53 -0400
Message-ID: <20190527203713.GA58392@gust.leadboat.com> (raw)
In-Reply-To: <20181119155315.vsrgj2droozqgfqo@alvherre.pgsql>
References: <alpine.DEB.2.21.1811171703090.16105@lancre>
<20181119155315.vsrgj2droozqgfqo@alvherre.pgsql>
On Mon, Nov 19, 2018 at 12:53:15PM -0300, Alvaro Herrera wrote:
> commit 6e5f8d4
> Commit: Alvaro Herrera <alvherre@alvh.no-ip.org>
> CommitDate: Mon Nov 19 14:34:12 2018 -0300
>
> psql: Show IP address in \conninfo
> Discussion: https://postgr.es/m/alpine.DEB.2.21.1810261532380.27686@lancre
> https://postgr.es/m/alpine.DEB.2.21.1808201323020.13832@lancre> --- a/src/bin/psql/command.c
> +++ b/src/bin/psql/command.c
> @@ -2894,12 +2911,27 @@ do_connect(enum trivalue reuse_previous_specification,
> }
>
> /* grab missing values from the old connection */
> - if (!user && reuse_previous)
> - user = PQuser(o_conn);
> - if (!host && reuse_previous)
> - host = PQhost(o_conn);
> - if (!port && reuse_previous)
> - port = PQport(o_conn);
> + if (reuse_previous)
> + {
> + if (!user)
> + user = PQuser(o_conn);
> + if (host && strcmp(host, PQhost(o_conn)) == 0)
> + {
> + /*
> + * if we are targetting the same host, reuse its hostaddr for
> + * consistency
> + */
> + hostaddr = PQhostaddr(o_conn);
> + }
> + if (!host)
> + {
> + host = PQhost(o_conn);
> + /* also set hostaddr for consistency */
> + hostaddr = PQhostaddr(o_conn);
> + }
> + if (!port)
> + port = PQport(o_conn);
> + }
>
> /*
> * Any change in the parameters read above makes us discard the password.
The "hostaddr = PQhostaddr(o_conn)" branches make \connect use the same IP
address as the existing connection. I like that when the existing connection
uses a hostaddr= parameter, but I doubt it's the right thing otherwise. If
the server IP changes, \connect should find the server at its new IP. If the
server has multiple IPs, \connect should have the opportunity to try them all,
just like the original connection attempt could have.
Other than that, the \connect behavior change makes sense to me. However,
nothing updated \connect documentation. (Even the commit log message said
nothing about \connect.)
> @@ -3071,14 +3108,27 @@ do_connect(enum trivalue reuse_previous_specification,
> /* If the host is an absolute path, the connection is via socket */
This comment is no longer correct. Copy the other "via socket" comment.
> --- a/src/interfaces/libpq/fe-connect.c
> +++ b/src/interfaces/libpq/fe-connect.c
> @@ -1471,6 +1471,39 @@ connectNoDelay(PGconn *conn)
> return 1;
> }
>
> +/* ----------
> + * Write currently connected IP address into host_addr (of len host_addr_len).
> + * If unable to, set it to the empty string.
> + * ----------
> + */
> +static void
> +getHostaddr(PGconn *conn, char *host_addr, int host_addr_len)
> +{
> + struct sockaddr_storage *addr = &conn->raddr.addr;
> +
> + if (conn->connhost[conn->whichhost].type == CHT_HOST_ADDRESS)
> + strlcpy(host_addr, conn->connhost[conn->whichhost].hostaddr, host_addr_len);
I recommend removing this branch; inet_net_ntop() would work fine in the
CHT_HOST_ADDRESS case, and that has the advantage of putting the address in
standard form. Currently, hostaddr in nonstandard form stays that way
(trailing space, in this example):
[nm@gust 8:1 2019-05-23T13:21:54 postgresql 0]$ psql -X "hostaddr='127.0.7.1 '" -c '\conninfo'
You are connected to database "test" as user "nm" on host "127.0.7.1 " at port "5433".
> @@ -6173,6 +6202,25 @@ PQhost(const PGconn *conn)
> }
>
> char *
> +PQhostaddr(const PGconn *conn)
> +{
> + if (!conn)
> + return NULL;
> +
> + if (conn->connhost != NULL)
> + {
> + if (conn->connhost[conn->whichhost].hostaddr != NULL &&
> + conn->connhost[conn->whichhost].hostaddr[0] != '\0')
> + return conn->connhost[conn->whichhost].hostaddr;
> +
> + if (conn->connip != NULL)
> + return conn->connip;
> + }
> +
> + return "";
> +}
Similar to my comment on getHostaddr(), why ever use hostaddr? connip should
always be equivalent to hostaddr when hostaddr is set. (connip may be NULL if
a malloc failed, but if we really cared, we'd fail the connection attempt when
that happens. If connip differs in any other material way, I'd want the user
to see connip.)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: noah@leadboat.com, alvherre@2ndquadrant.com, coelho@cri.ensmp.fr, pgsql-hackers@lists.postgresql.org, a.zakirov@postgrespro.ru, robertmhaas@gmail.com
Subject: Re: fix psql \conninfo & \connect when using hostaddr
In-Reply-To: <20190527203713.GA58392@gust.leadboat.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox