Sorry for late answer.

On 9/30/18 10:21 AM, Fabien COELHO wrote:
sh> psql "host=127.0.0.2 hostaddr=127.0.0.1"

I'm not sure that is is the issue. User defined the host name and psql
show it.

The issue is that "host" is an ip, "\conninfo" will inform wrongly that you are connected to "127.0.0.2", but the actual connection is really to "127.0.0.1", this is plain misleading, and I consider this level of unhelpfullness more a bug than a feature.

I didn't think that this is an issue, because I determined "host" as just a host's display name when "hostaddr" is defined. So user may determine 127.0.0.1 (hostaddr) as "happy_host", for example. It shouldn't be a real host.

I searched for another use cases of PQhost(). In PostgreSQL source code I found that it is used in pg_dump and psql to connect to some instance.

There is the next issue with PQhost() and psql (pg_dump could have it too, see CloneArchive() in pg_backup_archiver.c and _connectDB() in pg_backup_db.c):

$ psql "host=host_1,host_2 hostaddr=127.0.0.1,127.0.0.3 dbname=postgres"
=# \conninfo
You are connected to database "postgres" as user "artur" on host "host_1" at port "5432".
=# \connect test
could not translate host name "host_1" to address: Неизвестное имя или служба
Previous connection kept

So in the example above you cannot reuse connection string with \connect. What do you think?

I cannot agree with you. When I've learned libpq before I found
host/hostaddr rules description useful. And I disagree that it is good
to remove it (as the patch does).
Of course it is only my point of view and others may have another opinion.

I'm not sure I understand your concern.

Do you mean that you would prefer the document to keep describing that host/hostaddr/port accepts one value, and then have in some other place or at the end of the option documentation a line that say, "by the way, we really accept lists, and they must be somehow consistent between host/hostaddr/port"?

I wrote about the following part of the documentation:

-        Using <literal>hostaddr</literal> instead of <literal>host</literal> 
allows the
-        application to avoid a host name look-up, which might be important
-        in applications with time constraints. However, a host name is
-        required for GSSAPI or SSPI authentication
-        methods, as well as for <literal>verify-full</literal> SSL
-        certificate verification.  The following rules are used:
-        <itemizedlist>
> ...

So I think description of these rules is useful here and shouldn't be removed. Your patch removes it and maybe it shouldn't do that. But now I realised that the patch breaks this behavior and backward compatibility is broken.

Patch gives me an error if I specified only hostaddr:

psql -d "hostaddr=127.0.0.1"
psql: host "/tmp" cannot have an hostaddr "127.0.0.1"

This is the expected modified behavior: hostaddr can only be specified on a host when it is a name, which is not the case here.

See the comment above about backward compatibility. psql without the patch can connect to an instance if I specify only hostaddr.

--
Arthur Zakirov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company

Reply via email to