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

From David G. Johnston
Subject Re: PQHost() undefined behavior if connecting string contains bothhost and hostaddr types
Date
Msg-id CAKFQuwaats6dndVT73xrFqBu+RJ4tsBopCNON5XuV+xT8tGVkQ@mail.gmail.com
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  (Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>)
List pgsql-hackers
On Mon, Mar 26, 2018 at 10:47 PM, Haribabu Kommi <kommi.haribabu@gmail.com> wrote:
updated patch attached with additional doc updates as per the suggestion from the upthreads.
​---------------------------------------------------------
Some comments if the patch remains in-tact:
​Lower-case "i" in "It is not" in the second paragraphs

The ... function returns NULL when the input conn parameter is NULL or an empty string if conn cannot be evaluated.  Otherwise, the return value is the first non-NULL, non-empty, value of either the host or hostaddr property of conn.  <omit the entire last sentence, Callers of this function..., for PQport too>

Omit "properly" after established - if the connection is established one can assume it was done "properly".

Add "the" after calling:  "can be checked by calling the <function>PQstatus</function> function.

------------------------------------------------------
I'm having a bit of concern about the actual specification and wording...but don't have time right now to thoroughly read the thread history, docs, and/or code at this moment to know whether its just inexperience on my part or an actual confusion.  But here's where I'm at presently...

I'm not sure why there is confusion here in the first place...the docs[1] say:

"The following functions return parameter values established at connection."

At which point there is only one meaningful value for them - the value that pertains to the established connection.

As far as "or an empty string if conn cannot be evaluated" should just become "an empty string if conn is inactive".

----------------------------
Another odd piece is:

"The value for host is ignored unless the authentication method requires it, in which case it will be used as the host name."

There is no coverage of "1 host, many hostaddr; n host, n hostaddr; and n host, m hostaddr" combinations - namely which are valid and how the NxM combination would even work if it is even allowed.

Then, if we do connect using hostaddr, and authenticate with host, should we indicate that fact in the PQhost output or not?

Potentially PQhost would be defined to return an actual hostname if it can figure one out - even if the active connection was made using hostaddr.  My take is that PQhost is meant to be user-informative rather than technical, and should ever only return the single most appropriate value it can compute.  Just need to decide on the logic for determining "appropriate" then document and implement it.

-----------------------------
"The following functions return parameter values established at connection. These values are fixed for the life of the connection. If a multi-host connection string is used, the values of PQhost, PQport, and PQpass can change if a new connection is established using the same PGconn object. Other values are fixed for the lifetime of the PGconn object."

Maybe something like:

The following functions return parameters values established at connection and, except for the multi-valued fail-over accessors PQhost and PQport, as well as PQpass, cannot change during the lifetime of the PGconn object.

PQpass is a bit odd here given its not multi-valued...not sure what if anything to make of that.

David J.

pgsql-hackers by date:

Previous
From: Andrew Dunstan
Date:
Subject: Re: Parallel Aggregates for string_agg and array_agg
Next
From: Michael Paquier
Date:
Subject: Re: Problem while setting the fpw with SIGHUP