Re: [HACKERS] Patch: libpq new connect function (PQconnectParams)

2010-01-28 Thread Guillaume Lelarge
Le 28/01/2010 07:32, Joe Conway a écrit :
> On 01/26/2010 02:55 PM, Guillaume Lelarge wrote:
>> Le 26/01/2010 19:43, Joe Conway a écrit :
>>> On 01/25/2010 03:21 PM, Guillaume Lelarge wrote:
 I didn't put any documentation before knowing which one will be choosen.
 So we still need to work on the manual.
>>>
>>> Please send the documentation as a separate patch. Once I have that I
>>> will commit the posted patch, barring any objections in the meantime.
>>
>> You'll find it attached with this mail. Please read it carefully, my
>> written english is not that good.
> 
> Final committed patch attached.
> 
> One last code correction -- in psql/startup.c the original patch defines
> the keywords array in the body of the code, rather than at the top of
> the block.
> 
> Minor improvements ( hopefully ;-)) to the documentation as well.
> 

Thanks a lot.


-- 
Guillaume.
 http://www.postgresqlfr.org
 http://dalibo.com

-- 
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] Patch: libpq new connect function (PQconnectParams)

2010-01-27 Thread Joe Conway
On 01/26/2010 02:55 PM, Guillaume Lelarge wrote:
> Le 26/01/2010 19:43, Joe Conway a écrit :
>> On 01/25/2010 03:21 PM, Guillaume Lelarge wrote:
>>> I didn't put any documentation before knowing which one will be choosen.
>>> So we still need to work on the manual.
>>
>> Please send the documentation as a separate patch. Once I have that I
>> will commit the posted patch, barring any objections in the meantime.
> 
> You'll find it attached with this mail. Please read it carefully, my
> written english is not that good.

Final committed patch attached.

One last code correction -- in psql/startup.c the original patch defines
the keywords array in the body of the code, rather than at the top of
the block.

Minor improvements ( hopefully ;-)) to the documentation as well.

Joe
Index: doc/src/sgml/libpq.sgml
===
RCS file: /cvsroot/pgsql/doc/src/sgml/libpq.sgml,v
retrieving revision 1.295
diff -c -r1.295 libpq.sgml
*** doc/src/sgml/libpq.sgml	21 Jan 2010 14:58:52 -	1.295
--- doc/src/sgml/libpq.sgml	28 Jan 2010 06:26:29 -
***
*** 56,62 
 one time.  (One reason to do that is to access more than one
 database.)  Each connection is represented by a
 PGconnPGconn object, which
!is obtained from the function PQconnectdb or
 PQsetdbLogin.  Note that these functions will always
 return a non-null object pointer, unless perhaps there is too
 little memory even to allocate the PGconn object.
--- 56,63 
 one time.  (One reason to do that is to access more than one
 database.)  Each connection is represented by a
 PGconnPGconn object, which
!is obtained from the function PQconnectdb,
!PQconnectdbParams, or
 PQsetdbLogin.  Note that these functions will always
 return a non-null object pointer, unless perhaps there is too
 little memory even to allocate the PGconn object.
***
*** 91,125 
  
 
  
!  PQconnectdbPQconnectdb
   

 Makes a new connection to the database server.
  
 
! PGconn *PQconnectdb(const char *conninfo);
 

  

 This function opens a new database connection using the parameters taken
!from the string conninfo.  Unlike PQsetdbLogin below,
!the parameter set can be extended without changing the function signature,
!so use of this function (or its nonblocking analogues PQconnectStart
!and PQconnectPoll) is preferred for new application programming.

  

!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 \\.

  

--- 92,124 
  
 
  
!  PQconnectdbParamsPQconnectdbParams
   

 Makes a new connection to the database server.
  
 
! PGconn *PQconnectdbParams(const char **keywords, const char **values);
 

  

 This function opens a new database connection using the parameters taken
!from two NULL-terminated arrays. The first,
!keywords, is defined as an array of strings, each one
!being a key word. The second, values, gives the value
!for each key word. Unlike PQsetdbLogin below, the parameter
!set can be extended without changing the function signature, so use of
!this function (or its nonblocking analogs PQconnectStartParams
!and PQconnectPoll) is preferred for new application
!programming.

  

!The passed arrays can be empty to use all default parameters, or can
!contain one or more parameter settings. They should be matched in length.
!Processing will stop with the last non-NULL element
!of the keywords array.

  

***
*** 478,483 
--- 477,515 
  
  
  
+  PQconnectdbPQconnectdb
+  
+   
+Makes a new connection to the database server.
+ 
+
+ PGconn *PQconnectdb(const char *conninfo);
+
+   
+ 
+   
+This function opens a new database connection using the parameters taken
+from the string conninfo.
+   
+ 
+   
+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 con

Re: [HACKERS] Patch: libpq new connect function (PQconnectParams)

2010-01-26 Thread Guillaume Lelarge
Le 26/01/2010 19:43, Joe Conway a écrit :
> On 01/25/2010 03:21 PM, Guillaume Lelarge wrote:
>> I didn't put any documentation before knowing which one will be choosen.
>> So we still need to work on the manual.
> 
> Please send the documentation as a separate patch. Once I have that I
> will commit the posted patch, barring any objections in the meantime.
> 

You'll find it attached with this mail. Please read it carefully, my
written english is not that good.

Thanks.


-- 
Guillaume.
 http://www.postgresqlfr.org
 http://dalibo.com
Index: doc/src/sgml/libpq.sgml
===
RCS file: /opt/cvsroot_postgresql/pgsql/doc/src/sgml/libpq.sgml,v
retrieving revision 1.295
diff -c -p -c -r1.295 libpq.sgml
*** doc/src/sgml/libpq.sgml	21 Jan 2010 14:58:52 -	1.295
--- doc/src/sgml/libpq.sgml	26 Jan 2010 22:52:52 -
***
*** 56,62 
 one time.  (One reason to do that is to access more than one
 database.)  Each connection is represented by a
 PGconnPGconn object, which
!is obtained from the function PQconnectdb or
 PQsetdbLogin.  Note that these functions will always
 return a non-null object pointer, unless perhaps there is too
 little memory even to allocate the PGconn object.
--- 56,63 
 one time.  (One reason to do that is to access more than one
 database.)  Each connection is represented by a
 PGconnPGconn object, which
!is obtained from the function PQconnectdb,
!PQconnectdbParams or
 PQsetdbLogin.  Note that these functions will always
 return a non-null object pointer, unless perhaps there is too
 little memory even to allocate the PGconn object.
***
*** 91,125 
  
 
  
!  PQconnectdbPQconnectdb
   

 Makes a new connection to the database server.
  
 
! PGconn *PQconnectdb(const char *conninfo);
 

  

 This function opens a new database connection using the parameters taken
!from the string conninfo.  Unlike PQsetdbLogin below,
 the parameter set can be extended without changing the function signature,
!so use of this function (or its nonblocking analogues PQconnectStart
 and PQconnectPoll) is preferred for new application programming.

  

!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 \\.

  

--- 92,121 
  
 
  
!  PQconnectdbParamsPQconnectdbParams
   

 Makes a new connection to the database server.
  
 
! PGconn *PQconnectdbParams(const char **keywords, const char **values);
 

  

 This function opens a new database connection using the parameters taken
!from two arrays.  The first one, keywords, is defined
!as an array of strings, each one being a keyword.  The second one,
!values, gives the value for each keyword. Unlike
!PQsetdbLogin below,
 the parameter set can be extended without changing the function signature,
!so use of this function (or its nonblocking analogues PQconnectStartParams
 and PQconnectPoll) is preferred for new application programming.

  

!The passed arrays can be empty to use all default parameters, or it can
!contain one or more parameter settings.

  

***
*** 478,483 
--- 474,518 
  
  
  
+  PQconnectdbPQconnectdb
+  
+   
+Makes a new connection to the database server.
+ 
+
+ PGconn *PQconnectdb(const char *conninfo);
+
+   
+ 
+   
+This function opens a new database connection using the parameters taken
+from the string conninfo.
+   
+ 
+   
+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 \\.
+   
+ 
+   
+The currently recognized parameter key words are available above.
+If  any  parameter is unspecified, then th

Re: [HACKERS] Patch: libpq new connect function (PQconnectParams)

2010-01-26 Thread Joe Conway
On 01/25/2010 03:21 PM, Guillaume Lelarge wrote:
> I didn't put any documentation before knowing which one will be choosen.
> So we still need to work on the manual.

Please send the documentation as a separate patch. Once I have that I
will commit the posted patch, barring any objections in the meantime.

Joe



signature.asc
Description: OpenPGP digital signature


Re: [HACKERS] Patch: libpq new connect function (PQconnectParams)

2010-01-25 Thread Guillaume Lelarge
Le 26/01/2010 00:04, Joe Conway a écrit :
> I'm reviewing the patch posted here:
>   http://archives.postgresql.org/pgsql-hackers/2010-01/msg01579.php
> for this commitfest item:
>   https://commitfest.postgresql.org/action/patch_view?id=259
> 

First, thanks for reviewing my patch.

> Patch attached - a few minor changes:
> -
> 1) Updated to apply cleanly against cvs tip

Sorry about this. I already updated it twice. I didn't think a new
update was needed.

> 2) Improved comments

Sure.

> 3) Moved much of what was in PQconnectStartParams() to a new
>conninfo_array_parse() to be more consistent with existing code

You're right. It also makes the code more readable and understandable. I
should have done that.

> Questions/comments:
> ---
> a) Do we want an analog to PQconninfoParse(), e.g.
>PQconninfoParseParams()? If not, it isn't worth keeping use_defaults
>as an argument to conninfo_array_parse().

No, I don't think so. I can't find a use case for it.

> b) I refrained from further consolidation even though there is room.
>For example, I considered leaving only the real parsing code in
>conninfo_parse(), and having it return keywords and values arrays.
>If we did that, the rest of the code could be modified to accept
>keywords and values instead of conninfo, and therefore shared. I was
>concerned about the probably small performance hit to the existing
>code path. Thoughts?
> c) Obviously I liked the "two-arrays approach" better -- any objections
>to that?

No objection. I prefer the other one, but it's just not that important.

I didn't put any documentation before knowing which one will be choosen.
So we still need to work on the manual.

Thanks again.


-- 
Guillaume.
 http://www.postgresqlfr.org
 http://dalibo.com

-- 
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] Patch: libpq new connect function (PQconnectParams)

2010-01-25 Thread Joe Conway
I'm reviewing the patch posted here:
  http://archives.postgresql.org/pgsql-hackers/2010-01/msg01579.php
for this commitfest item:
  https://commitfest.postgresql.org/action/patch_view?id=259

Patch attached - a few minor changes:
-
1) Updated to apply cleanly against cvs tip
2) Improved comments
3) Moved much of what was in PQconnectStartParams() to a new
   conninfo_array_parse() to be more consistent with existing code

Questions/comments:
---
a) Do we want an analog to PQconninfoParse(), e.g.
   PQconninfoParseParams()? If not, it isn't worth keeping use_defaults
   as an argument to conninfo_array_parse().
b) I refrained from further consolidation even though there is room.
   For example, I considered leaving only the real parsing code in
   conninfo_parse(), and having it return keywords and values arrays.
   If we did that, the rest of the code could be modified to accept
   keywords and values instead of conninfo, and therefore shared. I was
   concerned about the probably small performance hit to the existing
   code path. Thoughts?
c) Obviously I liked the "two-arrays approach" better -- any objections
   to that?

Thanks,

