Re: [HACKERS] Another review of URI for libpq, v7 submission

2012-04-11 Thread Alvaro Herrera
Excerpts from Alvaro Herrera's message of lun abr 09 16:41:50 -0300 2012: There are three minor things that need to be changed for this to be committable: Committed this patch after some more editorialization; in particular the test was rewritten so that instead of trying to connect, it uses

Re: [HACKERS] Another review of URI for libpq, v7 submission

2012-04-09 Thread Alvaro Herrera
Excerpts from Peter Eisentraut's message of vie abr 06 03:09:10 -0300 2012: On fre, 2012-04-06 at 00:25 -0300, Alvaro Herrera wrote: Some moments of radical thinking later, I became unhappy with the fact that the conninfo stuff and parameter keywords are all crammed in the PQconnectdbParams

Re: [HACKERS] Another review of URI for libpq, v7 submission

2012-04-06 Thread Peter Eisentraut
On fre, 2012-04-06 at 00:25 -0300, Alvaro Herrera wrote: Some moments of radical thinking later, I became unhappy with the fact that the conninfo stuff and parameter keywords are all crammed in the PQconnectdbParams description. This feels wrong to me, even more so after we expand it even

Re: [HACKERS] Another review of URI for libpq, v7 submission

2012-04-05 Thread Peter Eisentraut
On ons, 2012-03-28 at 01:49 +0300, Alex wrote: Attached is a gzipped v10, complete with the above fixes and fixes to bugs found by Heikki. Documentation and code comments are also finally updated. The compiler warning is still there: fe-connect.c: In function ‘conninfo_parse’:

Re: [HACKERS] Another review of URI for libpq, v7 submission

2012-04-05 Thread Alvaro Herrera
Excerpts from Peter Eisentraut's message of jue abr 05 13:52:13 -0300 2012: On ons, 2012-03-28 at 01:49 +0300, Alex wrote: Attached is a gzipped v10, complete with the above fixes and fixes to bugs found by Heikki. Documentation and code comments are also finally updated. The compiler

Re: [HACKERS] Another review of URI for libpq, v7 submission

2012-03-27 Thread Heikki Linnakangas
On 22.03.2012 23:42, Alex wrote: Okay, at last here's v9, rebased against current master branch. Some quick comments on this patch: I see a compiler warning: fe-connect.c: In function ‘conninfo_parse’: fe-connect.c:4113: warning: unused variable ‘option’ Docs are missing. I wonder if you

Re: [HACKERS] Another review of URI for libpq, v7 submission

2012-03-27 Thread Alex Shulgin
Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes: On 22.03.2012 23:42, Alex wrote: Okay, at last here's v9, rebased against current master branch. Some quick comments on this patch: Heikki, thank you for taking a look at this! I see a compiler warning: fe-connect.c: In

Re: [HACKERS] Another review of URI for libpq, v7 submission

2012-03-27 Thread Peter Eisentraut
On tor, 2012-03-22 at 23:42 +0200, Alex wrote: Okay, at last here's v9, rebased against current master branch. Attached is a patch on top of your v9 with two small fixes: - Don't provide a check target in libpq/Makefile if it's not implemented. - Use the configured port number for running the

Re: [HACKERS] Another review of URI for libpq, v7 submission

2012-03-27 Thread Alex
Peter Eisentraut pete...@gmx.net writes: On tor, 2012-03-22 at 23:42 +0200, Alex wrote: Okay, at last here's v9, rebased against current master branch. Attached is a patch on top of your v9 with two small fixes: - Don't provide a check target in libpq/Makefile if it's not implemented. -

Re: [HACKERS] Another review of URI for libpq, v7 submission

