Re: PQHost() undefined behavior if connecting string contains both host and hostaddr types

On Wed, Mar 21, 2018 at 10:33:19AM +1100, Haribabu Kommi wrote:
> On Wed, Mar 21, 2018 at 6:06 AM, Peter Eisentraut <
> peter(dot)eisentraut(at)2ndquadrant(dot)com> wrote:
>
>> On 3/16/18 00:03, Kyotaro HORIGUCHI wrote:
>>> I agree to the conclusion that PQhost() shouldn't return hostaddr
>>> "if it has any host name to return". But I still haven't found
>>> the reason for returning '/tmp' for IP connection.
>>>
>>> The attached patch is revised version of that in the following thread.
>>
>> That patch looks good to me.

The case where we are complaining is this one.
"hostaddr=127.0.0.1,127.0.0.1 port=5432,5433"
This makes PQhost return "127.0.0.1" with the patch, and "local" on
HEAD.

>> Moreover, I wonder whether we shouldn't remove the branch where
>> conn->connhost is NULL. When would that be the case? The current
>> behavior is to sometimes return the actual host connected to, and
>> sometimes the host list. That doesn't make sense.
>
> Scenarios where the connection is not yet established, in that scenario
> the PQhost() can return the provided connection host information.
>
> Other than the above, it always returns the proper host details.

That remark is from me upthread. In the case of a non-established
connection, I think that we ought to return that.

> Even with the above PQhost() patch, If user provides both host and
> hostaddr as options, the PQhost() function returns host and not the
> hostaddr. In commit 7b02ba62, the support of "Allow multiple
> hostaddrs to go with multiple hostnames".

Sentence is unfinished here?

> If it is fine to display the host in combination of both host and
> hostaddr, then it is fine to remove the commit
> 64f86fb11e20b55fb742af72d55806f8bdd9cd2d.

Showing host when both host and hostaddr are present is more intuitive
in my opinion.
--
Michael