Re: libpq host/hostaddr/conninfo inconsistencies - Mailing list pgsql-hackers

From Fabien COELHO
Subject Re: libpq host/hostaddr/conninfo inconsistencies
Date
Msg-id alpine.DEB.2.21.1810260804280.27686@lancre
Whole thread Raw
In response to Re: libpq host/hostaddr/conninfo inconsistencies  (Robert Haas <robertmhaas@gmail.com>)
Responses Re: libpq host/hostaddr/conninfo inconsistencies  (Dmitry Dolgov <9erthalion6@gmail.com>)
Re: libpq host/hostaddr/conninfo inconsistencies  (Andres Freund <andres@anarazel.de>)
List 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@]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

pgsql-hackers by date:

Previous
From: Andrey Lepikhov
Date:
Subject: Re: [PATCH] XLogReadRecord returns pointer to currently read page
Next
From: Amit Langote
Date:
Subject: Re: CVE-2017-7484-induced bugs, or, btree cmp functions are notleakproof?