2012-03-27 Thread Alex
Alex a...@commandprompt.com writes: Peter Eisentraut pete...@gmx.net writes: Attached is a patch on top of your v9 with two small fixes: - Don't provide a check target in libpq/Makefile if it's not implemented. - Use the configured port number for running the tests (otherwise it runs

Re: Trivial libpq refactoring patch (was: Re: [HACKERS] Another review of URI for libpq, v7 submission)

2012-03-22 Thread Tom Lane
Alex Shulgin a...@commandprompt.com writes: While working on this fix I've figured out that I need my conninfo_uri_parse to support use_defaults parameter, like conninfo(_array)_parse functions do. The problem is that the block of code which does the defaults handling is duplicated in both

Re: [HACKERS] Another review of URI for libpq, v7 submission

2012-03-22 Thread Alex
Alex a...@commandprompt.com writes: Marko Kreen mark...@gmail.com writes: On Thu, Mar 15, 2012 at 11:29:31PM +0200, Alex wrote: https://github.com/a1exsh/postgres/commits/uri The point of the patch is to have one string with all connection options, in standard format, yes? So why does not

Trivial libpq refactoring patch (was: Re: [HACKERS] Another review of URI for libpq, v7 submission)

2012-03-21 Thread Alex Shulgin
Alex a...@commandprompt.com writes: Marko Kreen mark...@gmail.com writes: On Thu, Mar 15, 2012 at 11:29:31PM +0200, Alex wrote: https://github.com/a1exsh/postgres/commits/uri The point of the patch is to have one string with all connection options, in standard format, yes? So why does not

Re: [HACKERS] Another review of URI for libpq, v7 submission

2012-03-20 Thread Alex
Marko Kreen mark...@gmail.com writes: On Thu, Mar 15, 2012 at 11:29:31PM +0200, Alex wrote: https://github.com/a1exsh/postgres/commits/uri The point of the patch is to have one string with all connection options, in standard format, yes? So why does not this work: db =

Re: [HACKERS] Another review of URI for libpq, v7 submission

2012-03-17 Thread Marko Kreen
On Thu, Mar 15, 2012 at 11:29:31PM +0200, Alex wrote: https://github.com/a1exsh/postgres/commits/uri The point of the patch is to have one string with all connection options, in standard format, yes? So why does not this work: db = PQconnectdb(postgres://localhost); ? -- marko -- Sent

[HACKERS] Another review of URI for libpq, v7 submission

2012-03-15 Thread Daniel Farina
I reviewed this and so far have not found any serious problems, although as is par for the course it contains some of the fiddly bits involved in any string manipulations in C. I made a few edits -- none strictly necessary for correctness -- that the original author is free audit and/or

Re: [HACKERS] Another review of URI for libpq, v7 submission

2012-03-15 Thread Alvaro Herrera
Excerpts from Daniel Farina's message of jue mar 15 05:49:50 -0300 2012: One thing I found puzzling was that in the latest revision the tests appeared to be broken for me: all @ signs were translated to (at). Is that mangling applied by the archives, or something? Ugh, ouch. Yeah, that was

Re: [HACKERS] Another review of URI for libpq, v7 submission

2012-03-15 Thread Alex Shulgin
Daniel Farina dan...@heroku.com writes: I reviewed this and so far have not found any serious problems, although as is par for the course it contains some of the fiddly bits involved in any string manipulations in C. I made a few edits -- none strictly necessary for correctness -- that the

Re: [HACKERS] Another review of URI for libpq, v7 submission

2012-03-15 Thread Daniel Farina
On Thu, Mar 15, 2012 at 7:36 AM, Alex Shulgin a...@commandprompt.com wrote: I wonder if there's any evidence as to that mangling the email addresses helps to reduce spam at all?  I mean replacing (at) back to @ and (dot) to . is piece of cake for a spam crawler. I suspect we're long past the

Re: [HACKERS] Another review of URI for libpq, v7 submission

2012-03-15 Thread Alex
Daniel Farina dan...@heroku.com writes: Finally, attached is v8.  Hopefully I didn't mess things up too much. I'll give it another look-over. Do you have these in git somewhere? It will help me save time on some of the incremental changes. Yes, I've just pushed my dev branch to this fork of