Re: [HACKERS] new version of PQconnectdb was:(Re: [HACKERS] Determining client_encoding from client locale)
On Tue, Nov 3, 2009 at 6:31 AM, Peter Eisentraut pete...@gmx.net wrote: Is anyone planning to do further work on this? This appears to be blocking the client_encoding=auto feature. yes, i'm planning to make an attempt to do it as soon as i get some time... but if you think it's important enough please go for it -- Atentamente, Jaime Casanova Soporte y capacitación de PostgreSQL Asesoría y desarrollo de sistemas Guayaquil - Ecuador Cel. +59387171157 -- 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] new version of PQconnectdb was:(Re: [HACKERS] Determining client_encoding from client locale)
On Sun, 2009-09-27 at 21:49 -0400, Robert Haas wrote: On Wed, Sep 23, 2009 at 3:26 PM, Tom Lane t...@sss.pgh.pa.us wrote: Jaime Casanova jcasa...@systemguards.com.ec writes: i extracted the functions to connect that Heikki put on psql in his patch for determining client_encoding from client locale and put it in libpq so i follow the PQconnectdbParams(* params[]) approach. I got around to looking at the actual patch a bit. I hadn't understood why it was so small, but now I do: it's implemented as a wrapper around PQconnectdb. That is, it takes the given keywords and values, and builds a conninfo string, which PQconnectdb then has to pull apart again. Based on this review, it sounds like this patch will need a major rewrite before it can be seriously reviewed. Given that, I think it makes sense to postpone this to the next CommitFest, so I am going to mark it as Returned with Feedback. Is anyone planning to do further work on this? This appears to be blocking the client_encoding=auto feature. -- 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] new version of PQconnectdb was:(Re: [HACKERS] Determining client_encoding from client locale)
Peter Eisentraut pete...@gmx.net writes: Is anyone planning to do further work on this? This appears to be blocking the client_encoding=auto feature. Huh? Why would there be any linkage? This is just offering an alternative way to set connection options, it has nothing to do with what the set of options is. 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] new version of PQconnectdb was:(Re: [HACKERS] Determining client_encoding from client locale)
On Tue, 2009-11-03 at 09:32 -0500, Tom Lane wrote: Peter Eisentraut pete...@gmx.net writes: Is anyone planning to do further work on this? This appears to be blocking the client_encoding=auto feature. Huh? Why would there be any linkage? This is just offering an alternative way to set connection options, it has nothing to do with what the set of options is. Right, but we got stuck when we realized that we would need to switch psql's connection handling from PQsetdb to PQconnectdb, which would be annoying. And at the moment everyone involved appears to be waiting on this hypothetical newer version of PQconnectdb to make the original patch easier. -- 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] new version of PQconnectdb was:(Re: [HACKERS] Determining client_encoding from client locale)
Tom Lane wrote: Peter Eisentraut pete...@gmx.net writes: Is anyone planning to do further work on this? This appears to be blocking the client_encoding=auto feature. Huh? Why would there be any linkage? This is just offering an alternative way to set connection options, it has nothing to do with what the set of options is. The client_encoding=auto feature would use the new function in psql to set the client_encoding to 'auto'. Otherwise it needs to construct a properly quoted connection string and pass it to the existing PQconnectdb, which is a bit laborious. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.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] new version of PQconnectdb was:(Re: [HACKERS] Determining client_encoding from client locale)
On Wed, Sep 23, 2009 at 3:26 PM, Tom Lane t...@sss.pgh.pa.us wrote: Jaime Casanova jcasa...@systemguards.com.ec writes: i extracted the functions to connect that Heikki put on psql in his patch for determining client_encoding from client locale and put it in libpq so i follow the PQconnectdbParams(* params[]) approach. I got around to looking at the actual patch a bit. I hadn't understood why it was so small, but now I do: it's implemented as a wrapper around PQconnectdb. That is, it takes the given keywords and values, and builds a conninfo string, which PQconnectdb then has to pull apart again. This seems, well, dumb. I'll admit that the wasted cycles are probably not much compared to all the other costs of establishing a connection. But it means that you're still exposed to all the other limitations of PQconnectdb, eg any possible bugs or restrictions in the quoting/escaping logic. It seems to me that a non-bogus patch for this would involve refactoring the code within PQconnectdb so that the keywords and values could be plugged in directly without the escaping and de-escaping logic. It doesn't look that hard to pull apart conninfo_parse into two or three functions that would support this. Another reason why it needs refactoring is that this way doesn't expose all the functionality that ought to be available; in particular not the ability to do an async connection while passing the parameters in this style. You need analogs to PQconnectStart and PQconnectPoll too. (Actually I guess the existing PQconnectPoll would work fine, but you definitely need an analog to PQconnectStart.) Based on this review, it sounds like this patch will need a major rewrite before it can be seriously reviewed. Given that, I think it makes sense to postpone this to the next CommitFest, so I am going to mark it as Returned with Feedback. ...Robert -- 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] new version of PQconnectdb was:(Re: [HACKERS] Determining client_encoding from client locale)
Jaime Casanova jcasa...@systemguards.com.ec writes: i extracted the functions to connect that Heikki put on psql in his patch for determining client_encoding from client locale and put it in libpq so i follow the PQconnectdbParams(* params[]) approach. I got around to looking at the actual patch a bit. I hadn't understood why it was so small, but now I do: it's implemented as a wrapper around PQconnectdb. That is, it takes the given keywords and values, and builds a conninfo string, which PQconnectdb then has to pull apart again. This seems, well, dumb. I'll admit that the wasted cycles are probably not much compared to all the other costs of establishing a connection. But it means that you're still exposed to all the other limitations of PQconnectdb, eg any possible bugs or restrictions in the quoting/escaping logic. It seems to me that a non-bogus patch for this would involve refactoring the code within PQconnectdb so that the keywords and values could be plugged in directly without the escaping and de-escaping logic. It doesn't look that hard to pull apart conninfo_parse into two or three functions that would support this. Another reason why it needs refactoring is that this way doesn't expose all the functionality that ought to be available; in particular not the ability to do an async connection while passing the parameters in this style. You need analogs to PQconnectStart and PQconnectPoll too. (Actually I guess the existing PQconnectPoll would work fine, but you definitely need an analog to PQconnectStart.) 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] new version of PQconnectdb was:(Re: [HACKERS] Determining client_encoding from client locale)
On Thu, Sep 10, 2009 at 12:01 AM, Jaime Casanova jcasa...@systemguards.com.ec wrote: On Mon, Jul 6, 2009 at 10:00 AM, Heikki Linnakangasheikki.linnakan...@enterprisedb.com wrote: Could we have a version of PQconnectdb() with an API more suited for setting the params programmatically? The PQsetdbLogin() approach doesn't scale as parameters are added/removed in future versions, but we could have something like this: PGconn *PQconnectParams(const char **params) Where params is an array with an even number of parameters, forming key/value pairs. Usage example: i extracted the functions to connect that Heikki put on psql in his patch for determining client_encoding from client locale and put it in libpq so i follow the PQconnectdbParams(* params[]) approach. i put the new function at the end of the exports.txt file, there's a reason to renumber the exports to put it at the beginning with the other PQconnectdb function? this patch still lacks documentation, i will add it in the next days but want to know if you have any comments about this... -- Atentamente, Jaime Casanova Soporte y capacitación de PostgreSQL Asesoría y desarrollo de sistemas Guayaquil - Ecuador Cel. +59387171157 Index: src/bin/psql/command.c === RCS file: /home/postgres/pgrepo/pgsql/src/bin/psql/command.c,v retrieving revision 1.206 diff -c -r1.206 command.c *** src/bin/psql/command.c 11 Jun 2009 14:49:07 - 1.206 --- src/bin/psql/command.c 14 Sep 2009 17:34:00 - *** *** 1239,1246 while (true) { ! n_conn = PQsetdbLogin(host, port, NULL, NULL, ! dbname, user, password); /* We can immediately discard the password -- no longer needed */ if (password) --- 1239,1254 while (true) { ! const char *params[] = { ! host, host, ! port, port, ! dbname, dbname, ! user, user, ! password, password, ! NULL, NULL ! }; ! ! n_conn = PQconnectdbParams(params); /* We can immediately discard the password -- no longer needed */ if (password) Index: src/bin/psql/startup.c === RCS file: /home/postgres/pgrepo/pgsql/src/bin/psql/startup.c,v retrieving revision 1.156 diff -c -r1.156 startup.c *** src/bin/psql/startup.c 5 Apr 2009 04:19:58 - 1.156 --- src/bin/psql/startup.c 14 Sep 2009 17:33:43 - *** *** 171,181 /* 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) --- 171,189 /* loop until we have a password if requested by backend */ do { + const char *params[] = { + host, options.host, + port, options.port, + dbname, (options.action == ACT_LIST_DB +options.dbname == NULL) ? postgres : options.dbname, + user, options.username, + password, password, + NULL, NULL + }; + new_pass = false; ! ! pset.db = PQconnectdbParams(params); if (PQstatus(pset.db) == CONNECTION_BAD PQconnectionNeedsPassword(pset.db) Index: src/interfaces/libpq/exports.txt === RCS file: /home/postgres/pgrepo/pgsql/src/interfaces/libpq/exports.txt,v retrieving revision 1.23 diff -c -r1.23 exports.txt *** src/interfaces/libpq/exports.txt 31 Mar 2009 01:41:27 - 1.23 --- src/interfaces/libpq/exports.txt 14 Sep 2009 17:33:03 - *** *** 153,155 --- 153,156 PQfireResultCreateEvents 151 PQconninfoParse 152 PQinitOpenSSL 153 + PQconnectdbParams 154 Index: src/interfaces/libpq/fe-connect.c === RCS file: /home/postgres/pgrepo/pgsql/src/interfaces/libpq/fe-connect.c,v retrieving revision 1.376 diff -c -r1.376 fe-connect.c *** src/interfaces/libpq/fe-connect.c 24 Jul 2009 17:58:31 - 1.376 --- src/interfaces/libpq/fe-connect.c 14 Sep 2009 17:34:49 - *** *** 211,216 --- 211,219 GSS-library, , 7}, /* sizeof(gssapi) = 7 */ #endif + {appname, NULL, NULL, NULL, + Client-application, , 45}, + /* Terminating entry --- MUST BE LAST */ {NULL, NULL, NULL, NULL, NULL, NULL, 0} *** *** 283,288 --- 286,333 */ /* + * PQconnectdbParams + */ + PGconn * + PQconnectdbParams(const char * const *params) + { + PGconn *ret; + PQExpBufferData buf; + + initPQExpBuffer(buf); + + while(*params) + { + const char *option = params[0]; + const char *value = params[1]; + + if (value != NULL) + { + /* write option name */ + appendPQExpBuffer(buf, %s = ', option); +
Re: [HACKERS] new version of PQconnectdb was:(Re: [HACKERS] Determining client_encoding from client locale)
Jaime Casanova wrote: On Thu, Sep 10, 2009 at 12:01 AM, Jaime Casanova jcasa...@systemguards.com.ec wrote: On Mon, Jul 6, 2009 at 10:00 AM, Heikki Linnakangasheikki.linnakan...@enterprisedb.com wrote: Could we have a version of PQconnectdb() with an API more suited for setting the params programmatically? The PQsetdbLogin() approach doesn't scale as parameters are added/removed in future versions, but we could have something like this: PGconn *PQconnectParams(const char **params) Where params is an array with an even number of parameters, forming key/value pairs. Usage example: i extracted the functions to connect that Heikki put on psql in his patch for determining client_encoding from client locale and put it in libpq so i follow the PQconnectdbParams(* params[]) approach. I was following this and never saw any firm decision on the prototype for this function. Although, I can say the single argument version did not appear to win any votes. The below posts agreed on a two argument version of parallel arrays (keywords, values): http://archives.postgresql.org/pgsql-hackers/2009-09/msg00533.php http://archives.postgresql.org/pgsql-hackers/2009-09/msg00559.php There is also the idea of passing an array of structs floating around, NULL terminated list or include an additional argument specifying element count. -- Andrew Chernow eSilo, LLC every bit counts http://www.esilo.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] new version of PQconnectdb was:(Re: [HACKERS] Determining client_encoding from client locale)
On Mon, Sep 14, 2009 at 1:34 PM, Andrew Chernow a...@esilo.com wrote: Jaime Casanova wrote: i extracted the functions to connect that Heikki put on psql in his patch for determining client_encoding from client locale and put it in libpq so i follow the PQconnectdbParams(* params[]) approach. [...] The below posts agreed on a two argument version of parallel arrays (keywords, values): http://archives.postgresql.org/pgsql-hackers/2009-09/msg00533.php http://archives.postgresql.org/pgsql-hackers/2009-09/msg00559.php actually, Tom said: it's hard to be sure which way is actually more convenient without having tried coding some likely calling scenarios both ways. so i tried one scenario. :) do you think is worth the trouble make the other approach? i could make the patch if someone is interested... personally, i think it will cause more problems than solve because you have to be sure your arrays have relationship between them... There is also the idea of passing an array of structs floating around, NULL terminated list or include an additional argument specifying element count. one more variable to the equation, more innecesary complexity and another source of errors, IMO... -- Atentamente, Jaime Casanova Soporte y capacitación de PostgreSQL Asesoría y desarrollo de sistemas Guayaquil - Ecuador Cel. +59387171157 -- 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] new version of PQconnectdb was:(Re: [HACKERS] Determining client_encoding from client locale)
Jaime Casanova jcasa...@systemguards.com.ec writes: i put the new function at the end of the exports.txt file, there's a reason to renumber the exports to put it at the beginning with the other PQconnectdb function? Exports.txt numbers do not change. EVER. 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] new version of PQconnectdb was:(Re: [HACKERS] Determining client_encoding from client locale)
Jaime Casanova wrote: On Mon, Sep 14, 2009 at 1:34 PM, Andrew Chernow a...@esilo.com wrote: Jaime Casanova wrote: i extracted the functions to connect that Heikki put on psql in his patch for determining client_encoding from client locale and put it in libpq so i follow the PQconnectdbParams(* params[]) approach. [...] The below posts agreed on a two argument version of parallel arrays (keywords, values): http://archives.postgresql.org/pgsql-hackers/2009-09/msg00533.php http://archives.postgresql.org/pgsql-hackers/2009-09/msg00559.php actually, Tom said: it's hard to be sure which way is actually more convenient without having tried coding some likely calling scenarios both ways. Aahhh, correct you are Daniel son :) personally, i think it will cause more problems than solve because you have to be sure your arrays have relationship between them... A strict relationship exists either way. There is also the idea of passing an array of structs floating around, NULL terminated list or include an additional argument specifying element count. one more variable to the equation, more innecesary complexity and another source of errors, IMO... one more variable or one more element, both of which cause problems if omitted/incorrect. const char *params[] = {host, blah.com, port, 6262, NULL, NULL}; // compiler enforces relationship const PGopotion opts[] = {{host, blah.com}, {port, 6262}, {NULL, NULL}}; IMHO, the struct approach seems like a cleaner solution. Any chance of using a term other than params? Maybe options or props? -- Andrew Chernow eSilo, LLC every bit counts http://www.esilo.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] new version of PQconnectdb was:(Re: [HACKERS] Determining client_encoding from client locale)
On Mon, Sep 14, 2009 at 1:55 PM, Tom Lane t...@sss.pgh.pa.us wrote: Jaime Casanova jcasa...@systemguards.com.ec writes: i put the new function at the end of the exports.txt file, there's a reason to renumber the exports to put it at the beginning with the other PQconnectdb function? Exports.txt numbers do not change. EVER. i didn't find any info about it, not even in the sources... should we document that we need to put some functions in that file and for what reasons? actually, i was very confused when the psql fails to compile until i understood i need to put the function in that file -- Atentamente, Jaime Casanova Soporte y capacitación de PostgreSQL Asesoría y desarrollo de sistemas Guayaquil - Ecuador Cel. +59387171157 -- 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] new version of PQconnectdb was:(Re: [HACKERS] Determining client_encoding from client locale)
On Mon, Sep 14, 2009 at 2:20 PM, Andrew Chernow a...@esilo.com wrote: Jaime Casanova wrote: On Mon, Sep 14, 2009 at 1:34 PM, Andrew Chernow a...@esilo.com wrote: Jaime Casanova wrote: i extracted the functions to connect that Heikki put on psql in his patch for determining client_encoding from client locale and put it in libpq so i follow the PQconnectdbParams(* params[]) approach. [...] The below posts agreed on a two argument version of parallel arrays (keywords, values): http://archives.postgresql.org/pgsql-hackers/2009-09/msg00533.php http://archives.postgresql.org/pgsql-hackers/2009-09/msg00559.php actually, Tom said: it's hard to be sure which way is actually more convenient without having tried coding some likely calling scenarios both ways. Aahhh, correct you are Daniel son :) ??? don't understand you ??? personally, i think it will cause more problems than solve because you have to be sure your arrays have relationship between them... A strict relationship exists either way. [...] IMHO, the struct approach seems like a cleaner solution. i agree Any chance of using a term other than params? Maybe options or props? i don't have any problems with options -- Atentamente, Jaime Casanova Soporte y capacitación de PostgreSQL Asesoría y desarrollo de sistemas Guayaquil - Ecuador Cel. +59387171157 -- 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] new version of PQconnectdb was:(Re: [HACKERS] Determining client_encoding from client locale)
actually, Tom said: it's hard to be sure which way is actually more convenient without having tried coding some likely calling scenarios both ways. Aahhh, correct you are Daniel son :) ??? don't understand you ??? From the movie karate kid; oopps, should be Daniel San. I was trying to be cute but that apparently failed :( -- Andrew Chernow eSilo, LLC every bit counts http://www.esilo.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] new version of PQconnectdb was:(Re: [HACKERS] Determining client_encoding from client locale)
Jaime Casanova jcasa...@systemguards.com.ec writes: On Mon, Sep 14, 2009 at 1:55 PM, Tom Lane t...@sss.pgh.pa.us wrote: Exports.txt numbers do not change. Â EVER. i didn't find any info about it, not even in the sources... should we document that we need to put some functions in that file and for what reasons? Every function that is meant to be exported from libpq. 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] new version of PQconnectdb was:(Re: [HACKERS] Determining client_encoding from client locale)
On Mon, Sep 14, 2009 at 3:31 PM, Andrew Chernow a...@esilo.com wrote: actually, Tom said: it's hard to be sure which way is actually more convenient without having tried coding some likely calling scenarios both ways. Aahhh, correct you are Daniel son :) ??? don't understand you ??? From the movie karate kid; oopps, should be Daniel San. ah! got it... ;) -- Atentamente, Jaime Casanova Soporte y capacitación de PostgreSQL Asesoría y desarrollo de sistemas Guayaquil - Ecuador Cel. +59387171157 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] new version of PQconnectdb was:(Re: [HACKERS] Determining client_encoding from client locale)
On Mon, Jul 6, 2009 at 10:00 AM, Heikki Linnakangasheikki.linnakan...@enterprisedb.com wrote: Could we have a version of PQconnectdb() with an API more suited for setting the params programmatically? The PQsetdbLogin() approach doesn't scale as parameters are added/removed in future versions, but we could have something like this: PGconn *PQconnectParams(const char **params) Where params is an array with an even number of parameters, forming key/value pairs. Usage example: [...] Another idea is to use an array of PQconninfoOption structs: PQconn *PQconnectParams(PQconninfoOption *params); this sounds like a good idea, specially if we add new parameters to the conninfo string and want postgresql's client applications to use them. any one have a preference here? -- Atentamente, Jaime Casanova Soporte y capacitación de PostgreSQL Asesoría y desarrollo de sistemas Guayaquil - Ecuador Cel. +59387171157 -- 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] new version of PQconnectdb was:(Re: [HACKERS] Determining client_encoding from client locale)
PGconn *PQconnectParams(const char **params) Where params is an array with an even number of parameters, forming key/value pairs. Usage example: Maybe use the term properties (props for short) or options instead of params? Params is already in heavy use. How about PQconnectProps(...) or PQconnectOptions(...)? Another idea is to use an array of PQconninfoOption structs: PQconn *PQconnectParams(PQconninfoOption *params); this sounds like a good idea, specially if we add new parameters to Here's another idea, parallel arrays: PGconn *PQconnectProps(const char **names, const char **values); PGconn *PQconnectOptions(const char **names, const char **values); To build on the struct idea, maybe PGprop or PGoption instead of PQconninfoOption. Also, add an argument specifying the number of props/options. PGconn *PQconnectProps(const PGprop *props, int nProps); PGconn *PQconnectOptions(const PGoption *options, int nOptions); any one have a preference here? I like the struct approach. I personally prefer specifying the element count of an array, rather than using a NULL terminating element. -- Andrew Chernow eSilo, LLC every bit counts http://www.esilo.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers