On Wed, Oct 19, 2016 at 12:04 PM, Robert Haas <robertmh...@gmail.com> wrote: > On Thu, Oct 13, 2016 at 5:53 AM, Victor Wagner <vi...@wagner.pp.ru> wrote: >> On Thu, 13 Oct 2016 12:30:59 +0530 >> Mithun Cy <mithun...@enterprisedb.com> wrote: >>> On Fri, Sep 30, 2016 at 2:14 PM, Victor Wagner <vi...@wagner.pp.ru> >>> wrote: >>> Okay but for me consistency is also important. Since we agree to >>> disagree on some of the comments and others have not expressed any >>> problem I am moving it to committer. >> >> Thank you for your efforts improving my patch > > I haven't deeply reviewed this patch, but on a quick read-through it > certainly seems to need a lot of cleanup work.
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. - 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. - 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 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. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (email@example.com) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers