Re: Re: libpq host/hostaddr/conninfo inconsistencies

2019-09-11 Thread Alvaro Herrera from 2ndQuadrant
On 2019-Sep-10, Alvaro Herrera from 2ndQuadrant wrote:

> I propose to settle this issue by applying "Robert's changes two and
> three", which has been +1'd by two people already and I also accept
> myself as improvement.  I don't think any further changes are required.

Applied, marked item committed.

Thanks,

-- 
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services




Re: Re: libpq host/hostaddr/conninfo inconsistencies

2019-09-10 Thread Alvaro Herrera from 2ndQuadrant
I propose to settle this issue by applying "Robert's changes two and
three", which has been +1'd by two people already and I also accept
myself as improvement.  I don't think any further changes are required.

diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index c1d1b6b2db..7afe7bf68d 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -993,10 +993,14 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
 your machine supports IPv6, you can also use those addresses.
 TCP/IP communication is
 always used when a nonempty string is specified for this parameter.
+If this parameter is not specified, the value of 
host
+will be looked up to find the corresponding IP address - or, if
+host specifies an IP address, that value will be
+used directly.

 

-Using hostaddr instead of host 
allows the
+Using hostaddr allows the
 application to avoid a host name look-up, which might be important
 in applications with time constraints. However, a host name is
 required for GSSAPI or SSPI authentication

In absence of objections or better ideas, I'll get this pushed tomorrow.
If Robert as patch author wants to push himself, I'm happy to stand
aside.

-- 
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services




Re: Re: libpq host/hostaddr/conninfo inconsistencies

2019-03-21 Thread David Steele

On 2/22/19 9:44 PM, Fabien COELHO wrote:


Hmmm, I do not buy the typing argument: "host" is actually for 
everything, including directories. I do not think that adding "hostdir" 
would be desirable.



In any case, the existing doco never comes out and states either
rule set in so many words.  Maybe it should.


Personally I like the second and third edit from Robert's patch, but not 
the first one.  I'm having a hard time seeing why you would specify host 
*and* hostaddr as this seems to imply.


I also agree with Fabien's comment that host can be a path -- it's 
really a path to a socket file, but it's certainly not a host name or IP 
address.  Perhaps that should be called out explicitly.


Regards,
--
-David
da...@pgmasters.net



Re: libpq host/hostaddr/conninfo inconsistencies

2019-02-22 Thread Fabien COELHO



Hello Tom,


However, it does not discuss that an IP can (and should, IMHO) be given
through "host" if the point is to specify the target by its IP rather than
a lookup shortcut.


Ah, that's the crux of the problem.


Yep!

There are two ways that you could consider to be "best practice" for use 
of these parameters.  The one that is currently documented is:


1. If you want to give a host name, put it in "host".
2. If you want to give a host IP address (to skip DNS), put it in
  "hostaddr".
3. ... unless your security arrangements require specifying a host name,
  in which case provide the host IP address in "hostaddr" and
  the host name in "host".

What Fabien is basically proposing is replacing rule 2 with

2. If you want to give a host IP address (to skip DNS), put it in
  "host".


More or less. I'd rather rephrase it in two steps rather than 3, to 
emphasize that it is simpler:


  1. use "host" to provide the target whatever its form (name, ip, dir).
 basically, always use host, which has always worked.

  2. use "hostaddr" only to provide a ns shortcut on a provided name,
 which can be real (the lookup would have provided the same answer)
 or false (eg for testing purposes you really connect to another
 host). Basically nobody should ever do that but in special use cases.


While that would perhaps be an equally good best practice if we'd
started there, it's not clear to me that it has any advantage that
would justify changing the recommendation.


Simplicity and clarity: less thing to remember, just always use "host"
for the target.

In particular, the existing rule is a lot clearer from a data-type 
standpoint: host is for names, hostaddr is for IP addresses.


Hmmm, I do not buy the typing argument: "host" is actually for everything, 
including directories. I do not think that adding "hostdir" would be 
desirable.



In any case, the existing doco never comes out and states either
rule set in so many words.  Maybe it should.


--
Fabien.



Re: libpq host/hostaddr/conninfo inconsistencies

2019-02-21 Thread Robert Haas
On Thu, Feb 21, 2019 at 10:57 AM Tom Lane  wrote:
> True, but isn't that because we fail to document at all that you
> can put an IP address in "host"?  Which your proposed patch didn't
> change, IIRC.

Well, that's another way to tackle the problem.  Personally, I see
pretty much no downside in approaching this by encouraging people to
use only 'host' in normal cases and adding 'hostaddr' as an additional
field only when necessary, so that's the approach I took.  Now you
seem to think that it's important for people to know that they could
use 'hostaddr' without specifying 'host', but I think that's a detail
that nobody really needs to know.  I'm looking for a way to give
people a clearer suggestion that they should just use 'host' and
forget the rest.  Perhaps we could get there via what you propose
here, namely documenting that 'host' can be either a name or an IP
address, but I'm worried that it won't come through clearly enough and
that people will still get confused.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



Re: libpq host/hostaddr/conninfo inconsistencies

2019-02-21 Thread Tom Lane
Robert Haas  writes:
> From my point of view, the issue here is that the way the
> documentation is written right now tends to lead people to believe
> that if they have a host name, they must pass it via host, and if they
> have an IP address, they must pass it via hostaddr.  Thus pgAdmin 3,
> at least, had code to check whether the input looked like an IP
> address and put it into one field or the other accordingly.  And
> that's stupid.

True, but isn't that because we fail to document at all that you
can put an IP address in "host"?  Which your proposed patch didn't
change, IIRC.

regards, tom lane



Re: libpq host/hostaddr/conninfo inconsistencies

2019-02-21 Thread Tom Lane
Fabien COELHO  writes:
> However, it does not discuss that an IP can (and should, IMHO) be given 
> through "host" if the point is to specify the target by its IP rather than 
> a lookup shortcut.

Ah, that's the crux of the problem.  There are two ways that you could
consider to be "best practice" for use of these parameters.  The one
that is currently documented is:

1. If you want to give a host name, put it in "host".
2. If you want to give a host IP address (to skip DNS), put it in
   "hostaddr".
3. ... unless your security arrangements require specifying a host name,
   in which case provide the host IP address in "hostaddr" and
   the host name in "host".

What Fabien is basically proposing is replacing rule 2 with

2. If you want to give a host IP address (to skip DNS), put it in
   "host".

While that would perhaps be an equally good best practice if we'd
started there, it's not clear to me that it has any advantage that
would justify changing the recommendation.  In particular, the
existing rule is a lot clearer from a data-type standpoint: host
is for names, hostaddr is for IP addresses.

In any case, the existing doco never comes out and states either
rule set in so many words.  Maybe it should.

regards, tom lane



Re: libpq host/hostaddr/conninfo inconsistencies

2019-02-21 Thread Robert Haas
On Thu, Feb 21, 2019 at 10:33 AM Tom Lane  wrote:
> Sure, you only need it if you want to bypass DNS lookup, but if you
> do that, you don't necessarily need to give "host" as well.  Changing
> the documentation to imply that you do would not be an improvement.

