[PATCH] Add PQconninfoParseParams and PQconninfodefaultsMerge to libpq

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

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...

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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:
/messages/by-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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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:
/messages/by-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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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:
/messages/by-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.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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:
/messages/by-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.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

-----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.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/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.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.

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

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers