Re: [HACKERS] Patch: libpq new connect function (PQconnectParams)
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)
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 structnamePGconn/indextermprimaryPGconn// object, which !is obtained from the function functionPQconnectdb/ or functionPQsetdbLogin/. Note that these functions will always return a non-null object pointer, unless perhaps there is too little memory even to allocate the structnamePGconn/ object. --- 56,63 one time. (One reason to do that is to access more than one database.) Each connection is represented by a structnamePGconn/indextermprimaryPGconn// object, which !is obtained from the function functionPQconnectdb/, !functionPQconnectdbParams/, or functionPQsetdbLogin/. Note that these functions will always return a non-null object pointer, unless perhaps there is too little memory even to allocate the structnamePGconn/ object. *** *** 91,125 variablelist varlistentry ! termfunctionPQconnectdb/functionindextermprimaryPQconnectdb///term listitem para Makes a new connection to the database server. synopsis ! PGconn *PQconnectdb(const char *conninfo); /synopsis /para para This function opens a new database connection using the parameters taken !from the string literalconninfo/literal. Unlike functionPQsetdbLogin/ below, !the parameter set can be extended without changing the function signature, !so use of this function (or its nonblocking analogues functionPQconnectStart/ !and functionPQconnectPoll/function) is preferred for new application programming. /para para !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 literalkeyword = value/literal. !Spaces around the equal sign are optional. !To write an empty value or a value containing !spaces, surround it with single quotes, e.g., !literalkeyword = 'a value'/literal. !Single quotes and backslashes within the value must be escaped with a !backslash, i.e., literal\'/literal and literal\\/literal. /para para --- 92,124 variablelist varlistentry ! termfunctionPQconnectdbParams/functionindextermprimaryPQconnectdbParams///term listitem para Makes a new connection to the database server. synopsis ! PGconn *PQconnectdbParams(const char **keywords, const char **values); /synopsis /para para This function opens a new database connection using the parameters taken !from two symbolNULL/symbol-terminated arrays. The first, !literalkeywords/literal, is defined as an array of strings, each one !being a key word. The second, literalvalues/literal, gives the value !for each key word. Unlike functionPQsetdbLogin/ below, the parameter !set can be extended without changing the function signature, so use of !this function (or its nonblocking analogs functionPQconnectStartParams/ !and functionPQconnectPoll/function) is preferred for new application !programming. /para para !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-symbolNULL/symbol element !of the literalkeywords/literal array. /para para *** *** 478,483 --- 477,515 /varlistentry varlistentry +
Re: [HACKERS] Patch: libpq new connect function (PQconnectParams)
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)
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 structnamePGconn/indextermprimaryPGconn// object, which !is obtained from the function functionPQconnectdb/ or functionPQsetdbLogin/. Note that these functions will always return a non-null object pointer, unless perhaps there is too little memory even to allocate the structnamePGconn/ object. --- 56,63 one time. (One reason to do that is to access more than one database.) Each connection is represented by a structnamePGconn/indextermprimaryPGconn// object, which !is obtained from the function functionPQconnectdb/, !functionPQconnectdbParams/ or functionPQsetdbLogin/. Note that these functions will always return a non-null object pointer, unless perhaps there is too little memory even to allocate the structnamePGconn/ object. *** *** 91,125 variablelist varlistentry ! termfunctionPQconnectdb/functionindextermprimaryPQconnectdb///term listitem para Makes a new connection to the database server. synopsis ! PGconn *PQconnectdb(const char *conninfo); /synopsis /para para This function opens a new database connection using the parameters taken !from the string literalconninfo/literal. Unlike functionPQsetdbLogin/ below, the parameter set can be extended without changing the function signature, !so use of this function (or its nonblocking analogues functionPQconnectStart/ and functionPQconnectPoll/function) is preferred for new application programming. /para para !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 literalkeyword = value/literal. !Spaces around the equal sign are optional. !To write an empty value or a value containing !spaces, surround it with single quotes, e.g., !literalkeyword = 'a value'/literal. !Single quotes and backslashes within the value must be escaped with a !backslash, i.e., literal\'/literal and literal\\/literal. /para para --- 92,121 variablelist varlistentry ! termfunctionPQconnectdbParams/functionindextermprimaryPQconnectdbParams///term listitem para Makes a new connection to the database server. synopsis ! PGconn *PQconnectdbParams(const char **keywords, const char **values); /synopsis /para para This function opens a new database connection using the parameters taken !from two arrays. The first one, literalkeywords/literal, is defined !as an array of strings, each one being a keyword. The second one, !literalvalues/literal, gives the value for each keyword. Unlike !functionPQsetdbLogin/ below, the parameter set can be extended without changing the function signature, !so use of this function (or its nonblocking analogues functionPQconnectStartParams/ and functionPQconnectPoll/function) is preferred for new application programming. /para para !The passed arrays can be empty to use all default parameters, or it can !contain one or more parameter settings. /para para *** *** 478,483 --- 474,518 /varlistentry varlistentry + termfunctionPQconnectdb/functionindextermprimaryPQconnectdb///term + listitem + para +Makes a new connection to the database server. + +synopsis + PGconn *PQconnectdb(const char *conninfo); +/synopsis + /para + + para +This function opens a new database connection using the parameters taken +from the string literalconninfo/literal. + /para + + para +
Re: [HACKERS] Patch: libpq new connect function (PQconnectParams)
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 passing of options to the
*
Re: [HACKERS] Patch: libpq new connect function (PQconnectParams)
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