>From my point of view, the issue here is that the way the
documentation is written right now tends to lead people to believe
that if they have a host name, they must pass it via host, and if they
have an IP address, they must pass it via hostaddr.  Thus pgAdmin 3,
at least, had code to check whether the input looked like an IP
address and put it into one field or the other accordingly.  And
that's stupid.

My feeling is that it doesn't really matter whether people think that
both host and hostaddr are required.  What's more important is that
they understand that whatever form of host identifier they've got can
be put in host, and that hostaddr doesn't normally need to be set at
all.  That's less clear than it could be at present.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



Re: libpq host/hostaddr/conninfo inconsistencies

2019-02-21 Thread Tom Lane
Fabien COELHO  writes:
>> My only complaint about this is that it makes it sound like you *must*
>> provide "host", even when giving "hostaddr":

> That is the idea, "hostaddr" is moslty never needed.

Sure, you only need it if you want to bypass DNS lookup, but if you
do that, you don't necessarily need to give "host" as well.  Changing
the documentation to imply that you do would not be an improvement.

> It is the initial misleading issue I've been complaining about: one can 
> specify an IP *both* in host (although it is not documented) and in 
> hostaddr... and when both are provided, things started being messy

Indeed, but the verbiage being suggested here actually encourages
people to do that, by falsely implying that they have to supply
both parameters.

regards, tom lane



Re: libpq host/hostaddr/conninfo inconsistencies

2019-02-21 Thread Fabien COELHO



Hello Tom,


My only complaint about this is that it makes it sound like you *must*
provide "host", even when giving "hostaddr":


That is the idea, "hostaddr" is moslty never needed.

It is the initial misleading issue I've been complaining about: one can 
specify an IP *both* in host (although it is not documented) and in 
hostaddr... and when both are provided, things started being messy: the 
information displayed was plain wrong (eg telling you that you were 
connected to an IP when you were really connected to another), and one 
could not get the actual information about the current connection out of 
libpq.


A committed patch has fixed the display (\conninfo) and connection 
(\c) issues by extending libpq and being carefull about the message 
displayed or the information used to reconnect, which were basically bug 
fixes.


About host/hostaddr, my initial submission was to force "host" as the 
target (whether name, directory, or ip which already work) and "hostaddr" 
only as a lookup shortcut, which is I think its initial intended use. This 
has been rejected.


Now I'm just trying to improve the documentation so that at least it 
reflects what is done, and implicitely advise about how to use the 
features properly even if it is not enforced: Basically, a user should 
always used "host" for all purposes because it works, and that makes one 
parameter for one purpose, and "hostaddr" is only needed for lookup 
shortcut, which is basically a very rare and specialized use case.


--
Fabien.



Re: libpq host/hostaddr/conninfo inconsistencies

2019-02-21 Thread Fabien COELHO



Hello Robert,


(2) you are not against improving the documentation, although you find it
clear enough already, but you agree that some people could get confused.

The attached patch v4 only improves the documentation so that it reflects
what the implementation really does. I think it too bad to leave out the
user-friendly aspects of the patch, though.


Robert, any chance you could opine on the doc patch, given that's your
suggested direction?


I find it to be a more change than we really need, and I'm not sure
how much it helps to clarify the issue at hand. Here is a simpler
change which seems like it might do the trick (or maybe not, let's see
what others think).


It is a minimal diff on "hostaddr" documentation which clarifies what is 
its intended use. I'm okay with it.


However, it does not discuss that an IP can (and should, IMHO) be given 
through "host" if the point is to specify the target by its IP rather than 
a lookup shortcut.


--
Fabien.



Re: libpq host/hostaddr/conninfo inconsistencies

2019-02-20 Thread Michael Paquier
On Wed, Feb 20, 2019 at 08:12:55PM -0500, Tom Lane wrote:
> The second para explains the cases in which you actually do need to
> provide "host", but I'm afraid that the first sentence will have
> misled people enough that they won't get the point.
>
> I don't think there's anything very wrong with the existing wording
> of this sentence.

I am not seeing anything bad with the first sentence either.  Now if
people are willing to tweak its wording it may point out that
something is confusing in it.  Would it be an improvement with a
formulation like that?  Say cutting the apple in half like that:
"Numeric IP address that can be used in replacement of host."

> Robert's second and third changes seem fine, though.

Agreed.
--
Michael


signature.asc
Description: PGP signature


Re: libpq host/hostaddr/conninfo inconsistencies

2019-02-20 Thread Tom Lane
Robert Haas  writes:
>> Robert, any chance you could opine on the doc patch, given that's your
>> suggested direction?

> I find it to be a more change than we really need, and I'm not sure
> how much it helps to clarify the issue at hand. Here is a simpler
> change which seems like it might do the trick (or maybe not, let's see
> what others think).

My only complaint about this is that it makes it sound like you *must*
provide "host", even when giving "hostaddr":

-Numeric IP address of host to connect to.  This should be in the
+Numeric IP address that should be used for the server specified
+by host.  This should be in the

The second para explains the cases in which you actually do need to
provide "host", but I'm afraid that the first sentence will have
misled people enough that they won't get the point.

I don't think there's anything very wrong with the existing wording
of this sentence.

Robert's second and third changes seem fine, though.

regards, tom lane



Re: libpq host/hostaddr/conninfo inconsistencies

2019-02-20 Thread Robert Haas
On Thu, Feb 14, 2019 at 1:10 PM Andres Freund  wrote:
> > (2) you are not against improving the documentation, although you find it
> > clear enough already, but you agree that some people could get confused.
> >
> > The attached patch v4 only improves the documentation so that it reflects
> > what the implementation really does. I think it too bad to leave out the
> > user-friendly aspects of the patch, though.
>
> Robert, any chance you could opine on the doc patch, given that's your
> suggested direction?

I find it to be a more change than we really need, and I'm not sure
how much it helps to clarify the issue at hand. Here is a simpler
change which seems like it might do the trick (or maybe not, let's see
what others think).

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


clarify-hostaddr-rmh.patch
Description: Binary data


Re: libpq host/hostaddr/conninfo inconsistencies

2019-02-17 Thread Fabien COELHO



I am getting error while applying patch.I think the patch needs to be
redone on the latest code in master as there are some commits in master
after the patch is created


Possibly. Here is a v5 with a simplified text.

The key point is to tell that "host" expects names, ips or directories, 
and that "hostaddr" is for ip lookup shortcuts.


--
Fabien.diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index c1d1b6b2db..b0f8f5097f 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -964,49 +964,41 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
   host
   

-Name of host to connect to.host name
-If a host name begins with a slash, it specifies Unix-domain
-communication rather than TCP/IP communication; the value is the
-name of the directory in which the socket file is stored.
+Comma-separated list of hosts to connect to.
+Each item may be a host namehost name
+that will be resolved with a look-up,
+a numeric IP address that will be used directly, or
+the full path of a directory which contains the socket file for
+Unix-domain communication rather than TCP/IP communication;
+Each specified host will be tried in turn in the order given.
+See  for details.
+   
+
+   
 The default behavior when host is
-not specified, or is empty, is to connect to a Unix-domain
+not specified, or an item is empty, is to connect to a Unix-domain
 socketUnix domain socket in
 /tmp (or whatever socket directory was specified
-when PostgreSQL was built). On machines without
-Unix-domain sockets, the default is to connect to localhost.
+when PostgreSQL was built).
+On machines without Unix-domain sockets, the default is to connect to
+localhost.

-   
-A comma-separated list of host names is also accepted, in which case
-each host name in the list is tried in order; an empty item in the
-list selects the default behavior as explained above. See
- for details.
-   
-  
- 
 
- 
-  hostaddr
-  

-Numeric IP address of host to connect to.  This should be in the
-standard IPv4 address format, e.g., 172.28.40.9.  If
-your machine supports IPv6, you can also use those addresses.
-TCP/IP communication is
-always used when a nonempty string is specified for this parameter.
+To shortcut host name lookups, see parameter
+.

 

-Using hostaddr instead of host allows the
-application to avoid a host name look-up, which might be important
-in applications with time constraints. However, a host name is
-required for GSSAPI or SSPI authentication
-methods, as well as for verify-full SSL
-certificate verification.  The following rules are used:
+When connecting, the following rules apply:
 
  
   
-   If host is specified
-   without hostaddr, a host name lookup occurs.
+   If an item in host has a corresponding empty
+   item in hostaddr, it is used:
+   For an IP address or a Unix-domain socket, it is done directly.
+   For a host name, a lookup occurs, and the resulting set of IPs will
+   be tried in turn.
(When using PQconnectPoll, the lookup occurs
when PQconnectPoll first considers this host
name, and it may cause PQconnectPoll to block
@@ -1015,45 +1007,48 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
  
  
   
-   If hostaddr is specified without host,
-   the value for hostaddr gives the server network address.
-   The connection attempt will fail if the authentication
-   method requires a host name.
-  
- 
- 
-  
-   If both host and hostaddr are specified,
-   the value for hostaddr gives the server network address.
-   The value for host is ignored unless the
-   authentication method requires it, in which case it will be
-   used as the host name.
+   If an item in host has a corresponding non-empty
+   item in hostaddr, the later network address is
+   used and the host name may be used for authentication purposes.
   
  
 
-Note that authentication is likely to fail if host
+   
+  
+ 
+
+ 
+  hostaddr
+  
+   
+Comma-separated numeric IP addresses corresponding one-to-one to
+host, to avoid a host name look-up when non empty.
+This may be desirable for testing, to work around NAT settings, or for
+better performance.
+   
+
+   
+Parameter hostaddr must be either empty or a
+list of the same length as host.
+A 

Re: libpq host/hostaddr/conninfo inconsistencies

2019-02-16 Thread Ramanarayana
Hi,

I am getting error while applying patch.I think the patch needs to be
redone on the latest code in master as there are some commits in master
after the patch is created

On Sat, 16 Feb 2019 at 13:44, Fabien COELHO  wrote:

>
> Hello Kyotaro-san,
>
> > On such standpoint, the first hunk in the patch attracted my
> > eyes.
> >
> >   host
> >   
> >
> > -Name of host to connect to.host
> name
> > -If a host name begins with a slash, it specifies Unix-domain
> > -communication rather than TCP/IP communication; the value is the
> > -name of the directory in which the socket file is stored.
> > +   
> >
> > I don't think this is user-friendly since almost all of them don't write
> > multiple hosts there. So I prefer the previous organization.
>
> ISTM that specifying the expected syntax is the first information needed?
>
> The previous organization says "this is a host name (bla bla bla) btw I
> lied at the beginning this is a list".
>
> > The description about IP-address looks too verbose, especially we don't
> > need explain what is IP-address here.
>
> Ok.
>
> I agree that the order is not the best possible one. Here is a simplified
> and reordered version:
>
> """ Comma-separated list of hosts to connect to. Each item may be a host
> name that will be resolved with a look-up, a numeric IP address that will
> be used directly, or the name of a directory which contains the socket
> file for Unix-domain communication, if the specification begins with a
> slash. Each specified target will be tried in turn in the order given. See
>  for details. """
>
> What do you think about that version.
>
> --
> Fabien.
>
>

-- 
Cheers
Ram 4.0


Re: libpq host/hostaddr/conninfo inconsistencies

2019-02-16 Thread Fabien COELHO



Hello Kyotaro-san,


On such standpoint, the first hunk in the patch attracted my
eyes.

  host
  
   
-Name of host to connect to.host 
name
-If a host name begins with a slash, it specifies Unix-domain
-communication rather than TCP/IP communication; the value is the
-name of the directory in which the socket file is stored.
+   

I don't think this is user-friendly since almost all of them don't write 
multiple hosts there. So I prefer the previous organization.


ISTM that specifying the expected syntax is the first information needed?

The previous organization says "this is a host name (bla bla bla) btw I 
lied at the beginning this is a list".


The description about IP-address looks too verbose, especially we don't 
need explain what is IP-address here.


Ok.

I agree that the order is not the best possible one. Here is a simplified 
and reordered version:


""" Comma-separated list of hosts to connect to. Each item may be a host 
name that will be resolved with a look-up, a numeric IP address that will 
be used directly, or the name of a directory which contains the socket 
file for Unix-domain communication, if the specification begins with a 
slash. Each specified target will be tried in turn in the order given. See 
 for details. """


What do you think about that version.

--
Fabien.



Re: libpq host/hostaddr/conninfo inconsistencies

2019-02-14 Thread Kyotaro HORIGUCHI
Hello.

At Thu, 14 Feb 2019 22:51:40 +0100 (CET), Fabien COELHO  
wrote in 
> 
> > On 2018-10-26 09:21:51 +0200, Fabien COELHO wrote:
> >> (1) you are somehow against changing the current implementation, eg
> >> erroring
> >> out on possibly misleading configurations, because you do not think it
> >> is
> >> really useful to help users in those cases.
> >
> > I find this formulation somewhat passive aggressive.
> 
> I do not understand what you mean by that expression.
> 
> I was just trying to sum-up Robert's opposition to erroring on
> misleading configurations (eg "host=1.2.3.4 hostaddr=4.3.2.1") instead
> of complying to it whatever, as is currently done. Probably my
> phrasing could be improved, but I do not think that I misrepresented
> Robert's position.
> 
> Note that the issue is somehow mitigated by 6e5f8d489a: \conninfo now
> displays a more precise information, so that at least you are not told
> that you are connected to a socket when you a really connected to an
> ip, or to one ip when you a really connected to another.

I'm rather on (maybe) Robert's side in that not opposing to edit
it but documentation should be plain as far as it is not so
mis-leading for average readers. From the same viewpoint,
documentation is written general-and-important-first, then
special cases and trivials.

On such standpoint, the first hunk in the patch attracted my
eyes.

   host
   

-Name of host to connect to.host 
name
-If a host name begins with a slash, it specifies Unix-domain
-communication rather than TCP/IP communication; the value is the
-name of the directory in which the socket file is stored.
+Comma-separated list of hosts to connect to.host 
name
+Each specified host will be tried in turn in the order given.
+See  for details.
+Each item may be a host name that will be resolved with a look-up,
+a numeric IP address (IPv4 in the standard format, e.g.,
+172.28.40.9, or IPv6 if supported by your machine)
+that will be used directly, or
+the name of a directory which contains the socket file for Unix-domain
+communication rather than TCP/IP communication
+(the specification must then begin with a slash);
+   

I don't think this is user-friendly since almost all of them
don't write multiple hosts there. So I prefer the previous
organization. The description about IP-address looks too verbose,
especially we don't need explain what is IP-address here.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center




Re: libpq host/hostaddr/conninfo inconsistencies

2019-02-14 Thread Fabien COELHO




On 2018-10-26 09:21:51 +0200, Fabien COELHO wrote:

(1) you are somehow against changing the current implementation, eg erroring
out on possibly misleading configurations, because you do not think it is
really useful to help users in those cases.


I find this formulation somewhat passive aggressive.


I do not understand what you mean by that expression.

I was just trying to sum-up Robert's opposition to erroring on misleading 
configurations (eg "host=1.2.3.4 hostaddr=4.3.2.1") instead of complying 
to it whatever, as is currently done. Probably my phrasing could be 
improved, but I do not think that I misrepresented Robert's position.


Note that the issue is somehow mitigated by 6e5f8d489a: \conninfo now 
displays a more precise information, so that at least you are not told 
that you are connected to a socket when you a really connected to an ip, 
or to one ip when you a really connected to another.


--
Fabien.



Re: libpq host/hostaddr/conninfo inconsistencies

2019-02-14 Thread Andres Freund
Hi,

On 2018-10-26 09:21:51 +0200, Fabien COELHO wrote:
> (1) you are somehow against changing the current implementation, eg erroring
> out on possibly misleading configurations, because you do not think it is
> really useful to help users in those cases.

I find this formulation somewhat passive aggressive.


> (2) you are not against improving the documentation, although you find it
> clear enough already, but you agree that some people could get confused.
> 
> The attached patch v4 only improves the documentation so that it reflects
> what the implementation really does. I think it too bad to leave out the
> user-friendly aspects of the patch, though.

Robert, any chance you could opine on the doc patch, given that's your
suggested direction?

- Andres



Re: libpq host/hostaddr/conninfo inconsistencies

2019-02-03 Thread Michael Paquier
On Fri, Nov 30, 2018 at 01:08:51PM +0100, Dmitry Dolgov wrote:
> Why then not split the original proposal into two patches, one to improve the
> documentation, and another to make it more user friendly?

Moved to next CF for now.  From what I can see the latest patch
manipulates the same areas of the documentation, so keeping things
grouped would reduce the global amount of diffs.
--
Michael


signature.asc
Description: PGP signature


Re: libpq host/hostaddr/conninfo inconsistencies

2018-11-30 Thread Dmitry Dolgov
> On Fri, Oct 26, 2018 at 9:22 AM Fabien COELHO  wrote:
>
> To sum up:
>
> (1) you are somehow against changing the current implementation, eg
> erroring out on possibly misleading configurations, because you do not
> think it is really useful to help users in those cases.
>
> (2) you are not against improving the documentation, although you find it
> clear enough already, but you agree that some people could get confused.
>
> The attached patch v4 only improves the documentation so that it reflects
> what the implementation really does.

Thanks, it's definitely makes sense to propose documentation patch if there are
any concerns about how clear it is. For now I'm moving patch to the next CF.

> I think it too bad to leave out the user-friendly aspects of the patch,
> though.

Why then not split the original proposal into two patches, one to improve the
documentation, and another to make it more user friendly?



Re: libpq host/hostaddr/conninfo inconsistencies

2018-10-26 Thread Fabien COELHO


Hello Robert,


[...] that's a self-inflicted injury.


Sure. I'm trying to be more user friendly.

It's not that I am opposed to helping people avoid self-inflicted 
injuries, but this one doesn't seem either likely or serious.


If I'm trying to improve something, I tend to be thorough about it.


I see at least three actual defects:

  - \conninfo output does NOT reflect the actual status of a connection
some cases. I do not see how this can be defended as a powerful
feature.


Well, again, I think you're talking about the case where host and
hostaddr don't match.  But that's not an intended use case,


I disagree: it is an intended use case because it is documented that you 
can use both host & hostaddr. This feature has been added without telling 
conninfo about it, hence the confusion when it is used.


so I'm not sure it matters.  Perhaps extending the \conninfo output with 
the actual IP to which somebody connected wouldn't be a bad idea, but in 
at least 99% cases, it's just going to be clutter.


It helps when both host & hostaddr are used, or if a host name resolves to 
several IPs.


About clutter: if someone asks for \conninfo it is because they need it, 
so probably they can deal with a precise information, instead of an output 
that may or may not be what the connection really is.


Moreover, ISTM more likely that I would want to look at \conninfo if the 
connection parameters were complex, to know how it resolved, probably 
while debugging something, and then I would really want it to reflect the 
actual status.



  - \connect does NOT work in some trivial cases.

These two above issues are linked to the fact that libpq does not allow to
know what the actual connection is, so it cannot be described correctly
nor reused to create another connection.


Yeah, that's not great.


Indeed, I think it is a bug. Note that the patch does not address this 
issue, I'm keeping it for later. It should require extending libpq, which 
requires some more thinking.



[...] ssh ... [user@]hostname [command]

Well, are you confused?  That host name could really be an IP address.


Sure, but ssh does not give an alternate syntax to provide a target IP 
address, whereas libpq (apparently) provides one syntax for hostnames and 
one for IPs.



What is, arguably, a little confusing in the case of ssh is that
'hostname' could ALSO, instead of being a name that we can find in DNS
or an IP address, correspond to a Host entry in our ~/.ssh/config
file, which could remap the hostname we gave to some other hostname
for DNS lookup purposes, or to an IP address.


Sure. Now when you run "ssh -v", the output tells you that it used the 
config to redefine the connection, it does not say that it is directly 
connected to the target, contrary to \conninfo which provides plain false 
informations.



The documentation says that host should be used for host names or sockets,
hostaddr for IP addresses, and then there is a special case when both are
provided. The implementation does not really do that, as noted above.


You're not the first person to think that -- I believe the pgAdmin 3
developers were confused about the same point -- so it's probably not
as clear as it could be.


Yep. That is my point:-)


