Re: [PATCH] Add PQconninfoParseParams and PQconninfodefaultsMerge to libpq

From: Amit Kapila <amit(dot)kapila(at)huawei(dot)com>
To: "'Heikki Linnakangas'" <hlinnakangas(at)vmware(dot)com>
Cc: "'Phil Sorber'" <phil(at)omniti(dot)com>, "'Alvaro Herrera'" <alvherre(at)2ndquadrant(dot)com>, "'Magnus Hagander'" <magnus(at)hagander(dot)net>, "'PostgreSQL-development'" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [PATCH] Add PQconninfoParseParams and PQconninfodefaultsMerge to libpq
Date: 2013-02-20 09:42:04
Message-ID: 005101ce0f4e$8b76af80$a2640e80$@kapila@huawei.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> > 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(dot)kapila(at)huawei(dot)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.

Attachment Content-Type Size
pg_basebkup_recvxlog_dump_conn_str_v2.patch application/octet-stream 28.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Heikki Linnakangas 2013-02-20 10:29:20 Re: streaming header too small
Previous Message Heikki Linnakangas 2013-02-20 09:02:07 Re: 9.2.3 crashes during archive recovery