Thread

1. [PATCH] Add PQconninfoParseParams and PQconninfodefaultsMerge to libpq

   Phil Sorber <phil@omniti.com> — 2013-02-03T03:16:29Z

   This patch came up from discussion about pg_isready.
   
   PQconninfoParseParams is similar to PQconninfoParse but takes two
   arrays like PQconnectdbParams. It essentially exposes
   conninfo_array_parse().
   
   PQconninfodefaultsMerge essentially exposes conninfo_add_defaults().
   It allows you to pass a PQconninfoOption struct and it adds defaults
   for all NULL values.
   
   There are no docs yet. I assumed I would let bikeshedding ensue, and
   also debate on whether we even want these first.
   
   
   

2. Re: [PATCH] Add PQconninfoParseParams and PQconninfodefaultsMerge to libpq

   Magnus Hagander <magnus@hagander.net> — 2013-02-03T06:37:14Z

   On Feb 3, 2013 4:16 AM, "Phil Sorber" <phil@omniti.com> wrote:
   >
   > This patch came up from discussion about pg_isready.
   >
   > PQconninfoParseParams is similar to PQconninfoParse but takes two
   > arrays like PQconnectdbParams. It essentially exposes
   > conninfo_array_parse().
   >
   > PQconninfodefaultsMerge essentially exposes conninfo_add_defaults().
   > It allows you to pass a PQconninfoOption struct and it adds defaults
   > for all NULL values.
   >
   > There are no docs yet. I assumed I would let bikeshedding ensue, and
   > also debate on whether we even want these first.
   
   I think you forgot to attach the patch.
   
   /Magnus
   

3. Re: [PATCH] Add PQconninfoParseParams and PQconninfodefaultsMerge to libpq

   Phil Sorber <phil@omniti.com> — 2013-02-03T07:23:05Z

   On Sun, Feb 3, 2013 at 1:37 AM, Magnus Hagander <magnus@hagander.net> wrote:
   >
   > On Feb 3, 2013 4:16 AM, "Phil Sorber" <phil@omniti.com> wrote:
   >>
   >> This patch came up from discussion about pg_isready.
   >>
   >> PQconninfoParseParams is similar to PQconninfoParse but takes two
   >> arrays like PQconnectdbParams. It essentially exposes
   >> conninfo_array_parse().
   >>
   >> PQconninfodefaultsMerge essentially exposes conninfo_add_defaults().
   >> It allows you to pass a PQconninfoOption struct and it adds defaults
   >> for all NULL values.
   >>
   >> There are no docs yet. I assumed I would let bikeshedding ensue, and
   >> also debate on whether we even want these first.
   >
   > I think you forgot to attach the patch.
   >
   > /Magnus
   
   /me hangs head in shame :-(
   
   Here is is...
   

4. Re: [PATCH] Add PQconninfoParseParams and PQconninfodefaultsMerge to libpq

   Alvaro Herrera <alvherre@2ndquadrant.com> — 2013-02-04T14:13:26Z

   > > On Feb 3, 2013 4:16 AM, "Phil Sorber" <phil@omniti.com> wrote:
   > >>
   > >> This patch came up from discussion about pg_isready.
   > >>
   > >> PQconninfoParseParams is similar to PQconninfoParse but takes two
   > >> arrays like PQconnectdbParams. It essentially exposes
   > >> conninfo_array_parse().
   > >>
   > >> PQconninfodefaultsMerge essentially exposes conninfo_add_defaults().
   > >> It allows you to pass a PQconninfoOption struct and it adds defaults
   > >> for all NULL values.
   
   Uh, no existing code can use this new functionality?  That seems
   disappointing.
   
   
   -- 
   Álvaro Herrera                http://www.2ndQuadrant.com/
   PostgreSQL Development, 24x7 Support, Training & Services
   
   
   

5. Re: [PATCH] Add PQconninfoParseParams and PQconninfodefaultsMerge to libpq

   Phil Sorber <phil@omniti.com> — 2013-02-04T15:08:08Z

   On Mon, Feb 4, 2013 at 9:13 AM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
   >
   >> > On Feb 3, 2013 4:16 AM, "Phil Sorber" <phil@omniti.com> wrote:
   >> >>
   >> >> This patch came up from discussion about pg_isready.
   >> >>
   >> >> PQconninfoParseParams is similar to PQconninfoParse but takes two
   >> >> arrays like PQconnectdbParams. It essentially exposes
   >> >> conninfo_array_parse().
   >> >>
   >> >> PQconninfodefaultsMerge essentially exposes conninfo_add_defaults().
   >> >> It allows you to pass a PQconninfoOption struct and it adds defaults
   >> >> for all NULL values.
   >
   > Uh, no existing code can use this new functionality?  That seems
   > disappointing.
   >
   
   I wrote this because I wanted to use it in pg_isready. I also wrote
   something for pg_isready to get around not having this. I think adding
   these two functions to libpq would be the better option, but wanted
   something that could fix an existing issue without having to patch
   libpq so late in the 9.3 development process. Actually, I think it was
   you who suggested that approach.
   
   So long answer short, there is existing code that can use this
   functionality immediately.
   
   >
   > --
   > Álvaro Herrera                http://www.2ndQuadrant.com/
   > PostgreSQL Development, 24x7 Support, Training & Services
   
   
   

6. Re: [PATCH] Add PQconninfoParseParams and PQconninfodefaultsMerge to libpq

   Alvaro Herrera <alvherre@2ndquadrant.com> — 2013-02-04T15:16:55Z

   Phil Sorber wrote:
   > On Mon, Feb 4, 2013 at 9:13 AM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
   
   > > Uh, no existing code can use this new functionality?  That seems
   > > disappointing.
   > 
   > I wrote this because I wanted to use it in pg_isready. I also wrote
   > something for pg_isready to get around not having this. I think adding
   > these two functions to libpq would be the better option, but wanted
   > something that could fix an existing issue without having to patch
   > libpq so late in the 9.3 development process. Actually, I think it was
   > you who suggested that approach.
   
   Yes, I realize that (and yes, I did).  But is no code other than
   pg_isready doing this?  Not even the libpq URI test program?
   
   -- 
   Álvaro Herrera                http://www.2ndQuadrant.com/
   PostgreSQL Development, 24x7 Support, Training & Services
   
   
   

7. Re: [PATCH] Add PQconninfoParseParams and PQconninfodefaultsMerge to libpq

   Phil Sorber <phil@omniti.com> — 2013-02-04T15:25:18Z

   On Mon, Feb 4, 2013 at 10:16 AM, Alvaro Herrera
   <alvherre@2ndquadrant.com> wrote:
   > Phil Sorber wrote:
   >> On Mon, Feb 4, 2013 at 9:13 AM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
   >
   >> > Uh, no existing code can use this new functionality?  That seems
   >> > disappointing.
   >>
   >> I wrote this because I wanted to use it in pg_isready. I also wrote
   >> something for pg_isready to get around not having this. I think adding
   >> these two functions to libpq would be the better option, but wanted
   >> something that could fix an existing issue without having to patch
   >> libpq so late in the 9.3 development process. Actually, I think it was
   >> you who suggested that approach.
   >
   > Yes, I realize that (and yes, I did).  But is no code other than
   > pg_isready doing this?  Not even the libpq URI test program?
   
   I think it probably would be able to benefit from this. Are you
   suggesting I patch that too? I thought it was usually frowned upon to
   touch random bits of working code like that. I'd be more than happy to
   do it if it helps build the case for getting this added.
   
   >
   > --
   > Álvaro Herrera                http://www.2ndQuadrant.com/
   > PostgreSQL Development, 24x7 Support, Training & Services
   
   
   

8. Re: [PATCH] Add PQconninfoParseParams and PQconninfodefaultsMerge to libpq

   Alvaro Herrera <alvherre@2ndquadrant.com> — 2013-02-04T15:32:51Z

   Phil Sorber wrote:
   > On Mon, Feb 4, 2013 at 10:16 AM, Alvaro Herrera
   > <alvherre@2ndquadrant.com> wrote:
   > > Phil Sorber wrote:
   > >> On Mon, Feb 4, 2013 at 9:13 AM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
   > >
   > >> > Uh, no existing code can use this new functionality?  That seems
   > >> > disappointing.
   > >>
   > >> I wrote this because I wanted to use it in pg_isready. I also wrote
   > >> something for pg_isready to get around not having this. I think adding
   > >> these two functions to libpq would be the better option, but wanted
   > >> something that could fix an existing issue without having to patch
   > >> libpq so late in the 9.3 development process. Actually, I think it was
   > >> you who suggested that approach.
   > >
   > > Yes, I realize that (and yes, I did).  But is no code other than
   > > pg_isready doing this?  Not even the libpq URI test program?
   > 
   > I think it probably would be able to benefit from this. Are you
   > suggesting I patch that too? I thought it was usually frowned upon to
   > touch random bits of working code like that. I'd be more than happy to
   > do it if it helps build the case for getting this added.
   
   Well, this qualifies as refactoring, so yeah, it helps the case.
   
   -- 
   Álvaro Herrera                http://www.2ndQuadrant.com/
   PostgreSQL Development, 24x7 Support, Training & Services
   
   
   

9. Re: [PATCH] Add PQconninfoParseParams and PQconninfodefaultsMerge to libpq

   Heikki Linnakangas <hlinnakangas@vmware.com> — 2013-02-11T21:19:58Z

   On 04.02.2013 17:32, Alvaro Herrera wrote:
   > Phil Sorber wrote:
   >> On Mon, Feb 4, 2013 at 10:16 AM, Alvaro Herrera
   >> <alvherre@2ndquadrant.com>  wrote:
   >>> Phil Sorber wrote:
   >>>> On Mon, Feb 4, 2013 at 9:13 AM, Alvaro Herrera<alvherre@2ndquadrant.com>  wrote:
   >>>
   >>>>> Uh, no existing code can use this new functionality?  That seems
   >>>>> disappointing.
   >>>>
   >>>> I wrote this because I wanted to use it in pg_isready. I also wrote
   >>>> something for pg_isready to get around not having this. I think adding
   >>>> these two functions to libpq would be the better option, but wanted
   >>>> something that could fix an existing issue without having to patch
   >>>> libpq so late in the 9.3 development process. Actually, I think it was
   >>>> you who suggested that approach.
   >>>
   >>> Yes, I realize that (and yes, I did).  But is no code other than
   >>> pg_isready doing this?  Not even the libpq URI test program?
   >>
   >> I think it probably would be able to benefit from this. Are you
   >> suggesting I patch that too? I thought it was usually frowned upon to
   >> touch random bits of working code like that. I'd be more than happy to
   >> do it if it helps build the case for getting this added.
   >
   > Well, this qualifies as refactoring, so yeah, it helps the case.
   
   I think this patch would simplift the patch to pass a connection string 
   to pg_basebackup and pg_receivexlog: 
   http://www.postgresql.org/message-id/005501cdfa45$6b0eec80$412cc580$@kommi@huawei.com. 
   I note that pg_dumpall also has a similar issue as pg_basebackup and 
   pg_receivexlog; there's no way to pass a connection string to it either.
   
   If you could come up with a unified patch that takes care of all of 
   those, that would be great.
   
   - Heikki
   
   
   

10. Re: [PATCH] Add PQconninfoParseParams and PQconninfodefaultsMerge to libpq

    Phil Sorber <phil@omniti.com> — 2013-02-11T21:42:02Z

    On Mon, Feb 11, 2013 at 4:19 PM, Heikki Linnakangas
    <hlinnakangas@vmware.com> wrote:
    > On 04.02.2013 17:32, Alvaro Herrera wrote:
    >>
    >> Phil Sorber wrote:
    >>>
    >>> On Mon, Feb 4, 2013 at 10:16 AM, Alvaro Herrera
    >>> <alvherre@2ndquadrant.com>  wrote:
    >>>>
    >>>> Phil Sorber wrote:
    >>>>>
    >>>>> On Mon, Feb 4, 2013 at 9:13 AM, Alvaro
    >>>>> Herrera<alvherre@2ndquadrant.com>  wrote:
    >>>>
    >>>>
    >>>>>> Uh, no existing code can use this new functionality?  That seems
    >>>>>> disappointing.
    >>>>>
    >>>>>
    >>>>> I wrote this because I wanted to use it in pg_isready. I also wrote
    >>>>> something for pg_isready to get around not having this. I think adding
    >>>>> these two functions to libpq would be the better option, but wanted
    >>>>> something that could fix an existing issue without having to patch
    >>>>> libpq so late in the 9.3 development process. Actually, I think it was
    >>>>> you who suggested that approach.
    >>>>
    >>>>
    >>>> Yes, I realize that (and yes, I did).  But is no code other than
    >>>> pg_isready doing this?  Not even the libpq URI test program?
    >>>
    >>>
    >>> I think it probably would be able to benefit from this. Are you
    >>> suggesting I patch that too? I thought it was usually frowned upon to
    >>> touch random bits of working code like that. I'd be more than happy to
    >>> do it if it helps build the case for getting this added.
    >>
    >>
    >> Well, this qualifies as refactoring, so yeah, it helps the case.
    >
    >
    > I think this patch would simplift the patch to pass a connection string to
    > pg_basebackup and pg_receivexlog:
    > http://www.postgresql.org/message-id/005501cdfa45$6b0eec80$412cc580$@kommi@huawei.com.
    > I note that pg_dumpall also has a similar issue as pg_basebackup and
    > pg_receivexlog; there's no way to pass a connection string to it either.
    >
    > If you could come up with a unified patch that takes care of all of those,
    > that would be great.
    
    I will take a look.
    
    >
    > - Heikki
    
    
    

11. Re: [PATCH] Add PQconninfoParseParams and PQconninfodefaultsMerge to libpq

    Amit Kapila <amit.kapila@huawei.com> — 2013-02-17T06:35:05Z

    On Tuesday, February 12, 2013 2:49 AM Heikki Linnakangas wrote:
    On 04.02.2013 17:32, Alvaro Herrera wrote:
    > Phil Sorber wrote:
    >> On Mon, Feb 4, 2013 at 10:16 AM, Alvaro Herrera
    >> <alvherre@2ndquadrant.com>  wrote:
    >>> Phil Sorber wrote:
    >>>> On Mon, Feb 4, 2013 at 9:13 AM, Alvaro Herrera<alvherre@2ndquadrant.com>  wrote:
    >>>
    
    > I think this patch would simplift the patch to pass a connection string
    > to pg_basebackup and pg_receivexlog:
    > http://www.postgresql.org/message-id/005501cdfa45$6b0eec80$412cc580$@kommi@huawei.com.
    > I note that pg_dumpall also has a similar issue as pg_basebackup and
    > pg_receivexlog; there's no way to pass a connection string to it either.
    
    I have looked into both patches and my analysis is as below:
    
    In pg_basebackup patch, we have connection string and list of keywords (individual options specified by user),
    in the current available patch it has combined connection string and list of keywords as connection string 
    and called PQconnectdb() which takes connection string as input.
    
    Now the patch of Phil Sober provides 2 new API's PQconninfoParseParams(), and PQconninfodefaultsMerge(),
    using these API's I can think of below way for patch "pass a connection string to pg_basebackup, ..."
    
    1. Call existing function PQconinfoParse() with connection string input by user and get PQconninfoOption.
    
    2. Now use the existing keywords (individual options specified by user) and extract the keywords from 
       PQconninfoOption structure and call new API PQconninfoParseParams() which will return PQconninfoOption.
       The PQconninfoOption structure returned in this step will contain all keywords
    
    3. Call PQconninfodefaultsMerge() to merge any default values if exist. Not sure if this step is required?
    
    4. Extract individual keywords from PQconninfoOption structure and call PQconnectdbParams.
    
    
    Is this inline with what you have in mind or you have thought of some other simpler way of using new API's?
    
    With Regards,
    Amit Kapila.
    
    
    

12. Re: [PATCH] Add PQconninfoParseParams and PQconninfodefaultsMerge to libpq

    Phil Sorber <phil@omniti.com> — 2013-02-17T15:13:34Z

    On Sun, Feb 17, 2013 at 1:35 AM, Amit kapila <amit.kapila@huawei.com> wrote:
    > On Tuesday, February 12, 2013 2:49 AM Heikki Linnakangas wrote:
    > On 04.02.2013 17:32, Alvaro Herrera wrote:
    >> Phil Sorber wrote:
    >>> On Mon, Feb 4, 2013 at 10:16 AM, Alvaro Herrera
    >>> <alvherre@2ndquadrant.com>  wrote:
    >>>> Phil Sorber wrote:
    >>>>> On Mon, Feb 4, 2013 at 9:13 AM, Alvaro Herrera<alvherre@2ndquadrant.com>  wrote:
    >>>>
    >
    >> I think this patch would simplift the patch to pass a connection string
    >> to pg_basebackup and pg_receivexlog:
    >> http://www.postgresql.org/message-id/005501cdfa45$6b0eec80$412cc580$@kommi@huawei.com.
    >> I note that pg_dumpall also has a similar issue as pg_basebackup and
    >> pg_receivexlog; there's no way to pass a connection string to it either.
    >
    > I have looked into both patches and my analysis is as below:
    >
    > In pg_basebackup patch, we have connection string and list of keywords (individual options specified by user),
    > in the current available patch it has combined connection string and list of keywords as connection string
    > and called PQconnectdb() which takes connection string as input.
    >
    > Now the patch of Phil Sober provides 2 new API's PQconninfoParseParams(), and PQconninfodefaultsMerge(),
    > using these API's I can think of below way for patch "pass a connection string to pg_basebackup, ..."
    >
    > 1. Call existing function PQconinfoParse() with connection string input by user and get PQconninfoOption.
    >
    > 2. Now use the existing keywords (individual options specified by user) and extract the keywords from
    >    PQconninfoOption structure and call new API PQconninfoParseParams() which will return PQconninfoOption.
    >    The PQconninfoOption structure returned in this step will contain all keywords
    >
    > 3. Call PQconninfodefaultsMerge() to merge any default values if exist. Not sure if this step is required?
    >
    > 4. Extract individual keywords from PQconninfoOption structure and call PQconnectdbParams.
    >
    >
    > Is this inline with what you have in mind or you have thought of some other simpler way of using new API's?
    >
    > With Regards,
    > Amit Kapila.
    
    I think what would be nice is an additional connect function that took
    PQconninfoOption as a parameter. Or at least something that can
    convert from PQconninfoOption to a connection string or keyword/value
    arrays.
    
    
    

13. Re: [PATCH] Add PQconninfoParseParams and PQconninfodefaultsMerge to libpq

    Amit Kapila <amit.kapila@huawei.com> — 2013-02-18T04:07:12Z

    On Sunday, February 17, 2013 8:44 PM Phil Sorber wrote:
    > On Sun, Feb 17, 2013 at 1:35 AM, Amit kapila <amit.kapila@huawei.com>
    > wrote:
    > > On Tuesday, February 12, 2013 2:49 AM Heikki Linnakangas wrote:
    > > On 04.02.2013 17:32, Alvaro Herrera wrote:
    > >> Phil Sorber wrote:
    > >>> On Mon, Feb 4, 2013 at 10:16 AM, Alvaro Herrera
    > >>> <alvherre@2ndquadrant.com>  wrote:
    > >>>> Phil Sorber wrote:
    > >>>>> On Mon, Feb 4, 2013 at 9:13 AM, Alvaro
    > Herrera<alvherre@2ndquadrant.com>  wrote:
    > >>>>
    > >
    > >> I think this patch would simplift the patch to pass a connection
    > string
    > >> to pg_basebackup and pg_receivexlog:
    > >> http://www.postgresql.org/message-
    > id/005501cdfa45$6b0eec80$412cc580$@kommi@huawei.com.
    > >> I note that pg_dumpall also has a similar issue as pg_basebackup and
    > >> pg_receivexlog; there's no way to pass a connection string to it
    > either.
    > >
    > > I have looked into both patches and my analysis is as below:
    > >
    > > In pg_basebackup patch, we have connection string and list of
    > keywords (individual options specified by user),
    > > in the current available patch it has combined connection string and
    > list of keywords as connection string
    > > and called PQconnectdb() which takes connection string as input.
    > >
    > > Now the patch of Phil Sober provides 2 new API's
    > PQconninfoParseParams(), and PQconninfodefaultsMerge(),
    > > using these API's I can think of below way for patch "pass a
    > connection string to pg_basebackup, ..."
    > >
    > > 1. Call existing function PQconinfoParse() with connection string
    > input by user and get PQconninfoOption.
    > >
    > > 2. Now use the existing keywords (individual options specified by
    > user) and extract the keywords from
    > >    PQconninfoOption structure and call new API
    > PQconninfoParseParams() which will return PQconninfoOption.
    > >    The PQconninfoOption structure returned in this step will contain
    > all keywords
    > >
    > > 3. Call PQconninfodefaultsMerge() to merge any default values if
    > exist. Not sure if this step is required?
    > >
    > > 4. Extract individual keywords from PQconninfoOption structure and
    > call PQconnectdbParams.
    > >
    > >
    > > Is this inline with what you have in mind or you have thought of some
    > other simpler way of using new API's?
    > >
    > > With Regards,
    > > Amit Kapila.
    > 
    > I think what would be nice is an additional connect function that took
    > PQconninfoOption as a parameter. Or at least something that can
    > convert from PQconninfoOption to a connection string or keyword/value
    > arrays.
    
    Yes, it would be better if we would like to use new API's, I think it can
    save step-4 and some part in step-2. 
    I am not sure for this purpose would it be okay to introduce new Connect
    API?
    
    I also feel it will increase the scope of patch. 
    
    Heikki, would you be more specific that what in the patch you want to see
    simplified.
    Is the combining of keywords and connection string makes you feel the area
    where patch can be improved.
    
    
    With Regards,
    Amit Kapila.
    
    
    
    

14. Re: [PATCH] Add PQconninfoParseParams and PQconninfodefaultsMerge to libpq

    Heikki Linnakangas <hlinnakangas@vmware.com> — 2013-02-18T08:11:08Z

    On 18.02.2013 06:07, Amit Kapila wrote:
    > On Sunday, February 17, 2013 8:44 PM Phil Sorber wrote:
    >> On Sun, Feb 17, 2013 at 1:35 AM, Amit kapila<amit.kapila@huawei.com>
    >> wrote:
    >>> Now the patch of Phil Sober provides 2 new API's
    >> PQconninfoParseParams(), and PQconninfodefaultsMerge(),
    >>> using these API's I can think of below way for patch "pass a
    >> connection string to pg_basebackup, ..."
    >>>
    >>> 1. Call existing function PQconinfoParse() with connection string
    >> input by user and get PQconninfoOption.
    >>>
    >>> 2. Now use the existing keywords (individual options specified by
    >> user) and extract the keywords from
    >>>     PQconninfoOption structure and call new API
    >> PQconninfoParseParams() which will return PQconninfoOption.
    >>>     The PQconninfoOption structure returned in this step will contain
    >> all keywords
    >>>
    >>> 3. Call PQconninfodefaultsMerge() to merge any default values if
    >> exist. Not sure if this step is required?
    >>>
    >>> 4. Extract individual keywords from PQconninfoOption structure and
    >> call PQconnectdbParams.
    >>>
    >>> Is this inline with what you have in mind or you have thought of some
    >> other simpler way of using new API's?
    
    Yep, that's roughly what I had in mind. I don't think it's necessary to 
    merge defaults in step 3, but it needs to add the "replication=true" and 
    "dbname=replication" options.
    
    >> I think what would be nice is an additional connect function that took
    >> PQconninfoOption as a parameter. Or at least something that can
    >> convert from PQconninfoOption to a connection string or keyword/value
    >> arrays.
    >
    > Yes, it would be better if we would like to use new API's, I think it can
    > save step-4 and some part in step-2.
    
    pg_basebackup needs to add options to the array, so I don't think a new 
    connect function would help much. It's not a lot of code to iterate 
    through the PGconnInfoOption array and turn it back into keywords and 
    values arrays, so I'd just do that straight in the client code.
    
    - Heikki
    
    
    

15. Re: [PATCH] Add PQconninfoParseParams and PQconninfodefaultsMerge to libpq

    Amit Kapila <amit.kapila@huawei.com> — 2013-02-18T08:55:23Z

    On Monday, February 18, 2013 1:41 PM Heikki Linnakangas wrote:
    > On 18.02.2013 06:07, Amit Kapila wrote:
    > > On Sunday, February 17, 2013 8:44 PM Phil Sorber wrote:
    > >> On Sun, Feb 17, 2013 at 1:35 AM, Amit kapila<amit.kapila@huawei.com>
    > >> wrote:
    > >>> Now the patch of Phil Sober provides 2 new API's
    > >> PQconninfoParseParams(), and PQconninfodefaultsMerge(),
    > >>> using these API's I can think of below way for patch "pass a
    > >> connection string to pg_basebackup, ..."
    > >>>
    > >>> 1. Call existing function PQconinfoParse() with connection string
    > >> input by user and get PQconninfoOption.
    > >>>
    > >>> 2. Now use the existing keywords (individual options specified by
    > >> user) and extract the keywords from
    > >>>     PQconninfoOption structure and call new API
    > >> PQconninfoParseParams() which will return PQconninfoOption.
    > >>>     The PQconninfoOption structure returned in this step will
    > contain
    > >> all keywords
    > >>>
    > >>> 3. Call PQconninfodefaultsMerge() to merge any default values if
    > >> exist. Not sure if this step is required?
    > >>>
    > >>> 4. Extract individual keywords from PQconninfoOption structure and
    > >> call PQconnectdbParams.
    > >>>
    > >>> Is this inline with what you have in mind or you have thought of
    > some
    > >> other simpler way of using new API's?
    > 
    > Yep, that's roughly what I had in mind. I don't think it's necessary to
    > merge defaults in step 3, but it needs to add the "replication=true"
    > and
    > "dbname=replication" options.
    > 
    > >> I think what would be nice is an additional connect function that
    > took
    > >> PQconninfoOption as a parameter. Or at least something that can
    > >> convert from PQconninfoOption to a connection string or
    > keyword/value
    > >> arrays.
    > >
    > > Yes, it would be better if we would like to use new API's, I think it
    > can
    > > save step-4 and some part in step-2.
    > 
    > pg_basebackup needs to add options to the array, so I don't think a new
    > connect function would help much. It's not a lot of code to iterate
    > through the PGconnInfoOption array and turn it back into keywords and
    > values arrays, so I'd just do that straight in the client code.
    
    Okay, got the point.
    
    Phil, I will try to finish the combined patch. Is it possible for you to
    complete
    the documentation for the new API's.
    
    With Regards,
    Amit Kapila.
    
    
    
    

16. Re: [PATCH] Add PQconninfoParseParams and PQconninfodefaultsMerge to libpq

    Amit Kapila <amit.kapila@huawei.com> — 2013-02-18T13:08:27Z

    On Monday, February 18, 2013 1:41 PM Heikki Linnakangas wrote:
    > On 18.02.2013 06:07, Amit Kapila wrote:
    > > On Sunday, February 17, 2013 8:44 PM Phil Sorber wrote:
    > >> On Sun, Feb 17, 2013 at 1:35 AM, Amit kapila<amit.kapila@huawei.com>
    > >> wrote:
    > >>> Now the patch of Phil Sober provides 2 new API's
    > >> PQconninfoParseParams(), and PQconninfodefaultsMerge(),
    > >>> using these API's I can think of below way for patch "pass a
    > >> connection string to pg_basebackup, ..."
    > >>>
    > >>> 1. Call existing function PQconinfoParse() with connection string
    > >> input by user and get PQconninfoOption.
    > >>>
    > >>> 2. Now use the existing keywords (individual options specified by
    > >> user) and extract the keywords from
    > >>>     PQconninfoOption structure and call new API
    > >> PQconninfoParseParams() which will return PQconninfoOption.
    > >>>     The PQconninfoOption structure returned in this step will
    > contain
    > >> all keywords
    > >>>
    > >>> 3. Call PQconninfodefaultsMerge() to merge any default values if
    > >> exist. Not sure if this step is required?
    > >>>
    > >>> 4. Extract individual keywords from PQconninfoOption structure and
    > >> call PQconnectdbParams.
    > >>>
    > >>> Is this inline with what you have in mind or you have thought of
    > some
    > >> other simpler way of using new API's?
    > 
    > Yep, that's roughly what I had in mind. I don't think it's necessary to
    > merge defaults in step 3, but it needs to add the "replication=true"
    > and
    > "dbname=replication" options.
    
    I could see the advantage of calling PQconninfoParseParams() in step-2 is
    that 
    it will remove the duplicate values by overriding the values for conflicting
    keywords.
    This is done in function conninfo_array_parse() which is called from
    PQconninfoParseParams().
    Am I right or there is any other advantage of calling
    PQconninfoParseParams()?
    
    If there is no other advantage then this is done in PQconnectdbParams()
    also, so can't we avoid calling PQconninfoParseParams()?
    
    With Regards,
    Amit Kapila.
    
    
    
    

17. Re: [PATCH] Add PQconninfoParseParams and PQconninfodefaultsMerge to libpq

    Amit Kapila <amit.kapila@huawei.com> — 2013-02-19T12:52:40Z

    
    > -----Original Message-----
    > From: pgsql-hackers-owner@postgresql.org [mailto:pgsql-hackers-
    > owner@postgresql.org] On Behalf Of Amit Kapila
    > Sent: Monday, February 18, 2013 6:38 PM
    > To: 'Heikki Linnakangas'
    > Cc: 'Phil Sorber'; 'Alvaro Herrera'; 'Magnus Hagander'; 'PostgreSQL-
    > development'
    > Subject: Re: [HACKERS] [PATCH] Add PQconninfoParseParams and
    > PQconninfodefaultsMerge to libpq
    > 
    > On Monday, February 18, 2013 1:41 PM Heikki Linnakangas wrote:
    > > On 18.02.2013 06:07, Amit Kapila wrote:
    > > > On Sunday, February 17, 2013 8:44 PM Phil Sorber wrote:
    > > >> On Sun, Feb 17, 2013 at 1:35 AM, Amit
    > kapila<amit.kapila@huawei.com>
    > > >> wrote:
    > > >>> Now the patch of Phil Sober provides 2 new API's
    > > >> PQconninfoParseParams(), and PQconninfodefaultsMerge(),
    > > >>> using these API's I can think of below way for patch "pass a
    > > >> connection string to pg_basebackup, ..."
    > > >>>
    > > >>> 1. Call existing function PQconinfoParse() with connection string
    > > >> input by user and get PQconninfoOption.
    > > >>>
    > > >>> 2. Now use the existing keywords (individual options specified by
    > > >> user) and extract the keywords from
    > > >>>     PQconninfoOption structure and call new API
    > > >> PQconninfoParseParams() which will return PQconninfoOption.
    > > >>>     The PQconninfoOption structure returned in this step will
    > > contain
    > > >> all keywords
    > > >>>
    > > >>> 3. Call PQconninfodefaultsMerge() to merge any default values if
    > > >> exist. Not sure if this step is required?
    > > >>>
    > > >>> 4. Extract individual keywords from PQconninfoOption structure
    > and
    > > >> call PQconnectdbParams.
    > > >>>
    > > >>> Is this inline with what you have in mind or you have thought of
    > > some
    > > >> other simpler way of using new API's?
    > >
    > > Yep, that's roughly what I had in mind. I don't think it's necessary
    > to
    > > merge defaults in step 3, but it needs to add the "replication=true"
    > > and
    > > "dbname=replication" options.
    > 
    > I could see the advantage of calling PQconninfoParseParams() in step-2
    > is
    > that
    > it will remove the duplicate values by overriding the values for
    > conflicting
    > keywords.
    > This is done in function conninfo_array_parse() which is called from
    > PQconninfoParseParams().
    > Am I right or there is any other advantage of calling
    > PQconninfoParseParams()?
    > 
    > If there is no other advantage then this is done in PQconnectdbParams()
    > also, so can't we avoid calling PQconninfoParseParams()?
    
    ----
    > I note that pg_dumpall also has a similar issue as pg_basebackup and
    > pg_receivexlog; there's no way to pass a connection string to it
    > either.
    
    I think not only pg_dumpall, but we need to add it to pg_dump.
    As -C is already used option in pg_dump, I need to use something different.
    I am planning to use -K as new option(available ones were
    d,g,j,k,l,m,p,q,y).
    
    I am planning to keep option same for pg_dumpall, as pg_dumpall internally
    calls pg_dump with the options supplied by user.
    In fact, I think we can hack the string passed to pg_dump to change the
    option from -C to -K, but I am not able see if it will be way better than
    using -K for both.
    
    Suggestions?
    
    With Regards,
    Amit Kapila.
    
    
    
    

18. Re: [PATCH] Add PQconninfoParseParams and PQconninfodefaultsMerge to libpq

    Amit Kapila <amit.kapila@huawei.com> — 2013-02-20T09:42:04Z

    > > Tuesday, February 19, 2013 6:23 PM Amit Kapila wrote:
    > > On Monday, February 18, 2013 1:41 PM Heikki Linnakangas wrote:
    > > > On 18.02.2013 06:07, Amit Kapila wrote:
    > > > > On Sunday, February 17, 2013 8:44 PM Phil Sorber wrote:
    > > > >> On Sun, Feb 17, 2013 at 1:35 AM, Amit
    > > kapila<amit.kapila@huawei.com>
    > > > >> wrote:
    > > > >>> Now the patch of Phil Sober provides 2 new API's
    > > > >> PQconninfoParseParams(), and PQconninfodefaultsMerge(),
    > > > >>> using these API's I can think of below way for patch "pass a
    > > > >> connection string to pg_basebackup, ..."
    > > > >>>
    > > > >>> 1. Call existing function PQconinfoParse() with connection
    > string
    > > > >> input by user and get PQconninfoOption.
    > > > >>>
    > > > >>> 2. Now use the existing keywords (individual options specified
    > by
    > > > >> user) and extract the keywords from
    > > > >>>     PQconninfoOption structure and call new API
    > > > >> PQconninfoParseParams() which will return PQconninfoOption.
    > > > >>>     The PQconninfoOption structure returned in this step will
    > > > contain
    > > > >> all keywords
    > > > >>>
    > > > >>> 3. Call PQconninfodefaultsMerge() to merge any default values
    > if
    > > > >> exist. Not sure if this step is required?
    > > > >>>
    > > > >>> 4. Extract individual keywords from PQconninfoOption structure
    > > and
    > > > >> call PQconnectdbParams.
    > > > >>>
    > > > >>> Is this inline with what you have in mind or you have thought
    > of
    > > > some
    > > > >> other simpler way of using new API's?
    > > >
    > > > Yep, that's roughly what I had in mind. I don't think it's
    > necessary
    > > to
    > > > merge defaults in step 3, but it needs to add the
    > "replication=true"
    > > > and
    > > > "dbname=replication" options.
    > >
    > > I could see the advantage of calling PQconninfoParseParams() in step-
    > 2
    > > is
    > > that
    > > it will remove the duplicate values by overriding the values for
    > > conflicting
    > > keywords.
    > > This is done in function conninfo_array_parse() which is called from
    > > PQconninfoParseParams().
    > > Am I right or there is any other advantage of calling
    > > PQconninfoParseParams()?
    > >
    > > If there is no other advantage then this is done in
    > PQconnectdbParams()
    > > also, so can't we avoid calling PQconninfoParseParams()?
    > 
    > ----
    > > I note that pg_dumpall also has a similar issue as pg_basebackup and
    > > pg_receivexlog; there's no way to pass a connection string to it
    > > either.
    > 
    > I think not only pg_dumpall, but we need to add it to pg_dump.
    > As -C is already used option in pg_dump, I need to use something
    > different.
    > I am planning to use -K as new option(available ones were
    > d,g,j,k,l,m,p,q,y).
    > 
    > I am planning to keep option same for pg_dumpall, as pg_dumpall
    > internally
    > calls pg_dump with the options supplied by user.
    > In fact, I think we can hack the string passed to pg_dump to change the
    > option from -C to -K, but I am not able see if it will be way better
    > than
    > using -K for both.
    
    The patch for providing connection string for pg_basebackup, pg_receivexlog,
    pg_dump and pg_dumpall is attached with this mail.
    
    With Regards,
    Amit Kapila.
    

19. Re: [PATCH] Add PQconninfoParseParams and PQconninfodefaultsMerge to libpq

    Heikki Linnakangas <hlinnakangas@vmware.com> — 2013-02-20T19:16:14Z

    On 20.02.2013 11:42, Amit Kapila wrote:
    > The patch for providing connection string for pg_basebackup, pg_receivexlog,
    > pg_dump and pg_dumpall is attached with this mail.
    
    Thanks. Now that I look at this patch, I realize that we don't actually 
    need these new functions for pg_basebackup and friends after all. We 
    already have PQconninfoParse(), we can just use that.
    
    pg_dump can already take a connection string:
    
    pg_dump "dbname=postgres port=5432"
    
    For consistency with psql and other tools, perhaps we should add a "-d" 
    option to pg_dump, so that you could do:
    
    pg_dump -d "dbname=postgres port=5432"
    
    It'd be nice to call the option -d or --dbname in all the tools. That's 
    a bit confusing for pg_basebackup and pg_receivexlog, as it can *not* 
    actually be a database name, but it would be otherwise consistent with 
    the other commands.
    
    
    I came up with the attached three patches. The first adds -d/--dbname 
    option to pg_basebackup and pg_receivexlog. The second adds it to 
    pg_dump, per above. The third adds it to pg_dumpall.
    
    The third patch is a bit complicated. It first parses the user-specified 
    connection string using PQconninfoParse, so that it can merge in some 
    extra keywords: user, host, password, dbname and 
    fallback_application_name. It then calls PQconnectdbParams with the 
    keyword/value pairs. After making the initial connection to postgres or 
    template1 database, it calls PQconninfo() to again extract the 
    keyword/value pairs in effect in the connection, and constructs a new 
    connection string from them. That new connection string is then passed 
    to pg_dump on the command line, with the database name appended to it.
    
    That seems to work, although it's perhaps a bit Rube Goldbergian. One 
    step of deparsing and parsing could be avoided by keeping the 
    keyword/value pairs from the first PQconninfoParse() call, instead of 
    constructing them again with PQconninfo(). I'll experiment with that 
    tomorrow.
    
    
    The docs need some improvement. In those commands where you can't pass a 
    database name to the -d/--dbname option, only a connection string, I 
    kept your wording in the docs. But it ought to explain the seemingly 
    strange name for the option, and more. I'll take another whack at that 
    tomorrow as well.
    
    
    Where does this leave the PQconninfoParseParams/PQconninfodefaultsMerge 
    patch? I'm not sure. Somehow I thought it would be necessary for this 
    work, but it wasn't. I didn't remember that we already have 
    PQconninfoParse() function, which was enough. So, what's the use case 
    for those functions?
    
    - Heikki
    

20. Re: [PATCH] Add PQconninfoParseParams and PQconninfodefaultsMerge to libpq

    Phil Sorber <phil@omniti.com> — 2013-02-20T20:16:56Z

    On Wed, Feb 20, 2013 at 2:16 PM, Heikki Linnakangas
    <hlinnakangas@vmware.com> wrote:
    > Where does this leave the PQconninfoParseParams/PQconninfodefaultsMerge
    > patch? I'm not sure. Somehow I thought it would be necessary for this work,
    > but it wasn't. I didn't remember that we already have PQconninfoParse()
    > function, which was enough. So, what's the use case for those functions?
    >
    
    I don't think that there is an immediate case. I still think they are
    useful, and would be more useful if we had some other functions that
    took PQconninfoOption. But the original reason for their being has
    been circumvented and I think we should just push them off to next
    release commit fest and discuss them then.
    
    > - Heikki
    
    
    

21. Re: [PATCH] Add PQconninfoParseParams and PQconninfodefaultsMerge to libpq

    Amit Kapila <amit.kapila@huawei.com> — 2013-02-21T14:09:48Z

    On Thursday, February 21, 2013 12:46 AM Heikki Linnakangas wrote:
    > On 20.02.2013 11:42, Amit Kapila wrote:
    > > The patch for providing connection string for pg_basebackup,
    > > pg_receivexlog, pg_dump and pg_dumpall is attached with this mail.
    > 
    > Thanks. Now that I look at this patch, I realize that we don't actually
    > need these new functions for pg_basebackup and friends after all. We
    > already have PQconninfoParse(), we can just use that.
    > 
    > pg_dump can already take a connection string:
    > 
    > pg_dump "dbname=postgres port=5432"
    > 
    > For consistency with psql and other tools, perhaps we should add a "-d"
    > option to pg_dump, so that you could do:
    > 
    > pg_dump -d "dbname=postgres port=5432"
    > 
    > It'd be nice to call the option -d or --dbname in all the tools. That's
    > a bit confusing for pg_basebackup and pg_receivexlog, as it can *not*
    > actually be a database name, but it would be otherwise consistent with
    > the other commands.
    > 
    > 
    > I came up with the attached three patches. The first adds -d/--dbname
    > option to pg_basebackup and pg_receivexlog. 
    
    Patch-1 is working fine.
    
    >The second adds it to
    > pg_dump, per above. The third adds it to pg_dumpall.
    > 
    > The third patch is a bit complicated. It first parses the user-
    > specified connection string using PQconninfoParse, so that it can merge
    > in some extra keywords: user, host, password, dbname and
    > fallback_application_name. It then calls PQconnectdbParams with the
    > keyword/value pairs. After making the initial connection to postgres or
    > template1 database, it calls PQconninfo() to again extract the
    > keyword/value pairs in effect in the connection, and constructs a new
    > connection string from them. That new connection string is then passed
    > to pg_dump on the command line, with the database name appended to it.
    > 
    > That seems to work, although it's perhaps a bit Rube Goldbergian. One
    > step of deparsing and parsing could be avoided by keeping the
    > keyword/value pairs from the first PQconninfoParse() call, instead of
    > constructing them again with PQconninfo(). I'll experiment with that
    > tomorrow.
    
    I think this whole new logic in pg_dumpall is needed because in
    pg_dump(Patch-2),
    we have used dbname for new option.
    In pg_dump, if for new option (-d) we use new variable conn_str similar as
    we used at other places and change
    the ConnectDatabase() in file pg_backup_db.c similar to GetConnection of
    pg_basebackup, we might not
    need the new logic of deparsing and parsing in pg_dumpall.
    
    Am I missing something or do you feel this will be more cumbersome than
    current Patch-2 & Patch-3?
     
    With Regards,
    Amit Kapila.
    
    
    
    

22. Re: [PATCH] Add PQconninfoParseParams and PQconninfodefaultsMerge to libpq

    Heikki Linnakangas <hlinnakangas@vmware.com> — 2013-02-25T17:56:12Z

    On 21.02.2013 16:09, Amit Kapila wrote:
    > On Thursday, February 21, 2013 12:46 AM Heikki Linnakangas wrote:
    >> The second adds it to
    >> pg_dump, per above. The third adds it to pg_dumpall.
    >>
    >> The third patch is a bit complicated. It first parses the user-
    >> specified connection string using PQconninfoParse, so that it can merge
    >> in some extra keywords: user, host, password, dbname and
    >> fallback_application_name. It then calls PQconnectdbParams with the
    >> keyword/value pairs. After making the initial connection to postgres or
    >> template1 database, it calls PQconninfo() to again extract the
    >> keyword/value pairs in effect in the connection, and constructs a new
    >> connection string from them. That new connection string is then passed
    >> to pg_dump on the command line, with the database name appended to it.
    >>
    >> That seems to work, although it's perhaps a bit Rube Goldbergian. One
    >> step of deparsing and parsing could be avoided by keeping the
    >> keyword/value pairs from the first PQconninfoParse() call, instead of
    >> constructing them again with PQconninfo(). I'll experiment with that
    >> tomorrow.
    
    I did the above, keeping the keyword/value pairs from the 
    PQconninfoParse() step instead.
    
    > I think this whole new logic in pg_dumpall is needed because in
    > pg_dump(Patch-2),
    > we have used dbname for new option.
    > In pg_dump, if for new option (-d) we use new variable conn_str similar as
    > we used at other places and change
    > the ConnectDatabase() in file pg_backup_db.c similar to GetConnection of
    > pg_basebackup, we might not
    > need the new logic of deparsing and parsing in pg_dumpall.
    
    Yeah, could be. However, it's good to call the option -d for the sake of 
    consistency. Also, there would be some confusion if there were two ways 
    to pass a connection string (we'd stil have to support passing a 
    connection string as the database name, for backwards-compatibility).
    
    I've committed those patches, with some further changes. If you have the 
    time, please take another look at the committed patches. Thanks!
    
    - Heikki
    
    
    

23. Re: [PATCH] Add PQconninfoParseParams and PQconninfodefaultsMerge to libpq

    Amit Kapila <amit.kapila@huawei.com> — 2013-02-26T07:06:24Z

    On Monday, February 25, 2013 11:26 PM Heikki Linnakangas wrote:
    > On 21.02.2013 16:09, Amit Kapila wrote:
    > > On Thursday, February 21, 2013 12:46 AM Heikki Linnakangas wrote:
    > I've committed those patches, with some further changes. If you have
    > the
    > time, please take another look at the committed patches. Thanks!
    
    1. For pg_dumpall, incase user gives dbname in connection string, for ex.
    "user=amit dbname=db_regress port=5434"
       it will be ignored and postgres (default database name) will be used.
    
    2. For pg_basebackup, no need to check if(conn_opts) before PQconninfoFree
    as it will internally take care of
       NULL. This will not cause any harm, just an observation.
    
    With Regards,
    Amit Kapila
    
    
    
    

24. Re: [PATCH] Add PQconninfoParseParams and PQconninfodefaultsMerge to libpq

    Heikki Linnakangas <hlinnakangas@vmware.com> — 2013-02-26T08:06:08Z

    On 26.02.2013 09:06, Amit Kapila wrote:
    > On Monday, February 25, 2013 11:26 PM Heikki Linnakangas wrote:
    >> On 21.02.2013 16:09, Amit Kapila wrote:
    >>> On Thursday, February 21, 2013 12:46 AM Heikki Linnakangas wrote:
    >> I've committed those patches, with some further changes. If you have
    >> the
    >> time, please take another look at the committed patches. Thanks!
    >
    > 1. For pg_dumpall, incase user gives dbname in connection string, for ex.
    > "user=amit dbname=db_regress port=5434"
    >     it will be ignored and postgres (default database name) will be used.
    
    Yeah, that is mentioned in the docs. The other option would be to use 
    the database name from the connection string for the initial connection, 
    ie. the same as specifying a database name with the -l option. Yet 
    another option is to throw an error if database name was given. If we 
    make it an error, then we should make it an error in pg_basebackup and 
    pg_receivexlog too. I'm all ears for opinions on this.
    
    - Heikki
    
    
    

25. Re: [PATCH] Add PQconninfoParseParams and PQconninfodefaultsMerge to libpq

    Amit Kapila <amit.kapila@huawei.com> — 2013-02-26T10:21:04Z

    On Tuesday, February 26, 2013 1:36 PM Heikki Linnakangas wrote:
    > On 26.02.2013 09:06, Amit Kapila wrote:
    > > On Monday, February 25, 2013 11:26 PM Heikki Linnakangas wrote:
    > >> On 21.02.2013 16:09, Amit Kapila wrote:
    > >>> On Thursday, February 21, 2013 12:46 AM Heikki Linnakangas wrote:
    > >> I've committed those patches, with some further changes. If you have
    > >> the
    > >> time, please take another look at the committed patches. Thanks!
    > >
    > > 1. For pg_dumpall, incase user gives dbname in connection string, for
    > ex.
    > > "user=amit dbname=db_regress port=5434"
    > >     it will be ignored and postgres (default database name) will be
    > used.
    > 
    > Yeah, that is mentioned in the docs. 
    
    Sorry, I have missed that part, this just came to me when I was looking at
    the code.
    
    >The other option would be to use
    > the database name from the connection string for the initial
    > connection,
    > ie. the same as specifying a database name with the -l option.
    
    This is what come to my mind initially, as if it is allowed with -l option
    it is better if the same should be allowed through connection string as
    well.
    We are ignoring for pg_basebackup as well, but in pg_basebackup there is no
    way for user to
    specify database name.
    
    
    With Regards,
    Amit Kapila.