[...] Maybe we should explicitly say the opposite e.g. host Name or IP 
address of host to connect to. hostaddr Numeric IP address of host to 
connect to.  Normally not needed, because PostgreSQL will perform a 
lookup on the value specified for host if necessary.  If specified, this 
should be...


Well, that is one of my point, trying to improve the documentation to make 
it less confusing...



It seems that the documentation does not say what you think it says.


Or maybe it doesn't say what YOU think it says.  :-)


Hmmm. I have re-read the current host/hostaddr doc before replying to your 
email. I find it confusing because of what it says and not says and 
somehow suggests. Moreover, people get regularly confused, as you pointed 
out.


Probably I'm below par at understanding English technical documentations, 
but I'm afraid I'm not the only average Joe around.


To sum up:

(1) you are somehow against changing the current implementation, eg 
erroring out on possibly misleading configurations, because you do not 
think it is really useful to help users in those cases.


(2) you are not against improving the documentation, although you find it 
clear enough already, but you agree that some people could get confused.


The attached patch v4 only improves the documentation so that it reflects 
what the implementation really does. I think it too bad to leave out the 
user-friendly aspects of the patch, though.


--
Fabien.diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 82a440531b..e4e8549d8a 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -964,49 +964,43 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
   host
   

-Name of host to connect 

