On Wed, Feb 6, 2013 at 11:36 AM, Fujii Masao <masao.fu...@gmail.com> wrote: > On Thu, Feb 7, 2013 at 1:15 AM, Phil Sorber <p...@omniti.com> wrote: >> On Wed, Feb 6, 2013 at 11:11 AM, Fujii Masao <masao.fu...@gmail.com> wrote: >>> On Thu, Feb 7, 2013 at 12:05 AM, Phil Sorber <p...@omniti.com> wrote: >>>> On Tue, Feb 5, 2013 at 12:44 PM, Phil Sorber <p...@omniti.com> wrote: >>>>> On Tue, Feb 5, 2013 at 9:08 AM, Phil Sorber <p...@omniti.com> wrote: >>>>>> On Tue, Feb 5, 2013 at 9:06 AM, Alvaro Herrera >>>>>> <alvhe...@2ndquadrant.com> wrote: >>>>>>> Phil Sorber escribió: >>>>>>>> On Tue, Feb 5, 2013 at 6:41 AM, Robert Haas <robertmh...@gmail.com> >>>>>>>> wrote: >>>>>>>> > On Sat, Feb 2, 2013 at 9:55 PM, Phil Sorber <p...@omniti.com> wrote: >>>>>>>> >> OK, here is the patch that handles the connection string in dbname. >>>>>>>> >> I'll post the other patch under a different posting because I am >>>>>>>> >> sure >>>>>>>> >> it will get plenty of debate on it's own. >>>>>>>> > >>>>>>>> > I'm sorry, can you remind me what this does for us vs. the existing >>>>>>>> > coding? >>>>>>>> > >>>>>>>> >>>>>>>> It's supposed to handle the connection string passed as dbname case to >>>>>>>> be able to get the right output for host:port. >>>>>>> >>>>>>> Surely the idea is that you can also give it a postgres:// URI, right? >>>>>> >>>>>> Absolutely. >>>>> >>>>> Here is it. I like this approach more than the previous one, but I'd >>>>> like some feedback. >>> >>> The patch looks complicated to me. I was thinking that we can address >>> the problem >>> just by using PQconninfoParse() and PQconndefaults() like uri-regress.c >>> does. >>> The patch should be very simple. Why do we need so complicated code? >> >> Did you like the previous version better? >> >> http://www.postgresql.org/message-id/cadakt-hnb3ohcpkr+pcg1c_bjrsb7j__bpv+-jrjs5opjr2...@mail.gmail.com > > Yes because that version is simpler. But which version is better depends on > the reason why you implemented new version. If you have some idea about > the merit and demerit of each version, could you elaborate them?
I didn't like the way that I had to hard code the options in the first one as you pointed out below. I also was looking through the code for something else and saw that a lot of the apps were starting with defaults then building from there, rather than trying to add the defaults at the end. I think they were still doing it wrong because they were using getenv() on their own rather than asking libpq for the defaults though. So the new version gets the defaults at the beginning and also makes it easy to add new params without changing function definitions. > > + set_connect_options(connect_options, > &pgdbname, &pghost, > &pgport, &connect_timeout, &pguser); > > This code prevents us from giving options other than the above, for example > application_name, in the conninfo. I think that pg_isready should accept all > the libpq options. > I'm with you there. The new version fixes that as well. > When more than one -d options are specified, psql always prefer the last one > and ignore the others. OTOH, pg_isready with this patch seems to merge them. > I'm not sure if there is specific rule about the priority order of -d > option. But > it seems better to follow the existing way, i.e., always prefer the > last -d option. > The problem I am having here is resolving the differences between different -d options and other command line options. For example: -h foo -p 4321 -d "host=bar port=1234" -d "host=baz" I would expect that to be 'baz:1234' but you are saying it should be 'baz:4321'? I look at -d as just a way to pass in multiple options (when you aren't strictly passing in dbname) and should be able to expand the above example to: -h foo -p 4321 -h bar -p 1234 -h baz If we hold off on parsing the value of -d until the end so we are sure we have the last one, then we might lose other parameters that were after the -d option. For example: -h foo -p 4321 -d "host=bar port=1234" -d "host=baz user=you" -U me Should this be 'me@baz:1234' or 'you@baz:4321' or 'me@baz:4321'? So we would have to track the last instance of a parameter as well as the order those final versions came in. Sound to me like there is no clear answer there, but maybe there is a project consensus that has already been reached with regard to this? Or some general computing wisdom that applies? > Regards, > > -- > Fujii Masao -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
