Re: PQHost() undefined behavior if connecting string contains bothhost and hostaddr types - Mailing list pgsql-hackers

From Michael Paquier
Subject Re: PQHost() undefined behavior if connecting string contains bothhost and hostaddr types
Date
Msg-id 20180321074011.GE2288@paquier.xyz
Whole thread Raw
In response to Re: PQHost() undefined behavior if connecting string contains bothhost and hostaddr types  (Haribabu Kommi <kommi.haribabu@gmail.com>)
Responses Re: PQHost() undefined behavior if connecting string contains bothhost and hostaddr types  (Peter Eisentraut <peter.eisentraut@2ndquadrant.com>)
List pgsql-hackers
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.eisentraut@2ndquadrant.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

Attachment

pgsql-hackers by date:

Previous
From: Catalin Iacob
Date:
Subject: Re: JIT compiling with LLVM v12.2
Next
From: Michael Banck
Date:
Subject: Re: [PoC PATCH] Parallel dump to /dev/null