Re: libpq host/hostaddr/conninfo inconsistencies

2018-10-25 Thread Robert Haas
On Thu, Oct 25, 2018 at 1:06 PM Fabien COELHO  wrote:
> If you do it, even by accident, there is no way to guess what is wrong
> because the reported informations are inconsistent and does not reflect
> the actual status.

Meh.  The reported information is fine.  If you tell the system that
foo.com has an IP of 127.0.0.1 when it really doesn't, and then you
get confused because it reports a failure to connect to foo.com when
you really failed to connect to 127.0.0.1, that's a self-inflicted
injury.  It's not that I am opposed to helping people avoid
self-inflicted injuries, but this one doesn't seem either likely or
serious.

> I see at least three actual defects:
>
>   - \conninfo output does NOT reflect the actual status of a connection
> some cases. I do not see how this can be defended as a powerful
> feature.

Well, again, I think you're talking about the case where host and
hostaddr don't match.  But that's not an intended use case, so I'm not
sure it matters.  Perhaps extending the \conninfo output with the
actual IP to which somebody connected wouldn't be a bad idea, but in
at least 99% cases, it's just going to be clutter.

>   - \connect does NOT work in some trivial cases.
>
> These two above issues are linked to the fact that libpq does not allow to
> know what the actual connection is, so it cannot be described correctly
> nor reused to create another connection.

Yeah, that's not great.

>   - the documentation does not say that "host" accepts IPs,
> and implicitely says that hostaddr should be used for IPs.
>
> Once it is clear that "host" accepts IPs, then the host/hostaddr duality
> becomes much less clear, which is the conceptual issue I'm trying to
> solve by improving the documentation.

All I can really say here is that I don't find the current
documentation very confusing, but I agree with you that some people
have been confused by it. I'm not direly opposed to making it more
clear, but I'm not sure that necessitates all of the behavior changes
you are proposing.

I mean, the ssh syntax synopsis says:

 ssh [-1246AaCfGgKkMNnqsTtVvXxYy] [-b bind_address] [-c cipher_spec]
 [-D [bind_address:]port] [-E log_file] [-e escape_char]
 [-F configfile] [-I pkcs11] [-i identity_file]
 [-J [user@]host[:port]] [-L address] [-l login_name] [-m mac_spec]
 [-O ctl_cmd] [-o option] [-p port] [-Q query_option] [-R address]
 [-S ctl_path] [-W host:port] [-w local_tun[:remote_tun]]
 [user@]hostname [command]

Well, are you confused?  That host name could really be an IP address.
But I don't think that's really confusing, because I think it's pretty
widely understood that a hostname is just a proxy for an IP address,
and therefore it's expected that any place where a hostname is
requested, you could instead supply the IP address directly.

What is, arguably, a little confusing in the case of ssh is that
'hostname' could ALSO, instead of being a name that we can find in DNS
or an IP address, correspond to a Host entry in our ~/.ssh/config
file, which could remap the hostname we gave to some other hostname
for DNS lookup purposes, or to an IP address.  But we don't have that
problem, because we picked a different keyword for that kind of
functionality -- service=whatever vs. host=whatever.

> The documentation says that host should be used for host names or sockets,
> hostaddr for IP addresses, and then there is a special case when both are
> provided. The implementation does not really do that, as noted above.

You're not the first person to think that -- I believe the pgAdmin 3
developers were confused about the same point -- so it's probably not
as clear as it could be.  But I actually do not see that in the
documentation anywhere.  It says that the value of hostaddr must be an
IP address, but I do not see that it says that if what you have is an
IP address, you should stuff that in hostaddr rather than host.  Maybe
we should explicitly say the opposite e.g.

host Name or IP address of host to connect to.

hostaddr Numeric IP address of host to connect to.  Normally not
needed, because PostgreSQL will perform a lookup on the value
specified for host if necessary.  If specified, this should be...

> > But the documentation already says that, so I don't really see why we
> > need to change anything here.
>
> It seems that the documentation does not say what you think it says.

Or maybe it doesn't say what YOU think it says.  :-)

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



Re: libpq host/hostaddr/conninfo inconsistencies

2018-10-25 Thread Fabien COELHO



Hello Robert,


I think this patch is a solution in search of a problem.


I take note of this negative opinion.

[...] It's true that the above example is very confusing, but there's no 
reason for everybody to ever do that.


If you do it, even by accident, there is no way to guess what is wrong 
because the reported informations are inconsistent and does not reflect 
the actual status.



Well, no.  The fact that a programming language -- or a connection
string -- can be used to create incomprehensible constructs is an
artifact of it being powerful and flexible, not a defect.


I see at least three actual defects:

 - \conninfo output does NOT reflect the actual status of a connection
   some cases. I do not see how this can be defended as a powerful
   feature.

 - \connect does NOT work in some trivial cases.

These two above issues are linked to the fact that libpq does not allow to 
know what the actual connection is, so it cannot be described correctly 
nor reused to create another connection.


 - the documentation does not say that "host" accepts IPs,
   and implicitely says that hostaddr should be used for IPs.

Once it is clear that "host" accepts IPs, then the host/hostaddr duality 
becomes much less clear, which is the conceptual issue I'm trying to 
solve by improving the documentation.



What users should do is just use host.  If that causes name lookups
they want to avoid, they should instead use both host and hostaddr.


THANKS!

This is exactly the simple approach what I'm trying to promote:-) However, 
this is NOT what is actually said in the documentation.


The documentation says that host should be used for host names or sockets, 
hostaddr for IP addresses, and then there is a special case when both are 
provided. The implementation does not really do that, as noted above.



If they do that, they'll be fine.


Sure.

If they do strange things like specify host and hostaddr strings that 
don't match, then yes, it won't work very well.


Indeed. I think that we should be a bit more user friendly by catching 
obvious misconfigurations.


But the documentation already says that, so I don't really see why we 
need to change anything here.


It seems that the documentation does not say what you think it says.

--
Fabien.



Re: libpq host/hostaddr/conninfo inconsistencies

2018-10-25 Thread Robert Haas
On Mon, Aug 20, 2018 at 7:32 AM Fabien COELHO  wrote:
>sh> psql "host=localhost,127.0.0.2,, hostaddr=127.0.0.1,,127.0.0.3,"
># attempt 1 is 127.0.0.1 identified as localhost
># attempt 2 is 127.0.0.2
># attempt 3 is 127.0.0.3 identified as the default, whatever it is
># attempt 4 is really the default

I think this patch is a solution in search of a problem.  It's true
that the above example is very confusing, but there's no reason for
everybody to ever do that.  It's like saying that C is a bad
programming language because people can do this:

