On Wed, 19 Oct 2016 20:15:38 -0400
Robert Haas <robertmhaas@gmail.com> wrote:
>
> Some more comments:
>
> - I am pretty doubtful that the changes to connectOptions2() work as
> intended. I think that's only going to be called before actualhost
> and actualport could possibly get set. I don't think we keep
> reinvoking this function for every new host we try.
I would investigate this. Probably test cases for .pgpass file should
be added.
> - It's pretty clear that this isn't going to work if the host list
> includes a mix of hostnames and UNIX socket addresses. The code that
> handles the UNIX socket address case is totally separate from the code
> that handles the multiple-hostname case.
Muitihost feature was never intended to work with unix-domain socket.
May be it should be clarified in the docs, or even improved error
message emitted when unix socket occurs in the multiple host list.
> - This creates a sort of definitional problem for functions like
> PQhost(). The documentation says of this and other related functions:
> "These values are fixed for the life of the PGconn object." But with
> this patch, that would no longer be true. So at least the
> documentation needs an update. Actually, though, I think the problem
I've missed this phrase, documentation should be updated.
> is more fundamental than that. If the user asks for PQhost(conn), do
> they want the list of all hosts, or the one to which we're currently
> connected? It might be either, depending on what they intend to do
> with the information. If they mean to construct a new connection
> string, they might well want them all; but something like psql's
> \conninfo only shows the current one. There's also the question of
> what "the current one" means if we're not connected to any server at
> all. I'm not sure exactly how to solve these problems, but they need
> some thought.
I've thought that no one would need to reconstruct connect string from
conninfo object. May be I've missed some use cases.
Name PQhost suggest that it'll return just one host. So, I've decided
to interpret it as 'currently choosen host'.
If we really need function which lists all the potentially connected
hosts, it should be new function.
I hope this would introduce no backward incompatibility, because no
existing setup uses multihost connect strings.
Thanks for your critique.
Victor.
