Re: libpq host/hostaddr/conninfo inconsistencies

From: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: libpq host/hostaddr/conninfo inconsistencies
Date: 2018-10-26 07:21:51
Message-ID: alpine.DEB.2.21.1810260804280.27686@lancre
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


Hello Robert,

> [...] that's a self-inflicted injury.

Sure. I'm trying to be more user friendly.

> It's not that I am opposed to helping people avoid self-inflicted
> injuries, but this one doesn't seem either likely or serious.

If I'm trying to improve something, I tend to be thorough about it.

>> I see at least three actual defects:
>>
>> - \conninfo output does NOT reflect the actual status of a connection
>> some cases. I do not see how this can be defended as a powerful
>> feature.
>
> Well, again, I think you're talking about the case where host and
> hostaddr don't match. But that's not an intended use case,

I disagree: it is an intended use case because it is documented that you
can use both host & hostaddr. This feature has been added without telling
conninfo about it, hence the confusion when it is used.

> so I'm not sure it matters. Perhaps extending the \conninfo output with
> the actual IP to which somebody connected wouldn't be a bad idea, but in
> at least 99% cases, it's just going to be clutter.

It helps when both host & hostaddr are used, or if a host name resolves to
several IPs.

About clutter: if someone asks for \conninfo it is because they need it,
so probably they can deal with a precise information, instead of an output
that may or may not be what the connection really is.

Moreover, ISTM more likely that I would want to look at \conninfo if the
connection parameters were complex, to know how it resolved, probably
while debugging something, and then I would really want it to reflect the
actual status.

>> - \connect does NOT work in some trivial cases.
>>
>> These two above issues are linked to the fact that libpq does not allow to
>> know what the actual connection is, so it cannot be described correctly
>> nor reused to create another connection.
>
> Yeah, that's not great.

Indeed, I think it is a bug. Note that the patch does not address this
issue, I'm keeping it for later. It should require extending libpq, which
requires some more thinking.

> [...] ssh ... [user(at)]hostname [command]
>
> Well, are you confused? That host name could really be an IP address.

Sure, but ssh does not give an alternate syntax to provide a target IP
address, whereas libpq (apparently) provides one syntax for hostnames and
one for IPs.

> What is, arguably, a little confusing in the case of ssh is that
> 'hostname' could ALSO, instead of being a name that we can find in DNS
> or an IP address, correspond to a Host entry in our ~/.ssh/config
> file, which could remap the hostname we gave to some other hostname
> for DNS lookup purposes, or to an IP address.

Sure. Now when you run "ssh -v", the output tells you that it used the
config to redefine the connection, it does not say that it is directly
connected to the target, contrary to \conninfo which provides plain false
informations.

>> The documentation says that host should be used for host names or sockets,
>> hostaddr for IP addresses, and then there is a special case when both are
>> provided. The implementation does not really do that, as noted above.
>
> You're not the first person to think that -- I believe the pgAdmin 3
> developers were confused about the same point -- so it's probably not
> as clear as it could be.

Yep. That is my point:-)

> [...] Maybe we should explicitly say the opposite e.g. host Name or IP
> address of host to connect to. hostaddr Numeric IP address of host to
> connect to. Normally not needed, because PostgreSQL will perform a
> lookup on the value specified for host if necessary. If specified, this
> should be...

Well, that is one of my point, trying to improve the documentation to make
it less confusing...

>> It seems that the documentation does not say what you think it says.
>
> Or maybe it doesn't say what YOU think it says. :-)

Hmmm. I have re-read the current host/hostaddr doc before replying to your
email. I find it confusing because of what it says and not says and
somehow suggests. Moreover, people get regularly confused, as you pointed
out.

Probably I'm below par at understanding English technical documentations,
but I'm afraid I'm not the only average Joe around.

To sum up:

(1) you are somehow against changing the current implementation, eg
erroring out on possibly misleading configurations, because you do not
think it is really useful to help users in those cases.

(2) you are not against improving the documentation, although you find it
clear enough already, but you agree that some people could get confused.

The attached patch v4 only improves the documentation so that it reflects
what the implementation really does. I think it too bad to leave out the
user-friendly aspects of the patch, though.

--
Fabien.

Attachment Content-Type Size
libpq-host-ip-4.patch text/x-diff 9.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Langote 2018-10-26 07:42:30 Re: CVE-2017-7484-induced bugs, or, btree cmp functions are not leakproof?
Previous Message Andrey Lepikhov 2018-10-26 06:43:14 Re: [PATCH] XLogReadRecord returns pointer to currently read page