https://www.ioccc.org/2018/anderson/prog.c

Well, no.  The fact that a programming language -- or a connection
string -- can be used to create incomprehensible constructs is an
artifact of it being powerful and flexible, not a defect.

What users should do is just use host.  If that causes name lookups
they want to avoid, they should instead use both host and hostaddr.
If they do that, they'll be fine.  If they do strange things like
specify host and hostaddr strings that don't match, then yes, it won't
work very well.  But the documentation already says that, so I don't
really see why we need to change anything here.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



Re: libpq host/hostaddr/conninfo inconsistencies

2018-10-23 Thread Fabien COELHO


Hello Arthur,


sh> psql "host=127.0.0.2 hostaddr=127.0.0.1"


I'm not sure that is is the issue. User defined the host name and psql
show it.


The issue is that "host" is an ip, "\conninfo" will inform wrongly that you 
are connected to "127.0.0.2", but the actual connection is really to 
"127.0.0.1", this is plain misleading, and I consider this level of 
unhelpfullness more a bug than a feature.


I didn't think that this is an issue, because I determined "host" as just a 
host's display name when "hostaddr" is defined.


When I type "\conninfo", I do not expect to have false clues that must be 
interpreted depending on a fine knowledge of the documentation and the 
connection parameters possibly typed hours earlier, I would just expect to 
have a direct answer describing in a self contained way what the 
connection actually is.


So user may determine 127.0.0.1 (hostaddr) as "happy_host", for example. 
It shouldn't be a real host.


They may determine it if they can access the initial connection 
information, which means an careful inquest because \conninfo does not say 
what it is... If they just read what is said, they just get wrong 
informations.


I searched for another use cases of PQhost(). In PostgreSQL source code I 
found that it is used in pg_dump and psql to connect to some instance.


There is the next issue with PQhost() and psql (pg_dump could have it too, 
see CloneArchive() in pg_backup_archiver.c and _connectDB() in 
pg_backup_db.c):


$ psql "host=host_1,host_2 hostaddr=127.0.0.1,127.0.0.3 dbname=postgres"
=# \conninfo
You are connected to database "postgres" as user "artur" on host "host_1" at 
port "5432".

=# \connect test
could not translate host name "host_1" to address: Неизвестное имя или служба
Previous connection kept

So in the example above you cannot reuse connection string with \connect. 
What do you think?


I think that this is another connection related "feature", aka bug, that 
should be fixed as well:-(



I cannot agree with you. When I've learned libpq before I found
host/hostaddr rules description useful. And I disagree that it is good
to remove it (as the patch does).




Of course it is only my point of view and others may have another opinion.


I'm not sure I understand your concern.

Do you mean that you would prefer the document to keep describing that 
host/hostaddr/port accepts one value, and then have in some other place or 
at the end of the option documentation a line that say, "by the way, we 
really accept lists, and they must be somehow consistent between 
host/hostaddr/port"?


I wrote about the following part of the documentation:


-  Using hostaddr instead of host 
allows the
-  application to avoid a host name look-up, which might be important
-  in applications with time constraints. However, a host name is
-  required for GSSAPI or SSPI authentication
-  methods, as well as for verify-full SSL
-  certificate verification.  The following rules are used:
-  
...


So I think description of these rules is useful here and shouldn't be 
removed.


Ok, I have put back a summary description of which rules apply, which are
somehow simpler & saner, at least this is the aim of this patch.

Your patch removes it and maybe it shouldn't do that. But now I 
realised that the patch breaks this behavior and backward compatibility 
is broken.


Indeed. The incompatible changes are that "host" must always be provided, 
instead of letting the user providing an IP either in host or hostaddr 
(currently both work although undocumented), and that "hostaddr" can only 
be provided for a host name, not for an IP or socket.



Patch gives me an error if I specified only hostaddr:

psql -d "hostaddr=127.0.0.1"
psql: host "/tmp" cannot have an hostaddr "127.0.0.1"


This is the expected modified behavior: hostaddr can only be specified on a 
host when it is a name, which is not the case here.


See the comment above about backward compatibility. psql without the patch 
can connect to an instance if I specify only hostaddr.


Yes, that is intentional and is the purpose of this patch: to provide a 
simple connection model for the user: use "host" to connect to a target 
server, and "hostaddr" as a lookup shortcut only.


For a reminder, my main issues with the current status are:

(1) the documentation is inconsistent with the implementation:
"host" can be given an IP, but this is not documented.
"hostaddr" can be provided for anything, and overshadows the initial
specification, but:

(2) "\conninfo" does not give a clue about what the connection
really is in such cases.

Moreover, you found another issue with psql's "\connect" which does not 
work properly when both "host" & "hostaddr" are given.


In the attached patch, I tried to clarify the documentation further and 
fix some rebase issues I had. ISTM that all relevant informations provided 
in the previous version are still there.


The backward incompatibility is 

Re: libpq host/hostaddr/conninfo inconsistencies

2018-10-03 Thread Arthur Zakirov

Sorry for late answer.

On 9/30/18 10:21 AM, Fabien COELHO wrote:

sh> psql "host=127.0.0.2 hostaddr=127.0.0.1"


I'm not sure that is is the issue. User defined the host name and psql
show it.


The issue is that "host" is an ip, "\conninfo" will inform wrongly that 
you are connected to "127.0.0.2", but the actual connection is really to 
"127.0.0.1", this is plain misleading, and I consider this level of 
unhelpfullness more a bug than a feature.


I didn't think that this is an issue, because I determined "host" as 
just a host's display name when "hostaddr" is defined. So user may 
determine 127.0.0.1 (hostaddr) as "happy_host", for example. It 
shouldn't be a real host.


I searched for another use cases of PQhost(). In PostgreSQL source code 
I found that it is used in pg_dump and psql to connect to some instance.


There is the next issue with PQhost() and psql (pg_dump could have it 
too, see CloneArchive() in pg_backup_archiver.c and _connectDB() in 
pg_backup_db.c):


$ psql "host=host_1,host_2 hostaddr=127.0.0.1,127.0.0.3 dbname=postgres"
=# \conninfo
You are connected to database "postgres" as user "artur" on host 
"host_1" at port "5432".

=# \connect test
could not translate host name "host_1" to address: Неизвестное имя или 
служба

Previous connection kept

So in the example above you cannot reuse connection string with 
\connect. What do you think?



I cannot agree with you. When I've learned libpq before I found
host/hostaddr rules description useful. And I disagree that it is good
to remove it (as the patch does).
Of course it is only my point of view and others may have another 
opinion.


I'm not sure I understand your concern.

Do you mean that you would prefer the document to keep describing that 
host/hostaddr/port accepts one value, and then have in some other place 
or at the end of the option documentation a line that say, "by the way, 
we really accept lists, and they must be somehow consistent between 
host/hostaddr/port"?


I wrote about the following part of the documentation:


-Using hostaddr instead of host 
allows the
-application to avoid a host name look-up, which might be important
-in applications with time constraints. However, a host name is
-required for GSSAPI or SSPI authentication
-methods, as well as for verify-full SSL
-certificate verification.  The following rules are used:
-

> ...

So I think description of these rules is useful here and shouldn't be 
removed. Your patch removes it and maybe it shouldn't do that. But now I 
realised that the patch breaks this behavior and backward compatibility 
is broken.



Patch gives me an error if I specified only hostaddr:

psql -d "hostaddr=127.0.0.1"
psql: host "/tmp" cannot have an hostaddr "127.0.0.1"


This is the expected modified behavior: hostaddr can only be specified 
on a host when it is a name, which is not the case here.


See the comment above about backward compatibility. psql without the 
patch can connect to an instance if I specify only hostaddr.


--
Arthur Zakirov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company



Re: libpq host/hostaddr/conninfo inconsistencies

2018-09-30 Thread Fabien COELHO




So I'm speculating that the cfbot is using a version of patch(1) that
doesn't have strip-trailing-CRs logic.  Which bemuses me, because
I thought they all did.


Huh.  Yeah.  I have now switched it over to GNU patch.  It seems to be
happier with Fabien's patches so far, but will take a few minutes to
catch up with all of them.


Thanks for the fix. I gather that I'm the only one on the list who uses a 
MIME-conformant MUA.


--
Fabien.



Re: libpq host/hostaddr/conninfo inconsistencies

2018-09-30 Thread Fabien COELHO



Hello Arthur,

Thanks for the comments.


However, the actual capability is slightly different: specifying an ip
address to "host" does work, without ensuing any name or reverse name
look-ups, even if this is undocumented.


Agree it may have more details within the documentation.


sh> psql "host=/tmp hostaddr=127.0.0.1"


Yeah this example shows that user may be confused by output of
\conninfo. I think it is psql issue and libpq issue.


Yep. I'd add that there is a documentation issue as well.

psql in exec_command_conninfo() rely only on the PQhost() result. Can we 
add a function PQhostType() to solve this issue?


I did not attempt to fix "\conninfo" yet, I focussed on the host/hostaddr 
documentation and consistency checks in libpq.


I agree that at least one additional PQ function is needed.

What to do with a "host type" function is unclear, because it would not 
change the output of PQhost() which returns the "host" value even if it 
was ignored by the connection, there is no access to "hostaddr"... it is 
not enough.


I was thinking that maybe a function could return the full description as 
a string, so that the connection logic choices and display are implemented 
in libpq only, but this is debatable.


Otherwise a collection of functions, including a host type function, would 
be necessary for the client to have full information about the actual 
current connection.



sh> psql "host=127.0.0.2 hostaddr=127.0.0.1"


I'm not sure that is is the issue. User defined the host name and psql
show it.


The issue is that "host" is an ip, "\conninfo" will inform wrongly that 
you are connected to "127.0.0.2", but the actual connection is really to 
"127.0.0.1", this is plain misleading, and I consider this level of 
unhelpfullness more a bug than a feature.



sh> psql "hostaddr=127.0.0.1"


I cannot reproduce it. It gives me the message:
You are connected to database "artur" as user "artur" on host "127.0.0.1" at port 
"5432".

I think it is because of the environment (I didn't define PGHOST
variable, for example). If so, depending on PGHOST variable value
("/tmp" or "127.0.0.1") it is related with first or second issue.


Indeed, hostaddr superseedes the default, whatever it is, so it depends on 
the default, which can be overriden with PGHOST.



* Another issue with \conninfo is that if a host resolves to multiple ips,
there is no way to know which was chosen and/or worked, although on errors
some messages show the failing ip.


Can you explain it please? You can use PQhost() to know choosed host.


Indeed PQhost will tell the name. My point is that there will be no clue 
about the actual ip used among those possible.



* The documentation about host/hostaddr/port accepting lists is really
added as an afterthought: the features are presented for one, and then the
list is mentionned.


I cannot agree with you. When I've learned libpq before I found
host/hostaddr rules description useful. And I disagree that it is good
to remove it (as the patch does).
Of course it is only my point of view and others may have another opinion.


I'm not sure I understand your concern.

Do you mean that you would prefer the document to keep describing that 
host/hostaddr/port accepts one value, and then have in some other place or 
at the end of the option documentation a line that say, "by the way, we 
really accept lists, and they must be somehow consistent between 
host/hostaddr/port"?



(3) checking that hostaddr non empty addresses are only accepted if the
corresponding host is a name. The user must use the "host=ip" syntax
to connect to an ip.




Patch gives me an error if I specified only hostaddr:

psql -d "hostaddr=127.0.0.1"
psql: host "/tmp" cannot have an hostaddr "127.0.0.1"


This is the expected modified behavior: hostaddr can only be specified on 
a host when it is a name, which is not the case here.


Changing the name to "resolve", has it would maybe help the user realize 
it is not expected to be used to provide the target host, it is just a dns 
shortcut.


If the user wants to connect to 127.0.0.1, they have to use 
"host=127.0.0.1".



It is wrong, because I didn't specified host=/tmp.


You did not, but this is the default value when you do not specify "host" 
explicitely, so it was specified behind your back.


--
Fabien.



Re: libpq host/hostaddr/conninfo inconsistencies

2018-09-29 Thread Thomas Munro
On Sat, Aug 25, 2018 at 7:25 AM Tom Lane  wrote:
> Fabien COELHO  writes:
> > Attached is a rebase after 5ca00774.
>
> I notice that the cfbot thinks that *none* of your pending patches apply
> successfully.  I tried this one locally and what I get is
>
> $ patch -p1 <~/libpq-host-ip-2.patch
> (Stripping trailing CRs from patch.)
> patching file doc/src/sgml/libpq.sgml
> (Stripping trailing CRs from patch.)
> patching file src/interfaces/libpq/fe-connect.c
>
> as compared to the cfbot report, in which every hunk is rejected:
>
> === applying patch ./libpq-host-ip-2.patch
> Hmm...  Looks like a unified diff to me...
> The text leading up to this was:
> --
> |diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
> |index 5e7931ba90..086172d4f0 100644
> |--- a/doc/src/sgml/libpq.sgml
> |+++ b/doc/src/sgml/libpq.sgml
> --
> Patching file doc/src/sgml/libpq.sgml using Plan A...
> Hunk #1 failed at 964.
> Hunk #2 failed at 994.
> 2 out of 2 hunks failed--saving rejects to doc/src/sgml/libpq.sgml.rej
> Hmm...  The next patch looks like a unified diff to me...
> The text leading up to this was:
> --
> |diff --git a/src/interfaces/libpq/fe-connect.c 
> b/src/interfaces/libpq/fe-connect.c
> |index a8048ffad2..34025ba041 100644
> |--- a/src/interfaces/libpq/fe-connect.c
> |+++ b/src/interfaces/libpq/fe-connect.c
> --
> Patching file src/interfaces/libpq/fe-connect.c using Plan A...
> Hunk #1 failed at 908.
> Hunk #2 failed at 930.
> Hunk #3 failed at 943.
> Hunk #4 failed at 974.
> Hunk #5 failed at 1004.
> Hunk #6 failed at 1095.
> Hunk #7 failed at 2098.
> Hunk #8 failed at 2158.
> Hunk #9 failed at 6138.
> 9 out of 9 hunks failed--saving rejects to 
> src/interfaces/libpq/fe-connect.c.rej
> done
>
> So I'm speculating that the cfbot is using a version of patch(1) that
> doesn't have strip-trailing-CRs logic.  Which bemuses me, because
> I thought they all did.

Huh.  Yeah.  I have now switched it over to GNU patch.  It seems to be
happier with Fabien's patches so far, but will take a few minutes to
catch up with all of them.

-- 
Thomas Munro
http://www.enterprisedb.com



Re: libpq host/hostaddr/conninfo inconsistencies

2018-09-29 Thread Arthur Zakirov
Hello,

On Fri, Aug 24, 2018 at 11:22:47AM +0200, Fabien COELHO wrote:
> Attached is a rebase after 5ca00774.

I looked a little bit the patch. And I have a few notes.

> However, the actual capability is slightly different: specifying an ip
> address to "host" does work, without ensuing any name or reverse name
> look-ups, even if this is undocumented.

Agree it may have more details within the documentation.

> sh> psql "host=/tmp hostaddr=127.0.0.1"

Yeah this example shows that user may be confused by output of
\conninfo. I think it is psql issue and libpq issue. psql in
exec_command_conninfo() rely only on the PQhost() result. Can we add a
function PQhostType() to solve this issue?

> sh> psql "host=127.0.0.2 hostaddr=127.0.0.1"

I'm not sure that is is the issue. User defined the host name and psql
show it.

> sh> psql "hostaddr=127.0.0.1"

I cannot reproduce it. It gives me the message:

You are connected to database "artur" as user "artur" on host "127.0.0.1" at 
port "5432".

I think it is because of the environment (I didn't define PGHOST
variable, for example). If so, depending on PGHOST variable value
("/tmp" or "127.0.0.1") it is related with first or second issue.

> * Another issue with \conninfo is that if a host resolves to multiple ips,
> there is no way to know which was chosen and/or worked, although on errors
> some messages show the failing ip.

Can you explain it please? You can use PQhost() to know choosed host.

> * The documentation about host/hostaddr/port accepting lists is really
> added as an afterthought: the features are presented for one, and then the
> list is mentionned.

I cannot agree with you. When I've learned libpq before I found
host/hostaddr rules description useful. And I disagree that it is good
to remove it (as the patch does).
Of course it is only my point of view and others may have another opinion.

> (3) checking that hostaddr non empty addresses are only accepted if the
> corresponding host is a name. The user must use the "host=ip" syntax
> to connect to an ip.

Patch gives me an error if I specified only hostaddr:

psql -d "hostaddr=127.0.0.1"
psql: host "/tmp" cannot have an hostaddr "127.0.0.1"

It is wrong, because I didn't specified host=/tmp.

-- 
Arthur Zakirov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company



Re: libpq host/hostaddr/conninfo inconsistencies

2018-08-24 Thread Fabien COELHO




I notice that the cfbot thinks that *none* of your pending patches apply
successfully.  I tried this one locally and what I get is


Hmmm. :-(

I've reverted to sending MIME conformant "text/x-diff" CRLF attachements, 
as "text/plain" did the same and you complained rightfully that 
"application/octet-stream" was a bad choice.


I do not know how to force my MUA to send MIME-broken text attachments 
with LF only, which are indeed sent by other MUAs (eg thunderbird on macos 
does it, and Tom your mailer seems to do it as well, dunno what it is, 
though).


So I'm out of choices:-(

--
Fabien.



Re: libpq host/hostaddr/conninfo inconsistencies

2018-08-24 Thread Fabien COELHO



The attached patch does 1-3 (2 without renaming, though).


Attached is a rebase after 5ca00774.

--
Fabien.diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 5e7931ba90..086172d4f0 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -964,22 +964,28 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
   host
   

-Name of host to connect to.host name
-If a host name begins with a slash, it specifies Unix-domain
-communication rather than TCP/IP communication; the value is the
-name of the directory in which the socket file is stored.
+Comma-separated list of hosts to connect to.host name
+Each item may be a host name that will be resolved with a look-up,
+a numeric IP address (IPv4 in the standard format, e.g.,
+172.28.40.9, or IPv6 if supported by your machine)
+that will be used directly, or
+the name of a directory which contains the socket file for Unix-domain
+communication rather than TCP/IP communication
+(the specification must then begin with a slash);
+   
+
+   
 The default behavior when host is
-not specified, or is empty, is to connect to a Unix-domain
+not specified, or an item is empty, is to connect to a Unix-domain
 socketUnix domain socket in
 /tmp (or whatever socket directory was specified
 when PostgreSQL was built). On machines without
 Unix-domain sockets, the default is to connect to localhost.

+

-A comma-separated list of host names is also accepted, in which case
-each host name in the list is tried in order; an empty item in the
-list selects the default behavior as explained above. See
- for details.
+If multiple host names are specified, each will be tried in turn in
+the order given.  See  for details.

   
  
@@ -988,49 +994,22 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
   hostaddr
   

-Numeric IP address of host to connect to.  This should be in the
-standard IPv4 address format, e.g., 172.28.40.9.  If
-your machine supports IPv6, you can also use those addresses.
-TCP/IP communication is
-always used when a nonempty string is specified for this parameter.
+Comma-separated numeric IP addresses corresponding one-to-one to
+host, to avoid a host name look-up when non empty.
+This may be desirable for testing, to work around NAT settings, or for
+better performance.
+TCP/IP communication is always used when a nonempty string is specified
+for this parameter.
+   
+
+   
+If a host name is required for GSSAPI or SSPI authentication
+certificate verification, the corresponding host name is used.
+The host name is also used to identify the connection in
+a password file (see ).

 

-Using hostaddr instead of host allows the
-application to avoid a host name look-up, which might be important
-in applications with time constraints. However, a host name is
-required for GSSAPI or SSPI authentication
-methods, as well as for verify-full SSL
-certificate verification.  The following rules are used:
-
- 
-  
-   If host is specified
-   without hostaddr, a host name lookup occurs.
-   (When using PQconnectPoll, the lookup occurs
-   when PQconnectPoll first considers this host
-   name, and it may cause PQconnectPoll to block
-   for a significant amount of time.)
-  
- 
- 
-  
-   If hostaddr is specified without host,
-   the value for hostaddr gives the server network address.
-   The connection attempt will fail if the authentication
-   method requires a host name.
-  
- 
- 
-  
-   If both host and hostaddr are specified,
-   the value for hostaddr gives the server network address.
-   The value for host is ignored unless the
-   authentication method requires it, in which case it will be
-   used as the host name.
-  
- 
-
 Note that authentication is likely to fail if host
 is not the name of the server at network address hostaddr.
 Also, when both host and hostaddr
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index a8048ffad2..34025ba041 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -908,14 +908,15 @@ count_comma_separated_elems(const char *input)
 /*
  * Parse a simple comma-separated list.
  *
- * On each call, returns a malloc'd copy of the next element, and sets *more
- * to indicate whether there are any more elements in the list after