psql commandline conninfo

Started by Andrew Dunstanabout 19 years ago24 messages
#1Andrew Dunstan
andrew@dunslane.net

I have been working on providing psql with the ability to accept a libpq
conninfo string, so that the following now works for me:

psql "conn:service=sname user=uname"

Instead of providing yet another switch, I overloaded the dbname
parameter so that if it has the recognised prefix the remainder is
treated as a conninfo string. I have 3 questions:

1. Is this a good way to go, or should we just provide yet another switch?
2. If this is ok, what should the prefix be? is "conn:" ok?
3. Should we append settings from other switches to the conninfo (e.g.
-U or -h), or should we just ignore them? If we ignore them should we
warn about that if they are present?

cheers

andrew

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Dunstan (#1)
Re: psql commandline conninfo

Andrew Dunstan <andrew@dunslane.net> writes:

I have been working on providing psql with the ability to accept a libpq
conninfo string, so that the following now works for me:
psql "conn:service=sname user=uname"

Perhaps this should be implemented in libpq, not at the psql level?
Otherwise you're going to have to do it over for each client program.

2. If this is ok, what should the prefix be? is "conn:" ok?

I'd prefer to dispense with the conn:, so that this looks somehow
designed in rather than bolted on after the fact.

I'm tempted to suggest that if the "dbname" includes "=" it should be
considered a conninfo string; perhaps also after checking keyword
validity.

3. Should we append settings from other switches to the conninfo (e.g.
-U or -h), or should we just ignore them? If we ignore them should we
warn about that if they are present?

Do we complain about duplicate keywords in conninfo now? I think not,
so appending the other switches would have the result of overriding what
is in conninfo, which is probably reasonable. (Actually, if you
implement this in libpq, there's probably no need to form the appended
string explicitly --- just process the other options of PQsetdbLogin()
after the conninfo.)

regards, tom lane

#3Andrew Dunstan
andrew@dunslane.net
In reply to: Tom Lane (#2)
Re: psql commandline conninfo

Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

I have been working on providing psql with the ability to accept a libpq
conninfo string, so that the following now works for me:
psql "conn:service=sname user=uname"

Perhaps this should be implemented in libpq, not at the psql level?
Otherwise you're going to have to do it over for each client program.

Just as well I haven't spent much time on it, eh?

2. If this is ok, what should the prefix be? is "conn:" ok?

I'd prefer to dispense with the conn:, so that this looks somehow
designed in rather than bolted on after the fact.

well, I thought this made it look slightly URLish, a bit like a jbdc
URL. But OK. no big deal.

I'm tempted to suggest that if the "dbname" includes "=" it should be
considered a conninfo string; perhaps also after checking keyword
validity.

Now I look at fe-connect.c more closely, I'm tempted just to try parsing
the dbname param as a conninfo string, and if it doesn't work fall back
on a plain dbname. I could greatly reduce the chance of following the
failure path by just looking for an = but I think anything more is
likely to be redundant.

3. Should we append settings from other switches to the conninfo (e.g.
-U or -h), or should we just ignore them? If we ignore them should we
warn about that if they are present?

Do we complain about duplicate keywords in conninfo now? I think not,
so appending the other switches would have the result of overriding what
is in conninfo, which is probably reasonable. (Actually, if you
implement this in libpq, there's probably no need to form the appended
string explicitly --- just process the other options of PQsetdbLogin()
after the conninfo.)

OK. I think this just falls out.

cheers

andrew

#4Martijn van Oosterhout
kleptog@svana.org
In reply to: Andrew Dunstan (#3)
Re: psql commandline conninfo

On Tue, Dec 12, 2006 at 05:44:07PM -0500, Andrew Dunstan wrote:

Now I look at fe-connect.c more closely, I'm tempted just to try parsing
the dbname param as a conninfo string, and if it doesn't work fall back
on a plain dbname. I could greatly reduce the chance of following the
failure path by just looking for an = but I think anything more is
likely to be redundant.

Does that mean that:

psql -d "service=myservice"

should Just Work(tm)? That would be nice.

Have a nice day,
--
Martijn van Oosterhout <kleptog@svana.org> http://svana.org/kleptog/

Show quoted text

From each according to his ability. To each according to his ability to litigate.

#5Tom Lane
tgl@sss.pgh.pa.us
In reply to: Martijn van Oosterhout (#4)
Re: psql commandline conninfo

Martijn van Oosterhout <kleptog@svana.org> writes:

Does that mean that:
psql -d "service=myservice"
should Just Work(tm)? That would be nice.

Even more to the point,

psql "service=myservice"

which is why we want to overload dbname rather than any of the other
PQsetdbLogin parameters for this --- dbname has pride of place in the
command line syntax for several of the client programs.

regards, tom lane

#6Andrew Dunstan
andrew@dunslane.net
In reply to: Tom Lane (#5)
1 attachment(s)
Re: [HACKERS] psql commandline conninfo

Tom Lane wrote:

Martijn van Oosterhout <kleptog@svana.org> writes:

Does that mean that:
psql -d "service=myservice"
should Just Work(tm)? That would be nice.

Even more to the point,

psql "service=myservice"

which is why we want to overload dbname rather than any of the other
PQsetdbLogin parameters for this --- dbname has pride of place in the
command line syntax for several of the client programs.

regards, tom lane

Right. Here's the patch I just knocked up, which seems to Just Work (tm) ;-)

cheers

andrew

Attachments:

dsnpatchtext/plain; name=dsnpatchDownload
Index: src/interfaces/libpq/fe-connect.c
===================================================================
RCS file: /cvsroot/pgsql/src/interfaces/libpq/fe-connect.c,v
retrieving revision 1.339
diff -c -r1.339 fe-connect.c
*** src/interfaces/libpq/fe-connect.c	21 Nov 2006 16:28:00 -0000	1.339
--- src/interfaces/libpq/fe-connect.c	12 Dec 2006 22:49:28 -0000
***************
*** 567,572 ****
--- 567,573 ----
  			 const char *pwd)
  {
  	PGconn	   *conn;
+ 	bool       have_conninfo = false;
  
  	/*
  	 * Allocate memory for the conn structure
***************
*** 575,585 ****
  	if (conn == NULL)
  		return NULL;
  
  	/*
  	 * Parse an empty conninfo string in order to set up the same defaults
! 	 * that PQconnectdb() would use.
  	 */
! 	if (!connectOptions1(conn, ""))
  		return conn;
  
  	/*
--- 576,609 ----
  	if (conn == NULL)
  		return NULL;
  
+ 	/* 
+ 	 * Have we got something that might be a conninfo string? 
+ 	 * If so, try that first.
+ 	 */
+ 	if (dbName && strchr(dbName,'='))
+ 	{
+ 		if(connectOptions1(conn,dbName))
+ 		{
+ 			/* it was a conninfo string */
+ 			have_conninfo = true;
+ 		}
+ 		else
+ 		{
+ 			/* put things back the way they were so we can try again */
+ 			freePGconn(conn);
+ 			conn = makeEmptyPGconn();
+ 			if (conn == NULL)
+ 				return NULL;
+ 			
+ 		}
+ 	}
+ 
  	/*
  	 * Parse an empty conninfo string in order to set up the same defaults
! 	 * that PQconnectdb() would use. Skip this if we already found a 
! 	 * conninfo string.
  	 */
! 	if (!have_conninfo && !connectOptions1(conn, ""))
  		return conn;
  
  	/*
***************
*** 613,619 ****
  		conn->pgtty = strdup(pgtty);
  	}
  
! 	if (dbName && dbName[0] != '\0')
  	{
  		if (conn->dbName)
  			free(conn->dbName);
--- 637,643 ----
  		conn->pgtty = strdup(pgtty);
  	}
  
! 	if (!have_conninfo && dbName && dbName[0] != '\0')
  	{
  		if (conn->dbName)
  			free(conn->dbName);
#7Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Dunstan (#6)
Re: [HACKERS] psql commandline conninfo

Andrew Dunstan <andrew@dunslane.net> writes:

Right. Here's the patch I just knocked up, which seems to Just Work (tm) ;-)

The main objection I can see to this is that you'd get a fairly
unhelpful message if you intended a conninfo string and there was
anything wrong with your syntax (eg, misspelled keyword). Maybe we
should go with the conn: bit, although really that doesn't seem any
less likely to collide with actual dbnames than the "does it contain
"="" idea. Anyone have other ideas how to disambiguate?

regards, tom lane

#8Casey Duncan
casey@pandora.com
In reply to: Tom Lane (#7)
Re: [HACKERS] psql commandline conninfo

On Dec 12, 2006, at 3:37 PM, Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

Right. Here's the patch I just knocked up, which seems to Just
Work (tm) ;-)

The main objection I can see to this is that you'd get a fairly
unhelpful message if you intended a conninfo string and there was
anything wrong with your syntax (eg, misspelled keyword). Maybe we
should go with the conn: bit, although really that doesn't seem any
less likely to collide with actual dbnames than the "does it contain
"="" idea. Anyone have other ideas how to disambiguate?

I would personally prefer a real option over a prefix, i.e. --
dbconn="service=foo" though the inline conninfo string in place of
the dbname would be ideal.

Perhaps like Tom suggests, if the value matches a conninfo regex
(slightly more rigid than just containing an equals character) then
we assume it is a conninfo string, but never try it as a dbname. If
someone has a database named like a conninfo string (c'mon folks ;^)
then they would need to pass it as explicitly an argument to '-d' or
'--dbname', not as a bare argument.

This is not completely b/w compatible of course, but IMO the added
convenience outweighs the incompatibility.

-Casey

#9Andrew Dunstan
andrew@dunslane.net
In reply to: Casey Duncan (#8)
Re: psql commandline conninfo

Casey Duncan wrote:

On Dec 12, 2006, at 3:37 PM, Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

Right. Here's the patch I just knocked up, which seems to Just
Work (tm) ;-)

The main objection I can see to this is that you'd get a fairly
unhelpful message if you intended a conninfo string and there was
anything wrong with your syntax (eg, misspelled keyword). Maybe we
should go with the conn: bit, although really that doesn't seem any
less likely to collide with actual dbnames than the "does it contain
"="" idea. Anyone have other ideas how to disambiguate?

I would personally prefer a real option over a prefix, i.e. --
dbconn="service=foo" though the inline conninfo string in place of
the dbname would be ideal.

Perhaps like Tom suggests, if the value matches a conninfo regex
(slightly more rigid than just containing an equals character) then
we assume it is a conninfo string, but never try it as a dbname. If
someone has a database named like a conninfo string (c'mon folks ;^)
then they would need to pass it as explicitly an argument to '-d' or
'--dbname', not as a bare argument.

You are confusing two things here. The way the patch is written it simply
interprets the parameter passed to libpq - it has no idea what was used
(if anything) on the commandline. The alternative, as Tom pointed out, is
to patch every client.

I'm inclined to say we should go back almost to my original suggestion: a
param that starts with conn: and contains an = is conclusively presumed to
be a conninfo string.

The workaround for a db name like that (say conn:foo=bar) is to use
"conn:dbname='conn:foo=bar'". You'll soon get tired of that and rename the
db to something sane :-)

cheers

andrew

#10Casey Duncan
casey@pandora.com
In reply to: Andrew Dunstan (#9)
Re: psql commandline conninfo

On Dec 12, 2006, at 5:16 PM, Andrew Dunstan wrote:

Casey Duncan wrote:

On Dec 12, 2006, at 3:37 PM, Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

Right. Here's the patch I just knocked up, which seems to Just
Work (tm) ;-)

The main objection I can see to this is that you'd get a fairly
unhelpful message if you intended a conninfo string and there was
anything wrong with your syntax (eg, misspelled keyword). Maybe we
should go with the conn: bit, although really that doesn't seem any
less likely to collide with actual dbnames than the "does it contain
"="" idea. Anyone have other ideas how to disambiguate?

I would personally prefer a real option over a prefix, i.e. --
dbconn="service=foo" though the inline conninfo string in place of
the dbname would be ideal.

Perhaps like Tom suggests, if the value matches a conninfo regex
(slightly more rigid than just containing an equals character) then
we assume it is a conninfo string, but never try it as a dbname. If
someone has a database named like a conninfo string (c'mon folks ;^)
then they would need to pass it as explicitly an argument to '-d' or
'--dbname', not as a bare argument.

You are confusing two things here. The way the patch is written it
simply
interprets the parameter passed to libpq - it has no idea what was
used
(if anything) on the commandline. The alternative, as Tom pointed
out, is
to patch every client.

I was speaking from and end-user point of view, but I see your point.
It's certainly attractive to just patch libpq and be done. However,
that does have the side-effect of implicitly propagating the behavior
to all libpg client software. That may be more unpleasantly
surprising to more people then just changing the built-in postgresql
client utilities. But then again it could also be considered a
feature 8^)

-Casey

#11Andrew Dunstan
andrew@dunslane.net
In reply to: Casey Duncan (#10)
Re: psql commandline conninfo

Casey Duncan wrote:

I was speaking from and end-user point of view, but I see your point.
It's certainly attractive to just patch libpq and be done. However,
that does have the side-effect of implicitly propagating the behavior
to all libpg client software. That may be more unpleasantly
surprising to more people then just changing the built-in postgresql
client utilities. But then again it could also be considered a
feature 8^)

We change libpq from time to time. Besides, how many DBs are there that
match the name pattern /^conn:.*=/ ? My guess is mighty few. So I don't
expect lots of surprise.

cheers

andrew

#12Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Dunstan (#11)
Re: psql commandline conninfo

"Andrew Dunstan" <andrew@dunslane.net> writes:

We change libpq from time to time. Besides, how many DBs are there that
match the name pattern /^conn:.*=/ ? My guess is mighty few. So I don't
expect lots of surprise.

Um, but how many DB names have an "=" in them at all?

Basically what this proposal is about is migrating from separated
dbname/user/host/port/etc parameters to a unified conninfo parameter.
That seems to me like a good long-term objective, and so I'm willing
to break a few eggs on the way to the omelet, as long as we're not
breaking any very likely usages.

So: who here has a database with "=" in the name? And hands up if
you've got a database whose name begins with "conn:"?

I'm betting zero response rate on both of those, so see no reason to
contort the long-term definition for a very marginal difference in
the extent of backwards compatibility ...

regards, tom lane

#13Albe Laurenz
all@adv.magwien.gv.at
In reply to: Tom Lane (#12)
Re: psql commandline conninfo

Tom Lane wrote:

We change libpq from time to time. Besides, how many DBs are there

that

match the name pattern /^conn:.*=/ ? My guess is mighty few. So I

don't

expect lots of surprise.

Um, but how many DB names have an "=" in them at all?

Basically what this proposal is about is migrating from separated
dbname/user/host/port/etc parameters to a unified conninfo parameter.
That seems to me like a good long-term objective, and so I'm willing
to break a few eggs on the way to the omelet, as long as we're not
breaking any very likely usages.

So: who here has a database with "=" in the name? And hands up if
you've got a database whose name begins with "conn:"?

I'm betting zero response rate on both of those, so see no reason to
contort the long-term definition for a very marginal difference in
the extent of backwards compatibility ...

I second the idea to have libpq interpret a database name with "=" in
it as a connection parameter string.

The "conn:" seems artificial and difficult to remember to me.

As to the problem of cryptic error messages from psql, can't we improve
libpq's error response if it gets a database name that causes problems
when parsed as a connection parameter string? That would take care of
that.

Yours,
Laurenz Albe

#14Andrew Dunstan
andrew@dunslane.net
In reply to: Tom Lane (#12)
Re: psql commandline conninfo

Tom Lane wrote:

"Andrew Dunstan" <andrew@dunslane.net> writes:

We change libpq from time to time. Besides, how many DBs are there that
match the name pattern /^conn:.*=/ ? My guess is mighty few. So I don't
expect lots of surprise.

Um, but how many DB names have an "=" in them at all?

Basically what this proposal is about is migrating from separated
dbname/user/host/port/etc parameters to a unified conninfo parameter.
That seems to me like a good long-term objective, and so I'm willing
to break a few eggs on the way to the omelet, as long as we're not
breaking any very likely usages.

So: who here has a database with "=" in the name? And hands up if
you've got a database whose name begins with "conn:"?

I'm betting zero response rate on both of those, so see no reason to
contort the long-term definition for a very marginal difference in
the extent of backwards compatibility ...

I'm not sure -hackers is the most representative group to poll regarding
dbnames in use ...

Anyway, if I understand your current position, the only change needed to
my current patch would be that if we fail to parse a dbname parameter
that contains an = we simply fail at that point, rather than retrying it
as a straight database name.

I'm OK with that.

cheers

andrew

#15Andrew Dunstan
andrew@dunslane.net
In reply to: Andrew Dunstan (#14)
1 attachment(s)
Re: [HACKERS] psql commandline conninfo

Andrew Dunstan wrote:

Tom Lane wrote:

"Andrew Dunstan" <andrew@dunslane.net> writes:

We change libpq from time to time. Besides, how many DBs are there that
match the name pattern /^conn:.*=/ ? My guess is mighty few. So I don't
expect lots of surprise.

Um, but how many DB names have an "=" in them at all?

Basically what this proposal is about is migrating from separated
dbname/user/host/port/etc parameters to a unified conninfo parameter.
That seems to me like a good long-term objective, and so I'm willing
to break a few eggs on the way to the omelet, as long as we're not
breaking any very likely usages.

So: who here has a database with "=" in the name? And hands up if
you've got a database whose name begins with "conn:"?

I'm betting zero response rate on both of those, so see no reason to
contort the long-term definition for a very marginal difference in
the extent of backwards compatibility ...

I'm not sure -hackers is the most representative group to poll regarding
dbnames in use ...

Anyway, if I understand your current position, the only change needed to
my current patch would be that if we fail to parse a dbname parameter
that contains an = we simply fail at that point, rather than retrying it
as a straight database name.

I'm OK with that.

Here's the patch for what I think is the consensus position. If there's no
objection I will apply this and document it.

cheers

andrew

Attachments:

dsnpatchapplication/octet-stream; name=dsnpatchDownload
Index: fe-connect.c
===================================================================
RCS file: /cvsroot/pgsql/src/interfaces/libpq/fe-connect.c,v
retrieving revision 1.339
diff -c -r1.339 fe-connect.c
*** fe-connect.c	21 Nov 2006 16:28:00 -0000	1.339
--- fe-connect.c	16 Dec 2006 03:47:08 -0000
***************
*** 567,572 ****
--- 567,573 ----
  			 const char *pwd)
  {
  	PGconn	   *conn;
+ 	char       *conninit = "";
  
  	/*
  	 * Allocate memory for the conn structure
***************
*** 575,585 ****
  	if (conn == NULL)
  		return NULL;
  
  	/*
! 	 * Parse an empty conninfo string in order to set up the same defaults
! 	 * that PQconnectdb() would use.
  	 */
! 	if (!connectOptions1(conn, ""))
  		return conn;
  
  	/*
--- 576,595 ----
  	if (conn == NULL)
  		return NULL;
  
+ 	/* 
+ 	 * Have we got something that might be a conninfo string? 
+ 	 */
+ 	if (dbName && strchr(dbName,'='))
+ 	{
+ 		conninit = (char *) dbName;
+ 	}
+ 
  	/*
! 	 * Parse a string in order to set up the same defaults
! 	 * that PQconnectdb() would use. Use the info string if we
! 	 * found it, ortherwise an empty string.
  	 */
! 	if (!connectOptions1(conn, conninit))
  		return conn;
  
  	/*
***************
*** 613,619 ****
  		conn->pgtty = strdup(pgtty);
  	}
  
! 	if (dbName && dbName[0] != '\0')
  	{
  		if (conn->dbName)
  			free(conn->dbName);
--- 623,630 ----
  		conn->pgtty = strdup(pgtty);
  	}
  
! 	/* Skip the dbname param if we already usaed it as conninfo */
! 	if (!*conninit && dbName && dbName[0] != '\0')
  	{
  		if (conn->dbName)
  			free(conn->dbName);
#16Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Dunstan (#15)
Re: [HACKERS] psql commandline conninfo

"Andrew Dunstan" <andrew@dunslane.net> writes:

Here's the patch for what I think is the consensus position. If there's no
objection I will apply this and document it.

Please do something for the comment for the connectOptions1 call.
As you've coded it, that is doing two completely different things
and the comment is almost completely unhelpful at explaining this
complexity. Oh, and casting away const gets no points for style.

I think you could do worse than to split it into two independent code
paths:

/*
* If the dbName parameter contains '=', assume it's a conninfo
* string.
*/
if (dbName && strchr(dbName,'='))
{
if (!connectOptions1(conn, dbName))
return conn;
}
else
{
/*
* Old-style path: first, parse an empty conninfo string in
* order to set up the same defaults that PQconnectdb() would use.
*/
if (!connectOptions1(conn, ""))
return conn;

/* Insert dbName parameter value into struct */
if (dbName && dbName[0] != '\0')
{
if (conn->dbName)
free(conn->dbName);
conn->dbName = strdup(dbName);
}
}

/*
* Insert remaining parameters into struct, overriding defaults
* (as well as any conflicting data from dbName taken as a conninfo).
*/

(This works because there's no reason the dbName parameter can't be
moved up to process before the rest.)

regards, tom lane

#17Andrew Dunstan
andrew@dunslane.net
In reply to: Tom Lane (#16)
Re: [HACKERS] psql commandline conninfo

Tom Lane wrote:

"Andrew Dunstan" <andrew@dunslane.net> writes:

Here's the patch for what I think is the consensus position. If there's
no
objection I will apply this and document it.

Please do something for the comment for the connectOptions1 call.
As you've coded it, that is doing two completely different things
and the comment is almost completely unhelpful at explaining this
complexity. Oh, and casting away const gets no points for style.

ouch. :-)

Ok, I accept the reproof. In fact I got up this morning, had my coffee,
and thought "That's a silly way to do it, I could make it much neater by
moving the dbName processing up," and lo and behold when I read your email
you had done it :-) It shall be done as you wish.

BTW, what is the approved way to handle warnings about const? Copy the
object?

cheers

andrew

#18Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Dunstan (#17)
Re: [HACKERS] psql commandline conninfo

"Andrew Dunstan" <andrew@dunslane.net> writes:

BTW, what is the approved way to handle warnings about const? Copy the
object?

Well, in the revised code there shouldn't be any warning at all, but
I think the mistake in your original was to declare the local variable
as "char *" instead of "const char *".

If "const" is being used as intended then a const-violation warning
would indeed suggest that you needed to make a writable copy.
Sometimes the problem is that you're working in a chunk of inadequately
const-ified code, ie, you're passing a const argument to some other
functions that do indeed treat their inputs as read-only but don't
declare them const. In such cases you can either run around and try to
inject const everywhere it should be, or hold your nose and use a cast,
or give up on marking your own argument const :-(. But you weren't
presented with that problem here, because connectOptions1() is already
const-ified.

regards, tom lane

#19Bruce Momjian
bruce@momjian.us
In reply to: Andrew Dunstan (#15)
Re: [HACKERS] psql commandline conninfo

I assume this patch will still allow a database name with an equals
sign, right?

psql "dbname='a=b'"

The libpq documentation isn't clear that single-quotes also escape
equals, but I assume that is true looking at the code:

http://www.postgresql.org/docs/8.2/static/libpq-connect.html

The passed string can be empty to use all default parameters, or it can
contain one or more parameter settings separated by whitespace. Each
parameter setting is in the form keyword = value. Spaces around the
equal sign are optional. To write an empty value or a value containing
spaces, surround it with single quotes, e.g., keyword = 'a value'.
Single quotes and backslashes within the value must be escaped with a
backslash, i.e., \' and \\.

---------------------------------------------------------------------------

Andrew Dunstan wrote:

Andrew Dunstan wrote:

Tom Lane wrote:

"Andrew Dunstan" <andrew@dunslane.net> writes:

We change libpq from time to time. Besides, how many DBs are there that
match the name pattern /^conn:.*=/ ? My guess is mighty few. So I don't
expect lots of surprise.

Um, but how many DB names have an "=" in them at all?

Basically what this proposal is about is migrating from separated
dbname/user/host/port/etc parameters to a unified conninfo parameter.
That seems to me like a good long-term objective, and so I'm willing
to break a few eggs on the way to the omelet, as long as we're not
breaking any very likely usages.

So: who here has a database with "=" in the name? And hands up if
you've got a database whose name begins with "conn:"?

I'm betting zero response rate on both of those, so see no reason to
contort the long-term definition for a very marginal difference in
the extent of backwards compatibility ...

I'm not sure -hackers is the most representative group to poll regarding
dbnames in use ...

Anyway, if I understand your current position, the only change needed to
my current patch would be that if we fail to parse a dbname parameter
that contains an = we simply fail at that point, rather than retrying it
as a straight database name.

I'm OK with that.

Here's the patch for what I think is the consensus position. If there's no
objection I will apply this and document it.

cheers

andrew

[ Attachment, skipping... ]

---------------------------(end of broadcast)---------------------------
TIP 9: In versions below 8.0, the planner will ignore your desire to
choose an index scan if your joining column's datatypes do not
match

--
Bruce Momjian bruce@momjian.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

#20Andrew Dunstan
andrew@dunslane.net
In reply to: Bruce Momjian (#19)
Re: [PATCHES] psql commandline conninfo

Bruce Momjian wrote:

I assume this patch will still allow a database name with an equals
sign, right?

psql "dbname='a=b'"

Yes. In fact, reading the code it looks like the quotes are not necessary
in this case.

cheers

andrew

#21Bruce Momjian
bruce@momjian.us
In reply to: Andrew Dunstan (#20)
Re: [PATCHES] psql commandline conninfo

Andrew Dunstan wrote:

Bruce Momjian wrote:

I assume this patch will still allow a database name with an equals
sign, right?

psql "dbname='a=b'"

Yes. In fact, reading the code it looks like the quotes are not necessary
in this case.

OK, good to know. Does the patch need documentation too? Are we
deprecating the psql options now duplicated by the new functionality,
like host/port/username/password?

--
Bruce Momjian bruce@momjian.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

#22Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#21)
Re: [PATCHES] psql commandline conninfo

Bruce Momjian <bruce@momjian.us> writes:

OK, good to know. Does the patch need documentation too?

Certainly.

Are we
deprecating the psql options now duplicated by the new functionality,
like host/port/username/password?

I'd vote not. This is just another way to do it.

regards, tom lane

#23Andrew Dunstan
andrew@dunslane.net
In reply to: Tom Lane (#22)
Re: [PATCHES] psql commandline conninfo

Tom Lane wrote:

Bruce Momjian <bruce@momjian.us> writes:

OK, good to know. Does the patch need documentation too?

Certainly.

That's why I haven't committed it yet. I intend to put info in the psql
manual as well as in the libpq reference.

Are we
deprecating the psql options now duplicated by the new functionality,
like host/port/username/password?

I'd vote not. This is just another way to do it.

I entirely agree. It lets you do some nice things that aren't obvious
now, like:

psql 'service=foo sslmode=require'

cheers

andrew

#24Andrew Dunstan
andrew@dunslane.net
In reply to: Andrew Dunstan (#23)
1 attachment(s)
Re: [HACKERS] psql commandline conninfo

Andrew Dunstan wrote:

Tom Lane wrote:

Bruce Momjian <bruce@momjian.us> writes:

OK, good to know. Does the patch need documentation too?

Certainly.

That's why I haven't committed it yet. I intend to put info in the
psql manual as well as in the libpq reference.

Are we
deprecating the psql options now duplicated by the new functionality,
like host/port/username/password?

I'd vote not. This is just another way to do it.

I entirely agree. It lets you do some nice things that aren't obvious
now, like:

psql 'service=foo sslmode=require'

Patch (Tom's code, my docs) attached.

cheers

andrew

Attachments:

conninfo.patchtext/x-patch; name=conninfo.patchDownload
Index: doc/src/sgml/libpq.sgml
===================================================================
RCS file: /cvsroot/pgsql/doc/src/sgml/libpq.sgml,v
retrieving revision 1.220
diff -c -r1.220 libpq.sgml
*** doc/src/sgml/libpq.sgml	10 Nov 2006 22:15:26 -0000	1.220
--- doc/src/sgml/libpq.sgml	18 Dec 2006 16:52:39 -0000
***************
*** 324,336 ****
                       const char *login,
                       const char *pwd);
  </synopsis>
! </para>
  
! <para>
!    This is the predecessor of <function>PQconnectdb</function> with a fixed
!    set of parameters.  It has the same functionality except that the
!    missing parameters will always take on default values.  Write <symbol>NULL</symbol> or an
!    empty string for any one of the fixed parameters that is to be defaulted.
     </para>
    </listitem>
   </varlistentry>
--- 324,342 ----
                       const char *login,
                       const char *pwd);
  </synopsis>
!     </para>
  
!     <para>
!      This is the predecessor of <function>PQconnectdb</function> with a fixed
!      set of parameters.  It has the same functionality except that the
!      missing parameters will always take on default values.  Write <symbol>NULL</symbol> or an
!      empty string for any one of the fixed parameters that is to be defaulted.
!    </para>
!    <para>
!      If the <parameter>dbName</parameter> contains an <symbol>=</symbol> sign, it
! 	 is taken as a <parameter>conninfo</parameter> string in exactly the same way as
! 	 if it had been passed to <function>PQconnectdb</function>, and the remaining
! 	 parameters are then applied as above.
     </para>
    </listitem>
   </varlistentry>
Index: doc/src/sgml/ref/psql-ref.sgml
===================================================================
RCS file: /cvsroot/pgsql/doc/src/sgml/ref/psql-ref.sgml,v
retrieving revision 1.175
diff -c -r1.175 psql-ref.sgml
*** doc/src/sgml/ref/psql-ref.sgml	21 Nov 2006 17:01:58 -0000	1.175
--- doc/src/sgml/ref/psql-ref.sgml	18 Dec 2006 16:52:40 -0000
***************
*** 112,117 ****
--- 112,121 ----
        class="parameter">dbname</replaceable> as the first non-option
        argument on the command line.
        </para>
+ 	  <para>
+ 	  If this parameter contains an <symbol>=</symbol> sign, it it treated as a
+ 	  <parameter>conninfo</parameter> string. See <xref linkend="libpq-connect"> for more information.
+ 	  </para>
        </listitem>
      </varlistentry>
  
***************
*** 554,559 ****
--- 558,575 ----
      passwords. See <xref linkend="libpq-pgpass"> for more information.
      </para>
  
+ 	<para>
+     An alternative way to specify connection parameters is in a
+ 	<parameter>conninfo</parameter>	string, which is used instead of a 
+ 	database name. This mechanism give you very wide control over the 
+ 	connection. For example,
+ <programlisting>
+ $ <userinput>psql "service=myservice sslmode=require"</userinput>
+ </programlisting>
+     See <xref linkend="libpq-connect"> for more information on all the
+ 	available connection options.
+ 	</para>
+ 
      <para>
      If the connection could not be made for any reason (e.g., insufficient
      privileges, server is not running on the targeted host, etc.),
Index: src/interfaces/libpq/fe-connect.c
===================================================================
RCS file: /cvsroot/pgsql/src/interfaces/libpq/fe-connect.c,v
retrieving revision 1.339
diff -c -r1.339 fe-connect.c
*** src/interfaces/libpq/fe-connect.c	21 Nov 2006 16:28:00 -0000	1.339
--- src/interfaces/libpq/fe-connect.c	18 Dec 2006 16:52:43 -0000
***************
*** 574,589 ****
  	conn = makeEmptyPGconn();
  	if (conn == NULL)
  		return NULL;
! 
! 	/*
! 	 * Parse an empty conninfo string in order to set up the same defaults
! 	 * that PQconnectdb() would use.
! 	 */
! 	if (!connectOptions1(conn, ""))
! 		return conn;
! 
! 	/*
! 	 * Absorb specified options into conn structure, overriding defaults
  	 */
  	if (pghost && pghost[0] != '\0')
  	{
--- 574,609 ----
  	conn = makeEmptyPGconn();
  	if (conn == NULL)
  		return NULL;
!     /*
!      * If the dbName parameter contains '=', assume it's a conninfo
!      * string.
!      */
!     if (dbName && strchr(dbName,'='))
!     {
!         if (!connectOptions1(conn, dbName))
!             return conn;
!     }
!     else
!     {
!         /*
!          * Old-style path: first, parse an empty conninfo string in
!          * order to set up the same defaults that PQconnectdb() would use.
!          */
!         if (!connectOptions1(conn, ""))
!             return conn;
! 
!         /* Insert dbName parameter value into struct */
!         if (dbName && dbName[0] != '\0')
!         {
!             if (conn->dbName)
!                 free(conn->dbName);
!             conn->dbName = strdup(dbName);
!         }
!     }
! 
!     /*
!      * Insert remaining parameters into struct, overriding defaults
!      * (as well as any conflicting data from dbName taken as a conninfo).
  	 */
  	if (pghost && pghost[0] != '\0')
  	{
***************
*** 613,625 ****
  		conn->pgtty = strdup(pgtty);
  	}
  
- 	if (dbName && dbName[0] != '\0')
- 	{
- 		if (conn->dbName)
- 			free(conn->dbName);
- 		conn->dbName = strdup(dbName);
- 	}
- 
  	if (login && login[0] != '\0')
  	{
  		if (conn->pguser)
--- 633,638 ----