Re: [HACKERS] ecpglib use PQconnectdbParams

2012-02-13 Thread Michael Meskes
> Because connect_timeout is a separate libpq connection parameter, but
> now it's stuck into "options".  It might have worked more or less by
> accident before.

So it is not an option, right? But the old function accepted it as an option it 
seems.

> It's not clear to me why this only appears on checktcp.  And why we
> don't run those tests by default.  That should be clarified, because

This was decided when regression tests for ecpg were introduced to not depend
on the use of TCP ports.

For details see this thread:
http://archives.postgresql.org/pgsql-hackers/2006-08/msg00078.php
and in particular these emails: 
http://archives.postgresql.org/pgsql-hackers/2006-08/msg00118.php
http://archives.postgresql.org/pgsql-hackers/2006-08/msg00134.php

Michael
-- 
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org
Jabber: michael.meskes at googlemail dot com
VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL

-- 
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] ecpglib use PQconnectdbParams

2012-02-08 Thread Tom Lane
Peter Eisentraut  writes:
>   if (IsUnderPostmaster)
>   ereport(FATAL,
>   (errcode(ERRCODE_SYNTAX_ERROR),
> -  errmsg("invalid command-line arguments for 
> server process"),
> +  errmsg("invalid command-line arguments for 
> server process: %s", argv[optind]),
> errhint("Try \"%s --help\" for more information.", 
> progname)));
>   else
>   ereport(FATAL,

+1 for that change, but please s/arguments/argument/ in the text.  Also,
what about the other branch of the "if" that's just out of sight here?

regards, tom lane

-- 
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] ecpglib use PQconnectdbParams

2012-02-08 Thread Peter Eisentraut
On mån, 2012-02-06 at 21:11 +0100, Michael Meskes wrote:
> On Fri, Feb 03, 2012 at 01:15:30PM +0100, Christian Ullrich wrote:
> > Shouldn't these be [7]? You can have up to 6 items, plus the terminator.
> 
> I take it only keywords have to be [7], right? Committed, thanks for spotting 
> this.
> 
> There seems to be one more problem that I haven't had time to tackle yet. With
> this patch the connection regression test (make checktcp) doesn't run through
> anymore because one test connection cannot be made. It spits out the error
> message:
> 
> FATAL:  invalid command-line arguments for server process 
> HINT:  Try "postgres --help" for more information.
> 
> This connection used to work without the patch and besides the error message 
> is
> not really telling in this context.

You can get more information on that with the attached patch, which
might be a useful addition in general.  The problem comes from this:

exec sql connect to unix:postgresql://localhost/connectdb?connect_timeout=14 
user connectuser;

and the updated error message is:

ECPGconnect: could not open database: FATAL:  invalid command-line arguments 
for server process: connect_timeout=14

Because connect_timeout is a separate libpq connection parameter, but
now it's stuck into "options".  It might have worked more or less by
accident before.

It's not clear to me why this only appears on checktcp.  And why we
don't run those tests by default.  That should be clarified, because
there are also other failures in that test that show that it hasn't been
run in a while.

diff --git i/src/backend/tcop/postgres.c w/src/backend/tcop/postgres.c
index 49a3969..d5bd73f 100644
--- i/src/backend/tcop/postgres.c
+++ w/src/backend/tcop/postgres.c
@@ -3375,7 +3375,7 @@ process_postgres_switches(int argc, char *argv[], GucContext ctx)
 		if (IsUnderPostmaster)
 			ereport(FATAL,
 	(errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("invalid command-line arguments for server process"),
+ errmsg("invalid command-line arguments for server process: %s", argv[optind]),
 			  errhint("Try \"%s --help\" for more information.", progname)));
 		else
 			ereport(FATAL,

-- 
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] ecpglib use PQconnectdbParams

2012-02-06 Thread Michael Meskes
On Fri, Feb 03, 2012 at 01:15:30PM +0100, Christian Ullrich wrote:
> Shouldn't these be [7]? You can have up to 6 items, plus the terminator.

I take it only keywords have to be [7], right? Committed, thanks for spotting 
this.

There seems to be one more problem that I haven't had time to tackle yet. With
this patch the connection regression test (make checktcp) doesn't run through
anymore because one test connection cannot be made. It spits out the error
message:

FATAL:  invalid command-line arguments for server process 
HINT:  Try "postgres --help" for more information.

This connection used to work without the patch and besides the error message is
not really telling in this context.

So if anyone is interested in looking at this fine, if not I will as soon as I
find some spare time.

Michael
-- 
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org
Jabber: michael.meskes at googlemail dot com
VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL

-- 
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] ecpglib use PQconnectdbParams

2012-02-06 Thread Marko Kreen
On Fri, Feb 03, 2012 at 01:15:30PM +0100, Christian Ullrich wrote:
> * Peter Eisentraut wrote:
> >I noticed ecpglib still uses PQconnectdb() with a craftily assembled
> >connection string.  Here is a patch to use PQconnectdbParams() instead.
> 
> + const char *conn_keywords[6];
> + const char *conn_values[6];
> 
> Shouldn't these be [7]? You can have up to 6 items, plus the terminator.

This bug is now in GIT.

-- 
marko


-- 
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] ecpglib use PQconnectdbParams