Joe
Index: src/bin/psql/startup.c
===
RCS file: /opt/src/cvs/pgsql/src/bin/psql/startup.c,v
retrieving revision 1.158
diff -c -r1.158 startup.c
*** src/bin/psql/startup.c	2 Jan 2010 16:57:59 -	1.158
--- src/bin/psql/startup.c	25 Jan 2010 18:09:55 -
***
*** 168,181 
  	if (pset.getPassword == TRI_YES)
  		password = simple_prompt(password_prompt, 100, false);
  
  	/* loop until we have a password if requested by backend */
  	do
  	{
! 		new_pass = false;
! 		pset.db = PQsetdbLogin(options.host, options.port, NULL, NULL,
! 	options.action == ACT_LIST_DB && options.dbname == NULL ?
! 			   "postgres" : options.dbname,
! 			   options.username, password);
  
  		if (PQstatus(pset.db) == CONNECTION_BAD &&
  			PQconnectionNeedsPassword(pset.db) &&
--- 168,200 
  	if (pset.getPassword == TRI_YES)
  		password = simple_prompt(password_prompt, 100, false);
  
+ const char *keywords[] = {
+ "host",
+ "port",
+ "dbname",
+ "user",
+ "password",
+ "application_name",
+ NULL
+ };
+ 
  	/* loop until we have a password if requested by backend */
  	do
  	{
! const char *values[] = {
!   options.host,
!   options.port,
!   (options.action == ACT_LIST_DB && 
!options.dbname == NULL) ? "postgres" : options.dbname,
!   options.username,
!   password,
!   pset.progname,
!   NULL
!   };
! 
! new_pass = false;
! 
! pset.db = PQconnectdbParams(keywords, values);
  
  		if (PQstatus(pset.db) == CONNECTION_BAD &&
  			PQconnectionNeedsPassword(pset.db) &&
Index: src/interfaces/libpq/exports.txt
===
RCS file: /opt/src/cvs/pgsql/src/interfaces/libpq/exports.txt,v
retrieving revision 1.24
diff -c -r1.24 exports.txt
*** src/interfaces/libpq/exports.txt	21 Jan 2010 14:58:53 -	1.24
--- src/interfaces/libpq/exports.txt	25 Jan 2010 18:09:55 -
***
*** 155,157 
--- 155,159 
  PQinitOpenSSL 153
  PQescapeLiteral   154
  PQescapeIdentifier155
+ PQconnectdbParams 156
+ PQconnectStartParams  157
Index: src/interfaces/libpq/fe-connect.c
===
RCS file: /opt/src/cvs/pgsql/src/interfaces/libpq/fe-connect.c,v
retrieving revision 1.384
diff -c -r1.384 fe-connect.c
*** src/interfaces/libpq/fe-connect.c	20 Jan 2010 21:15:21 -	1.384
--- src/interfaces/libpq/fe-connect.c	25 Jan 2010 22:39:45 -
***
*** 262,271 
--- 262,275 
  static int	connectDBStart(PGconn *conn);
  static int	connectDBComplete(PGconn *conn);
  static PGconn *makeEmptyPGconn(void);
+ static void fillPGconn(PGconn *conn, PQconninfoOption *connOptions);
  static void freePGconn(PGconn *conn);
  static void closePGconn(PGconn *conn);
  static PQconninfoOption *conninfo_parse(const char *conninfo,
  			   PQExpBuffer errorMessage, bool use_defaults);
+ static PQconninfoOption *conninfo_array_parse(const char **keywords,
+ const char **values, PQExpBuffer errorMessage,
+ bool use_defaults);
  static char *conninfo_getval(PQconninfoOption *connOptions,
  const char *keyword);
  static void defaultNoticeReceiver(void *arg, const PGresult *res);
***
*** 290,312 
  /*
   *		Connecting to a Database
   *
!  * There are now four different ways a user of this API can connect to the
   * database.  Two are not recommended for use in new code, because of their
   * lack of extensibility with respect to the p