Re: [HACKERS] POLA violation with \c service=
On 04/02/2015 12:42 PM, Alvaro Herrera wrote: David Fetter wrote: I haven't checked yet, but could this be because people aren't using --enable-depend with ./configure ? BTW --- No, this can't be the answer; --enable-depend is meant to help with recompiling after updating the source tree, but lack of it cannot cause any failures (assuming proper usage of make distclean). And the buildfarm always builds with pristine sources. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] POLA violation with \c service=
David Fetter wrote: > I haven't checked yet, but could this be because people aren't using > --enable-depend with ./configure ? BTW --- No, this can't be the answer; --enable-depend is meant to help with recompiling after updating the source tree, but lack of it cannot cause any failures (assuming proper usage of make distclean). -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] POLA violation with \c service=
On Thu, Apr 02, 2015 at 11:46:53AM +0900, Michael Paquier wrote: > On Thu, Apr 2, 2015 at 9:38 AM, Alvaro Herrera > wrote: > > > > David Fetter wrote: > > > On Wed, Apr 01, 2015 at 08:13:02PM -0300, Alvaro Herrera wrote: > > > > I have pushed this after some rework. For instance, the 9.0 > > > > and 9.1 versions believed that URIs were accepted, but that > > > > stuff was introduced in 9.2. I changed some other minor > > > > issues -- I hope not to have broken too many other things in > > > > the process. Please give the whole thing a look, preferrably > > > > in all branches, and let me know if I've broken something in > > > > some horrible way. > > > > > > Thanks for taking the time to do this. Will test. I'm a little > > > unsure as to how regression tests involving different hosts > > > might work, but I'll see what I can do. > > > > Well, contrib/dblink is failing all over the place, for one thing. > > > OSX and Windows builds are broken as well, libpq missing > dependencies with connstrings.c. Sent a patch is here FWIW: > http://www.postgresql.org/message-id/cab7npqto1fn7inxaps2exdy4wxbncwnooaweptt-jvorli3...@mail.gmail.com I haven't checked yet, but could this be because people aren't using --enable-depend with ./configure ? Cheers, David. -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] POLA violation with \c service=
On Thu, Apr 2, 2015 at 9:38 AM, Alvaro Herrera wrote: > > David Fetter wrote: > > On Wed, Apr 01, 2015 at 08:13:02PM -0300, Alvaro Herrera wrote: > > > I have pushed this after some rework. For instance, the 9.0 and 9.1 > > > versions believed that URIs were accepted, but that stuff was introduced > > > in 9.2. I changed some other minor issues -- I hope not to have broken > > > too many other things in the process. Please give the whole thing a > > > look, preferrably in all branches, and let me know if I've broken > > > something in some horrible way. > > > > Thanks for taking the time to do this. Will test. I'm a little > > unsure as to how regression tests involving different hosts might > > work, but I'll see what I can do. > > Well, contrib/dblink is failing all over the place, for one thing. OSX and Windows builds are broken as well, libpq missing dependencies with connstrings.c. Sent a patch is here FWIW: http://www.postgresql.org/message-id/cab7npqto1fn7inxaps2exdy4wxbncwnooaweptt-jvorli3...@mail.gmail.com -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] POLA violation with \c service=
David Fetter wrote: > On Wed, Apr 01, 2015 at 08:13:02PM -0300, Alvaro Herrera wrote: > > I have pushed this after some rework. For instance, the 9.0 and 9.1 > > versions believed that URIs were accepted, but that stuff was introduced > > in 9.2. I changed some other minor issues -- I hope not to have broken > > too many other things in the process. Please give the whole thing a > > look, preferrably in all branches, and let me know if I've broken > > something in some horrible way. > > Thanks for taking the time to do this. Will test. I'm a little > unsure as to how regression tests involving different hosts might > work, but I'll see what I can do. Well, contrib/dblink is failing all over the place, for one thing. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] POLA violation with \c service=
On Wed, Apr 01, 2015 at 08:13:02PM -0300, Alvaro Herrera wrote: > I have pushed this after some rework. For instance, the 9.0 and 9.1 > versions believed that URIs were accepted, but that stuff was introduced > in 9.2. I changed some other minor issues -- I hope not to have broken > too many other things in the process. Please give the whole thing a > look, preferrably in all branches, and let me know if I've broken > something in some horrible way. Thanks for taking the time to do this. Will test. I'm a little unsure as to how regression tests involving different hosts might work, but I'll see what I can do. Cheers, David. -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] POLA violation with \c service=
I have pushed this after some rework. For instance, the 9.0 and 9.1 versions believed that URIs were accepted, but that stuff was introduced in 9.2. I changed some other minor issues -- I hope not to have broken too many other things in the process. Please give the whole thing a look, preferrably in all branches, and let me know if I've broken something in some horrible way. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] POLA violation with \c service=
On Fri, Feb 27, 2015 at 08:42:29AM -0800, David Fetter wrote: > On Mon, Feb 23, 2015 at 05:56:12PM -0300, Alvaro Herrera wrote: > > > > David Fetter wrote: > > > > > My thinking behind this was that the patch is a bug fix and intended > > > to be back-patched, so I wanted to mess with as little infrastructure > > > as possible. A new version of libpq seems like a very big ask for > > > such a case. You'll recall that the original problem was that > > > > > > \c service=foo > > > > > > only worked accidentally for some pretty narrow use cases and broke > > > without much of a clue for the rest. It turned out that the general > > > problem was that options given to psql on the command line were not > > > even remotely equivalent to \c, even though they were documented to > > > be. > > > > So, in view of these arguments and those put forward by Pavel > > downthread, I think the attached is an acceptable patch for the master > > branch. It doesn't apply to back branches though; 9.4 and 9.3 have a > > conflict in tab-complete.c, 9.2 has additional conflicts in command.c, > > and 9.1 and 9.0 are problematic all over because they don't have > > src/common. Could you please submit patches adapted for each group of > > branches? > > Please find patches attached for each live branch. Is this getting into the upcoming bug fix releases? Does it need rework to do so? Cheers, David. -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] POLA violation with \c service=
On Wed, Mar 4, 2015 at 8:33 AM, David Fetter wrote: > On Tue, Mar 03, 2015 at 09:52:55PM -0500, Robert Haas wrote: >> On Mon, Mar 2, 2015 at 5:05 PM, David Fetter wrote: >> > So just to clarify, are you against back-patching the behavior >> > change, or the addition to src/common? >> >> Mostly the latter. > > So you're saying the former isn't a problem? To recap, the behavior I > dug up was that sending a conninfo string or a URI to \c resulted in > \c's silently mangling same in a way that could only work accidentally. Well, the thing is, I'm not sure that's actually documented to work anywhere. psql says this: \c[onnect] [DBNAME|- USER|- HOST|- PORT|-] The psql documentation looks like this: \c or \connect [ dbname [ username ] [ host ] [ port ] ] Neither says anything about being able to use a conninfo string or a URI. So I am not sure why we shouldn't regard this as a new feature --- which, by the way, should be documented. Arguably the right thing to do is back-patch a change that prevents the dbname from being interpreted as anything other than a literal database name, and then in master make it work as you suggest. Now, if those two patches are substantially equal in size and risk, then you could argue that's just silly, and that we should just make this work all the way back. I'm willing to accept that argument if it is in fact true. But I'm not very excited about doing what amounts to a refactoring exercise in the back-branches. Shuffling code around from one file to another seems like something that we really ought to only be doing in master unless there's a really compelling reason to do otherwise, and making something work that is not documented to work does not compel me. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] POLA violation with \c service=
On Tue, Mar 03, 2015 at 09:52:55PM -0500, Robert Haas wrote: > On Mon, Mar 2, 2015 at 5:05 PM, David Fetter wrote: > > So just to clarify, are you against back-patching the behavior > > change, or the addition to src/common? > > Mostly the latter. So you're saying the former isn't a problem? To recap, the behavior I dug up was that sending a conninfo string or a URI to \c resulted in \c's silently mangling same in a way that could only work accidentally. Anyhow, patches have been posted for both approaches to all relevant branches. Cheers, David. -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] POLA violation with \c service=
On Mon, Mar 2, 2015 at 5:05 PM, David Fetter wrote: > So just to clarify, are you against back-patching the behavior change, > or the addition to src/common? Mostly the latter. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] POLA violation with \c service=
David Fetter wrote: > On Mon, Mar 02, 2015 at 04:52:37PM -0500, Robert Haas wrote: > > On Mon, Feb 23, 2015 at 3:56 PM, Alvaro Herrera > > wrote: > > > David Fetter wrote: > > > > > >> My thinking behind this was that the patch is a bug fix and intended > > >> to be back-patched, so I wanted to mess with as little infrastructure > > >> as possible. A new version of libpq seems like a very big ask for > > >> such a case. You'll recall that the original problem was that > > >> > > >> \c service=foo > > >> > > >> only worked accidentally for some pretty narrow use cases and broke > > >> without much of a clue for the rest. It turned out that the general > > >> problem was that options given to psql on the command line were not > > >> even remotely equivalent to \c, even though they were documented to > > >> be. > > > > > > So, in view of these arguments and those put forward by Pavel > > > downthread, I think the attached is an acceptable patch for the master > > > branch. It doesn't apply to back branches though; 9.4 and 9.3 have a > > > conflict in tab-complete.c, 9.2 has additional conflicts in command.c, > > > and 9.1 and 9.0 are problematic all over because they don't have > > > src/common. Could you please submit patches adapted for each group of > > > branches? > > > > I'm fine with this change in master, but I vote against back-patching > > it. This is not such an important problem that we need to take the > > risk of destabilizing existing installations. > > So just to clarify, are you against back-patching the behavior change, > or the addition to src/common? Not sure I agree with that. Currently \c is pretty useless with services and URIs. (The recently introduced behavior that it "forgets" the old connection info when the connection is lost, for example the server crashes, is pretty unhelpful too.) > > (Also, src/common is only 2 years old, so how would we back-patch > > anything touching that past 9.3 anyway?) > > I was hacking something together to add it. Should I stop? I think I wasn't very clear. What I was trying to say was that for all branches that have src/common we should put the duplicated functions there, and for older branches we'd just accept the duplication. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] POLA violation with \c service=
On Mon, Mar 02, 2015 at 04:52:37PM -0500, Robert Haas wrote: > On Mon, Feb 23, 2015 at 3:56 PM, Alvaro Herrera > wrote: > > David Fetter wrote: > > > >> My thinking behind this was that the patch is a bug fix and intended > >> to be back-patched, so I wanted to mess with as little infrastructure > >> as possible. A new version of libpq seems like a very big ask for > >> such a case. You'll recall that the original problem was that > >> > >> \c service=foo > >> > >> only worked accidentally for some pretty narrow use cases and broke > >> without much of a clue for the rest. It turned out that the general > >> problem was that options given to psql on the command line were not > >> even remotely equivalent to \c, even though they were documented to > >> be. > > > > So, in view of these arguments and those put forward by Pavel > > downthread, I think the attached is an acceptable patch for the master > > branch. It doesn't apply to back branches though; 9.4 and 9.3 have a > > conflict in tab-complete.c, 9.2 has additional conflicts in command.c, > > and 9.1 and 9.0 are problematic all over because they don't have > > src/common. Could you please submit patches adapted for each group of > > branches? > > I'm fine with this change in master, but I vote against back-patching > it. This is not such an important problem that we need to take the > risk of destabilizing existing installations. So just to clarify, are you against back-patching the behavior change, or the addition to src/common? > (Also, src/common is only 2 years old, so how would we back-patch > anything touching that past 9.3 anyway?) I was hacking something together to add it. Should I stop? Cheers, David. -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] POLA violation with \c service=
On Mon, Feb 23, 2015 at 3:56 PM, Alvaro Herrera wrote: > David Fetter wrote: > >> My thinking behind this was that the patch is a bug fix and intended >> to be back-patched, so I wanted to mess with as little infrastructure >> as possible. A new version of libpq seems like a very big ask for >> such a case. You'll recall that the original problem was that >> >> \c service=foo >> >> only worked accidentally for some pretty narrow use cases and broke >> without much of a clue for the rest. It turned out that the general >> problem was that options given to psql on the command line were not >> even remotely equivalent to \c, even though they were documented to >> be. > > So, in view of these arguments and those put forward by Pavel > downthread, I think the attached is an acceptable patch for the master > branch. It doesn't apply to back branches though; 9.4 and 9.3 have a > conflict in tab-complete.c, 9.2 has additional conflicts in command.c, > and 9.1 and 9.0 are problematic all over because they don't have > src/common. Could you please submit patches adapted for each group of > branches? I'm fine with this change in master, but I vote against back-patching it. This is not such an important problem that we need to take the risk of destabilizing existing installations. (Also, src/common is only 2 years old, so how would we back-patch anything touching that past 9.3 anyway?) -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] POLA violation with \c service=
On Fri, Feb 27, 2015 at 02:51:18PM -0300, Alvaro Herrera wrote: > I don't understand. Why don't these patches move anything to > src/common? Because I misunderstood the scope. Hope to get to those this evening. Cheers, David. -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] POLA violation with \c service=
I don't understand. Why don't these patches move anything to src/common? -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] POLA violation with \c service=
On Mon, Feb 23, 2015 at 05:56:12PM -0300, Alvaro Herrera wrote:
>
> David Fetter wrote:
>
> > My thinking behind this was that the patch is a bug fix and intended
> > to be back-patched, so I wanted to mess with as little infrastructure
> > as possible. A new version of libpq seems like a very big ask for
> > such a case. You'll recall that the original problem was that
> >
> > \c service=foo
> >
> > only worked accidentally for some pretty narrow use cases and broke
> > without much of a clue for the rest. It turned out that the general
> > problem was that options given to psql on the command line were not
> > even remotely equivalent to \c, even though they were documented to
> > be.
>
> So, in view of these arguments and those put forward by Pavel
> downthread, I think the attached is an acceptable patch for the master
> branch. It doesn't apply to back branches though; 9.4 and 9.3 have a
> conflict in tab-complete.c, 9.2 has additional conflicts in command.c,
> and 9.1 and 9.0 are problematic all over because they don't have
> src/common. Could you please submit patches adapted for each group of
> branches?
Please find patches attached for each live branch.
Cheers,
David.
--
David Fetter http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fet...@gmail.com
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
>From 64c7d3bcfad80dc80857b2ea06c9f2b7fff8f2a5 Mon Sep 17 00:00:00 2001
From: David Fetter
Date: Wed, 25 Feb 2015 13:51:17 -0800
Subject: [PATCH] From Alvaro Herrera
Resolved conflicts:
src/bin/psql/tab-complete.c
Resolved conflicts:
src/bin/psql/command.c
Resolved conflicts:
doc/src/sgml/ref/psql-ref.sgml
src/bin/psql/command.c
---
doc/src/sgml/ref/psql-ref.sgml | 61 ++---
src/bin/psql/command.c | 77 --
src/bin/psql/common.c | 36
src/bin/psql/common.h | 2 ++
src/bin/psql/tab-complete.c| 12 ++-
5 files changed, 150 insertions(+), 38 deletions(-)
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 15bbd7a..b7f0601 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -751,33 +751,20 @@ testdb=>
-\C [ title
]
-
-
-Sets the title of any tables being printed as the result of a
-query or unset any such title. This command is equivalent to
-\pset title title. (The name of
-this command derives from caption, as it was
-previously only used to set the caption in an
-HTML table.)
-
-
-
-
-
-\connect (or \c)
[ dbname [ username ] [ host ] [ port ] ]
+\c or \connect [
{ [ dbname [ username ] [ host ] [ port ] ] | conninfo string | URI } ]
Establishes a new connection to a PostgreSQL
-server. If the new connection is successfully made, the
-previous connection is closed. If any of conninfo string, or a URI.
If the new connection is
+successfully made, the
+previous connection is closed. When using positional parameters, if
any of dbname, username, host or port are omitted or specified
as -, the value of that parameter from the
-previous connection is used. If there is no previous
+previous connection is used. If using positional parameters and there
is no previous
connection, the libpq default for
the parameter's value is used.
@@ -792,6 +779,42 @@ testdb=>
mechanism that scripts are not accidentally acting on the
wrong database on the other hand.
+
+
+Positional syntax:
+
+=> \c mydb myuser host.dom 6432
+
+
+
+
+The conninfo form takes two forms: keyword-value pairs
+
+
+=> \c service=foo
+=> \c "host=localhost port=5432 dbname=mydb connect_timeout=10
sslmode=disable"
+
+
+and URIs:
+
+
+=> \c postgresql://tom@localhost/mydb?application_name=myapp
+
+
+
+
+
+\C [ title
]
+
+
+Sets the title of any tables being printed as the result of a
+query or unset any such title. This command is equivalent to
+\pset title title. (The name of
+this command derives from caption, as it was
+previously only used to set the caption in an
+HTML table.)
+
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index d2967ed..49c20bd 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -1240,16 +1240,46 @@ do_connect(char *dbname, char *user, char *host, char
*port)
PGconn *o_conn = pset.db,
*n_conn;
char *password = NULL;
+ boolke
Re: [HACKERS] POLA violation with \c service=
Here's the real attachment. Previous one was a misguided shell
redirection. Meh.
--
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>From 830d41b9d23716af29491cbab59808c35e25ec12 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera
Date: Fri, 20 Feb 2015 14:36:41 -0300
Subject: [PATCH] Fix psql's \c to work with URIs and conninfo strings
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
This makes it more consistent with what can be given in the command
line.
Authors: Andrew Dunstan, David Fetter, Andrew Gierth, Pavel Stěhule
---
doc/src/sgml/ref/psql-ref.sgml| 30 ++---
src/bin/psql/command.c| 69 +++
src/bin/psql/tab-complete.c | 20 ++--
src/common/Makefile | 2 +-
src/common/connstrings.c | 52 +
src/include/common/connstrings.h | 16 +
src/interfaces/libpq/fe-connect.c | 36 +---
7 files changed, 162 insertions(+), 63 deletions(-)
create mode 100644 src/common/connstrings.c
create mode 100644 src/include/common/connstrings.h
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index a637001..a218af8 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -796,18 +796,23 @@ testdb=>
-\c or \connect [ dbname [ username ] [ host ] [ port ] ]
+\c [ { [ dbname [ username ] [ host ] [ port ] ] | conninfo string } ]
+\connect [ { [ dbname [ username ] [ host ] [ port ] ] | conninfo string } ]
Establishes a new connection to a PostgreSQL
-server. If the new connection is successfully made, the
-previous connection is closed. If any of conninfo string, or a URI.
+If the new connection is successfully made, the
+previous connection is closed.
+When using positional parameters, if any of dbname, username, host or port are omitted or specified
as -, the value of that parameter from the
-previous connection is used. If there is no previous
+previous connection is used.
+If using positional parameters and there is no previous
connection, the libpq default for
the parameter's value is used.
@@ -822,6 +827,23 @@ testdb=>
mechanism that scripts are not accidentally acting on the
wrong database on the other hand.
+
+
+Positional syntax:
+
+=> \c mydb myuser host.dom 6432
+
+
+
+
+The conninfo form detailed in
+takes two forms: keyword-value pairs, and URIs:
+
+
+=> \c service=foo
+=> \c "host=localhost port=5432 dbname=mydb connect_timeout=10 sslmode=disable"
+=> \c postgresql://tom@localhost/mydb?application_name=myapp
+
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 7c9f28d..7d8d5bb 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -1607,6 +1607,8 @@ do_connect(char *dbname, char *user, char *host, char *port)
PGconn *o_conn = pset.db,
*n_conn;
char *password = NULL;
+ bool keep_password = true;
+ bool has_connection_string = false;
if (!o_conn && (!dbname || !user || !host || !port))
{
@@ -1620,15 +1622,31 @@ do_connect(char *dbname, char *user, char *host, char *port)
return false;
}
- if (!dbname)
- dbname = PQdb(o_conn);
if (!user)
user = PQuser(o_conn);
+
if (!host)
host = PQhost(o_conn);
+
if (!port)
port = PQport(o_conn);
+ if (dbname)
+ has_connection_string = recognized_connection_string(dbname);
+
+ keep_password =
+ ((strcmp(user, PQuser(o_conn)) == 0) &&
+ (!host || strcmp(host, PQhost(o_conn)) == 0) &&
+ (strcmp(port, PQport(o_conn)) == 0) &&
+ !has_connection_string);
+
+ /*
+ * Unlike the previous stanzas, changing only the dbname shouldn't trigger
+ * throwing away the password.
+ */
+ if (!dbname)
+ dbname = PQdb(o_conn);
+
/*
* If the user asked to be prompted for a password, ask for one now. If
* not, use the password from the old connection, provided the username
@@ -1643,9 +1661,13 @@ do_connect(char *dbname, char *user, char *host, char *port)
{
password = prompt_for_password(user);
}
- else if (o_conn && user && strcmp(PQuser(o_conn), user) == 0)
+ else if (o_conn && keep_password)
{
- password = pg_strdup(PQpass(o_conn));
+ password = PQpass(o_conn);
+ if (password && *password)
+ password = pg_strdup(password);
+ else
+ password = NULL;
}
while (true)
@@ -1653,23 +1675,28 @@ do_connect(char *dbname, char *user, char *host, char *port)
#define PARAMS_ARRAY_SIZE 8
const char **keywords = pg_malloc(PARAMS_ARRAY_SIZE * sizeof(*keywords));
const char **values = pg_malloc(PARAMS_ARRAY_SIZE * sizeof(*values));
-
- keywords[0] = "host";
- values[0] = host;
-
Re: [HACKERS] POLA violation with \c service=
David Fetter wrote: > My thinking behind this was that the patch is a bug fix and intended > to be back-patched, so I wanted to mess with as little infrastructure > as possible. A new version of libpq seems like a very big ask for > such a case. You'll recall that the original problem was that > > \c service=foo > > only worked accidentally for some pretty narrow use cases and broke > without much of a clue for the rest. It turned out that the general > problem was that options given to psql on the command line were not > even remotely equivalent to \c, even though they were documented to > be. So, in view of these arguments and those put forward by Pavel downthread, I think the attached is an acceptable patch for the master branch. It doesn't apply to back branches though; 9.4 and 9.3 have a conflict in tab-complete.c, 9.2 has additional conflicts in command.c, and 9.1 and 9.0 are problematic all over because they don't have src/common. Could you please submit patches adapted for each group of branches? -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] POLA violation with \c service=
2015-02-21 7:04 GMT+01:00 Pavel Stehule : > Hi > > 2015-02-20 21:55 GMT+01:00 Alvaro Herrera : > >> Pavel Stehule wrote: >> > 2015-02-20 8:22 GMT+01:00 David Fetter : >> > >> > > On Fri, Feb 20, 2015 at 07:10:29AM +0100, Pavel Stehule wrote: >> > > > Hi >> > > > >> > > > I am happy with doc changes now. >> > > > >> > > > When I test last patch, I found sigfault bug, because host = >> > > > PQhost(o_conn); returns NULL. I fexed it - please, see patch 007 >> > > > >> > > > If you are agree with fix, I'll mark this patch as ready for commit. >> > > >> > > Thanks for fixing the bug. Let's go with this. >> > > >> > >> > marked as "ready for commit" >> >> Gave this patch a look. In general it looks pretty good, but there is >> one troublesome point: it duplicates two functions from libpq into psql, >> including the URI designators. This doesn't look very nice. I thought >> about just creating a new src/common (say connstring.c) to host those >> two functions and the URI designators, but then on closer look I noticed >> that libpq's facilities for URI parsing become severed: two very small >> functions become part of libpgcommon, while the more complex parts >> remain in libpq. >> >> On the other hand, if we see that psql needs this functionality, isn't >> this a clue that other client programs might find it useful too? >> (Honestly I'm not completely sure about this point -- other opinions?) >> >> I see three[four] ways forward from here: >> >> 1. export this functionality in libpq as one or two new functions. This >> would need proper docs, exports.txt, etc. >> > > I don't think so it is preferable way - me (as developer) doesn't interest > a format of connection string - and if somebody would to check the format, > then he use a simply regexp. It is task for libpq to check and detect used > format correctly. "psql" works on very low level and needs these > functionality almost all for autocomplete - and it is not usual task for > database based applications. > and libpq should not bloat too much > > >> >> 2. export it in libpgcommon. If we choose this option we should >> probably rename those functions, as in the attached patch. >> > > +1 > > >> >> 3. accept the patch as is, i.e. duplicate the libq-internal functions in >> psql. >> >> [4. reject the whole thing] >> >> I lean towards (2) myself, but what do others think? >> > > aggree with you > > Regards > > Pavel > > >> >> -- >> Álvaro Herrerahttp://www.2ndQuadrant.com/ >> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services >> > >
Re: [HACKERS] POLA violation with \c service=
2015-02-20 22:25 GMT+01:00 Alvaro Herrera : > David Fetter wrote: > > On Fri, Feb 20, 2015 at 05:55:20PM -0300, Alvaro Herrera wrote: > > > > Gave this patch a look. In general it looks pretty good, but there is > > > one troublesome point: it duplicates two functions from libpq into > psql, > > > including the URI designators. This doesn't look very nice. > > > > My thinking behind this was that the patch is a bug fix and intended > > to be back-patched, so I wanted to mess with as little infrastructure > > as possible. > > Oh, so this is to be backpatched? Yeah, I guess in the backbranch case > it's acceptable to duplicate some bits, but I don't think this gives us > blanket permission to do it in master. So we need two versions of the > patch. > > > > 2. export it in libpgcommon. If we choose this option we should > > > probably rename those functions, as in the attached patch. > > > > I'm not super attached to the names. > > Me neither. Suggestions welcome. > I have not any problem with these names - it is related what it does. "libpq_connstring_uri_prefix_length" is 100% correct "libpq_connstring_is_recognized" is correct too .. the name is maybe long, but this functions are not used 100x per day Regards Pavel > > -- > Álvaro Herrerahttp://www.2ndQuadrant.com/ > PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services >
Re: [HACKERS] POLA violation with \c service=
Hi 2015-02-20 21:55 GMT+01:00 Alvaro Herrera : > Pavel Stehule wrote: > > 2015-02-20 8:22 GMT+01:00 David Fetter : > > > > > On Fri, Feb 20, 2015 at 07:10:29AM +0100, Pavel Stehule wrote: > > > > Hi > > > > > > > > I am happy with doc changes now. > > > > > > > > When I test last patch, I found sigfault bug, because host = > > > > PQhost(o_conn); returns NULL. I fexed it - please, see patch 007 > > > > > > > > If you are agree with fix, I'll mark this patch as ready for commit. > > > > > > Thanks for fixing the bug. Let's go with this. > > > > > > > marked as "ready for commit" > > Gave this patch a look. In general it looks pretty good, but there is > one troublesome point: it duplicates two functions from libpq into psql, > including the URI designators. This doesn't look very nice. I thought > about just creating a new src/common (say connstring.c) to host those > two functions and the URI designators, but then on closer look I noticed > that libpq's facilities for URI parsing become severed: two very small > functions become part of libpgcommon, while the more complex parts > remain in libpq. > > On the other hand, if we see that psql needs this functionality, isn't > this a clue that other client programs might find it useful too? > (Honestly I'm not completely sure about this point -- other opinions?) > > I see three[four] ways forward from here: > > 1. export this functionality in libpq as one or two new functions. This > would need proper docs, exports.txt, etc. > I don't think so it is preferable way - me (as developer) doesn't interest a format of connection string - and if somebody would to check the format, then he use a simply regexp. It is task for libpq to check and detect used format correctly. "psql" works on very low level and needs these functionality almost all for autocomplete - and it is not usual task for database based applications. > > 2. export it in libpgcommon. If we choose this option we should > probably rename those functions, as in the attached patch. > +1 > > 3. accept the patch as is, i.e. duplicate the libq-internal functions in > psql. > > [4. reject the whole thing] > > I lean towards (2) myself, but what do others think? > aggree with you Regards Pavel > > -- > Álvaro Herrerahttp://www.2ndQuadrant.com/ > PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services >
Re: [HACKERS] POLA violation with \c service=
David Fetter wrote: > On Fri, Feb 20, 2015 at 05:55:20PM -0300, Alvaro Herrera wrote: > > Gave this patch a look. In general it looks pretty good, but there is > > one troublesome point: it duplicates two functions from libpq into psql, > > including the URI designators. This doesn't look very nice. > > My thinking behind this was that the patch is a bug fix and intended > to be back-patched, so I wanted to mess with as little infrastructure > as possible. Oh, so this is to be backpatched? Yeah, I guess in the backbranch case it's acceptable to duplicate some bits, but I don't think this gives us blanket permission to do it in master. So we need two versions of the patch. > > 2. export it in libpgcommon. If we choose this option we should > > probably rename those functions, as in the attached patch. > > I'm not super attached to the names. Me neither. Suggestions welcome. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] POLA violation with \c service=
On Fri, Feb 20, 2015 at 05:55:20PM -0300, Alvaro Herrera wrote: > Pavel Stehule wrote: > > 2015-02-20 8:22 GMT+01:00 David Fetter : > > > > > On Fri, Feb 20, 2015 at 07:10:29AM +0100, Pavel Stehule wrote: > > > > Hi > > > > > > > > I am happy with doc changes now. > > > > > > > > When I test last patch, I found sigfault bug, because host = > > > > PQhost(o_conn); returns NULL. I fexed it - please, see patch 007 > > > > > > > > If you are agree with fix, I'll mark this patch as ready for commit. > > > > > > Thanks for fixing the bug. Let's go with this. > > > > > > > marked as "ready for commit" > > Gave this patch a look. In general it looks pretty good, but there is > one troublesome point: it duplicates two functions from libpq into psql, > including the URI designators. This doesn't look very nice. My thinking behind this was that the patch is a bug fix and intended to be back-patched, so I wanted to mess with as little infrastructure as possible. A new version of libpq seems like a very big ask for such a case. You'll recall that the original problem was that \c service=foo only worked accidentally for some pretty narrow use cases and broke without much of a clue for the rest. It turned out that the general problem was that options given to psql on the command line were not even remotely equivalent to \c, even though they were documented to be. > I thought about just creating a new src/common (say connstring.c) to > host those two functions and the URI designators, but then on closer > look I noticed that libpq's facilities for URI parsing become > severed: two very small functions become part of libpgcommon, while > the more complex parts remain in libpq. Right. > On the other hand, if we see that psql needs this functionality, isn't > this a clue that other client programs might find it useful too? > (Honestly I'm not completely sure about this point -- other opinions?) It might well. > I see three[four] ways forward from here: > > 1. export this functionality in libpq as one or two new functions. This > would need proper docs, exports.txt, etc. That sounds like a great thing going forward. > 2. export it in libpgcommon. If we choose this option we should > probably rename those functions, as in the attached patch. I'm not super attached to the names. > 3. accept the patch as is, i.e. duplicate the libq-internal functions in > psql. Again, my chief concern was to produce a back-patchable bug fix. The internal functions could be in the going-backward versions, and the shared ones in the going-forward (9.5+) versions. > [4. reject the whole thing] > > I lean towards (2) myself, but what do others think? Obviously, I'm biased, as the original impulse to fix this bug was mine. Cheers, David. -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] POLA violation with \c service=
Pavel Stehule wrote:
> 2015-02-20 8:22 GMT+01:00 David Fetter :
>
> > On Fri, Feb 20, 2015 at 07:10:29AM +0100, Pavel Stehule wrote:
> > > Hi
> > >
> > > I am happy with doc changes now.
> > >
> > > When I test last patch, I found sigfault bug, because host =
> > > PQhost(o_conn); returns NULL. I fexed it - please, see patch 007
> > >
> > > If you are agree with fix, I'll mark this patch as ready for commit.
> >
> > Thanks for fixing the bug. Let's go with this.
> >
>
> marked as "ready for commit"
Gave this patch a look. In general it looks pretty good, but there is
one troublesome point: it duplicates two functions from libpq into psql,
including the URI designators. This doesn't look very nice. I thought
about just creating a new src/common (say connstring.c) to host those
two functions and the URI designators, but then on closer look I noticed
that libpq's facilities for URI parsing become severed: two very small
functions become part of libpgcommon, while the more complex parts
remain in libpq.
On the other hand, if we see that psql needs this functionality, isn't
this a clue that other client programs might find it useful too?
(Honestly I'm not completely sure about this point -- other opinions?)
I see three[four] ways forward from here:
1. export this functionality in libpq as one or two new functions. This
would need proper docs, exports.txt, etc.
2. export it in libpgcommon. If we choose this option we should
probably rename those functions, as in the attached patch.
3. accept the patch as is, i.e. duplicate the libq-internal functions in
psql.
[4. reject the whole thing]
I lean towards (2) myself, but what do others think?
--
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index a637001..173b6c3 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -796,18 +796,21 @@ testdb=>
-\c or \connect [ dbname [ username ] [ host ] [ port ] ]
+\c [ { [ dbname [ username ] [ host ] [ port ] ] | conninfo string } ]
+\connect [ { [ dbname [ username ] [ host ] [ port ] ] | conninfo string } ]
Establishes a new connection to a PostgreSQL
-server. If the new connection is successfully made, the
-previous connection is closed. If any of conninfo string, or a URI. If the new connection is
+successfully made, the
+previous connection is closed. When using positional parameters, if any of dbname, username, host or port are omitted or specified
as -, the value of that parameter from the
-previous connection is used. If there is no previous
+previous connection is used. If using positional parameters and there is no previous
connection, the libpq default for
the parameter's value is used.
@@ -822,6 +825,22 @@ testdb=>
mechanism that scripts are not accidentally acting on the
wrong database on the other hand.
+
+
+Positional syntax:
+
+=> \c mydb myuser host.dom 6432
+
+
+
+
+The conninfo form detailed in takes two forms: keyword-value pairs, and URIs:
+
+
+=> \c service=foo
+=> \c "host=localhost port=5432 dbname=mydb connect_timeout=10 sslmode=disable"
+=> \c postgresql://tom@localhost/mydb?application_name=myapp
+
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 4201956..64210bf 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -31,6 +31,7 @@
#include /* for stat() */
#endif
+#include "common/connstrings.h"
#include "portability/instr_time.h"
#include "libpq-fe.h"
@@ -1608,6 +1609,8 @@ do_connect(char *dbname, char *user, char *host, char *port)
PGconn *o_conn = pset.db,
*n_conn;
char *password = NULL;
+ bool keep_password = true;
+ bool has_connection_string = false;
if (!o_conn && (!dbname || !user || !host || !port))
{
@@ -1621,15 +1624,31 @@ do_connect(char *dbname, char *user, char *host, char *port)
return false;
}
- if (!dbname)
- dbname = PQdb(o_conn);
if (!user)
user = PQuser(o_conn);
+
if (!host)
host = PQhost(o_conn);
+
if (!port)
port = PQport(o_conn);
+ if (dbname)
+ has_connection_string = libpq_connstring_is_recognized(dbname);
+
+ keep_password =
+ ((strcmp(user, PQuser(o_conn)) == 0) &&
+ (!host || strcmp(host, PQhost(o_conn)) == 0) &&
+ (strcmp(port, PQport(o_conn)) == 0) &&
+ !has_connection_string);
+
+ /*
+ * Unlike the previous stanzas, changing only the dbname shouldn't trigger
+ * throwing away the password.
+ */
+ if (!dbname)
+ dbname = PQdb(o_conn);
+
/*
* If the user asked to be prompted for a password, ask for one now. If
* not, use the password from the old connection, provided the username
@@ -1644,9 +1663,13 @
Re: [HACKERS] POLA violation with \c service=
2015-02-20 8:22 GMT+01:00 David Fetter : > On Fri, Feb 20, 2015 at 07:10:29AM +0100, Pavel Stehule wrote: > > Hi > > > > I am happy with doc changes now. > > > > When I test last patch, I found sigfault bug, because host = > > PQhost(o_conn); returns NULL. I fexed it - please, see patch 007 > > > > If you are agree with fix, I'll mark this patch as ready for commit. > > Thanks for fixing the bug. Let's go with this. > marked as "ready for commit" Thank you for patch Regards Pavel > > Cheers, > David. > -- > David Fetter http://fetter.org/ > Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter > Skype: davidfetter XMPP: david.fet...@gmail.com > > Remember to vote! > Consider donating to Postgres: http://www.postgresql.org/about/donate >
Re: [HACKERS] POLA violation with \c service=
The following review has been posted through the commitfest application: make installcheck-world: tested, passed Implements feature: tested, passed Spec compliant: tested, passed Documentation:tested, passed no issues with last 007 patch -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] POLA violation with \c service=
On Fri, Feb 20, 2015 at 07:10:29AM +0100, Pavel Stehule wrote: > Hi > > I am happy with doc changes now. > > When I test last patch, I found sigfault bug, because host = > PQhost(o_conn); returns NULL. I fexed it - please, see patch 007 > > If you are agree with fix, I'll mark this patch as ready for commit. Thanks for fixing the bug. Let's go with this. Cheers, David. -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] POLA violation with \c service=
Hi
I am happy with doc changes now.
When I test last patch, I found sigfault bug, because host =
PQhost(o_conn); returns NULL. I fexed it - please, see patch 007
If you are agree with fix, I'll mark this patch as ready for commit.
Regards
Pavel
2015-02-19 23:33 GMT+01:00 David Fetter :
> On Thu, Feb 19, 2015 at 09:32:29PM +0100, Pavel Stehule wrote:
> > 2015-02-19 19:51 GMT+01:00 David Fetter :
> >
> > > On Sun, Feb 01, 2015 at 08:38:24AM +0100, Pavel Stehule wrote:
> > >
> > > I'm not sure how best to illustrate those. Are you thinking of one
> > > example each for the URI and conninfo cases?
> > >
> >
> > some like
> >
> > "most common form is:
> >
> > "\c mydb"
> >
> > but you can use any connection format described
> > (libpq-connect.html#LIBPQ-PARAMKEYWORDS) like
> >
> > "\c postgresql://tom@localhost/mydb?application_name=myapp"
>
> Examples added.
>
> Cheers,
> David.
> --
> David Fetter http://fetter.org/
> Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
> Skype: davidfetter XMPP: david.fet...@gmail.com
>
> Remember to vote!
> Consider donating to Postgres: http://www.postgresql.org/about/donate
>
commit f0e71c50590b94da5dafb72a377c7c70b49ce488
Author: root
Date: Fri Feb 20 07:04:53 2015 +0100
fix segfault due empty variable host
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index a637001..340a5d5 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -796,18 +796,20 @@ testdb=>
-\c or \connect [ dbname [ username ] [ host ] [ port ] ]
+\c or \connect [ { [ dbname [ username ] [ host ] [ port ] ] | conninfo string | URI } ]
Establishes a new connection to a PostgreSQL
-server. If the new connection is successfully made, the
-previous connection is closed. If any of conninfo string, or a URI. If the new connection is
+successfully made, the
+previous connection is closed. When using positional parameters, if any of dbname, username, host or port are omitted or specified
as -, the value of that parameter from the
-previous connection is used. If there is no previous
+previous connection is used. If using positional parameters and there is no previous
connection, the libpq default for
the parameter's value is used.
@@ -822,6 +824,27 @@ testdb=>
mechanism that scripts are not accidentally acting on the
wrong database on the other hand.
+
+
+Positional syntax:
+
+=> \c mydb myuser host.dom 6432
+
+
+
+
+The conninfo form detailed in takes two forms: keyword-value pairs
+
+
+=> \c service=foo
+=> \c "host=localhost port=5432 dbname=mydb connect_timeout=10 sslmode=disable"
+
+
+and URIs:
+
+
+=> \c postgresql://tom@localhost/mydb?application_name=myapp
+
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 7c9f28d..dd9350e 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -1607,6 +1607,8 @@ do_connect(char *dbname, char *user, char *host, char *port)
PGconn *o_conn = pset.db,
*n_conn;
char *password = NULL;
+ bool keep_password = true;
+ bool has_connection_string = false;
if (!o_conn && (!dbname || !user || !host || !port))
{
@@ -1620,15 +1622,31 @@ do_connect(char *dbname, char *user, char *host, char *port)
return false;
}
- if (!dbname)
- dbname = PQdb(o_conn);
if (!user)
user = PQuser(o_conn);
+
if (!host)
host = PQhost(o_conn);
+
if (!port)
port = PQport(o_conn);
+ if (dbname)
+ has_connection_string = recognized_connection_string(dbname);
+
+ keep_password = (
+ (strcmp(user, PQuser(o_conn)) == 0) &&
+ (!host || strcmp(host, PQhost(o_conn)) == 0) &&
+ (strcmp(port, PQport(o_conn)) == 0) &&
+ !has_connection_string);
+
+ /*
+ * Unlike the previous stanzas, changing only the dbname shouldn't
+ * trigger throwing away the password.
+ */
+ if (!dbname)
+ dbname = PQdb(o_conn);
+
/*
* If the user asked to be prompted for a password, ask for one now. If
* not, use the password from the old connection, provided the username
@@ -1643,9 +1661,13 @@ do_connect(char *dbname, char *user, char *host, char *port)
{
password = prompt_for_password(user);
}
- else if (o_conn && user && strcmp(PQuser(o_conn), user) == 0)
+ else if (o_conn && keep_password)
{
- password = pg_strdup(PQpass(o_conn));
+ password = PQpass(o_conn);
+ if (password && *password)
+ password = pg_strdup(password);
+ else
+ password = NULL;
}
while (true)
@@ -1653,23 +1675,28 @@ do_connect(char *dbname, char *user, char *host, char *port)
#define PARAMS_ARRAY_SIZE 8
const char **keywords = pg_malloc(PARAMS_ARRAY_SIZE * sizeof(*keywords));
const char **values = pg_malloc(PARAMS_ARRAY_SIZE * sizeof(*values));
-
- keywords[0] = "host";
- values[0] = host;
Re: [HACKERS] POLA violation with \c service=
On Thu, Feb 19, 2015 at 09:32:29PM +0100, Pavel Stehule wrote:
> 2015-02-19 19:51 GMT+01:00 David Fetter :
>
> > On Sun, Feb 01, 2015 at 08:38:24AM +0100, Pavel Stehule wrote:
> >
> > I'm not sure how best to illustrate those. Are you thinking of one
> > example each for the URI and conninfo cases?
> >
>
> some like
>
> "most common form is:
>
> "\c mydb"
>
> but you can use any connection format described
> (libpq-connect.html#LIBPQ-PARAMKEYWORDS) like
>
> "\c postgresql://tom@localhost/mydb?application_name=myapp"
Examples added.
Cheers,
David.
--
David Fetter http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fet...@gmail.com
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index a637001..340a5d5 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -796,18 +796,20 @@ testdb=>
-\c or \connect [
dbname [ username ] [ host ] [ port ] ]
+\c or \connect [
{ [ dbname [ username ] [ host ] [ port ] ] | conninfo string | URI } ]
Establishes a new connection to a PostgreSQL
-server. If the new connection is successfully made, the
-previous connection is closed. If any of conninfo string, or a URI.
If the new connection is
+successfully made, the
+previous connection is closed. When using positional parameters, if
any of dbname, username, host or port are omitted or specified
as -, the value of that parameter from the
-previous connection is used. If there is no previous
+previous connection is used. If using positional parameters and there
is no previous
connection, the libpq default for
the parameter's value is used.
@@ -822,6 +824,27 @@ testdb=>
mechanism that scripts are not accidentally acting on the
wrong database on the other hand.
+
+
+Positional syntax:
+
+=> \c mydb myuser host.dom 6432
+
+
+
+
+The conninfo form detailed in takes
two forms: keyword-value pairs
+
+
+=> \c service=foo
+=> \c "host=localhost port=5432 dbname=mydb connect_timeout=10
sslmode=disable"
+
+
+and URIs:
+
+
+=> \c postgresql://tom@localhost/mydb?application_name=myapp
+
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 7c9f28d..ec86e52 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -1607,6 +1607,8 @@ do_connect(char *dbname, char *user, char *host, char
*port)
PGconn *o_conn = pset.db,
*n_conn;
char *password = NULL;
+ boolkeep_password = true;
+ boolhas_connection_string = false;
if (!o_conn && (!dbname || !user || !host || !port))
{
@@ -1620,15 +1622,31 @@ do_connect(char *dbname, char *user, char *host, char
*port)
return false;
}
- if (!dbname)
- dbname = PQdb(o_conn);
if (!user)
user = PQuser(o_conn);
+
if (!host)
host = PQhost(o_conn);
+
if (!port)
port = PQport(o_conn);
+ if (dbname)
+ has_connection_string = recognized_connection_string(dbname);
+
+ keep_password = (
+ (strcmp(user, PQuser(o_conn)) == 0) &&
+ (strcmp(host, PQhost(o_conn)) == 0) &&
+ (strcmp(port, PQport(o_conn)) == 0) &&
+!has_connection_string);
+
+ /*
+* Unlike the previous stanzas, changing only the dbname shouldn't
+* trigger throwing away the password.
+*/
+ if (!dbname)
+ dbname = PQdb(o_conn);
+
/*
* If the user asked to be prompted for a password, ask for one now. If
* not, use the password from the old connection, provided the username
@@ -1643,9 +1661,13 @@ do_connect(char *dbname, char *user, char *host, char
*port)
{
password = prompt_for_password(user);
}
- else if (o_conn && user && strcmp(PQuser(o_conn), user) == 0)
+ else if (o_conn && keep_password)
{
- password = pg_strdup(PQpass(o_conn));
+ password = PQpass(o_conn);
+ if (password && *password)
+ password = pg_strdup(password);
+ else
+ password = NULL;
}
while (true)
@@ -1653,23 +1675,28 @@ do_connect(char *dbname, char *user, char *host, char
*port)
#define PARAMS_ARRAY_SIZE 8
const char **keywords = pg_malloc(PARAMS_ARRAY_SIZE *
sizeof(*keywords));
const char **values = pg_malloc(PARAMS_ARRAY_SIZE *
sizeof(*values));
+
Re: [HACKERS] POLA violation with \c service=
2015-02-19 19:51 GMT+01:00 David Fetter : > On Sun, Feb 01, 2015 at 08:38:24AM +0100, Pavel Stehule wrote: > > Hi all > > > > I am sending a review of this patch: > > > > * What it does? - Allow to connect to other db by \connect uri connection > > format > > > > postgres=# \c postgresql://localhost?service=old > > psql (9.5devel, server 9.2.9) > > You are now connected to database "postgres" as user "pavel". > > > > * Would we this feature? - yes, it eliminate inconsistency between cmd > line > > connect and \connect. It is good idea without any objections. > > > > * This patch is cleanly applicable, later compilation without any issues > > > > * All regress tests passed > > > > * A psql documentation is updated -- this feature (and format) is not > > widely known, so maybe some more examples are welcome > > > > * When I tested this feature, it worked as expected > > > > * Code respects PostgreSQL coding rules. I prefer a little bit different > > test if keep password. Current code is little bit harder to understand. > But > > I can live with David's code well too. > > > > if > > (!user) > > > > user = PQuser(o_conn); > > > > if > > (!host) > > > > host = > > PQhost(o_conn); > > > > > > if > > (!port) > > > > port = > > PQport(o_conn); > > > > > > if > > (dbname) > > > > has_connection_string = > > recognized_connection_string(dbname); > > > > > >/* we should not to keep password if some connection property is > changed > > */ > > > > > > keep_password = strcmp(user, PQuser(o_conn)) == 0 && strcmp(host, > > PQhost(o_conn)) == 0 > > && strcmp(port, PQport(o_conn)) == 0 && > > !has_connection_string; > > Changed. This is cleaner. > > > I have not any other comments. > > > > Possible questions: > > 1. more examples in doc > > I'm not sure how best to illustrate those. Are you thinking of one > example each for the URI and conninfo cases? > some like "most common form is: "\c mydb" but you can use any connection format described (libpq-connect.html#LIBPQ-PARAMKEYWORDS) like "\c postgresql://tom@localhost/mydb?application_name=myapp" > > > 2. small change how to check keep_password > > Done. > > Cheers, > David. > -- > David Fetter http://fetter.org/ > Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter > Skype: davidfetter XMPP: david.fet...@gmail.com > > Remember to vote! > Consider donating to Postgres: http://www.postgresql.org/about/donate >
Re: [HACKERS] POLA violation with \c service=
On Sun, Feb 01, 2015 at 08:38:24AM +0100, Pavel Stehule wrote:
> Hi all
>
> I am sending a review of this patch:
>
> * What it does? - Allow to connect to other db by \connect uri connection
> format
>
> postgres=# \c postgresql://localhost?service=old
> psql (9.5devel, server 9.2.9)
> You are now connected to database "postgres" as user "pavel".
>
> * Would we this feature? - yes, it eliminate inconsistency between cmd line
> connect and \connect. It is good idea without any objections.
>
> * This patch is cleanly applicable, later compilation without any issues
>
> * All regress tests passed
>
> * A psql documentation is updated -- this feature (and format) is not
> widely known, so maybe some more examples are welcome
>
> * When I tested this feature, it worked as expected
>
> * Code respects PostgreSQL coding rules. I prefer a little bit different
> test if keep password. Current code is little bit harder to understand. But
> I can live with David's code well too.
>
> if
> (!user)
>
> user = PQuser(o_conn);
>
> if
> (!host)
>
> host =
> PQhost(o_conn);
>
>
> if
> (!port)
>
> port =
> PQport(o_conn);
>
>
> if
> (dbname)
>
> has_connection_string =
> recognized_connection_string(dbname);
>
>
>/* we should not to keep password if some connection property is changed
> */
>
>
> keep_password = strcmp(user, PQuser(o_conn)) == 0 && strcmp(host,
> PQhost(o_conn)) == 0
> && strcmp(port, PQport(o_conn)) == 0 &&
> !has_connection_string;
Changed. This is cleaner.
> I have not any other comments.
>
> Possible questions:
> 1. more examples in doc
I'm not sure how best to illustrate those. Are you thinking of one
example each for the URI and conninfo cases?
> 2. small change how to check keep_password
Done.
Cheers,
David.
--
David Fetter http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fet...@gmail.com
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index a637001..cae6bf4 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -796,18 +796,20 @@ testdb=>
-\c or \connect [
dbname [ username ] [ host ] [ port ] ]
+\c or \connect [
{ [ dbname [ username ] [ host ] [ port ] ] | conninfo string | URI } ]
Establishes a new connection to a PostgreSQL
-server. If the new connection is successfully made, the
-previous connection is closed. If any of conninfo string, or a URI.
If the new connection is
+successfully made, the
+previous connection is closed. When using positional parameters, if
any of dbname, username, host or port are omitted or specified
as -, the value of that parameter from the
-previous connection is used. If there is no previous
+previous connection is used. If using positional parameters and there
is no previous
connection, the libpq default for
the parameter's value is used.
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 7c9f28d..ec86e52 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -1607,6 +1607,8 @@ do_connect(char *dbname, char *user, char *host, char
*port)
PGconn *o_conn = pset.db,
*n_conn;
char *password = NULL;
+ boolkeep_password = true;
+ boolhas_connection_string = false;
if (!o_conn && (!dbname || !user || !host || !port))
{
@@ -1620,15 +1622,31 @@ do_connect(char *dbname, char *user, char *host, char
*port)
return false;
}
- if (!dbname)
- dbname = PQdb(o_conn);
if (!user)
user = PQuser(o_conn);
+
if (!host)
host = PQhost(o_conn);
+
if (!port)
port = PQport(o_conn);
+ if (dbname)
+ has_connection_string = recognized_connection_string(dbname);
+
+ keep_password = (
+ (strcmp(user, PQuser(o_conn)) == 0) &&
+ (strcmp(host, PQhost(o_conn)) == 0) &&
+ (strcmp(port, PQport(o_conn)) == 0) &&
+!has_connection_string);
+
+ /*
+* Unlike the previous stanzas, changing only the dbname shouldn't
+* trigger throwing away the password.
+*/
+ if (!dbname)
+ dbname = PQdb(o_conn);
+
/*
* If the user asked to be prompted for a password, ask for one now. If
* not, use the password from the old connection, provided the username
@@ -1643,9 +1661,13 @@ do_connect(char *dbname, char *user, char *host, char
*port)
{
password = prompt_for_password(user);
Re: [HACKERS] POLA violation with \c service=
Hi all I am sending a review of this patch: * What it does? - Allow to connect to other db by \connect uri connection format postgres=# \c postgresql://localhost?service=old psql (9.5devel, server 9.2.9) You are now connected to database "postgres" as user "pavel". * Would we this feature? - yes, it eliminate inconsistency between cmd line connect and \connect. It is good idea without any objections. * This patch is cleanly applicable, later compilation without any issues * All regress tests passed * A psql documentation is updated -- this feature (and format) is not widely known, so maybe some more examples are welcome * When I tested this feature, it worked as expected * Code respects PostgreSQL coding rules. I prefer a little bit different test if keep password. Current code is little bit harder to understand. But I can live with David's code well too. if (!user) user = PQuser(o_conn); if (!host) host = PQhost(o_conn); if (!port) port = PQport(o_conn); if (dbname) has_connection_string = recognized_connection_string(dbname); /* we should not to keep password if some connection property is changed */ keep_password = strcmp(user, PQuser(o_conn)) == 0 && strcmp(host, PQhost(o_conn)) == 0 && strcmp(port, PQport(o_conn)) == 0 && !has_connection_string; I have not any other comments. Possible questions: 1. more examples in doc 2. small change how to check keep_password Regards Pavel 2015-01-13 15:00 GMT+01:00 David Fetter : > On Sat, Jan 10, 2015 at 04:41:16PM -0800, David Fetter wrote: > > On Sat, Jan 10, 2015 at 09:30:57AM +0100, Erik Rijkers wrote: > > > On Fri, January 9, 2015 20:15, David Fetter wrote: > > > > [psql_fix_uri_service_003.patch] > > > > > > Applies on master; the feature (switching services) works well but a > \c without any parameters produces a segfault: > > > > > > (centos 6.6, 4.9.2, 64-bit) > > > > > > > > > $ echo -en "$PGSERVICEFILE\n$PGSERVICE\n$PGPORT\n" > > > /home/aardvark/.pg_service > > > service_pola > > > 6968 > > > > > > $ psql > > > Timing is on. > > > psql (9.5devel_service_pola_20150109_2340_ac7009abd228) > > > Type "help" for help. > > > > > > testdb=# \c service=HEAD > > > You are now connected to database "testdb" as user "aardvark" via > socket in "/tmp" at port "6545". > > > testdb=# \c service=service_pola > > > You are now connected to database "testdb" as user "aardvark" via > socket in "/tmp" at port "6968". > > > testdb=# \c > > > Segmentation fault (core dumped) > > > > Fixed by running that function only if the argument exists. > > > > More C cleanups, too. > > Added to the upcoming commitfest. > > Cheers, > David. > -- > David Fetter http://fetter.org/ > Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter > Skype: davidfetter XMPP: david.fet...@gmail.com > > Remember to vote! > Consider donating to Postgres: http://www.postgresql.org/about/donate > > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers >
Re: [HACKERS] POLA violation with \c service=
On Sat, Jan 10, 2015 at 04:41:16PM -0800, David Fetter wrote: > On Sat, Jan 10, 2015 at 09:30:57AM +0100, Erik Rijkers wrote: > > On Fri, January 9, 2015 20:15, David Fetter wrote: > > > [psql_fix_uri_service_003.patch] > > > > Applies on master; the feature (switching services) works well but a \c > > without any parameters produces a segfault: > > > > (centos 6.6, 4.9.2, 64-bit) > > > > > > $ echo -en "$PGSERVICEFILE\n$PGSERVICE\n$PGPORT\n" > > /home/aardvark/.pg_service > > service_pola > > 6968 > > > > $ psql > > Timing is on. > > psql (9.5devel_service_pola_20150109_2340_ac7009abd228) > > Type "help" for help. > > > > testdb=# \c service=HEAD > > You are now connected to database "testdb" as user "aardvark" via socket in > > "/tmp" at port "6545". > > testdb=# \c service=service_pola > > You are now connected to database "testdb" as user "aardvark" via socket in > > "/tmp" at port "6968". > > testdb=# \c > > Segmentation fault (core dumped) > > Fixed by running that function only if the argument exists. > > More C cleanups, too. Added to the upcoming commitfest. Cheers, David. -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] POLA violation with \c service=
On Sat, Jan 10, 2015 at 09:30:57AM +0100, Erik Rijkers wrote:
> On Fri, January 9, 2015 20:15, David Fetter wrote:
> > [psql_fix_uri_service_003.patch]
>
> Applies on master; the feature (switching services) works well but a \c
> without any parameters produces a segfault:
>
> (centos 6.6, 4.9.2, 64-bit)
>
>
> $ echo -en "$PGSERVICEFILE\n$PGSERVICE\n$PGPORT\n"
> /home/aardvark/.pg_service
> service_pola
> 6968
>
> $ psql
> Timing is on.
> psql (9.5devel_service_pola_20150109_2340_ac7009abd228)
> Type "help" for help.
>
> testdb=# \c service=HEAD
> You are now connected to database "testdb" as user "aardvark" via socket in
> "/tmp" at port "6545".
> testdb=# \c service=service_pola
> You are now connected to database "testdb" as user "aardvark" via socket in
> "/tmp" at port "6968".
> testdb=# \c
> Segmentation fault (core dumped)
Fixed by running that function only if the argument exists.
More C cleanups, too.
Cheers,
David.
--
David Fetter http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fet...@gmail.com
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index bdfb67c..eb6a57b 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -796,18 +796,20 @@ testdb=>
-\c or \connect [
dbname [ username ] [ host ] [ port ] ]
+\c or \connect [
{ [ dbname [ username ] [ host ] [ port ] ] | conninfo string | URI } ]
Establishes a new connection to a PostgreSQL
-server. If the new connection is successfully made, the
-previous connection is closed. If any of conninfo string, or a URI.
If the new connection is
+successfully made, the
+previous connection is closed. When using positional parameters, if
any of dbname, username, host or port are omitted or specified
as -, the value of that parameter from the
-previous connection is used. If there is no previous
+previous connection is used. If using positional parameters and there
is no previous
connection, the libpq default for
the parameter's value is used.
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 4ac21f2..bcdf278 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -1610,6 +1610,8 @@ do_connect(char *dbname, char *user, char *host, char
*port)
PGconn *o_conn = pset.db,
*n_conn;
char *password = NULL;
+ boolkeep_password = true;
+ boolhas_connection_string = false;
if (!o_conn && (!dbname || !user || !host || !port))
{
@@ -1623,14 +1625,33 @@ do_connect(char *dbname, char *user, char *host, char
*port)
return false;
}
- if (!dbname)
- dbname = PQdb(o_conn);
if (!user)
user = PQuser(o_conn);
+ else if (strcmp(user, PQuser(o_conn)) != 0)
+ keep_password = false;
+
if (!host)
host = PQhost(o_conn);
+ else if (strcmp(host, PQhost(o_conn)) != 0)
+ keep_password = false;
+
if (!port)
port = PQport(o_conn);
+ else if (strcmp(port, PQport(o_conn)) != 0)
+ keep_password = false;
+
+ if (dbname)
+ has_connection_string = recognized_connection_string(dbname);
+
+ if (has_connection_string)
+ keep_password = false;
+
+ /*
+* Unlike the previous stanzas, changing only the dbname shouldn't
+* trigger throwing away the password.
+*/
+ if (!dbname)
+ dbname = PQdb(o_conn);
/*
* If the user asked to be prompted for a password, ask for one now. If
@@ -1646,9 +1667,13 @@ do_connect(char *dbname, char *user, char *host, char
*port)
{
password = prompt_for_password(user);
}
- else if (o_conn && user && strcmp(PQuser(o_conn), user) == 0)
+ else if (o_conn && keep_password)
{
- password = pg_strdup(PQpass(o_conn));
+ password = PQpass(o_conn);
+ if (password && *password)
+ password = pg_strdup(password);
+ else
+ password = NULL;
}
while (true)
@@ -1656,23 +1681,28 @@ do_connect(char *dbname, char *user, char *host, char
*port)
#define PARAMS_ARRAY_SIZE 8
const char **keywords = pg_malloc(PARAMS_ARRAY_SIZE *
sizeof(*keywords));
const char **values = pg_malloc(PARAMS_ARRAY_SIZE *
sizeof(*values));
+ int paramnum = 0;
- keywords[0] = "host";
- values[0] = host;
- keywords[1] = "port";
- values[1] = port;
-
Re: [HACKERS] POLA violation with \c service=
On 2015-01-10 09:49:52 -0500, Andrew Dunstan wrote: > >Save static inline functions, that is. > > Yeah, but not normally data items. (I did say "in general"). As a general > rule for novice C programmers I think my rule of thumb is reasonable. Agreed. I just tried to preempt somebody grepping for static in src/include and crying foul ;) Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] POLA violation with \c service=
On 01/10/2015 09:32 AM, Andres Freund wrote: On 2015-01-10 09:16:07 -0500, Andrew Dunstan wrote: +static const char uri_designator[] = "postgresql://"; +static const char short_uri_designator[] = "postgres://"; These declarations in common.h would cause a separate instance of these pieces of storage to occur in every object file where the .h file had been #included. In general, you should not expect to see any static declarations in .h files. Save static inline functions, that is. Yeah, but not normally data items. (I did say "in general"). As a general rule for novice C programmers I think my rule of thumb is reasonable. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] POLA violation with \c service=
On 2015-01-10 09:16:07 -0500, Andrew Dunstan wrote: >+static const char uri_designator[] = "postgresql://"; >+static const char short_uri_designator[] = "postgres://"; > > These declarations in common.h would cause a separate instance of these > pieces of storage to occur in every object file where the .h file had been > #included. In general, you should not expect to see any static declarations > in .h files. Save static inline functions, that is. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] POLA violation with \c service=
On 01/09/2015 02:15 PM, David Fetter wrote: Some C cleanups... Not quite enough cleanup. As I told you on IRC, the only addition to common.h should be the declaration of recognized_connection_string. These do not belong there (they belong in common.c): +static const char uri_designator[] = "postgresql://"; +static const char short_uri_designator[] = "postgres://"; These declarations in common.h would cause a separate instance of these pieces of storage to occur in every object file where the .h file had been #included. In general, you should not expect to see any static declarations in .h files. In addition, you need to ensure that recognized_connection_string() is not called with a NULL argument. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] POLA violation with \c service=
On Fri, January 9, 2015 20:15, David Fetter wrote: > [psql_fix_uri_service_003.patch] Applies on master; the feature (switching services) works well but a \c without any parameters produces a segfault: (centos 6.6, 4.9.2, 64-bit) $ echo -en "$PGSERVICEFILE\n$PGSERVICE\n$PGPORT\n" /home/aardvark/.pg_service service_pola 6968 $ psql Timing is on. psql (9.5devel_service_pola_20150109_2340_ac7009abd228) Type "help" for help. testdb=# \c service=HEAD You are now connected to database "testdb" as user "aardvark" via socket in "/tmp" at port "6545". testdb=# \c service=service_pola You are now connected to database "testdb" as user "aardvark" via socket in "/tmp" at port "6968". testdb=# \c Segmentation fault (core dumped) or, under gdb: (gdb) run Starting program: /home/aardvark/pg_stuff/pg_installations/pgsql.service_pola/bin/psql [Thread debugging using libthread_db enabled] Using host libthread_db library "/lib64/libthread_db.so.1". Timing is on. psql (9.5devel_service_pola_20150109_2340_ac7009abd228) Type "help" for help. testdb=# \c Program received signal SIGSEGV, Segmentation fault. 0x0040b1f5 in uri_prefix_length (connstr=0x0) at common.c:1785 1785if (strncmp(connstr, uri_designator, (gdb) bt #0 0x0040b1f5 in uri_prefix_length (connstr=0x0) at common.c:1785 #1 recognized_connection_string (connstr=connstr@entry=0x0) at common.c:1805 #2 0x00406592 in do_connect (port=0x676960 "6968", host=0x676940 "/tmp", user=0x676900 "aardvark", dbname=0x0) at command.c:1643 #3 exec_command (query_buf=0x678150, scan_state=0x677fc0, cmd=0x68f730 "c") at command.c:247 #4 HandleSlashCmds (scan_state=scan_state@entry=0x677fc0, query_buf=0x678150) at command.c:112 #5 0x00411283 in MainLoop (source=0x32d4b8e6c0 <_IO_2_1_stdin_>) at mainloop.c:335 #6 0x00413b4e in main (argc=, argv=0x7fffd9f8) at startup.c:340 (gdb) I hope that helps to pinpoint the problem. thanks, Erik Rijkers -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] POLA violation with \c service=
On Thu, Jan 08, 2015 at 08:04:47PM -0800, David Fetter wrote:
> On Mon, Jan 05, 2015 at 02:26:59PM -0800, David Fetter wrote:
> > On Tue, Dec 30, 2014 at 04:48:11PM -0800, David Fetter wrote:
> > > On Wed, Dec 17, 2014 at 08:14:04AM -0500, Andrew Dunstan wrote:
> > > >
> > > > Yeah, that's the correct solution. It should not be terribly difficult
> > > > to
> > > > create a test for a conninfo string in the dbname parameter. That's what
> > > > libpq does after all. We certainly don't want psql to have to try to
> > > > interpret the service file. psql just needs to let libpq do its work in
> > > > this
> > > > situation.
> > >
> > > This took a little longer to get time to polish than I'd hoped, but
> > > please find attached a patch which:
> > >
> > > - Correctly connects to service= and postgres(ql)?:// with \c
> > > - Disallows tab completion in the above cases
> > >
> > > I'd like to see about having tab completion actually work correctly in
> > > at least the service= case, but that's a matter for a follow-on patch.
> > >
> > > Thanks to Andrew Dunstan for the original patch, and to Andrew Gierth
> > > for his help getting it into shape.
> > >
> > > Cheers,
> > > David.
> >
> > I should mention that the patch also corrects a problem where the
> > password was being saved/discarded at inappropriate times. Please
> > push this patch to the back branches :)
>
> Per discussion with Stephen Frost, I've documented the previously
> undocumented behavior with conninfo strings and URIs.
Some C cleanups...
I think longer term we should see about having libpq export the
functions I've put in common.[ch], but that's for a later patch.
Cheers,
David.
--
David Fetter http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fet...@gmail.com
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index bdfb67c..eb6a57b 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -796,18 +796,20 @@ testdb=>
-\c or \connect [
dbname [ username ] [ host ] [ port ] ]
+\c or \connect [
{ [ dbname [ username ] [ host ] [ port ] ] | conninfo string | URI } ]
Establishes a new connection to a PostgreSQL
-server. If the new connection is successfully made, the
-previous connection is closed. If any of conninfo string, or a URI.
If the new connection is
+successfully made, the
+previous connection is closed. When using positional parameters, if
any of dbname, username, host or port are omitted or specified
as -, the value of that parameter from the
-previous connection is used. If there is no previous
+previous connection is used. If using positional parameters and there
is no previous
connection, the libpq default for
the parameter's value is used.
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 4ac21f2..f290fbc 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -1610,6 +1610,8 @@ do_connect(char *dbname, char *user, char *host, char
*port)
PGconn *o_conn = pset.db,
*n_conn;
char *password = NULL;
+ boolkeep_password = true;
+ boolhas_connection_string = false;
if (!o_conn && (!dbname || !user || !host || !port))
{
@@ -1623,14 +1625,32 @@ do_connect(char *dbname, char *user, char *host, char
*port)
return false;
}
- if (!dbname)
- dbname = PQdb(o_conn);
if (!user)
user = PQuser(o_conn);
+ else if (strcmp(user, PQuser(o_conn)) != 0)
+ keep_password = false;
+
if (!host)
host = PQhost(o_conn);
+ else if (strcmp(host, PQhost(o_conn)) != 0)
+ keep_password = false;
+
if (!port)
port = PQport(o_conn);
+ else if (strcmp(port, PQport(o_conn)) != 0)
+ keep_password = false;
+
+ has_connection_string = recognized_connection_string(dbname);
+
+ if (has_connection_string)
+ keep_password = false;
+
+ /*
+* Unlike the previous stanzas, changing only the dbname shouldn't
+* trigger throwing away the password.
+*/
+ if (!dbname)
+ dbname = PQdb(o_conn);
/*
* If the user asked to be prompted for a password, ask for one now. If
@@ -1646,9 +1666,13 @@ do_connect(char *dbname, char *user, char *host, char
*port)
{
password = prompt_for_password(user);
}
- else if (o_conn && user && strcmp(PQuser(o_conn), user) == 0)
+ else if (o_conn && keep_password)
{
- password = pg_strdup(PQpass(o_conn));
+ pass
Re: [HACKERS] POLA violation with \c service=
On Mon, Jan 05, 2015 at 02:26:59PM -0800, David Fetter wrote:
> On Tue, Dec 30, 2014 at 04:48:11PM -0800, David Fetter wrote:
> > On Wed, Dec 17, 2014 at 08:14:04AM -0500, Andrew Dunstan wrote:
> > >
> > > Yeah, that's the correct solution. It should not be terribly difficult to
> > > create a test for a conninfo string in the dbname parameter. That's what
> > > libpq does after all. We certainly don't want psql to have to try to
> > > interpret the service file. psql just needs to let libpq do its work in
> > > this
> > > situation.
> >
> > This took a little longer to get time to polish than I'd hoped, but
> > please find attached a patch which:
> >
> > - Correctly connects to service= and postgres(ql)?:// with \c
> > - Disallows tab completion in the above cases
> >
> > I'd like to see about having tab completion actually work correctly in
> > at least the service= case, but that's a matter for a follow-on patch.
> >
> > Thanks to Andrew Dunstan for the original patch, and to Andrew Gierth
> > for his help getting it into shape.
> >
> > Cheers,
> > David.
>
> I should mention that the patch also corrects a problem where the
> password was being saved/discarded at inappropriate times. Please
> push this patch to the back branches :)
Per discussion with Stephen Frost, I've documented the previously
undocumented behavior with conninfo strings and URIs.
Cheers,
David.
--
David Fetter http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fet...@gmail.com
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index bdfb67c..eb6a57b 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -796,18 +796,20 @@ testdb=>
-\c or \connect [
dbname [ username ] [ host ] [ port ] ]
+\c or \connect [
{ [ dbname [ username ] [ host ] [ port ] ] | conninfo string | URI } ]
Establishes a new connection to a PostgreSQL
-server. If the new connection is successfully made, the
-previous connection is closed. If any of conninfo string, or a URI.
If the new connection is
+successfully made, the
+previous connection is closed. When using positional parameters, if
any of dbname, username, host or port are omitted or specified
as -, the value of that parameter from the
-previous connection is used. If there is no previous
+previous connection is used. If using positional parameters and there
is no previous
connection, the libpq default for
the parameter's value is used.
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 4ac21f2..f290fbc 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -1610,6 +1610,8 @@ do_connect(char *dbname, char *user, char *host, char
*port)
PGconn *o_conn = pset.db,
*n_conn;
char *password = NULL;
+ boolkeep_password = true;
+ boolhas_connection_string = false;
if (!o_conn && (!dbname || !user || !host || !port))
{
@@ -1623,14 +1625,32 @@ do_connect(char *dbname, char *user, char *host, char
*port)
return false;
}
- if (!dbname)
- dbname = PQdb(o_conn);
if (!user)
user = PQuser(o_conn);
+ else if (strcmp(user, PQuser(o_conn)) != 0)
+ keep_password = false;
+
if (!host)
host = PQhost(o_conn);
+ else if (strcmp(host, PQhost(o_conn)) != 0)
+ keep_password = false;
+
if (!port)
port = PQport(o_conn);
+ else if (strcmp(port, PQport(o_conn)) != 0)
+ keep_password = false;
+
+ has_connection_string = recognized_connection_string(dbname);
+
+ if (has_connection_string)
+ keep_password = false;
+
+ /*
+* Unlike the previous stanzas, changing only the dbname shouldn't
+* trigger throwing away the password.
+*/
+ if (!dbname)
+ dbname = PQdb(o_conn);
/*
* If the user asked to be prompted for a password, ask for one now. If
@@ -1646,9 +1666,13 @@ do_connect(char *dbname, char *user, char *host, char
*port)
{
password = prompt_for_password(user);
}
- else if (o_conn && user && strcmp(PQuser(o_conn), user) == 0)
+ else if (o_conn && keep_password)
{
- password = pg_strdup(PQpass(o_conn));
+ password = PQpass(o_conn);
+ if (password && *password)
+ password = pg_strdup(password);
+ else
+ password = NULL;
}
while (true)
@@ -1656,23 +1680,28 @@ do_connect(char *dbname, char *user, char *h
Re: [HACKERS] POLA violation with \c service=
On Tue, Dec 30, 2014 at 04:48:11PM -0800, David Fetter wrote: > On Wed, Dec 17, 2014 at 08:14:04AM -0500, Andrew Dunstan wrote: > > > > Yeah, that's the correct solution. It should not be terribly difficult to > > create a test for a conninfo string in the dbname parameter. That's what > > libpq does after all. We certainly don't want psql to have to try to > > interpret the service file. psql just needs to let libpq do its work in this > > situation. > > This took a little longer to get time to polish than I'd hoped, but > please find attached a patch which: > > - Correctly connects to service= and postgres(ql)?:// with \c > - Disallows tab completion in the above cases > > I'd like to see about having tab completion actually work correctly in > at least the service= case, but that's a matter for a follow-on patch. > > Thanks to Andrew Dunstan for the original patch, and to Andrew Gierth > for his help getting it into shape. > > Cheers, > David. I should mention that the patch also corrects a problem where the password was being saved/discarded at inappropriate times. Please push this patch to the back branches :) Cheers, David. -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] POLA violation with \c service=
On Wed, Dec 17, 2014 at 08:14:04AM -0500, Andrew Dunstan wrote:
>
> On 12/17/2014 04:11 AM, Heikki Linnakangas wrote:
> >On 12/17/2014 10:03 AM, Albe Laurenz wrote:
> >>David Fetter wrote:
> >>>I've noticed that psql's \c function handles service= requests in a
> >>>way that I can only characterize as broken.
> >>>
> >>>This came up in the context of connecting to a cloud hosting service
> >>>named after warriors or a river or something, whose default hostnames
> >>>are long, confusing, and easy to typo, so I suspect that service= may
> >>>come up more often going forward than it has until now.
> >>>
> >>>For example, when I try to use
> >>>
> >>>\c "service=foo"
> >>>
> >>>It will correctly figure out which database I'm trying to connect to,
> >>>but fail to notice that it's on a different host, port, etc., and
> >>>hence fail to connect with a somewhat unhelpful error message.
> >>>
> >>>I can think of a few approaches for fixing this:
> >>>
> >>>0. Leave it broken.
> >>>1. Disable "service=" requests entirely in \c context, and error out
> >>>if attempted.
> >>>2. Ensure that \c actually uses all of the available information.
> >>>
> >>>Is there another one I missed?
> >>>
> >>>If not, which of the approaches seems reasonable?
> >>
> >>#2 is the correct solution, #1 a band aid.
> >
> >It would be handy, if \c "service=foo" actually worked. We should do #3.
> >If the database name is actually a connection string, or a service
> >specification, it should not re-use the hostname and port from previous
> >connection, but use the values from the connection string or service file.
>
> Yeah, that's the correct solution. It should not be terribly difficult to
> create a test for a conninfo string in the dbname parameter. That's what
> libpq does after all. We certainly don't want psql to have to try to
> interpret the service file. psql just needs to let libpq do its work in this
> situation.
This took a little longer to get time to polish than I'd hoped, but
please find attached a patch which:
- Correctly connects to service= and postgres(ql)?:// with \c
- Disallows tab completion in the above cases
I'd like to see about having tab completion actually work correctly in
at least the service= case, but that's a matter for a follow-on patch.
Thanks to Andrew Dunstan for the original patch, and to Andrew Gierth
for his help getting it into shape.
Cheers,
David.
--
David Fetter http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fet...@gmail.com
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index c7a17d7..4ca50b3 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -1610,6 +1610,8 @@ do_connect(char *dbname, char *user, char *host, char
*port)
PGconn *o_conn = pset.db,
*n_conn;
char *password = NULL;
+ boolkeep_password = true;
+ boolhas_connection_string = false;
if (!o_conn && (!dbname || !user || !host || !port))
{
@@ -1623,14 +1625,32 @@ do_connect(char *dbname, char *user, char *host, char
*port)
return false;
}
- if (!dbname)
- dbname = PQdb(o_conn);
if (!user)
user = PQuser(o_conn);
+ else if (strcmp(user, PQuser(o_conn)) != 0)
+ keep_password = false;
+
if (!host)
host = PQhost(o_conn);
+ else if (strcmp(host, PQhost(o_conn)) != 0)
+ keep_password = false;
+
if (!port)
port = PQport(o_conn);
+ else if (strcmp(port, PQport(o_conn)) != 0)
+ keep_password = false;
+
+ has_connection_string = recognized_connection_string(dbname);
+
+ if (has_connection_string)
+ keep_password = false;
+
+ /*
+* Unlike the previous stanzas, changing only the dbname shouldn't
+* trigger throwing away the password.
+*/
+ if (!dbname)
+ dbname = PQdb(o_conn);
/*
* If the user asked to be prompted for a password, ask for one now. If
@@ -1646,9 +1666,13 @@ do_connect(char *dbname, char *user, char *host, char
*port)
{
password = prompt_for_password(user);
}
- else if (o_conn && user && strcmp(PQuser(o_conn), user) == 0)
+ else if (o_conn && keep_password)
{
- password = pg_strdup(PQpass(o_conn));
+ password = PQpass(o_conn);
+ if (password && *password)
+ password = pg_strdup(password);
+ else
+ password = NULL;
}
while (true)
@@ -1656,23 +1680,28 @@ do_connect(char *dbname, char *user, char *host, char
*port)
#define PARAMS_ARRAY_SIZE 8
const char **keywords = pg_malloc(PARAMS_AR
Re: [HACKERS] POLA violation with \c service=
On Fri, Dec 19, 2014 at 07:03:36PM -0700, David Johnston wrote: > On Wed, Dec 17, 2014 at 8:25 AM, David Fetter [via PostgreSQL] < > ml-node+s1045698n5831124...@n5.nabble.com> wrote: > > > On Wed, Dec 17, 2014 at 08:14:04AM -0500, Andrew Dunstan wrote: > > > > > > > > On 12/17/2014 04:11 AM, Heikki Linnakangas wrote: > > > >On 12/17/2014 10:03 AM, Albe Laurenz wrote: > > > >>David Fetter wrote: > > > >>>I've noticed that psql's \c function handles service= requests in a > > > >>>way that I can only characterize as broken. > > > >>> > > > >>>This came up in the context of connecting to a cloud hosting service > > > >>>named after warriors or a river or something, whose default hostnames > > > >>>are long, confusing, and easy to typo, so I suspect that service= may > > > >>>come up more often going forward than it has until now. > > > >>> > > > >>>For example, when I try to use > > > >>> > > > >>>\c "service=foo" > > > >>> > > > >>>It will correctly figure out which database I'm trying to connect to, > > > >>>but fail to notice that it's on a different host, port, etc., and > > > >>>hence fail to connect with a somewhat unhelpful error message. > > > >>> > > > >>>I can think of a few approaches for fixing this: > > > >>> > > > >>>0. Leave it broken. > > > >>>1. Disable "service=" requests entirely in \c context, and error out > > > >>>if attempted. > > > >>>2. Ensure that \c actually uses all of the available information. > > > >>> > > > >>>Is there another one I missed? > > > >>> > > > >>>If not, which of the approaches seems reasonable? > > > >> > > > >>#2 is the correct solution, #1 a band aid. > > > > > > > >It would be handy, if \c "service=foo" actually worked. We should do > > #3. > > > >If the database name is actually a connection string, or a service > > > >specification, it should not re-use the hostname and port from previous > > > >connection, but use the values from the connection string or service > > file. > > > > > > > > > Yeah, that's the correct solution. It should not be terribly difficult > > to > > > create a test for a conninfo string in the dbname parameter. That's what > > > libpq does after all. We certainly don't want psql to have to try to > > > interpret the service file. psql just needs to let libpq do its work in > > this > > > situation. > > > > letting libpq handle this is the only sane plan for fixing it. I'm > > looking into that today. > > > > > > On a tangentially related note; it is not outside the realm of possibility > that a user would want one pg_service entry > > to reference another one: You want entries in the service system to be able reference other entries, setting defaults, for example? Interesting idea. As you say, it's tangential to this. The bug I found, and I'm increasingly convinced it's a bug whose fix should be back-patched, is that psql fails to let libpq do its job with the extant service system, or more precisely prevents it from doing only part of its job, leading to broken behavior. Cheers, David. -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] POLA violation with \c service=
On Wed, Dec 17, 2014 at 8:25 AM, David Fetter [via PostgreSQL] < ml-node+s1045698n5831124...@n5.nabble.com> wrote: > On Wed, Dec 17, 2014 at 08:14:04AM -0500, Andrew Dunstan wrote: > > > > > On 12/17/2014 04:11 AM, Heikki Linnakangas wrote: > > >On 12/17/2014 10:03 AM, Albe Laurenz wrote: > > >>David Fetter wrote: > > >>>I've noticed that psql's \c function handles service= requests in a > > >>>way that I can only characterize as broken. > > >>> > > >>>This came up in the context of connecting to a cloud hosting service > > >>>named after warriors or a river or something, whose default hostnames > > >>>are long, confusing, and easy to typo, so I suspect that service= may > > >>>come up more often going forward than it has until now. > > >>> > > >>>For example, when I try to use > > >>> > > >>>\c "service=foo" > > >>> > > >>>It will correctly figure out which database I'm trying to connect to, > > >>>but fail to notice that it's on a different host, port, etc., and > > >>>hence fail to connect with a somewhat unhelpful error message. > > >>> > > >>>I can think of a few approaches for fixing this: > > >>> > > >>>0. Leave it broken. > > >>>1. Disable "service=" requests entirely in \c context, and error out > > >>>if attempted. > > >>>2. Ensure that \c actually uses all of the available information. > > >>> > > >>>Is there another one I missed? > > >>> > > >>>If not, which of the approaches seems reasonable? > > >> > > >>#2 is the correct solution, #1 a band aid. > > > > > >It would be handy, if \c "service=foo" actually worked. We should do > #3. > > >If the database name is actually a connection string, or a service > > >specification, it should not re-use the hostname and port from previous > > >connection, but use the values from the connection string or service > file. > > > > > > Yeah, that's the correct solution. It should not be terribly difficult > to > > create a test for a conninfo string in the dbname parameter. That's what > > libpq does after all. We certainly don't want psql to have to try to > > interpret the service file. psql just needs to let libpq do its work in > this > > situation. > > letting libpq handle this is the only sane plan for fixing it. I'm > looking into that today. > > On a tangentially related note; it is not outside the realm of possibility that a user would want one pg_service entry to reference another one: [realentry] user= dbname= [aliasentry] service=realentry furthermore, having a shareable entry like: [main-host] host=ip-address port=5433 [main-user1] user=user1 service=main-host [main-user2] user=user2 service=main-host also seems potentially useful. I just sent a -doc report that nothing in the documentation says this behavior is not implemented but a cursory attempt at it confirms the lack. While you are digging in there anything fundamental prohibiting the behavior and is it something you think would be useful in these complex environments you are working with? David J. Sorry about the oddball CC: but I don't have an e-mail with a full set of recipients... -- View this message in context: http://postgresql.nabble.com/POLA-violation-with-c-service-tp5831001p5831538.html Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.
Re: [HACKERS] POLA violation with \c service=
On Wed, Dec 17, 2014 at 08:14:04AM -0500, Andrew Dunstan wrote: > > On 12/17/2014 04:11 AM, Heikki Linnakangas wrote: > >On 12/17/2014 10:03 AM, Albe Laurenz wrote: > >>David Fetter wrote: > >>>I've noticed that psql's \c function handles service= requests in a > >>>way that I can only characterize as broken. > >>> > >>>This came up in the context of connecting to a cloud hosting service > >>>named after warriors or a river or something, whose default hostnames > >>>are long, confusing, and easy to typo, so I suspect that service= may > >>>come up more often going forward than it has until now. > >>> > >>>For example, when I try to use > >>> > >>>\c "service=foo" > >>> > >>>It will correctly figure out which database I'm trying to connect to, > >>>but fail to notice that it's on a different host, port, etc., and > >>>hence fail to connect with a somewhat unhelpful error message. > >>> > >>>I can think of a few approaches for fixing this: > >>> > >>>0. Leave it broken. > >>>1. Disable "service=" requests entirely in \c context, and error out > >>>if attempted. > >>>2. Ensure that \c actually uses all of the available information. > >>> > >>>Is there another one I missed? > >>> > >>>If not, which of the approaches seems reasonable? > >> > >>#2 is the correct solution, #1 a band aid. > > > >It would be handy, if \c "service=foo" actually worked. We should do #3. > >If the database name is actually a connection string, or a service > >specification, it should not re-use the hostname and port from previous > >connection, but use the values from the connection string or service file. > > > Yeah, that's the correct solution. It should not be terribly difficult to > create a test for a conninfo string in the dbname parameter. That's what > libpq does after all. We certainly don't want psql to have to try to > interpret the service file. psql just needs to let libpq do its work in this > situation. letting libpq handle this is the only sane plan for fixing it. I'm looking into that today. Cheers, David. -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] POLA violation with \c service=
On 12/17/2014 04:11 AM, Heikki Linnakangas wrote: On 12/17/2014 10:03 AM, Albe Laurenz wrote: David Fetter wrote: I've noticed that psql's \c function handles service= requests in a way that I can only characterize as broken. This came up in the context of connecting to a cloud hosting service named after warriors or a river or something, whose default hostnames are long, confusing, and easy to typo, so I suspect that service= may come up more often going forward than it has until now. For example, when I try to use \c "service=foo" It will correctly figure out which database I'm trying to connect to, but fail to notice that it's on a different host, port, etc., and hence fail to connect with a somewhat unhelpful error message. I can think of a few approaches for fixing this: 0. Leave it broken. 1. Disable "service=" requests entirely in \c context, and error out if attempted. 2. Ensure that \c actually uses all of the available information. Is there another one I missed? If not, which of the approaches seems reasonable? #2 is the correct solution, #1 a band aid. It would be handy, if \c "service=foo" actually worked. We should do #3. If the database name is actually a connection string, or a service specification, it should not re-use the hostname and port from previous connection, but use the values from the connection string or service file. Yeah, that's the correct solution. It should not be terribly difficult to create a test for a conninfo string in the dbname parameter. That's what libpq does after all. We certainly don't want psql to have to try to interpret the service file. psql just needs to let libpq do its work in this situation. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] POLA violation with \c service=
On 12/17/2014 10:03 AM, Albe Laurenz wrote: David Fetter wrote: I've noticed that psql's \c function handles service= requests in a way that I can only characterize as broken. This came up in the context of connecting to a cloud hosting service named after warriors or a river or something, whose default hostnames are long, confusing, and easy to typo, so I suspect that service= may come up more often going forward than it has until now. For example, when I try to use \c "service=foo" It will correctly figure out which database I'm trying to connect to, but fail to notice that it's on a different host, port, etc., and hence fail to connect with a somewhat unhelpful error message. I can think of a few approaches for fixing this: 0. Leave it broken. 1. Disable "service=" requests entirely in \c context, and error out if attempted. 2. Ensure that \c actually uses all of the available information. Is there another one I missed? If not, which of the approaches seems reasonable? #2 is the correct solution, #1 a band aid. It would be handy, if \c "service=foo" actually worked. We should do #3. If the database name is actually a connection string, or a service specification, it should not re-use the hostname and port from previous connection, but use the values from the connection string or service file. - Heikki -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] POLA violation with \c service=
David Fetter wrote: > I've noticed that psql's \c function handles service= requests in a > way that I can only characterize as broken. > > This came up in the context of connecting to a cloud hosting service > named after warriors or a river or something, whose default hostnames > are long, confusing, and easy to typo, so I suspect that service= may > come up more often going forward than it has until now. > > For example, when I try to use > > \c "service=foo" > > It will correctly figure out which database I'm trying to connect to, > but fail to notice that it's on a different host, port, etc., and > hence fail to connect with a somewhat unhelpful error message. > > I can think of a few approaches for fixing this: > > 0. Leave it broken. > 1. Disable "service=" requests entirely in \c context, and error out if > attempted. > 2. Ensure that \c actually uses all of the available information. > > Is there another one I missed? > > If not, which of the approaches seems reasonable? #2 is the correct solution, #1 a band aid. Yours, Laurenz Albe -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] POLA violation with \c service=
I do indeed see this behavior in some very quick testing using 9.3 David Fetter wrote > I've noticed that psql's \c function handles service= requests in a > way that I can only characterize as broken. Looking at the docs the fact it attempts to treat "service=foo" as anything other than a database name is broken... > I can think of a few approaches for fixing this: > > 0. Leave it broken. > 1. Disable "service=" requests entirely in \c context, and error out if > attempted. > 2. Ensure that \c actually uses all of the available information. > > Is there another one I missed? > > If not, which of the approaches seems reasonable? #2 has a few possible final implementations to consider given that both \c and service= can be incompletely specified and what happens if both \c-host and service-host, for instance, are specified...but I'm not in a position to reason out the various possibilities right now. Regardless, the ability to specify a service name is valuable (if one presumes \c is valuable) so the tasks are finding an implementer and, depending on that outcome, how to handle back-branches. I don't think the status-quo is safe enough to leave so for head either #1 or #2 get my vote. Leaving it broken in back branches is not appealing but maybe we can selectively break it if we cannot get a #2 implementation that can be back-patched. An aside - from the docs: "If there is no previous connection [...]" - how is this possible when issuing \c? David J. -- View this message in context: http://postgresql.nabble.com/POLA-violation-with-c-service-tp5831001p5831026.html Sent from the PostgreSQL - hackers mailing list archive at Nabble.com. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] POLA violation with \c service=
Folks, I've noticed that psql's \c function handles service= requests in a way that I can only characterize as broken. This came up in the context of connecting to a cloud hosting service named after warriors or a river or something, whose default hostnames are long, confusing, and easy to typo, so I suspect that service= may come up more often going forward than it has until now. For example, when I try to use \c "service=foo" It will correctly figure out which database I'm trying to connect to, but fail to notice that it's on a different host, port, etc., and hence fail to connect with a somewhat unhelpful error message. I can think of a few approaches for fixing this: 0. Leave it broken. 1. Disable "service=" requests entirely in \c context, and error out if attempted. 2. Ensure that \c actually uses all of the available information. Is there another one I missed? If not, which of the approaches seems reasonable? Cheers, David. -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