2012-02-03 Thread Michael Meskes
On Thu, Feb 02, 2012 at 08:01:48PM +0200, Peter Eisentraut wrote:
> I noticed ecpglib still uses PQconnectdb() with a craftily assembled
> connection string.  Here is a patch to use PQconnectdbParams() instead.

Thanks, committed. Will sync as soon as I'm online again.

Michael
-- 
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org
Jabber: michael.meskes at googlemail dot com
VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL

-- 
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] ecpglib use PQconnectdbParams

2012-02-03 Thread Christian Ullrich

* Peter Eisentraut wrote:


I noticed ecpglib still uses PQconnectdb() with a craftily assembled
connection string.  Here is a patch to use PQconnectdbParams() instead.


+   const char *conn_keywords[6];
+   const char *conn_values[6];

Shouldn't these be [7]? You can have up to 6 items, plus the terminator.

--
Christian Ullrich



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


[HACKERS] ecpglib use PQconnectdbParams

2012-02-02 Thread Peter Eisentraut
I noticed ecpglib still uses PQconnectdb() with a craftily assembled
connection string.  Here is a patch to use PQconnectdbParams() instead.
diff --git i/src/interfaces/ecpg/ecpglib/connect.c w/src/interfaces/ecpg/ecpglib/connect.c
index 909ba70..ea69e15 100644
--- i/src/interfaces/ecpg/ecpglib/connect.c
+++ w/src/interfaces/ecpg/ecpglib/connect.c
@@ -260,14 +260,6 @@ ECPGnoticeReceiver(void *arg, const PGresult *result)
 	ecpg_log("raising sqlcode %d\n", sqlcode);
 }
 
-static int
-strlen_or_null(const char *string)
-{
-	if (!string)
-		return 0;
-	return (strlen(string));
-}
-
 /* this contains some quick hacks, needs to be cleaned up, but it works */
 bool
 ECPGconnect(int lineno, int c, const char *name, const char *user, const char *passwd, const char *connection_name, int autocommit)
@@ -281,8 +273,9 @@ ECPGconnect(int lineno, int c, const char *name, const char *user, const char *p
 			   *tmp,
 			   *port = NULL,
 			   *realname = NULL,
-			   *options = NULL,
-			   *connect_string = NULL;
+			   *options = NULL;
+	const char *conn_keywords[6];
+	const char *conn_values[6];
 
 	ecpg_init_sqlca(sqlca);
 
@@ -482,34 +475,52 @@ ECPGconnect(int lineno, int c, const char *name, const char *user, const char *p
 			 options ? "with options " : "", options ? options : "",
 			 (user && strlen(user) > 0) ? "for user " : "", user ? user : "");
 
-	connect_string = ecpg_alloc(strlen_or_null(host)
-+ strlen_or_null(port)
-+ strlen_or_null(options)
-+ strlen_or_null(realname)
-+ strlen_or_null(user)
-+ strlen_or_null(passwd)
-			  + sizeof(" host = port = dbname = user = password ="), lineno);
-
-	if (options)/* replace '&' if tehre are any */
+	if (options)/* replace '&' if there are any */
 		for (i = 0; options[i]; i++)
 			if (options[i] == '&')
 options[i] = ' ';
 
-	sprintf(connect_string, "%s%s %s%s %s%s %s%s %s%s %s",
-			realname ? "dbname=" : "", realname ? realname : "",
-			host ? "host=" : "", host ? host : "",
-			port ? "port=" : "", port ? port : "",
-			(user && strlen(user) > 0) ? "user=" : "", user ? user : "",
-	 (passwd && strlen(passwd) > 0) ? "password=" : "", passwd ? passwd : "",
-			options ? options : "");
+	i = 0;
+	if (realname)
+	{
+		conn_keywords[i] = "dbname";
+		conn_values[i] = realname;
+		i++;
+	}
+	if (host)
+	{
+		conn_keywords[i] = "host";
+		conn_values[i] = host;
+		i++;
+	}
+	if (port)
+	{
+		conn_keywords[i] = "port";
+		conn_values[i] = port;
+		i++;
+	}
+	if (user && strlen(user) > 0)
+	{
+		conn_keywords[i] = "user";
+		conn_values[i] = user;
+		i++;
+	}
+	if (passwd && strlen(passwd) > 0)
+	{
+		conn_keywords[i] = "password";
+		conn_values[i] = passwd;
+		i++;
+	}
+	if (options)
+	{
+		conn_keywords[i] = "options";
+		conn_values[i] = options;
+		i++;
+	}
+	conn_keywords[i] = NULL;	/* terminator */
 
-	/*
-	 * this is deprecated this->connection = PQsetdbLogin(host, port, options,
-	 * NULL, realname, user, passwd);
-	 */
-	this->connection = PQconnectdb(connect_string);
+	this->connection = PQconnectdbParams(conn_keywords, conn_values, 0);
 
-	ecpg_free(connect_string);
 	if (host)
 		ecpg_free(host);
 	if (port)

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers