At Fri, 16 Mar 2018 09:50:41 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI <horiguchi.kyot...@lab.ntt.co.jp> wrote in <20180316.095041.241173653.horiguchi.kyot...@lab.ntt.co.jp> > I drifted to come here.. > > At Wed, 14 Mar 2018 11:17:35 +0900, Michael Paquier <mich...@paquier.xyz> > wrote in <20180314021735.gi1...@paquier.xyz> > > On Fri, Mar 09, 2018 at 04:42:30PM -0500, Peter Eisentraut wrote: > > > It seems, however, that PGhost() has always been broken for hostaddr > > > use. In 9.6 (before the multiple-hosts stuff was introduced), when > > > connecting to "hostaddr=127.0.0.1", PGhost() returns "/tmp". Urgh. > > > > > > I think we should decide what PGhost() is supposed to mean when hostaddr > > > is in use, and then make a fix for that consistently across all versions. > > > > Hm. The only inconsistency I can see here is when "host" is not defined > > but "hostaddr" is, so why not make PQhost return the value of "hostaddr" > > in this case?
I proposed that before and I strongly agree to that. I suppose that the return value of PQhost is to used for the following purpose. - It is for authentication subject. - It provides a string for a human to identify the connection peer. So it is incovenient that PQhost returns pghost before trying hostaddr. Attached patch does that based on Haribabu's patch at the beginning of this thread. ================ > https://www.postgresql.org/message-id/19297.1493660213%40sss.pgh.pa.us > > And I believe we already considered and rejected the idea of having it > > return the hostaddr string, back in some of the older discussions. > > (We could revisit that decision, no doubt, but let's go back and see > > what the reasoning was first.) > > But I'havent find where the decision made. I'm going to search > for that for a while. After all, I didn't find much.. I found this thread back to 2014. https://www.postgresql.org/message-id/cahgqgwhsnmxh97udxh5uif8eitd0wxdk_psb3tipagzjjvs...@mail.gmail.com It preserved the behavior that PQhost() returns '/tmp' for the case under discussion intentionally. After that, 274bb2b385 (mistakenly?) put priority on hostaddr to host in connectOptions2() so 11003eb556 reverted PQhost to return pghost if connhost[] is of CHT_HOST_ADDRESS. The last commit also reverted it to return /tmp if pghost is not specified. https://www.postgresql.org/message-id/ca+tgmob1y1kyk_twi9of3tj4p3j4c5yontauh7diajpewyn...@mail.gmail.com 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. https://www.postgresql.org/message-id/CA%2BTgmoZ%2B9h%3DmiD4%2BwYc9QztezgtLfeA59XtxVAL0NUjvfwKmaA%40mail.gmail.com > Kyotaro Horiguchi argues that the current behavior is "not useful" and > that's probably true, but I don't think it's the job of an API to try > as hard as possible to do something useful in every case. It's more > important that the behavior is predictable and understandable. In > short, if we're going to change the behavior of PQhost() here, then > there should be a documentation change to go with it, because the > current documentation around the interaction between host and hostaddr > does not make it at all clear that the current behavior is wrong, at > least not as far as I can see. To the contrary, it suggests that if > you use hostaddr without host, you may find the results surprising or > even unfortunate: I believe the behavior is not surprising and not a hard thing to do. https://www.postgresql.org/message-id/20170510.153403.28896042.horiguchi.kyotaro%40lab.ntt.co.jp > However, it might be a problem that the documentation doesn't > mention what the returned value from PQhost is. If it is the string that was given as host parameter, returning "" instead of NULL might be reasonable. If it is any string that points to the connecting host, I think that returning IP address is not surprising at all. regards, -- Kyotaro Horiguchi NTT Open Source Software Center
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c index 77eebb0ba1..8e450b21fe 100644 --- a/src/interfaces/libpq/fe-connect.c +++ b/src/interfaces/libpq/fe-connect.c @@ -6018,8 +6018,12 @@ PQhost(const PGconn *conn) if (!conn) return NULL; if (conn->connhost != NULL && - conn->connhost[conn->whichhost].type != CHT_HOST_ADDRESS) + conn->connhost[conn->whichhost].host != NULL && + conn->connhost[conn->whichhost].host[0] != '\0') return conn->connhost[conn->whichhost].host; + else if (conn->connhost[conn->whichhost].hostaddr != NULL && + conn->connhost[conn->whichhost].hostaddr[0] != '\0') + return conn->connhost[conn->whichhost].hostaddr; else if (conn->pghost != NULL && conn->pghost[0] != '\0') return conn->pghost; else
